On Wed, Nov 25, 2020 at 08:36:59AM +0100, Volodymyr Bendiuga wrote:
> Our observations so far:
> OC-master sends SYNC packet and it reaches TC;
> TC identifies it as 1-step SYNC, converts it to 2-step SYNC and
> creates a Followup message, which is backed by the following lines,
> excerpted from tc.c:tc_fwd_sync():
> TC goes forth and forwards newly converted 2-step SYNC to OC-slave
> TC then proceeds and tries to fetch a timestamp for our 2-step SYNC
> message and is nicely presented with error code from kernel. No
> surprise, hardware operates in 1-step mode, so no timestamp is
> available in socket error queue)

Nice investigation and story, however one thing remains open-ended,
which is: did you try the obvious? does it behave properly if you do?

A totally untested patch below, given that I don't have one-step
hardware.

>From 383c6a69f414b5a274c80f0699f54ea686ae15a6 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <olte...@gmail.com>
Date: Wed, 25 Nov 2020 11:54:13 +0200
Subject: [PATCH] tc: forward one-step sync messages as one-step if port
 supports it

IEEE 1588 defines the operation of a transparent clock as follows:

11.5.2.1 One-step transparent clocks
The <residenceTime> shall be added to the correctionField of the Sync
event message by the egress port of the clock as the Sync event message
is being transmitted. The egress port shall make any needed corrections
to checksums or other content dependent fields of the message.

11.5.2.2 Two-step transparent clocks
If the twoStepFlag of the received Sync message is FALSE indicating that
no Follow_Up message will be received, then:
(a) The twoStepFlag of the received Sync message shall be set to TRUE to
    indicate that a Follow_Up message will follow. The Sync message
    should be transmitted on the egress port as soon as possible, with
    any needed corrections to checksums or other content-dependent
    fields of the message. This modified Sync message shall be used to
    generate the egress timestamp for the computation of residence time
    for the Sync message.
(b) A Follow_Up message shall be prepared for transmission on the egress
    port as follows:
    (1) The originTimestamp of the received Sync message shall be copied
        into the preciseOriginTimestamp field of the Follow_Up message.
    (2) The sequenceId of the received Sync message shall be copied into
        the sequenceId of the Follow_Up message.
    (3) The sourcePortIdentity of the received Sync message shall be
        copied to the sourcePortIdentity of the Follow_Up message.
    (4) The domainNumber of the received Sync message shall be copied to
        the domainNumber of the Follow_Up message.
    (5) The logMessageInterval of the received Sync message shall be
        copied to the logMessageInterval of the Follow_Up message.
    (6) The header flagField of the received Sync message shall be
        copied to the flagField of the Follow_Up message but with the
        twoStepFlag set to TRUE.
    (7) The correctionField of the Follow_Up message shall be set to the
        <residenceTime>. For peer-to- peer transparent clocks, this
        shall be done prior to any corrections for <meanPathDelay>; see
        11.4.5.1.

Obviously ptp4l currently only implements the requirements of a two-step
transparent clock as per 11.5.2.2. Let's add some logic for ptp4l to
pass the sync message to the egress port in a way that is compatible
with the semantics of a one-step TC.

Signed-off-by: Vladimir Oltean <olte...@gmail.com>
---
 port.c |  5 +++++
 port.h |  2 ++
 tc.c   | 54 +++++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/port.c b/port.c
index c8d19aad97af..ee2dcc5aab93 100644
--- a/port.c
+++ b/port.c
@@ -3330,3 +3330,8 @@ enum bmca_select port_bmca(struct port *p)
 {
        return p->bmca;
 }
+
+int port_uses_onestep_sync(struct port *p)
+{
+       return p->timestamping == TS_ONESTEP || p->timestamping == TS_P2P1STEP;
+}
diff --git a/port.h b/port.h
index 0b07d5548904..efa58c7488f7 100644
--- a/port.h
+++ b/port.h
@@ -343,4 +343,6 @@ enum bmca_select port_bmca(struct port *p);
  */
 void tc_cleanup(void);
 
+int port_uses_onestep_sync(struct port *p);
+
 #endif
diff --git a/tc.c b/tc.c
index fb466031a7ca..6c94380c3e54 100644
--- a/tc.c
+++ b/tc.c
@@ -446,20 +446,48 @@ int tc_fwd_sync(struct port *q, struct ptp_message *msg)
        int err;
 
        if (one_step(msg)) {
-               fup = msg_allocate();
-               if (!fup) {
-                       return -1;
+               if (port_uses_onestep_sync(q)) {
+                       tmv_t origin = timestamp_to_tmv(msg->ts.pdu);
+                       tmv_t ingress = msg->hwts.ts;
+                       Integer64 c;
+
+                       /* The residence time (egress - ingress ts) needs
+                        * to be put in the correctionField of the Sync.
+                        * We know that the kernel convention for one-step sync
+                        * timestamping is that the driver/hardware will
+                        * already update the correctionField with the delta
+                        * between the originTimestamp and the actual hardware
+                        * TX timestamp.
+                        * So update the correctionField with the residence
+                        * time in two parts: ptp4l will first subtract the
+                        * ingress hardware timestamp relative to the
+                        * originTimestamp, then the driver will add the egress
+                        * timestamp (again relative to the originTimestamp) as
+                        * part of its normal operation of sending a Sync
+                        * message.
+                        */
+                       c = net2host64(msg->header.correction);
+                       c -= tmv_to_TimeInterval(tmv_sub(ingress, origin));
+                       msg->header.correction = host2net64(c);
+               } else {
+                       struct ptp_header *hdr;
+
+                       fup = msg_allocate();
+                       if (!fup) {
+                               return -1;
+                       }
+                       hdr = &fup->header;
+                       hdr->tsmt = FOLLOW_UP | (msg->header.tsmt & 0xf0);
+                       hdr->ver = msg->header.ver;
+                       hdr->messageLength = sizeof(struct follow_up_msg);
+                       hdr->domainNumber = msg->header.domainNumber;
+                       hdr->sourcePortIdentity = 
msg->header.sourcePortIdentity;
+                       hdr->sequenceId = msg->header.sequenceId;
+                       hdr->control = CTL_FOLLOW_UP;
+                       hdr->logMessageInterval = 
msg->header.logMessageInterval;
+                       fup->follow_up.preciseOriginTimestamp = 
msg->sync.originTimestamp;
+                       msg->header.flagField[0] |= TWO_STEP;
                }
-               fup->header.tsmt               = FOLLOW_UP | (msg->header.tsmt 
& 0xf0);
-               fup->header.ver                = msg->header.ver;
-               fup->header.messageLength      = sizeof(struct follow_up_msg);
-               fup->header.domainNumber       = msg->header.domainNumber;
-               fup->header.sourcePortIdentity = msg->header.sourcePortIdentity;
-               fup->header.sequenceId         = msg->header.sequenceId;
-               fup->header.control            = CTL_FOLLOW_UP;
-               fup->header.logMessageInterval = msg->header.logMessageInterval;
-               fup->follow_up.preciseOriginTimestamp = 
msg->sync.originTimestamp;
-               msg->header.flagField[0]      |= TWO_STEP;
        }
        err = tc_fwd_event(q, msg);
        if (err) {
-- 
2.25.1


_______________________________________________
Linuxptp-users mailing list
Linuxptp-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-users

Reply via email to