From a8a3ddec6ab31c8111bc3374fb7cbfe9957fcda5 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Fri, 28 Aug 2015 17:02:01 +0200 Subject: [PATCH 1/6] Fix integer overflow in the foreign master bookkeeping code. The logMessageInterval field has an improbable range from 2^-128 to 2^127 seconds. The extreme ends cause an integer overflow in the calculation of the "foreign master time window". Buggy or mis-configured foreign masters advertising extreme values will cause incorrect announce message aging. This patch fixes the issue by adding thresholds for the bogus extremes. Signed-off-by: Richard Cochran --- port.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/port.c b/port.c index 3984b78..ef2686d 100644 --- a/port.c +++ b/port.c @@ -163,10 +163,15 @@ static int msg_current(struct ptp_message *m, struct timespec now) t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec; t2 = now.tv_sec * NSEC2SEC + now.tv_nsec; - if (m->header.logMessageInterval < 0) + if (m->header.logMessageInterval < -63) { + tmo = 0; + } else if (m->header.logMessageInterval > 31) { + tmo = INT64_MAX; + } else if (m->header.logMessageInterval < 0) { tmo = 4LL * NSEC2SEC / (1 << -m->header.logMessageInterval); - else + } else { tmo = 4LL * (1 << m->header.logMessageInterval) * NSEC2SEC; + } return t2 - t1 < tmo; } From 0135e5344d52939b5c2514ac0e86bc599aa50be5 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Fri, 28 Aug 2015 17:55:32 +0200 Subject: [PATCH 2/6] port: constrain the master's logMinDelayReqInterval. Buggy or mis-configured masters can place bogus logMessageInterval values in their delay response messages. This patch places reasonable limits on the range of values that we will accept. Signed-off-by: Richard Cochran --- port.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/port.c b/port.c index ef2686d..91bb4e9 100644 --- a/port.c +++ b/port.c @@ -1666,12 +1666,18 @@ static void process_delay_resp(struct port *p, struct ptp_message *m) clock_path_delay(p->clock, t3, t4c); - if (p->logMinDelayReqInterval != rsp->hdr.logMessageInterval) { - // TODO - validate the input. - p->logMinDelayReqInterval = rsp->hdr.logMessageInterval; - pr_notice("port %hu: minimum delay request interval 2^%d", - portnum(p), p->logMinDelayReqInterval); + if (p->logMinDelayReqInterval == rsp->hdr.logMessageInterval) { + return; } + if (rsp->hdr.logMessageInterval < -10 || + rsp->hdr.logMessageInterval > 22) { + pr_debug("port %hu: ignore bogus delay request interval 2^%d", + portnum(p), rsp->hdr.logMessageInterval); + return; + } + p->logMinDelayReqInterval = rsp->hdr.logMessageInterval; + pr_notice("port %hu: minimum delay request interval 2^%d", + portnum(p), p->logMinDelayReqInterval); } static void process_follow_up(struct port *p, struct ptp_message *m) From 8d1b30d91cb7331f42e1796b3d7fe24b5036f057 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 29 Aug 2015 10:31:14 +0200 Subject: [PATCH 3/6] udp: set the destination port unconditionally. Even if the caller provides the destination address, still the port must depend on the passed 'event' value for correct delivery. Signed-off-by: Richard Cochran --- udp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/udp.c b/udp.c index b8aa76a..48d18d8 100644 --- a/udp.c +++ b/udp.c @@ -218,12 +218,12 @@ static int udp_send(struct transport *t, struct fdarray *fda, int event, addr_buf.sin.sin_family = AF_INET; addr_buf.sin.sin_addr = peer ? mcast_addr[MC_PDELAY] : mcast_addr[MC_PRIMARY]; - addr_buf.sin.sin_port = htons(event ? EVENT_PORT : - GENERAL_PORT); addr_buf.len = sizeof(addr_buf.sin); addr = &addr_buf; } + addr->sin.sin_port = htons(event ? EVENT_PORT : GENERAL_PORT); + /* * Extend the payload by two, for UDP checksum correction. * This is not really part of the standard, but it is the way From 1e35b91c6a05730e9e88d1009d59362113cd128f Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 29 Aug 2015 10:31:39 +0200 Subject: [PATCH 4/6] udp6: set the destination port unconditionally. Even if the caller provides the destination address, still the port must depend on the passed 'event' value for correct delivery. Signed-off-by: Richard Cochran --- udp6.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/udp6.c b/udp6.c index fdf5799..10ff166 100644 --- a/udp6.c +++ b/udp6.c @@ -223,9 +223,6 @@ static int udp6_send(struct transport *t, struct fdarray *fda, int event, addr_buf.sin6.sin6_family = AF_INET6; addr_buf.sin6.sin6_addr = peer ? mc6_addr[MC_PDELAY] : mc6_addr[MC_PRIMARY]; - addr_buf.sin6.sin6_port = htons(event ? EVENT_PORT : - GENERAL_PORT); - if (is_link_local(&addr_buf.sin6.sin6_addr)) addr_buf.sin6.sin6_scope_id = udp6->index; @@ -233,6 +230,8 @@ static int udp6_send(struct transport *t, struct fdarray *fda, int event, addr = &addr_buf; } + addr->sin6.sin6_port = htons(event ? EVENT_PORT : GENERAL_PORT); + len += 2; /* Extend the payload by two, for UDP checksum corrections. */ cnt = sendto(fd, buf, len, 0, &addr->sa, sizeof(addr->sin6)); From d0eb73c87b389bee54d3dd215694ac4438f1564a Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 29 Aug 2015 10:28:51 +0200 Subject: [PATCH 5/6] Use the standardized low level socket address format. The raw Ethernet transport code invented its own way of storing the MAC address into our "struct address" data structure. However, this private format is incompatible with the sockaddr_ll returned from the networking stack. This patch converts the code to use the proper format. Signed-off-by: Richard Cochran --- address.h | 2 ++ raw.c | 9 +++++---- sk.c | 6 ++++-- udp.c | 2 +- udp6.c | 2 +- util.c | 12 ++++++------ 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/address.h b/address.h index b780387..7578f91 100644 --- a/address.h +++ b/address.h @@ -21,6 +21,7 @@ #define HAVE_ADDRESS_H #include +#include #include #include @@ -28,6 +29,7 @@ struct address { socklen_t len; union { struct sockaddr_storage ss; + struct sockaddr_ll sll; struct sockaddr_in sin; struct sockaddr_in6 sin6; struct sockaddr_un sun; diff --git a/raw.c b/raw.c index 48c43b0..f51c829 100644 --- a/raw.c +++ b/raw.c @@ -188,14 +188,15 @@ no_socket: static void mac_to_addr(struct address *addr, void *mac) { - addr->sa.sa_family = AF_UNSPEC; - memcpy(&addr->sa.sa_data, mac, MAC_LEN); - addr->len = sizeof(addr->sa.sa_family) + MAC_LEN; + addr->sll.sll_family = AF_PACKET; + addr->sll.sll_halen = MAC_LEN; + memcpy(addr->sll.sll_addr, mac, MAC_LEN); + addr->len = sizeof(addr->sll); } static void addr_to_mac(void *mac, struct address *addr) { - memcpy(mac, &addr->sa.sa_data, MAC_LEN); + memcpy(mac, &addr->sll.sll_addr, MAC_LEN); } static int raw_open(struct transport *t, const char *name, diff --git a/sk.c b/sk.c index 847855a..e80f608 100644 --- a/sk.c +++ b/sk.c @@ -170,8 +170,10 @@ int sk_interface_macaddr(const char *name, struct address *mac) return -1; } - memcpy(&mac->sa, &ifreq.ifr_hwaddr, sizeof(ifreq.ifr_hwaddr)); - mac->len = sizeof(ifreq.ifr_hwaddr.sa_family) + MAC_LEN; + mac->sll.sll_family = AF_PACKET; + mac->sll.sll_halen = MAC_LEN; + memcpy(mac->sll.sll_addr, &ifreq.ifr_hwaddr.sa_data, MAC_LEN); + mac->len = sizeof(mac->sll); close(fd); return 0; } diff --git a/udp.c b/udp.c index 48d18d8..07277c7 100644 --- a/udp.c +++ b/udp.c @@ -256,7 +256,7 @@ static int udp_physical_addr(struct transport *t, uint8_t *addr) if (udp->mac.len) { len = MAC_LEN; - memcpy(addr, udp->mac.sa.sa_data, len); + memcpy(addr, udp->mac.sll.sll_addr, len); } return len; } diff --git a/udp6.c b/udp6.c index 10ff166..e6f3769 100644 --- a/udp6.c +++ b/udp6.c @@ -258,7 +258,7 @@ static int udp6_physical_addr(struct transport *t, uint8_t *addr) if (udp6->mac.len) { len = MAC_LEN; - memcpy(addr, udp6->mac.sa.sa_data, len); + memcpy(addr, udp6->mac.sll.sll_addr, len); } return len; } diff --git a/util.c b/util.c index 9202c55..4e74478 100644 --- a/util.c +++ b/util.c @@ -134,14 +134,14 @@ int generate_clock_identity(struct ClockIdentity *ci, const char *name) if (sk_interface_macaddr(name, &addr)) return -1; - ci->id[0] = addr.sa.sa_data[0]; - ci->id[1] = addr.sa.sa_data[1]; - ci->id[2] = addr.sa.sa_data[2]; + ci->id[0] = addr.sll.sll_addr[0]; + ci->id[1] = addr.sll.sll_addr[1]; + ci->id[2] = addr.sll.sll_addr[2]; ci->id[3] = 0xFF; ci->id[4] = 0xFE; - ci->id[5] = addr.sa.sa_data[3]; - ci->id[6] = addr.sa.sa_data[4]; - ci->id[7] = addr.sa.sa_data[5]; + ci->id[5] = addr.sll.sll_addr[3]; + ci->id[6] = addr.sll.sll_addr[4]; + ci->id[7] = addr.sll.sll_addr[5]; return 0; } From e85cb68320a36efb03aae97a536e1d7e9c75aefe Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Fri, 28 Aug 2015 20:41:50 +0200 Subject: [PATCH 6/6] Support hybrid E2E using unicast delay requests and responses. The draft Enterprise Profile [1] specifies a hybrid E2E delay mechanism, where the delay response message is sent "in kind". That is, if the request is unicast, then the response is also unicast. Apparently this scheme is already in widespread use in some industries. Also, it makes sense, because those messages are of no interest to the other slaves in the PTP network. Because of the address work already in place, in turns out that adding this mode is almost trivial. This patch introduces an "hybrid_e2e" option that enabled the new mode. 1. https://datatracker.ietf.org/doc/draft-ietf-tictoc-ptp-enterprise-profile Signed-off-by: Richard Cochran --- config.c | 1 + default.cfg | 1 + gPTP.cfg | 1 + port.c | 28 +++++++++++++++++++++++++++- ptp4l.8 | 9 +++++++++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 9fa2ecb..d462410 100644 --- a/config.c +++ b/config.c @@ -189,6 +189,7 @@ struct config_item config_tab[] = { GLOB_ITEM_INT("free_running", 0, 0, 1), PORT_ITEM_INT("freq_est_interval", 1, 0, INT_MAX), GLOB_ITEM_INT("gmCapable", 1, 0, 1), + PORT_ITEM_INT("hybrid_e2e", 0, 0, 1), PORT_ITEM_INT("ingressLatency", 0, INT_MIN, INT_MAX), GLOB_ITEM_INT("kernel_leap", 1, 0, 1), PORT_ITEM_INT("logAnnounceInterval", 1, INT8_MIN, INT8_MAX), diff --git a/default.cfg b/default.cfg index b46c0f6..d398d7c 100644 --- a/default.cfg +++ b/default.cfg @@ -31,6 +31,7 @@ assume_two_step 0 logging_level 6 path_trace_enabled 0 follow_up_info 0 +hybrid_e2e 0 tx_timestamp_timeout 1 use_syslog 1 verbose 0 diff --git a/gPTP.cfg b/gPTP.cfg index 34fa238..75e996c 100644 --- a/gPTP.cfg +++ b/gPTP.cfg @@ -31,6 +31,7 @@ assume_two_step 1 logging_level 6 path_trace_enabled 1 follow_up_info 1 +hybrid_e2e 0 tx_timestamp_timeout 1 use_syslog 1 verbose 0 diff --git a/port.c b/port.c index 91bb4e9..9c804cf 100644 --- a/port.c +++ b/port.c @@ -111,6 +111,7 @@ struct port { UInteger32 neighborPropDelayThresh; int follow_up_info; int freq_est_interval; + int hybrid_e2e; int min_neighbor_prop_delay; int path_trace_enabled; int rx_timestamp_offset; @@ -1223,6 +1224,12 @@ static int port_delay_request(struct port *p) msg->header.control = CTL_DELAY_REQ; msg->header.logMessageInterval = 0x7f; + if (p->hybrid_e2e) { + struct ptp_message *dst = TAILQ_FIRST(&p->best->messages); + msg->address = dst->address; + msg->header.flagField[0] |= UNICAST; + } + if (port_prepare_and_send(p, msg, 1)) { pr_err("port %hu: send delay request failed", portnum(p)); goto out; @@ -1630,6 +1637,12 @@ static int process_delay_req(struct port *p, struct ptp_message *m) msg->delay_resp.requestingPortIdentity = m->header.sourcePortIdentity; + if (p->hybrid_e2e && m->header.flagField[0] & UNICAST) { + msg->address = m->address; + msg->header.flagField[0] |= UNICAST; + msg->header.logMessageInterval = 0x7f; + } + err = port_prepare_and_send(p, msg, 0); if (err) pr_err("port %hu: send delay response failed", portnum(p)); @@ -1669,6 +1682,10 @@ static void process_delay_resp(struct port *p, struct ptp_message *m) if (p->logMinDelayReqInterval == rsp->hdr.logMessageInterval) { return; } + if (m->header.flagField[0] & UNICAST) { + /* Unicast responses have logMinDelayReqInterval set to 0x7F. */ + return; + } if (rsp->hdr.logMessageInterval < -10 || rsp->hdr.logMessageInterval > 22) { pr_debug("port %hu: ignore bogus delay request interval 2^%d", @@ -2319,7 +2336,11 @@ int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event) if (msg_pre_send(msg)) return -1; - cnt = transport_send(p->trp, &p->fda, event, msg); + if (msg->header.flagField[0] & UNICAST) { + cnt = transport_sendto(p->trp, &p->fda, event, msg); + } else { + cnt = transport_send(p->trp, &p->fda, event, msg); + } if (cnt <= 0) { return -1; } @@ -2550,6 +2571,7 @@ struct port *port_open(int phc_index, p->asymmetry <<= 16; p->follow_up_info = config_get_int(cfg, p->name, "follow_up_info"); p->freq_est_interval = config_get_int(cfg, p->name, "freq_est_interval"); + p->hybrid_e2e = config_get_int(cfg, p->name, "hybrid_e2e"); p->path_trace_enabled = config_get_int(cfg, p->name, "path_trace_enabled"); p->rx_timestamp_offset = config_get_int(cfg, p->name, "ingressLatency"); p->tx_timestamp_offset = config_get_int(cfg, p->name, "egressLatency"); @@ -2564,6 +2586,10 @@ struct port *port_open(int phc_index, p->delayMechanism = config_get_int(cfg, p->name, "delay_mechanism"); p->versionNumber = PTP_VERSION; + if (p->hybrid_e2e && p->delayMechanism != DM_E2E) { + pr_warning("port %d: hybrid_e2e only works with E2E", number); + } + /* Set fault timeouts to a default value */ for (i = 0; i < FT_CNT; i++) { p->flt_interval_pertype[i].type = FTMO_LOG2_SECONDS; diff --git a/ptp4l.8 b/ptp4l.8 index 14830c5..6eae40f 100644 --- a/ptp4l.8 +++ b/ptp4l.8 @@ -185,6 +185,15 @@ The default is 16 seconds. Select the delay mechanism. Possible values are E2E, P2P and Auto. The default is E2E. .TP +.B hybrid_e2e +Enables the "hybrid" delay mechanism from the draft Enterprise +Profile. When enabled, ports in the slave state send their delay +request messages to the unicast address taken from the master's +announce message. Ports in the master state will reply to unicast +delay requests using unicast delay responses. This option has no +effect if the delay_mechanism is set to P2P. +The default is 0 (disabled). +.TP .B ptp_dst_mac The MAC address to which PTP messages should be sent. Relevant only with L2 transport. The default is 01:1B:19:00:00:00.