[Linuxptp-devel] [PATCH v3 1/4] support TLVs with flexible size

[Geoff Salmon]

These flexible TLVs don't need to be represented as a struct directly
in the message buffer. Instead the TLV data is carried as a separate
struct in the message struct. The flexible TLV can only be the final
TLV in a message and the TLV type and length must still be written to
the message buffer (and also the management ID for management TLVs).
---
msg.c | 12 ++++++------
msg.h | 7 +++++++
tlv.c | 23 +++++++++++++++++------
tlv.h | 16 +++++++++++++---
4 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/msg.c b/msg.c
index 79fee6d..8fd2008 100644
--- a/msg.c
+++ b/msg.c
@@ -144,7 +144,7 @@ static void port_id_pre_send(struct PortIdentity *pid)
pid->portNumber = htons(pid->portNumber);
}

-static int suffix_post_recv(uint8_t *ptr, int len)
+static int suffix_post_recv(uint8_t *ptr, int len, struct tlv_extra *last)
{
int cnt;
struct TLV *tlv;
@@ -166,14 +166,14 @@ static int suffix_post_recv(uint8_t *ptr, int len)
}
len -= tlv->length;
ptr += tlv->length;
- if (tlv_post_recv(tlv)) {
+ if (tlv_post_recv(tlv, len ? NULL : last)) {
return -1;
}
}
return cnt;
}

-static void suffix_pre_send(uint8_t *ptr, int cnt)
+static void suffix_pre_send(uint8_t *ptr, int cnt, struct tlv_extra *last)
{
int i;
struct TLV *tlv;
@@ -183,7 +183,7 @@ static void suffix_pre_send(uint8_t *ptr, int cnt)

for (i = 0; i < cnt; i++) {
tlv = (struct TLV *) ptr;
- tlv_pre_send(tlv);
+ tlv_pre_send(tlv, i == cnt - 1 ? last : NULL);
ptr += sizeof(struct TLV) + tlv->length;
tlv->type = htons(tlv->type);
tlv->length = htons(tlv->length);
@@ -344,7 +344,7 @@ int msg_post_recv(struct ptp_message *m, int cnt)
return -1;
}

- m->tlv_count = suffix_post_recv(suffix, cnt - pdulen);
+ m->tlv_count = suffix_post_recv(suffix, cnt - pdulen, &m->last_tlv);
if (m->tlv_count == -1) {
return -1;
}
@@ -401,7 +401,7 @@ int msg_pre_send(struct ptp_message *m)
default:
return -1;
}
- suffix_pre_send(suffix, m->tlv_count);
+ suffix_pre_send(suffix, m->tlv_count, &m->last_tlv);
return 0;
}

diff --git a/msg.h b/msg.h
index 2feb804..279f3e6 100644
--- a/msg.h
+++ b/msg.h
@@ -26,6 +26,7 @@

#include "ddt.h"
#include "transport.h"
+#include "tlv.h"

#define PTP_VERSION 2

@@ -204,6 +205,12 @@ struct ptp_message {
* Contains the number of TLVs in the suffix.
*/
int tlv_count;
+ /**
+ * Used to hold the data of the last TLV in the message when
+ * the layout of the TLV makes it difficult to access the data
+ * directly from the message's buffer.
+ */
+ struct tlv_extra last_tlv;
};

/**
diff --git a/tlv.c b/tlv.c
index d1093ee..cd8a0a8 100644
--- a/tlv.c
+++ b/tlv.c
@@ -21,6 +21,7 @@

#include "port.h"
#include "tlv.h"
+#include "msg.h"

#define TLV_LENGTH_INVALID(tlv, type) \
(tlv->length < sizeof(struct type) - sizeof(struct TLV))
@@ -41,7 +42,7 @@ static void scaled_ns_h2n(ScaledNs *sns)
sns->fractional_nanoseconds = htons(sns->fractional_nanoseconds);
}

-static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
+static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv_extra *extra)
{
struct defaultDS *dds;
struct currentDS *cds;
@@ -49,6 +50,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
struct timePropertiesDS *tp;
struct portDS *p;
struct time_status_np *tsn;
+ int extra_len = 0;
switch (m->id) {
case DEFAULT_DATA_SET:
if (data_len != sizeof(struct defaultDS))
@@ -105,12 +107,18 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
tsn->gmPresent = ntohl(tsn->gmPresent);
break;
}
+ if (extra_len) {
+ if (extra_len % 2)
+ extra_len++;
+ if (extra_len + sizeof(m->id) != m->length)
+ goto bad_length;
+ }
return 0;
bad_length:
return -1;
}

-static void mgt_pre_send(struct management_tlv *m)
+static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
{
struct defaultDS *dds;
struct currentDS *cds;
@@ -209,12 +217,15 @@ static void org_pre_send(struct organization_tlv *org)
}
}

-int tlv_post_recv(struct TLV *tlv)
+int tlv_post_recv(struct TLV *tlv, struct tlv_extra *extra)
{
int result = 0;
struct management_tlv *mgt;
struct management_error_status *mes;
struct path_trace_tlv *ptt;
+ struct tlv_extra dummy_extra;
+ if (extra == 0)
+ extra = &dummy_extra;

switch (tlv->type) {
case TLV_MANAGEMENT:
@@ -223,7 +234,7 @@ int tlv_post_recv(struct TLV *tlv)
mgt = (struct management_tlv *) tlv;
mgt->id = ntohs(mgt->id);
if (tlv->length > sizeof(mgt->id))
- result = mgt_post_recv(mgt, tlv->length - sizeof(mgt->id));
+ result = mgt_post_recv(mgt, tlv->length - sizeof(mgt->id), extra);
break;
case TLV_MANAGEMENT_ERROR_STATUS:
if (TLV_LENGTH_INVALID(tlv, management_error_status))
@@ -261,7 +272,7 @@ bad_length:
return -1;
}

-void tlv_pre_send(struct TLV *tlv)
+void tlv_pre_send(struct TLV *tlv, struct tlv_extra *extra)
{
struct management_tlv *mgt;
struct management_error_status *mes;
@@ -270,7 +281,7 @@ void tlv_pre_send(struct TLV *tlv)
case TLV_MANAGEMENT:
mgt = (struct management_tlv *) tlv;
if (tlv->length > sizeof(mgt->id))
- mgt_pre_send(mgt);
+ mgt_pre_send(mgt, extra);
mgt->id = htons(mgt->id);
break;
case TLV_MANAGEMENT_ERROR_STATUS:
diff --git a/tlv.h b/tlv.h
index 8cce615..cfcab68 100644
--- a/tlv.h
+++ b/tlv.h
@@ -21,7 +21,6 @@
#define HAVE_TLV_H

#include "ddt.h"
-#include "msg.h"

/* TLV types */
#define TLV_MANAGEMENT 0x0001
@@ -177,17 +176,28 @@ struct time_status_np {
struct ClockIdentity gmIdentity;
} PACKED;

