Discussion:
[Linuxptp-devel] [PATCH RFC v1 0/4] Hybrid "Enterprise" E2E Mode
Richard Cochran
2015-08-29 09:44:21 UTC
Permalink
This series add support for the so-called hybrid mode, letting the
delay request/response messages be unicast while keeping the other
messages multicast.

The code works over all three transports, L2, UDPv4, and UDPv6,
although I did not test UDPv6.

This series requires two other recent patches:

port: constrain the master's logMinDelayReqInterval.
Fix integer overflow in the foreign master bookkeeping code.

Review, testing, and comments are welcome!

Thanks,
Richard


Richard Cochran (4):
udp: set the destination port unconditionally.
udp6: set the destination port unconditionally.
Use the standardized low level socket address format.
Support hybrid E2E using unicast delay requests and responses.

address.h | 2 ++
config.c | 1 +
default.cfg | 1 +
gPTP.cfg | 1 +
port.c | 28 +++++++++++++++++++++++++++-
ptp4l.8 | 9 +++++++++
raw.c | 9 +++++----
sk.c | 6 ++++--
udp.c | 6 +++---
udp6.c | 7 +++----
util.c | 12 ++++++------
11 files changed, 62 insertions(+), 20 deletions(-)
--
2.1.4


------------------------------------------------------------------------------
Richard Cochran
2015-08-29 09:44:22 UTC
Permalink
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 <***@gmail.com>
---
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
--
2.1.4


------------------------------------------------------------------------------
Richard Cochran
2015-08-29 09:44:24 UTC
Permalink
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 <***@gmail.com>
---
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 <netinet/in.h>
+#include <netpacket/packet.h>
#include <sys/socket.h>
#include <sys/un.h>

@@ -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;
}
--
2.1.4


------------------------------------------------------------------------------
Richard Cochran
2015-08-29 09:44:23 UTC
Permalink
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 <***@gmail.com>
---
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));
--
2.1.4


------------------------------------------------------------------------------
Richard Cochran
2015-08-29 09:44:25 UTC
Permalink
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 <***@gmail.com>
---
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.
--
2.1.4


------------------------------------------------------------------------------
Richard Cochran
2015-10-29 15:23:03 UTC
Permalink
Hello, does it make sense to back port this series of code changes from linuxptp version 1.6 into my local copy of version 1.4? The Hybrid Enterprise E2E Mode is what I need the most in release 1.6 and that would help reduce integration time.
Sure, go ahead. Only the config stuff is different, I think.

Thanks,
Richard

------------------------------------------------------------------------------
Loading...