Since commit 7fe69e7ba0c3bdce88d3a89d5ed43d0b382ef022 management messages are dropped in some cases because the tlv_count isn't 1. Further analysis shows that this is the case when the message is forwarded in clock_forward_mgmt_msg(). This is because msg_post_recv() will append TLVs and - in the forwarding case - there is already one TLV in the list, which results in tlv_count being 2 and thus dropped in subsequent code.
This patch fixes this behaviour by copying the message to avoid the network byte order to host byte order cycle. Signed-off-by: Michael Walle <[email protected]> --- This is the second variant of the "Drop all TLVs in clock_forward_mgmt_msg()" patch. I think this is more elegant than the previous one. It also drops the "pre_sent" stuff which isn't obvious anymore since the commit which splits port handling (48a8425f). Does anyone come up with a better name than msg_deepcopy()? Esp. since there is alread a msg_duplicate() function. A third alternative could be to use msg_duplicate() in bc_event and hand over the duplicate as a further parameter to clock_manage(). But if forwarding isn't needed this copy would be just wasting CPU time. clock.c | 30 +++++++++++++++--------------- msg.c | 27 +++++++++++++++++++++++++++ msg.h | 15 +++++++++++++++ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/clock.c b/clock.c index 7bbb848..1556c8b 100644 --- a/clock.c +++ b/clock.c @@ -1250,7 +1250,7 @@ void clock_fda_changed(struct clock *c) static int clock_do_forward_mgmt(struct clock *c, struct port *in, struct port *out, - struct ptp_message *msg, int *pre_sent) + struct ptp_message *msg) { if (in == out || !forwarding(c, out)) return 0; @@ -1265,34 +1265,34 @@ static int clock_do_forward_mgmt(struct clock *c, } } - if (!*pre_sent) { - /* delay calling msg_pre_send until - * actually forwarding */ - msg_pre_send(msg); - *pre_sent = 1; - } return port_forward(out, msg); } static void clock_forward_mgmt_msg(struct clock *c, struct port *p, struct ptp_message *msg) { struct port *piter; - int pdulen = 0, msg_ready = 0; + struct ptp_message *dup; if (forwarding(c, p) && msg->management.boundaryHops) { - pdulen = msg->header.messageLength; - msg->management.boundaryHops--; + dup = msg_deepcopy(msg); + if (!dup) { + return; + } + dup->management.boundaryHops--; + if (msg_pre_send(dup)) { + msg_put(dup); + return; + } LIST_FOREACH(piter, &c->ports, list) { - if (clock_do_forward_mgmt(c, p, piter, msg, &msg_ready)) + if (clock_do_forward_mgmt(c, p, piter, dup)) { pr_err("port %d: management forward failed", port_number(piter)); + } } - if (clock_do_forward_mgmt(c, p, c->uds_port, msg, &msg_ready)) + if (clock_do_forward_mgmt(c, p, c->uds_port, dup)) { pr_err("uds port: management forward failed"); - if (msg_ready) { - msg_post_recv(msg, pdulen); - msg->management.boundaryHops++; } + msg_put(dup); } } diff --git a/msg.c b/msg.c index 090715b..9b2d4b0 100644 --- a/msg.c +++ b/msg.c @@ -332,6 +332,33 @@ struct ptp_message *msg_duplicate(struct ptp_message *msg, int cnt) return dup; } +struct ptp_message *msg_deepcopy(struct ptp_message *msg) +{ + struct ptp_message *dup; + struct tlv_extra *extra, *extra_dup; + uint8_t *ptr; + + dup = msg_allocate(); + if (!dup) { + return NULL; + } + memcpy(dup, msg, sizeof(*dup)); + dup->refcnt = 1; + + /* duplicate tlv_extra list */ + dup->tlv_count = 0; + TAILQ_INIT(&dup->tlv_list); + ptr = msg_suffix(dup); + TAILQ_FOREACH(extra, &msg->tlv_list, list) { + extra_dup = tlv_extra_alloc(); + extra_dup->tlv = (struct TLV *) ptr; + ptr += sizeof(struct TLV) + extra->tlv->length; + msg_tlv_attach(dup, extra_dup); + } + + return dup; +} + void msg_get(struct ptp_message *m) { m->refcnt++; diff --git a/msg.h b/msg.h index 58a916c..7ac8733 100644 --- a/msg.h +++ b/msg.h @@ -337,6 +337,21 @@ void msg_cleanup(void); struct ptp_message *msg_duplicate(struct ptp_message *msg, int cnt); /** + * Copy a message instance. + * + * This function does a deep copy of a message in host byte order. + * + * Messages are reference counted, and newly allocated messages have a + * reference count of one. Allocated messages are freed using the + * function @ref msg_put(). + * + * @param msg A message obtained using @ref msg_allocate(). + * + * @return Pointer to a message on success, NULL otherwise. + */ +struct ptp_message *msg_deepcopy(struct ptp_message *msg); + +/** * Obtain a reference to a message, increasing its reference count by one. * @param m A message obtained using @ref msg_allocate(). */ -- 2.11.0 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linuxptp-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