+struct tlv_extra {
+ union {
+ /* Empty for now, but will contain structs for the
+ * TLVs that use the tlv_extra support. */
+ };
+};
+
/**
* Converts recognized value sub-fields into host byte order.
* @param tlv Pointer to a Type Length Value field.
+ * @param extra Additional struct where data from tlv will be saved,
+ * can be NULL.
* @return Zero if successful, otherwise non-zero
*/
-int tlv_post_recv(struct TLV *tlv);
+int tlv_post_recv(struct TLV *tlv, struct tlv_extra *extra);

/**
* Converts recognized value sub-fields into network byte order.
* @param tlv Pointer to a Type Length Value field.
+ * @param extra Additional struct containing tlv data to send, can be
+ * NULL.
*/
-void tlv_pre_send(struct TLV *tlv);
+void tlv_pre_send(struct TLV *tlv, struct tlv_extra *extra);

#endif

[Geoff Salmon]

---
pmc.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/pmc.c b/pmc.c
index 5e000bb..ac9526c 100644
--- a/pmc.c
+++ b/pmc.c
@@ -24,6 +24,7 @@
#include <string.h>
#include <unistd.h>
#include <inttypes.h>
+#include <arpa/inet.h>

#include "ds.h"
#include "fsm.h"
@@ -53,7 +54,7 @@ struct management_id {

struct management_id idtab[] = {
/* Clock management ID values */
- { "USER_DESCRIPTION", USER_DESCRIPTION, not_supported },
+ { "USER_DESCRIPTION", USER_DESCRIPTION, do_get_action },
{ "SAVE_IN_NON_VOLATILE_STORAGE", SAVE_IN_NON_VOLATILE_STORAGE, not_supported },
{ "RESET_NON_VOLATILE_STORAGE", RESET_NON_VOLATILE_STORAGE, not_supported },
{ "INITIALIZE", INITIALIZE, not_supported },
@@ -86,7 +87,7 @@ struct management_id idtab[] = {
{ "TIME_STATUS_NP", TIME_STATUS_NP, do_get_action },
/* Port management ID values */
{ "NULL_MANAGEMENT", NULL_MANAGEMENT, null_management },
- { "CLOCK_DESCRIPTION", CLOCK_DESCRIPTION, not_supported },
+ { "CLOCK_DESCRIPTION", CLOCK_DESCRIPTION, do_get_action },
{ "PORT_DATA_SET", PORT_DATA_SET, do_get_action },
{ "LOG_ANNOUNCE_INTERVAL", LOG_ANNOUNCE_INTERVAL, not_supported },
{ "ANNOUNCE_RECEIPT_TIMEOUT", ANNOUNCE_RECEIPT_TIMEOUT, not_supported },
@@ -114,6 +115,61 @@ static char *action_string[] = {

#define IFMT "\n\t\t"

+static char *text2str(struct PTPText *text) {
+ static struct static_ptp_text s;
+ s.max_symbols = -1;
+ static_ptp_text_copy(&s, text);
+ return (char*)(s.text);
+}
+
+#define MAX_PRINT_BYTES 16
+#define BIN_BUF_SIZE (MAX_PRINT_BYTES * 3 + 1)
+
+static char *bin2str_impl(Octet *data, int len, char *buf, int buf_len) {
+ int i, offset = 0;
+ if (len > MAX_PRINT_BYTES)
+ len = MAX_PRINT_BYTES;
+ buf[0] = '\0';
+ if (len)
+ offset += snprintf(buf, buf_len, "%02hhx", data[0]);
+ for (i = 1; i < len; i++) {
+ if (offset >= buf_len)
+ /* truncated output */
+ break;
+ offset += snprintf(buf + offset, buf_len - offset, ":%02hhx", data[i]);
+ }
+ return buf;
+}
+
+static char *bin2str(Octet *data, int len) {
+ static char buf[BIN_BUF_SIZE];
+ return bin2str_impl(data, len, buf, sizeof(buf));
+}
+
+static uint16_t align16(uint16_t *p) {
+ uint16_t v;
+ memcpy(&v, p, sizeof(v));
+ return v;
+}
+
+static char *portaddr2str(struct PortAddress *addr) {
+ static char buf[BIN_BUF_SIZE];
+ switch(align16(&addr->networkProtocol)) {
+ case TRANS_UDP_IPV4:
+ if (align16(&addr->addressLength) == 4
+ && inet_ntop(AF_INET, addr->address, buf, sizeof(buf)))
+ return buf;
+ break;
+ case TRANS_UDP_IPV6:
+ if (align16(&addr->addressLength) == 16
+ && inet_ntop(AF_INET6, addr->address, buf, sizeof(buf)))
+ return buf;
+ break;
+ }
+ bin2str_impl(addr->address, align16(&addr->addressLength), buf, sizeof(buf));
+ return buf;
+}
+
static void pmc_show(struct ptp_message *msg, FILE *fp)
{
int action;
@@ -124,6 +180,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
struct parentDS *pds;
struct timePropertiesDS *tp;
struct time_status_np *tsn;
+ struct mgmt_clock_description *cd;
struct portDS *p;
if (msg_type(msg) != MANAGEMENT) {
return;
@@ -149,6 +206,30 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
}
mgt = (struct management_tlv *) msg->management.suffix;
switch (mgt->id) {
+ case CLOCK_DESCRIPTION:
+ cd = &msg->last_tlv.cd;
+ fprintf(fp, "CLOCK_DESCRIPTION "
+ IFMT "clockType %hu"
+ IFMT "physicalLayerProtocol %s"
+ IFMT "physicalAddress %s"
+ IFMT "protocolAddress %hu %s",
+ align16(cd->clockType), text2str(cd->physicalLayerProtocol),
+ bin2str(cd->physicalAddress->address, align16(&cd->physicalAddress->length)),
+ align16(&cd->protocolAddress->networkProtocol),
+ portaddr2str(cd->protocolAddress));
+ fprintf(fp, IFMT "manufacturerId %s" IFMT "productDescription %s",
+ bin2str(cd->manufacturerIdentity, OUI_LEN),
+ text2str(cd->productDescription));
+ fprintf(fp, IFMT "revisionData %s", text2str(cd->revisionData));
+ fprintf(fp, IFMT "userDescription %s" IFMT "profileId %s",
+ text2str(cd->userDescription),
+ bin2str(cd->profileIdentity, PROFILE_ID_LEN));
+ break;
+ case USER_DESCRIPTION:
+ fprintf(fp, "USER_DESCRIPTION "
+ IFMT "userDescription %s",
+ text2str(msg->last_tlv.cd.userDescription));
+ break;
case DEFAULT_DATA_SET:
dds = (struct defaultDS *) mgt->data;
fprintf(fp, "DEFAULT_DATA_SET "

[Richard Cochran]

Post by Geoff Salmon
@@ -149,6 +206,30 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
}
mgt = (struct management_tlv *) msg->management.suffix;
switch (mgt->id) {
+ cd = &msg->last_tlv.cd;
+ fprintf(fp, "CLOCK_DESCRIPTION "
+ IFMT "clockType %hu"
+ IFMT "physicalLayerProtocol %s"
+ IFMT "physicalAddress %s"
+ IFMT "protocolAddress %hu %s",
+ align16(cd->clockType), text2str(cd->physicalLayerProtocol),
+ bin2str(cd->physicalAddress->address, align16(&cd->physicalAddress->length)),
+ align16(&cd->protocolAddress->networkProtocol),
+ portaddr2str(cd->protocolAddress));
+ fprintf(fp, IFMT "manufacturerId %s" IFMT "productDescription %s",
+ bin2str(cd->manufacturerIdentity, OUI_LEN),
+ text2str(cd->productDescription));
+ fprintf(fp, IFMT "revisionData %s", text2str(cd->revisionData));
+ fprintf(fp, IFMT "userDescription %s" IFMT "profileId %s",

These lines are a bit too long. Put each IFMT on its own line.

Thanks,
Richard

[Richard Cochran]

Post by Geoff Salmon
@@ -149,6 +206,30 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
}
mgt = (struct management_tlv *) msg->management.suffix;
switch (mgt->id) {
+ cd = &msg->last_tlv.cd;
+ fprintf(fp, "CLOCK_DESCRIPTION "
+ IFMT "clockType %hu"

This is a bit field, so format 0x%hx is probably better here.

Thanks,
Richard

[Geoff Salmon]

Modifies existing structs changing Octet *foo -> Octet foo[0] so the
message buffer can be cast to the structs.
---
ddt.h | 9 +++++++--
tlv.h | 26 ++++++++++++++++++++++++--
util.c | 17 +++++++++--------
3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/ddt.h b/ddt.h
index 760b443..9270858 100644
--- a/ddt.h
+++ b/ddt.h
@@ -51,7 +51,12 @@ struct PortIdentity {
struct PortAddress {
Enumeration16 networkProtocol;
UInteger16 addressLength;
- Octet *address;
+ Octet address[0];
+};
+
+struct PhysicalAddress {
+ UInteger16 length;
+ Octet address[0];
};

struct ClockQuality {
@@ -68,7 +73,7 @@ struct TLV {

struct PTPText {
UInteger8 length;
- Octet *text;
+ Octet text[0];
};

/* A static_ptp_text is like a PTPText but includes space to store the
diff --git a/tlv.h b/tlv.h
index cfcab68..b437286 100644
--- a/tlv.h
+++ b/tlv.h
@@ -21,6 +21,7 @@
#define HAVE_TLV_H

#include "ddt.h"
+#include "ds.h"

/* TLV types */
#define TLV_MANAGEMENT 0x0001
@@ -176,10 +177,31 @@ struct time_status_np {
struct ClockIdentity gmIdentity;
} PACKED;

+enum clock_type {
+ CLOCK_TYPE_ORDINARY = 0x80,
+ CLOCK_TYPE_BOUNDARY = 0x40,
+ CLOCK_TYPE_P2P = 0x20,
+ CLOCK_TYPE_E2E = 0x10,
+ CLOCK_TYPE_MANAGEMENT = 0x08,
+};
+
+#define PROFILE_ID_LEN 6
+
+struct mgmt_clock_description {
+ UInteger16 *clockType;
+ struct PTPText *physicalLayerProtocol;
+ struct PhysicalAddress *physicalAddress;
+ struct PortAddress *protocolAddress;
+ Octet *manufacturerIdentity;
+ struct PTPText *productDescription;
+ struct PTPText *revisionData;
+ struct PTPText *userDescription;
+ Octet *profileIdentity;
+};
+
struct tlv_extra {
union {
- /* Empty for now, but will contain structs for the
- * TLVs that use the tlv_extra support. */
+ struct mgmt_clock_description cd;
};
};

diff --git a/util.c b/util.c
index 57e9fbe..7699fac 100644
--- a/util.c
+++ b/util.c
@@ -119,7 +119,7 @@ int static_ptp_text_copy(struct static_ptp_text *dst, const struct PTPText *src)
void ptp_text_copy(struct PTPText *dst, const struct static_ptp_text *src)
{
dst->length = src->length;
- dst->text = (Octet *)src->text;
+ memcpy(dst->text, src->text, src->length);
}

int ptp_text_set(struct PTPText *dst, const char *src)
@@ -130,21 +130,22 @@ int ptp_text_set(struct PTPText *dst, const char *src)
if (len > MAX_PTP_OCTETS)
return -1;
dst->length = len;
- dst->text = (Octet *)src;
+ memcpy(dst->text, src, len);
} else {
dst->length = 0;
- dst->text = NULL;
}
return 0;
}

int static_ptp_text_set(struct static_ptp_text *dst, const char *src)
{
- struct PTPText t;
- size_t len = strlen(src);
+ int len = strlen(src);
if (len > MAX_PTP_OCTETS)
return -1;
- t.length = len;
- t.text = (Octet *)src;
- return static_ptp_text_copy(dst, &t);
+ if (dst->max_symbols > 0 && strlen_utf8((Octet *) src) > dst->max_symbols)
+ return -1;
+ dst->length = len;
+ memcpy(dst->text, src, len);
+ dst->text[len] = '\0';
+ return 0;
}

[Richard Cochran]

Post by Geoff Salmon
Modifies existing structs changing Octet *foo -> Octet foo[0] so the
message buffer can be cast to the structs.
---
ddt.h | 9 +++++++--
tlv.h | 26 ++++++++++++++++++++++++--
util.c | 17 +++++++++--------
3 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/ddt.h b/ddt.h
index 760b443..9270858 100644
--- a/ddt.h
+++ b/ddt.h
@@ -51,7 +51,12 @@ struct PortIdentity {
struct PortAddress {
Enumeration16 networkProtocol;
UInteger16 addressLength;
- Octet *address;
+ Octet address[0];
+};

We are now casting this to a buffer, so I think should have the
PACKED attribute.

Post by Geoff Salmon
+
+struct PhysicalAddress {
+ UInteger16 length;
+ Octet address[0];
};

Here, too.

Post by Geoff Salmon
struct ClockQuality {
@@ -68,7 +73,7 @@ struct TLV {
struct PTPText {
UInteger8 length;
- Octet *text;
+ Octet text[0];
};

And here.

Thanks,
Richard

[Geoff Salmon]

---
clock.c | 17 +++++++++++++++
clock.h | 7 ++++++
port.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tlv.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 161 insertions(+)

diff --git a/clock.c b/clock.c
index e4e4f30..87a2275 100644
--- a/clock.c
+++ b/clock.c
@@ -159,6 +159,7 @@ static int clock_management_get_response(struct clock *c, struct port *p,
struct ptp_message *rsp;
struct time_status_np *tsn;
struct PortIdentity pid = port_identity(p);
+ struct PTPText *text;

rsp = port_management_reply(pid, p, req);
if (!rsp) {
@@ -169,6 +170,13 @@ static int clock_management_get_response(struct clock *c, struct port *p,
tlv->id = id;

switch (id) {
+ case USER_DESCRIPTION:
+ text = (struct PTPText *) tlv->data;
+ text->length = c->desc.userDescription.length;
+ memcpy(text->text, c->desc.userDescription.text, text->length);
+ datalen = 1 + text->length;
+ respond = 1;
+ break;
case DEFAULT_DATA_SET:
memcpy(tlv->data, &c->dds, sizeof(c->dds));
datalen = sizeof(c->dds);
@@ -209,6 +217,10 @@ static int clock_management_get_response(struct clock *c, struct port *p,
break;
}
if (respond) {
+ if (datalen % 2) {
+ tlv->data[datalen] = 0;
+ datalen++;
+ }
tlv->length = sizeof(tlv->id) + datalen;
pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
rsp->header.messageLength = pdulen;
@@ -1082,3 +1094,8 @@ struct clock_description *clock_description(struct clock *c)
{
return &c->desc;
}
+
+int clock_num_ports(struct clock *c)
+{
+ return c->nports;
+}
diff --git a/clock.h b/clock.h
index 581fa14..7b4bfb0 100644
--- a/clock.h
+++ b/clock.h
@@ -222,4 +222,11 @@ struct timePropertiesDS *clock_time_properties(struct clock *c);
*/
struct clock_description *clock_description(struct clock *c);

+/**
+ * Obtain the number of ports a clock has, excluding the UDS port.
+ * @param c The clock instance.
+ * @return The number of ports.
+ */
+int clock_num_ports(struct clock *c);
+
#endif
diff --git a/port.c b/port.c
index 9899687..2260433 100644
--- a/port.c
+++ b/port.c
@@ -438,6 +438,9 @@ static void port_management_send_error(struct port *p, struct port *ingress,
pr_err("port %hu: management error failed", portnum(p));
}

+static const Octet profile_id_drr[] = {0x00, 0x1B, 0x19, 0x00, 0x01, 0x00};
+static const Octet profile_id_p2p[] = {0x00, 0x1B, 0x19, 0x00, 0x02, 0x00};
+
static int port_management_get_response(struct port *target,
struct port *ingress, int id,
struct ptp_message *req)
@@ -447,6 +450,10 @@ static int port_management_get_response(struct port *target,
struct ptp_message *rsp;
struct portDS *pds;
struct PortIdentity pid = port_identity(target);
+ struct clock_description *desc;
+ struct mgmt_clock_description *cd;
+ uint8_t *buf;
+ uint16_t u16;

rsp = port_management_reply(pid, ingress, req);
if (!rsp) {
@@ -461,6 +468,69 @@ static int port_management_get_response(struct port *target,
datalen = 0;
respond = 1;
break;
+ case CLOCK_DESCRIPTION:
+ cd = &rsp->last_tlv.cd;
+ buf = tlv->data;
+ cd->clockType = (UInteger16 *) buf;
+ buf += sizeof(*cd->clockType);
+ if (clock_num_ports(target->clock) > 1) {
+ *cd->clockType = CLOCK_TYPE_BOUNDARY;
+ } else {
+ *cd->clockType = CLOCK_TYPE_ORDINARY;
+ }
+
+ cd->physicalLayerProtocol = (struct PTPText *) buf;
+ switch(transport_type(target->trp)) {
+ case TRANS_UDP_IPV4:
+ case TRANS_UDP_IPV6:
+ case TRANS_IEEE_802_3:
+ ptp_text_set(cd->physicalLayerProtocol, "IEEE 802.3");
+ break;
+ default:
+ ptp_text_set(cd->physicalLayerProtocol, NULL);
+ break;
+ }
+ buf += sizeof(struct PTPText) + cd->physicalLayerProtocol->length;
+
+ cd->physicalAddress = (struct PhysicalAddress *) buf;
+ u16 = transport_physical_addr(target->trp, cd->physicalAddress->address);
+ memcpy(&cd->physicalAddress->length, &u16, 2);
+ buf += sizeof(struct PhysicalAddress) + u16;
+
+ cd->protocolAddress = (struct PortAddress *) buf;
+ u16 = transport_type(target->trp);
+ memcpy(&cd->protocolAddress->networkProtocol, &u16, 2);
+ u16 = transport_protocol_addr(target->trp, cd->protocolAddress->address);
+ memcpy(&cd->protocolAddress->addressLength, &u16, 2);
+ buf += sizeof(struct PortAddress) + u16;
+
+ desc = clock_description(target->clock);
+ cd->manufacturerIdentity = buf;
+ memcpy(cd->manufacturerIdentity, desc->manufacturerIdentity, OUI_LEN);
+ buf += OUI_LEN;
+ *(buf++) = 0; /* reserved */
+
+ cd->productDescription = (struct PTPText *) buf;
+ ptp_text_copy(cd->productDescription, &desc->productDescription);
+ buf += sizeof(struct PTPText) + cd->productDescription->length;
+
+ cd->revisionData = (struct PTPText *) buf;
+ ptp_text_copy(cd->revisionData, &desc->revisionData);
+ buf += sizeof(struct PTPText) + cd->revisionData->length;
+
+ cd->userDescription = (struct PTPText *) buf;
+ ptp_text_copy(cd->userDescription, &desc->userDescription);
+ buf += sizeof(struct PTPText) + cd->userDescription->length;
+
+ if (target->delayMechanism == DM_P2P) {
+ memcpy(buf, profile_id_p2p, PROFILE_ID_LEN);
+ } else {
+ memcpy(buf, profile_id_drr, PROFILE_ID_LEN);
+ }
+ buf += PROFILE_ID_LEN;
+ datalen = buf - tlv->data;
+ respond = 1;
+ break;
case PORT_DATA_SET:
pds = (struct portDS *) tlv->data;
pds->portIdentity = target->portIdentity;
@@ -486,6 +556,10 @@ static int port_management_get_response(struct port *target,
break;
}
if (respond) {
+ if (datalen % 2) {
+ tlv->data[datalen] = 0;
+ datalen++;
+ }
tlv->length = sizeof(tlv->id) + datalen;
pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
rsp->header.messageLength = pdulen;
diff --git a/tlv.c b/tlv.c
index cd8a0a8..3ab8a9b 100644
--- a/tlv.c
+++ b/tlv.c
@@ -42,6 +42,14 @@ static void scaled_ns_h2n(ScaledNs *sns)
sns->fractional_nanoseconds = htons(sns->fractional_nanoseconds);
}

+static uint16_t flip16(uint16_t *p) {
+ uint16_t v;
+ memcpy(&v, p, sizeof(v));
+ v = htons(v);
+ memcpy(p, &v, sizeof(v));
+ return v;
+}
+
static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv_extra *extra)
{
struct defaultDS *dds;
@@ -50,8 +58,53 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv
struct timePropertiesDS *tp;
struct portDS *p;
struct time_status_np *tsn;
+ struct mgmt_clock_description *cd;
int extra_len = 0;
+ uint8_t *buf;
+ uint16_t u16;
switch (m->id) {
+ case CLOCK_DESCRIPTION:
+ cd = &extra->cd;
+ buf = m->data;
+
+ cd->clockType = (UInteger16 *) buf;
+ flip16(cd->clockType);
+ buf += sizeof(*cd->clockType);
+
+ cd->physicalLayerProtocol = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->physicalLayerProtocol->length;
+
+ cd->physicalAddress = (struct PhysicalAddress *) buf;
+ u16 = flip16(&cd->physicalAddress->length);
+ if (u16 > TRANSPORT_ADDR_LEN)
+ goto bad_length;
+ buf += sizeof(struct PhysicalAddress) + u16;
+
+ cd->protocolAddress = (struct PortAddress *) buf;
+ flip16(&cd->protocolAddress->networkProtocol);
+ u16 = flip16(&cd->protocolAddress->addressLength);
+ if (u16 > TRANSPORT_ADDR_LEN)
+ goto bad_length;
+ buf += sizeof(struct PortAddress) + u16;
+
+ cd->manufacturerIdentity = buf;
+ buf += OUI_LEN + 1;
+
+ cd->productDescription = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->productDescription->length;
+ cd->revisionData = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->revisionData->length;
+ cd->userDescription = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->userDescription->length;
+
+ cd->profileIdentity = buf;
+ buf += PROFILE_ID_LEN;
+ extra_len = buf - m->data;
+ break;
+ case USER_DESCRIPTION:
+ extra->cd.userDescription = (struct PTPText *) m->data;
+ extra_len = sizeof(struct PTPText) + extra->cd.userDescription->length;
+ break;
case DEFAULT_DATA_SET:
if (data_len != sizeof(struct defaultDS))
goto bad_length;
@@ -126,7 +179,17 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
struct timePropertiesDS *tp;
struct portDS *p;
struct time_status_np *tsn;
+ struct mgmt_clock_description *cd;
switch (m->id) {
+ case CLOCK_DESCRIPTION:
+ if (extra) {
+ cd = &extra->cd;
+ flip16(cd->clockType);
+ flip16(&cd->physicalAddress->length);
+ flip16(&cd->protocolAddress->networkProtocol);
+ flip16(&cd->protocolAddress->addressLength);
+ }
+ break;
case DEFAULT_DATA_SET:
dds = (struct defaultDS *) m->data;
dds->numberPorts = htons(dds->numberPorts);

[Richard Cochran]

Geoff,

Below, I have marked a few lines that are a bit too long for my taste.

Post by Geoff Salmon
---
clock.c | 17 +++++++++++++++
clock.h | 7 ++++++
port.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tlv.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 161 insertions(+)
diff --git a/clock.c b/clock.c
index e4e4f30..87a2275 100644
--- a/clock.c
+++ b/clock.c
@@ -159,6 +159,7 @@ static int clock_management_get_response(struct clock *c, struct port *p,
struct ptp_message *rsp;
struct time_status_np *tsn;
struct PortIdentity pid = port_identity(p);
+ struct PTPText *text;
rsp = port_management_reply(pid, p, req);
if (!rsp) {
@@ -169,6 +170,13 @@ static int clock_management_get_response(struct clock *c, struct port *p,
tlv->id = id;
switch (id) {
+ text = (struct PTPText *) tlv->data;
+ text->length = c->desc.userDescription.length;
+ memcpy(text->text, c->desc.userDescription.text, text->length);
+ datalen = 1 + text->length;
+ respond = 1;
+ break;
memcpy(tlv->data, &c->dds, sizeof(c->dds));
datalen = sizeof(c->dds);
@@ -209,6 +217,10 @@ static int clock_management_get_response(struct clock *c, struct port *p,
break;
}
if (respond) {
+ if (datalen % 2) {
+ tlv->data[datalen] = 0;
+ datalen++;
+ }
tlv->length = sizeof(tlv->id) + datalen;
pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
rsp->header.messageLength = pdulen;
@@ -1082,3 +1094,8 @@ struct clock_description *clock_description(struct clock *c)
{
return &c->desc;
}
+
+int clock_num_ports(struct clock *c)
+{
+ return c->nports;
+}
diff --git a/clock.h b/clock.h
index 581fa14..7b4bfb0 100644
--- a/clock.h
+++ b/clock.h
@@ -222,4 +222,11 @@ struct timePropertiesDS *clock_time_properties(struct clock *c);
*/
struct clock_description *clock_description(struct clock *c);
+/**
+ * Obtain the number of ports a clock has, excluding the UDS port.
+ */
+int clock_num_ports(struct clock *c);
+
#endif
diff --git a/port.c b/port.c
index 9899687..2260433 100644
--- a/port.c
+++ b/port.c
@@ -438,6 +438,9 @@ static void port_management_send_error(struct port *p, struct port *ingress,
pr_err("port %hu: management error failed", portnum(p));
}
+static const Octet profile_id_drr[] = {0x00, 0x1B, 0x19, 0x00, 0x01, 0x00};
+static const Octet profile_id_p2p[] = {0x00, 0x1B, 0x19, 0x00, 0x02, 0x00};
+
static int port_management_get_response(struct port *target,
struct port *ingress, int id,
struct ptp_message *req)
@@ -447,6 +450,10 @@ static int port_management_get_response(struct port *target,
struct ptp_message *rsp;
struct portDS *pds;
struct PortIdentity pid = port_identity(target);
+ struct clock_description *desc;
+ struct mgmt_clock_description *cd;
+ uint8_t *buf;
+ uint16_t u16;
rsp = port_management_reply(pid, ingress, req);
if (!rsp) {
@@ -461,6 +468,69 @@ static int port_management_get_response(struct port *target,
datalen = 0;
respond = 1;
break;
+ cd = &rsp->last_tlv.cd;
+ buf = tlv->data;
+ cd->clockType = (UInteger16 *) buf;
+ buf += sizeof(*cd->clockType);
+ if (clock_num_ports(target->clock) > 1) {
+ *cd->clockType = CLOCK_TYPE_BOUNDARY;
+ } else {
+ *cd->clockType = CLOCK_TYPE_ORDINARY;
+ }
+
+ cd->physicalLayerProtocol = (struct PTPText *) buf;
+ switch(transport_type(target->trp)) {
+ ptp_text_set(cd->physicalLayerProtocol, "IEEE 802.3");
+ break;
+ ptp_text_set(cd->physicalLayerProtocol, NULL);
+ break;
+ }
+ buf += sizeof(struct PTPText) + cd->physicalLayerProtocol->length;
+
+ cd->physicalAddress = (struct PhysicalAddress *) buf;
+ u16 = transport_physical_addr(target->trp, cd->physicalAddress->address);

Here

Post by Geoff Salmon
+ memcpy(&cd->physicalAddress->length, &u16, 2);
+ buf += sizeof(struct PhysicalAddress) + u16;
+
+ cd->protocolAddress = (struct PortAddress *) buf;
+ u16 = transport_type(target->trp);
+ memcpy(&cd->protocolAddress->networkProtocol, &u16, 2);
+ u16 = transport_protocol_addr(target->trp, cd->protocolAddress->address);

Here

Post by Geoff Salmon
+ memcpy(&cd->protocolAddress->addressLength, &u16, 2);
+ buf += sizeof(struct PortAddress) + u16;
+
+ desc = clock_description(target->clock);
+ cd->manufacturerIdentity = buf;
+ memcpy(cd->manufacturerIdentity, desc->manufacturerIdentity, OUI_LEN);
+ buf += OUI_LEN;
+ *(buf++) = 0; /* reserved */
+
+ cd->productDescription = (struct PTPText *) buf;
+ ptp_text_copy(cd->productDescription, &desc->productDescription);
+ buf += sizeof(struct PTPText) + cd->productDescription->length;
+
+ cd->revisionData = (struct PTPText *) buf;
+ ptp_text_copy(cd->revisionData, &desc->revisionData);
+ buf += sizeof(struct PTPText) + cd->revisionData->length;
+
+ cd->userDescription = (struct PTPText *) buf;
+ ptp_text_copy(cd->userDescription, &desc->userDescription);
+ buf += sizeof(struct PTPText) + cd->userDescription->length;
+
+ if (target->delayMechanism == DM_P2P) {
+ memcpy(buf, profile_id_p2p, PROFILE_ID_LEN);
+ } else {
+ memcpy(buf, profile_id_drr, PROFILE_ID_LEN);
+ }
+ buf += PROFILE_ID_LEN;
+ datalen = buf - tlv->data;
+ respond = 1;
+ break;
pds = (struct portDS *) tlv->data;
pds->portIdentity = target->portIdentity;
@@ -486,6 +556,10 @@ static int port_management_get_response(struct port *target,
break;
}
if (respond) {
+ if (datalen % 2) {
+ tlv->data[datalen] = 0;
+ datalen++;
+ }
tlv->length = sizeof(tlv->id) + datalen;
pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
rsp->header.messageLength = pdulen;
diff --git a/tlv.c b/tlv.c
index cd8a0a8..3ab8a9b 100644
--- a/tlv.c
+++ b/tlv.c
@@ -42,6 +42,14 @@ static void scaled_ns_h2n(ScaledNs *sns)
sns->fractional_nanoseconds = htons(sns->fractional_nanoseconds);
}
+static uint16_t flip16(uint16_t *p) {
+ uint16_t v;
+ memcpy(&v, p, sizeof(v));
+ v = htons(v);
+ memcpy(p, &v, sizeof(v));
+ return v;
+}
+
static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv_extra *extra)
{
struct defaultDS *dds;
@@ -50,8 +58,53 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv
struct timePropertiesDS *tp;
struct portDS *p;
struct time_status_np *tsn;
+ struct mgmt_clock_description *cd;
int extra_len = 0;
+ uint8_t *buf;
+ uint16_t u16;
switch (m->id) {
+ cd = &extra->cd;
+ buf = m->data;
+
+ cd->clockType = (UInteger16 *) buf;
+ flip16(cd->clockType);
+ buf += sizeof(*cd->clockType);
+
+ cd->physicalLayerProtocol = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->physicalLayerProtocol->length;
+
+ cd->physicalAddress = (struct PhysicalAddress *) buf;
+ u16 = flip16(&cd->physicalAddress->length);
+ if (u16 > TRANSPORT_ADDR_LEN)
+ goto bad_length;
+ buf += sizeof(struct PhysicalAddress) + u16;
+
+ cd->protocolAddress = (struct PortAddress *) buf;
+ flip16(&cd->protocolAddress->networkProtocol);
+ u16 = flip16(&cd->protocolAddress->addressLength);
+ if (u16 > TRANSPORT_ADDR_LEN)
+ goto bad_length;
+ buf += sizeof(struct PortAddress) + u16;
+
+ cd->manufacturerIdentity = buf;
+ buf += OUI_LEN + 1;
+
+ cd->productDescription = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->productDescription->length;
+ cd->revisionData = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->revisionData->length;
+ cd->userDescription = (struct PTPText *) buf;
+ buf += sizeof(struct PTPText) + cd->userDescription->length;
+
+ cd->profileIdentity = buf;
+ buf += PROFILE_ID_LEN;
+ extra_len = buf - m->data;
+ break;
+ extra->cd.userDescription = (struct PTPText *) m->data;
+ extra_len = sizeof(struct PTPText) + extra->cd.userDescription->length;

and here.

Thanks,
Richard

[Richard Cochran]

Geoff,

This series has really shaped up well. I appreciate your following the
"pointer" direction. Looking at the diffstats, you were able to reduce
the scope of the changes, a good sign.

v1 9 files changed, 398 insertions(+), 17 deletions(-)
v2 9 files changed, 368 insertions(+), 17 deletions(-)
v3 10 files changed, 325 insertions(+), 27 deletions(-)

Thanks,
Richard

[Richard Cochran]

Post by Geoff Salmon
diff --git a/tlv.c b/tlv.c
index d1093ee..cd8a0a8 100644
--- a/tlv.c
+++ b/tlv.c
@@ -21,6 +21,7 @@
#include "port.h"
#include "tlv.h"
+#include "msg.h"
#define TLV_LENGTH_INVALID(tlv, type) \
(tlv->length < sizeof(struct type) - sizeof(struct TLV))
@@ -41,7 +42,7 @@ static void scaled_ns_h2n(ScaledNs *sns)
sns->fractional_nanoseconds = htons(sns->fractional_nanoseconds);
}
-static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
+static int mgt_post_recv(struct management_tlv *m, uint16_t data_len, struct tlv_extra *extra)

Please break this over two lines.

Post by Geoff Salmon
{
struct defaultDS *dds;
struct currentDS *cds;
@@ -49,6 +50,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
struct timePropertiesDS *tp;
struct portDS *p;
struct time_status_np *tsn;
+ int extra_len = 0;
switch (m->id) {
if (data_len != sizeof(struct defaultDS))
@@ -105,12 +107,18 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len)
tsn->gmPresent = ntohl(tsn->gmPresent);
break;
}
+ if (extra_len) {
+ if (extra_len % 2)
+ extra_len++;
+ if (extra_len + sizeof(m->id) != m->length)
+ goto bad_length;
+ }
return 0;
return -1;
}
-static void mgt_pre_send(struct management_tlv *m)
+static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
{
struct defaultDS *dds;
struct currentDS *cds;
@@ -209,12 +217,15 @@ static void org_pre_send(struct organization_tlv *org)
}
}
-int tlv_post_recv(struct TLV *tlv)
+int tlv_post_recv(struct TLV *tlv, struct tlv_extra *extra)
{
int result = 0;
struct management_tlv *mgt;
struct management_error_status *mes;
struct path_trace_tlv *ptt;
+ struct tlv_extra dummy_extra;
+ if (extra == 0)
+ extra = &dummy_extra;

Nicer style is:

if (!extra)
...

Thanks,
Richard