linuxptp-devel — Development channel for the linuxptp project

[Linuxptp-devel] The List Has Moved

[Richard C. <ric...@gm...>]

Dear LinuxPTP developers,

New posts to this list has been disabled.  If you want to continue to
participate on the mailing lists, please subscribe to the new list at
the Network Time Foundation.

https://lists.nwtime.org/sympa/info/linuxptp-devel

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Richard C. <ric...@gm...>]

On Wed, Dec 06, 2023 at 03:43:15PM +0100, Andrew Zaborowski wrote:
> Again feel free to include it or leave it for us to send separately.

I prefer to roll your fixes into this series.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Andrew Z. <and...@in...>]

On Tue, 5 Dec 2023 at 02:44, Andrew Zaborowski
<and...@in...> wrote:
>  * if I stop a CMLDS client and rerun it, it will receive the CMLDS
> notifications twice until the old subscription expires.  If I restart
> it again quickly enough it'll receive each notification 3 times.  This
> is because pmc_recv doesn't filter by .targetPortIdentity.  It could
> but it wouldn't be bullet proof anyway because the PMC port's identity
> is based on the pid, which is not unique, so I'm not sure if it's
> worth doing.

On a second thought, if the pid ends up being reused, when the client
re-subscribes to the CMLDS notifications, the server will update the
existing subscription so duplicate notifications wouldn't be sent.  So
at least this specific use case would be covered.

Additionally we found we needed to exit early from port_capable() so
as not to overwrite the p->asCapable value set by process_cmlds().
Again feel free to include it or leave it for us to send separately.

@@ -700,6 +700,11 @@ int port_capable(struct port *p)
         goto capable;
     }

+    if (p->delayMechanism == DM_COMMON_P2P) {
+        /* asCapable is calculated by the CMLDS. */
+        return p->asCapable != NOT_CAPABLE ? 1 : 0;
+    }
+
     if (tmv_to_nanoseconds(p->peer_delay) >    p->neighborPropDelayThresh) {
         if (p->asCapable)
             pr_debug("%s: peer_delay (%" PRId64 ") > neighborPropDelayThresh "

Best regards

Re: [Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

[Erez <ere...@gm...>]

On Tue, 5 Dec 2023 at 08:59, Miroslav Lichvar <mli...@re...> wrote:

> On Mon, Dec 04, 2023 at 11:25:00PM +0100, Andrew Zaborowski wrote:
> > On Sun, 3 Dec 2023 at 00:39, Richard Cochran <ric...@gm...>
> wrote:
> > > @@ -306,13 +307,15 @@ static void do_set_action(struct pmc *pmc, int
> action, int index, char *str)
> > >                              "duration          %hu "
> > >                              "NOTIFY_PORT_STATE %3s "
> > >                              "NOTIFY_TIME_SYNC  %3s "
> > > -                            "NOTIFY_PARENT_DATA_SET %3s ",
> > > +                            "NOTIFY_PARENT_DATA_SET %3s "
> > > +                            "NOTIFY_CMLDS %3s ",
> > >                              &sen.duration,
> > >                              onoff_port_state,
> > >                              onoff_time_status,
> > > -                            onoff_parent_data_set);
> > > -               if (cnt != 4) {
> > > -                       fprintf(stderr, "%s SET needs 4 values\n",
> > > +                            onoff_parent_data_set,
> > > +                            onoff_cmlds);
> > > +               if (cnt != 5) {
> > > +                       fprintf(stderr, "%s SET needs 5 values\n",
> >
> > Doing cnt != 4 && cnt != 5 should just work as sscanf() should stop
> > parsing when it doesn't find NOTIFY_CMLDS and leave onoff_cmlds as
> > "off".  Obviously this won't scale as more events are added but it'd
> > keep syntax backwards-compatible for scripts.
>
> Good idea. The PARENT_DATA_SET notification is new too (added after
> 4.1) and I guess there is no reason to require even the original two
> notifications, so change that to "cnt < 1" with "%s SET needs at least
> 1 value", requiring only the duration, which would effectively reset
> all existing notifications?
>

+


>
> --
> Miroslav Lichvar
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Richard C. <ric...@gm...>]

On Tue, Dec 05, 2023 at 02:44:18AM +0100, Andrew Zaborowski wrote:

>  * if I stop a CMLDS client and rerun it, it will receive the CMLDS
> notifications twice until the old subscription expires.  If I restart
> it again quickly enough it'll receive each notification 3 times.  This
> is because pmc_recv doesn't filter by .targetPortIdentity.  It could
> but it wouldn't be bullet proof anyway because the PMC port's identity
> is based on the pid, which is not unique, so I'm not sure if it's
> worth doing.

Yeah, super annoying.  Maybe the cure is to first cancel push
notifications unconditionally?
 
>  * inhibit_delay_req / logMinPdelayReqInterval /
> operLogPdelayReqInterval should not be used in the CMLDS client config
> (which wouldn't be useful anyway) because they'll affect the
> subscription renewals.

Right, a check should be added to avoid this combination.
 
>  * I had two extra changes, one to always calculate the NRR in the
> CMLDS port,

Can't this be taken from the Link Port?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Richard C. <ric...@gm...>]

On Mon, Dec 04, 2023 at 10:51:15PM +0100, Andrew Zaborowski wrote:

> I assume ignore_transport_specific can be dropped in this version.

Actually it is critical, if you care about being able to
inter-operate.  After all, nothing guarantees that the link partner is
also running CMLDS.  Experience teaches us that the very first thing
users will do is the thing we think they shouldn't.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

[Richard C. <ric...@gm...>]

On Tue, Dec 05, 2023 at 08:57:36AM +0100, Miroslav Lichvar wrote:
> On Mon, Dec 04, 2023 at 11:25:00PM +0100, Andrew Zaborowski wrote:

> > Doing cnt != 4 && cnt != 5 should just work as sscanf() should stop
> > parsing when it doesn't find NOTIFY_CMLDS and leave onoff_cmlds as
> > "off".  Obviously this won't scale as more events are added but it'd
> > keep syntax backwards-compatible for scripts.
> 
> Good idea. The PARENT_DATA_SET notification is new too (added after
> 4.1) and I guess there is no reason to require even the original two
> notifications, so change that to "cnt < 1" with "%s SET needs at least
> 1 value", requiring only the duration, which would effectively reset
> all existing notifications?

Makes sense to me.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

[Miroslav L. <mli...@re...>]

On Mon, Dec 04, 2023 at 11:25:00PM +0100, Andrew Zaborowski wrote:
> On Sun, 3 Dec 2023 at 00:39, Richard Cochran <ric...@gm...> wrote:
> > @@ -306,13 +307,15 @@ static void do_set_action(struct pmc *pmc, int action, int index, char *str)
> >                              "duration          %hu "
> >                              "NOTIFY_PORT_STATE %3s "
> >                              "NOTIFY_TIME_SYNC  %3s "
> > -                            "NOTIFY_PARENT_DATA_SET %3s ",
> > +                            "NOTIFY_PARENT_DATA_SET %3s "
> > +                            "NOTIFY_CMLDS %3s ",
> >                              &sen.duration,
> >                              onoff_port_state,
> >                              onoff_time_status,
> > -                            onoff_parent_data_set);
> > -               if (cnt != 4) {
> > -                       fprintf(stderr, "%s SET needs 4 values\n",
> > +                            onoff_parent_data_set,
> > +                            onoff_cmlds);
> > +               if (cnt != 5) {
> > +                       fprintf(stderr, "%s SET needs 5 values\n",
> 
> Doing cnt != 4 && cnt != 5 should just work as sscanf() should stop
> parsing when it doesn't find NOTIFY_CMLDS and leave onoff_cmlds as
> "off".  Obviously this won't scale as more events are added but it'd
> keep syntax backwards-compatible for scripts.

Good idea. The PARENT_DATA_SET notification is new too (added after
4.1) and I guess there is no reason to require even the original two
notifications, so change that to "cnt < 1" with "%s SET needs at least
1 value", requiring only the duration, which would effectively reset
all existing notifications?

-- 
Miroslav Lichvar

Re: [Linuxptp-devel] [PATCH v1 4/4] phc_ctl: Use util.h NSEC_PER_SEC macro instead of local macro

[Rahul R. <rra...@nv...>]

On Sat, 02 Dec, 2023 13:56:50 -0800 Richard Cochran <ric...@gm...> wrote:
> On Wed, Nov 22, 2023 at 09:36:36AM -0800, Rahul Rameshbabu via Linuxptp-devel wrote:
>> Use the common NSEC_PER_SEC macro in phc_ctl.
>> 
>> Signed-off-by: Rahul Rameshbabu <rra...@nv...>
>
> Series applied, but I fixed up one more NSEC2SEC that was added into
> phc_ctl.c since this series was posted.
>

Thanks for doing that. Will try to be more careful going forward to
account for the latest state of the tree when needing to send out
revisions to the list.

--
Thanks,

Rahul Rameshbabu

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Andrew Z. <and...@in...>]

I tested a basic 802.1AS type scenario with this v3 patchset on both
ends, and it worked!  We may have more test results in a few days if
my colleague runs a real test suite.

The only caveats were:
 * if I stop a CMLDS client and rerun it, it will receive the CMLDS
notifications twice until the old subscription expires.  If I restart
it again quickly enough it'll receive each notification 3 times.  This
is because pmc_recv doesn't filter by .targetPortIdentity.  It could
but it wouldn't be bullet proof anyway because the PMC port's identity
is based on the pid, which is not unique, so I'm not sure if it's
worth doing.

 * inhibit_delay_req / logMinPdelayReqInterval /
operLogPdelayReqInterval should not be used in the CMLDS client config
(which wouldn't be useful anyway) because they'll affect the
subscription renewals.

 * I had two extra changes, one to always calculate the NRR in the
CMLDS port, and one to handle an exception specified in 802.1AS-2020
section 11.2.17.1 above NOTE 3 -- this can be added later separately
from this CMLDS patchset

Best regards

 configs/802.1as-2020-master-cmlds.cfg    | 19 ++++++++++
 configs/802.1as-2020-master-domain-0.cfg | 43 ++++++++++++++++++++++
 configs/802.1as-2020-slave-cmlds.cfg     | 19 ++++++++++
 configs/802.1as-2020-slave-domain-0.cfg  | 45 ++++++++++++++++++++++++
 msg.h                                    |  3 +-
 port.c                                   |  7 ++--
 6 files changed, 130 insertions(+), 6 deletions(-)
 create mode 100644 configs/802.1as-2020-master-cmlds.cfg
 create mode 100644 configs/802.1as-2020-master-domain-0.cfg
 create mode 100644 configs/802.1as-2020-slave-cmlds.cfg
 create mode 100644 configs/802.1as-2020-slave-domain-0.cfg

diff --git a/configs/802.1as-2020-master-cmlds.cfg
b/configs/802.1as-2020-master-cmlds.cfg
new file mode 100644
index 0000000..2e09c6d
--- /dev/null
+++ b/configs/802.1as-2020-master-cmlds.cfg
@@ -0,0 +1,19 @@
+#
+# Common Mean Link Delay Service (CMLDS) example configuration,
+# containing those attributes which differ from the defaults.
+# See the file default.cfg for the complete list of available options.
+#
+[global]
+# Set this for CMLDS regardless of actual port roles on this node
+clientOnly            1
+free_running            1
+assume_two_step            1
+follow_up_info            1
+asCapable            true
+network_transport        L2
+uds_address            /var/run/master-cmlds
+
+transportSpecific        2
+clockIdentity            000001.0000.800000
+
+delay_mechanism            P2P
diff --git a/configs/802.1as-2020-master-domain-0.cfg
b/configs/802.1as-2020-master-domain-0.cfg
new file mode 100644
index 0000000..0e69565
--- /dev/null
+++ b/configs/802.1as-2020-master-domain-0.cfg
@@ -0,0 +1,43 @@
+#
+# 802.1AS configuration example for a single domain out of many (or not
+# so many) on a multidomain-capable node.  Uses a local CMLDS instance,
+# see configs/802.1as-2020-master-cmlds.cfg.
+#
+[global]
+gmCapable            1
+priority1            248
+priority2            248
+logAnnounceInterval        0
+logSyncInterval            -3
+syncReceiptTimeout        3
+neighborPropDelayThresh        800
+min_neighbor_prop_delay        -20000000
+assume_two_step            1
+path_trace_enabled        1
+follow_up_info            1
+asCapable            true
+network_transport        L2
+BMCA                noop
+# Required to quickly correct Time Jumps in master
+step_threshold            1
+inhibit_announce        1
+operLogSyncInterval        0
+operLogPdelayReqInterval    2
+msg_interval_request        1
+servo_offset_threshold        30
+servo_num_offset_values        10
+
+# Master only bits
+serverOnly            1
+inhibit_announce        1
+
+uds_address            /var/run/master-domain-0
+cmlds.server_address        /var/run/master-cmlds
+# FIXME: must set client_address per port
+cmlds.client_address        /var/run/master-client-0
+
+domainNumber            0
+transportSpecific        1
+clockIdentity            000001.0000.000000
+
+delay_mechanism            COMMON_P2P
diff --git a/configs/802.1as-2020-slave-cmlds.cfg
b/configs/802.1as-2020-slave-cmlds.cfg
new file mode 100644
index 0000000..e78e468
--- /dev/null
+++ b/configs/802.1as-2020-slave-cmlds.cfg
@@ -0,0 +1,19 @@
+#
+# Common Mean Link Delay Service (CMLDS) example configuration,
+# containing those attributes which differ from the defaults.
+# See the file default.cfg for the complete list of available options.
+#
+[global]
+# Set this for CMLDS regardless of actual port roles on this node
+clientOnly            1
+free_running            1
+assume_two_step            1
+follow_up_info            1
+asCapable            true
+network_transport        L2
+uds_address            /var/run/slave-cmlds
+
+transportSpecific        2
+clockIdentity            000002.0000.800000
+
+delay_mechanism            P2P
diff --git a/configs/802.1as-2020-slave-domain-0.cfg
b/configs/802.1as-2020-slave-domain-0.cfg
new file mode 100644
index 0000000..8656e60
--- /dev/null
+++ b/configs/802.1as-2020-slave-domain-0.cfg
@@ -0,0 +1,45 @@
+#
+# 802.1AS configuration example for a single domain out of many (or not
+# so many) on a multidomain-capable node.  Uses a local CMLDS instance,
+# see configs/802.1as-2020-slave-cmlds.cfg.
+#
+[global]
+gmCapable            1
+priority1            248
+priority2            248
+logAnnounceInterval        0
+logSyncInterval            -3
+syncReceiptTimeout        3
+neighborPropDelayThresh        800
+min_neighbor_prop_delay        -20000000
+assume_two_step            1
+path_trace_enabled        1
+follow_up_info            1
+asCapable            true
+network_transport        L2
+BMCA                noop
+# Required to quickly correct Time Jumps in master
+step_threshold            1
+inhibit_announce        1
+operLogSyncInterval        0
+operLogPdelayReqInterval    2
+msg_interval_request        1
+servo_offset_threshold        30
+servo_num_offset_values        10
+
+# Slave only bits
+clientOnly            1
+ignore_source_id        1
+servo_offset_threshold        30
+servo_num_offset_values        10
+
+uds_address            /var/run/slave-domain-0
+cmlds.server_address        /var/run/slave-cmlds
+# FIXME: must set client_address per port
+cmlds.client_address        /var/run/slave-client-0
+
+domainNumber            0
+transportSpecific        1
+clockIdentity            000002.0000.000000
+
+delay_mechanism            COMMON_P2P
diff --git a/msg.h b/msg.h
index 9c80f45..d4294ae 100644
--- a/msg.h
+++ b/msg.h
@@ -38,7 +38,8 @@
 #define MAJOR_VERSION_MASK    0x0f

 /* Values for the transportSpecific field */
-#define TS_IEEE_8021AS (1<<4)
+#define TS_IEEE_8021AS    (1<<4)
+#define TS_CMLDS    (2<<4)

 /* Values for the messageType field */
 #define SYNC                  0x0
diff --git a/port.c b/port.c
index e2ebaeb..c5c31d8 100644
--- a/port.c
+++ b/port.c
@@ -2424,9 +2424,6 @@ int process_pdelay_req(struct port *p, struct
ptp_message *m)
         return -1;
     }

-    if (p->delayMechanism == DM_COMMON_P2P) {
-        return 0;
-    }
     if (p->delayMechanism == DM_E2E) {
         pr_warning("%s: pdelay_req on E2E port", p->log_name);
         return 0;
@@ -2445,7 +2442,7 @@ int process_pdelay_req(struct port *p, struct
ptp_message *m)
             p->peer_portid_valid = 0;
             port_capable(p);
         }
-    } else {
+    } else if (p->delayMechanism != DM_COMMON_P2P) {
         p->peer_portid_valid = 1;
         p->peer_portid = m->header.sourcePortIdentity;
         pr_debug("%s: peer port id set to %s", p->log_name,
@@ -2597,7 +2594,7 @@ calc:
         t3c = tmv_add(t3, tmv_add(c1, c2));
     }

-    if (p->follow_up_info)
+    if (p->follow_up_info || p->transportSpecific == TS_CMLDS) /* FIXME */
         port_nrate_calculate(p, t3c, t4);

     tsproc_set_clock_rate_ratio(p->tsproc, p->nrate.ratio *
-- 
2.42.0

On Mon, 4 Dec 2023 at 22:51, Andrew Zaborowski
<and...@in...> wrote:
>
> On Sun, 3 Dec 2023 at 00:39, Richard Cochran <ric...@gm...> wrote:
> > +# Common Mean Link Delay Service (CMLDS) example configuration for a
> > +# CMLDS Link Port, containing those attributes which differ from the
>
> While there's only one Link Port here I wouldn't mention this because
> the CMLDS config basically has to cover all of the ports.
>
> > +# defaults.  See the file, default.cfg, for the complete list of
> > +# available options.
> > +#
> > +[global]
> > +clientOnly                     1
> > +clockIdentity                  C37D50.0000.000000
> > +free_running                   1
> > +ignore_transport_specific      1
>
> I assume ignore_transport_specific can be dropped in this version.
>
> > +transportSpecific              2
> > +uds_address                    /var/run/cmlds_server
> > +
> > +[eth1]
> > +delay_mechanism        P2P
>
> This line has to be the same for all interfaces in use by the CMLDS so
> I'd move this to [global] to avoid repetition.
>
> This way you basically have nothing port-specific to configure and can
> pass the interface names on the command line instead of listing here.
> On the client side cmlds.client_address will not allow this because I
> believe the source address has to be different for each port's socket.
> I thought (even before this came up) it would be nice for uds_address
> to default to something that includes the domainNumber, and now for
> cmlds.client_address to include the domainNumber and port number.
>
> I'm going to try test this patchset in an 802.1AS configuration and report back.
>
> Best regards

Re: [Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

[Andrew Z. <and...@in...>]

On Sun, 3 Dec 2023 at 00:39, Richard Cochran <ric...@gm...> wrote:
> @@ -306,13 +307,15 @@ static void do_set_action(struct pmc *pmc, int action, int index, char *str)
>                              "duration          %hu "
>                              "NOTIFY_PORT_STATE %3s "
>                              "NOTIFY_TIME_SYNC  %3s "
> -                            "NOTIFY_PARENT_DATA_SET %3s ",
> +                            "NOTIFY_PARENT_DATA_SET %3s "
> +                            "NOTIFY_CMLDS %3s ",
>                              &sen.duration,
>                              onoff_port_state,
>                              onoff_time_status,
> -                            onoff_parent_data_set);
> -               if (cnt != 4) {
> -                       fprintf(stderr, "%s SET needs 4 values\n",
> +                            onoff_parent_data_set,
> +                            onoff_cmlds);
> +               if (cnt != 5) {
> +                       fprintf(stderr, "%s SET needs 5 values\n",

Doing cnt != 4 && cnt != 5 should just work as sscanf() should stop
parsing when it doesn't find NOTIFY_CMLDS and leave onoff_cmlds as
"off".  Obviously this won't scale as more events are added but it'd
keep syntax backwards-compatible for scripts.

Best regards

Re: [Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Andrew Z. <and...@in...>]

On Sun, 3 Dec 2023 at 00:39, Richard Cochran <ric...@gm...> wrote:
> +# Common Mean Link Delay Service (CMLDS) example configuration for a
> +# CMLDS Link Port, containing those attributes which differ from the

While there's only one Link Port here I wouldn't mention this because
the CMLDS config basically has to cover all of the ports.

> +# defaults.  See the file, default.cfg, for the complete list of
> +# available options.
> +#
> +[global]
> +clientOnly                     1
> +clockIdentity                  C37D50.0000.000000
> +free_running                   1
> +ignore_transport_specific      1

I assume ignore_transport_specific can be dropped in this version.

> +transportSpecific              2
> +uds_address                    /var/run/cmlds_server
> +
> +[eth1]
> +delay_mechanism        P2P

This line has to be the same for all interfaces in use by the CMLDS so
I'd move this to [global] to avoid repetition.

This way you basically have nothing port-specific to configure and can
pass the interface names on the command line instead of listing here.
On the client side cmlds.client_address will not allow this because I
believe the source address has to be different for each port's socket.
I thought (even before this came up) it would be nice for uds_address
to default to something that includes the domainNumber, and now for
cmlds.client_address to include the domainNumber and port number.

I'm going to try test this patchset in an 802.1AS configuration and report back.

Best regards

Re: [Linuxptp-devel] [PATCH v7 1/2] pmc_agent: Add option to run callback for signaling messages

[Maciek M. <ma...@ma...>]

On 04/12/2023 17:46, Richard Cochran wrote:
> On Mon, Dec 04, 2023 at 08:39:47AM +0100, Maciek Machnikowski wrote:
>> Hey, please take a look at these patches and merge them if there are no
>> bugs in this code.
> 
> I don't understand the need for this series.
> What problem does it solve?
> 
> Thanks,
> Richard

If you have a remote node that is connected to a PPS signal and have no
trusted NTP or NMEA data you can use an incoming PTP traffic for
recovering the ToD for a given PPS.

It solves this problem for Assisted Partial Timing Support (APTS)
deployment where a node is connected to the PPS signal from a GNSS
receiver, but doesn't have the serial port (e.x. shared GNSS receiver).

Thanks,
Maciek

Re: [Linuxptp-devel] [PATCH v7 1/2] pmc_agent: Add option to run callback for signaling messages

[Richard C. <ric...@gm...>]

On Mon, Dec 04, 2023 at 08:39:47AM +0100, Maciek Machnikowski wrote:
> Hey, please take a look at these patches and merge them if there are no
> bugs in this code.

I don't understand the need for this series.
What problem does it solve?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v3 5/6] Implement the COMMON_P2P delay mechanism.

[Miroslav L. <mli...@re...>]

On Mon, Dec 04, 2023 at 04:06:31PM +0100, Miroslav Lichvar wrote:
> On Sat, Dec 02, 2023 at 03:38:18PM -0800, Richard Cochran wrote:
> > --- a/config.c
> > +++ b/config.c
> > @@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
> >  
> >  static struct config_enum delay_mech_enu[] = {
> >  	{ "Auto", DM_AUTO },
> > +	{ "COMMON_P2P", DM_COMMON_P2P },
> 
> The new option is missing in the ptp4l usage text.

My bad. I just realized there is no new getopt option to be documented.

-- 
Miroslav Lichvar

Re: [Linuxptp-devel] [PATCH v3 5/6] Implement the COMMON_P2P delay mechanism.

[Miroslav L. <mli...@re...>]

On Sat, Dec 02, 2023 at 03:38:18PM -0800, Richard Cochran wrote:
> --- a/config.c
> +++ b/config.c
> @@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
>  
>  static struct config_enum delay_mech_enu[] = {
>  	{ "Auto", DM_AUTO },
> +	{ "COMMON_P2P", DM_COMMON_P2P },

The new option is missing in the ptp4l usage text.

>  	{ "E2E",  DM_E2E },
>  	{ "P2P",  DM_P2P },
>  	{ "NONE", DM_NO_MECHANISM },
> @@ -249,6 +250,11 @@ struct config_item config_tab[] = {
>  	GLOB_ITEM_INT("clock_class_threshold", CLOCK_CLASS_THRESHOLD_DEFAULT, 6, CLOCK_CLASS_THRESHOLD_DEFAULT),
>  	GLOB_ITEM_ENU("clock_servo", CLOCK_SERVO_PI, clock_servo_enu),
>  	GLOB_ITEM_ENU("clock_type", CLOCK_TYPE_ORDINARY, clock_type_enu),
> +	PORT_ITEM_STR("cmlds.client_address", "/var/run/cmlds_cleint"),

Swapped e and i in "cleint" is not intentional, right? It's the same
in the man page and the example default config.

Other than that, the patchset looks ok to me. I didn't run any tests,
just a quick look at the code. I'll update the pmc test in the
testsuite.

-- 
Miroslav Lichvar

Re: [Linuxptp-devel] [PATCH v7 1/2] pmc_agent: Add option to run callback for signaling messages

[Maciek M. <ma...@ma...>]

Hey, please take a look at these patches and merge them if there are no
bugs in this code.

Thanks,
Maciek

On 10/11/2023 15:14, Maciek Machnikowski wrote:
> Add option to run callback that when the PMC agent receives signaling messages.
> 
> v2: changed pmc_agent implementation to not use a separate callback
> v3: updated poll thread to use less CPU
> v4: changed sleep routine to usleep, simplified poll thread
> v5: make sure management_tlv_id is not called on non-management messages
> v6: optimized code to not check signaling_cb_ena on management messages
> v7: rebase
> 
> Signed-off-by: Maciek Machnikowski <ma...@ma...>
> ---
>  pmc_agent.c | 22 ++++++++++++++++++++--
>  pmc_agent.h |  7 +++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/pmc_agent.c b/pmc_agent.c
> index 62d1a86..bea6b59 100644
> --- a/pmc_agent.c
> +++ b/pmc_agent.c
> @@ -42,6 +42,7 @@ struct pmc_agent {
>  	bool dds_valid;
>  	int leap;
>  	int pmc_ds_requested;
> +	bool signaling_cb_ena;
>  	bool stay_subscribed;
>  	int sync_offset;
>  	int utc_offset_traceable;
> @@ -127,6 +128,7 @@ static int run_pmc(struct pmc_agent *node, int timeout, int ds_id,
>  #define N_FD 1
>  	struct pollfd pollfd[N_FD];
>  	int cnt, res;
> +	bool skip_cb;
>  
>  	while (1) {
>  		pollfd[0].fd = pmc_get_transport_fd(node->pmc);
> @@ -178,9 +180,19 @@ static int run_pmc(struct pmc_agent *node, int timeout, int ds_id,
>  			node->pmc_ds_requested = 0;
>  			return RUN_PMC_NODEV;
>  		}
> -		if (res <= 0 ||
> +
> +		/* Skip callback if message is not management */
> +		skip_cb = (res <= 0) ? true : false;
> +
> +		/* Run the callback on signaling messages if configured */
> +		if (res == 0 && node->signaling_cb_ena &&
> +		    msg_type(*msg) == SIGNALING) {
> +			skip_cb = false;
> +		}
> +
> +		if (skip_cb ||
>  		    node->recv_subscribed(node->recv_context, *msg, ds_id) ||
> -		    management_tlv_id(*msg) != ds_id) {
> +		    (res == 1 && management_tlv_id(*msg) != ds_id)) {
>  			msg_put(*msg);
>  			*msg = NULL;
>  			continue;
> @@ -430,3 +442,9 @@ bool pmc_agent_utc_offset_traceable(struct pmc_agent *agent)
>  {
>  	return agent->utc_offset_traceable;
>  }
> +
> +void pmc_agent_enable_signaling_cb(struct pmc_agent *agent, bool enable)
> +{
> +	agent->signaling_cb_ena = enable;
> +}
> +
> diff --git a/pmc_agent.h b/pmc_agent.h
> index 2fb1cc8..9ae37f7 100644
> --- a/pmc_agent.h
> +++ b/pmc_agent.h
> @@ -170,4 +170,11 @@ int pmc_agent_update(struct pmc_agent *agent);
>   */
>  bool pmc_agent_utc_offset_traceable(struct pmc_agent *agent);
>  
> +/**
> + * Enables or disables callback on signaling messages
> + * @param agent  Pointer to a PMC instance obtained via @ref pmc_agent_create().
> + * @param enable - if set to true, callback will be called on signaling msgs
> + */
> +void pmc_agent_enable_signaling_cb(struct pmc_agent *agent, bool enable);
> +
>  #endif

Re: [Linuxptp-devel] [PATCH v1 5/5] Implement the COMMON_P2P delay mechanism.

[Richard C. <ric...@gm...>]

On Sun, Dec 03, 2023 at 06:15:52PM -0800, Kishen Maloor wrote:
> On 12/1/23 10:07 PM, Richard Cochran wrote:
> > Does the standard require that either ALL the nodes in a network use
> > CMLDS, or none do at all?
> > 
> 
> When the time-aware system supports multiple domains, IEEE
> 802.1as-2020/11.2.17 mandates CMLDS and the COMMON_P2P DM.

Very narrow minded and short sighted.

Oh well, I wouldn't expect 802.1as to actually be practical.

Pass me the crazy sauce, please!

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v1 5/5] Implement the COMMON_P2P delay mechanism.

[Kishen M. <kis...@in...>]

On 12/1/23 10:07 PM, Richard Cochran wrote:
> On Thu, Nov 30, 2023 at 04:52:44PM -0800, Kishen Maloor wrote:
> 
>> CMLDS ptp4l process would be configured with
>> transportSpecific/domain=0x2/0.
> 
> Question about the spec:
> 
> How on earth is this going to work?
> 
> I mean, if the link partner is not also using CMLDS, then it won't
> reply because of the majorSdoId mismatch.

Yes, as we understand, that is indeed the case. IEEE 
1588-2019/16.6.3.1/Note 3 emphasizes this point and 16.6.3.2 specifies 
the wire protocol variations for CMLDS. So yeah, the link partner is
expected to run an instance of the service and they would interoperate.

> 
> Does the standard require that either ALL the nodes in a network use
> CMLDS, or none do at all?
> 

When the time-aware system supports multiple domains, IEEE 
802.1as-2020/11.2.17 mandates CMLDS and the COMMON_P2P DM.

Thanks,
-Kishen.

Re: [Linuxptp-devel] [PATCH v2 0/6] Common Mean Link Delay Service (CMLDS)

[Richard C. <ric...@gm...>]

On Sat, Dec 02, 2023 at 02:28:54PM -0800, Richard Cochran wrote:
> On Sat, Dec 02, 2023 at 02:24:52PM -0800, Richard Cochran wrote:
> 
> > Changes in v2
> > ~~~~~~~~~~~~~
> > - Added timer to renew push subscription and detect missing server.
> > - Addressed Andrew and Kishen's review comments.
> > - Added example configuration files.
> 
> I also updated the man page with the new config options.
> Still need to add COMMON_P2P into man page.

And this v2 is buggy, please ignore in favor of v3.

Thanks,
Richard

[Linuxptp-devel] [PATCH v3 6/6] Add example Common Mean Link Delay Service configuration files.

[Richard C. <ric...@gm...>]

Signed-off-by: Richard Cochran <ric...@gm...>
---
 configs/CMLDS_client.cfg | 10 ++++++++++
 configs/CMLDS_server.cfg | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 configs/CMLDS_client.cfg
 create mode 100644 configs/CMLDS_server.cfg

diff --git a/configs/CMLDS_client.cfg b/configs/CMLDS_client.cfg
new file mode 100644
index 0000000..ef10fef
--- /dev/null
+++ b/configs/CMLDS_client.cfg
@@ -0,0 +1,10 @@
+#
+# Common Mean Link Delay Service (CMLDS) example configuration for a
+# CMLDS client, containing those attributes which differ from the
+# defaults.  See the file, default.cfg, for the complete list of
+# available options.
+#
+[global]
+
+[eth1]
+delay_mechanism	COMMON_P2P
diff --git a/configs/CMLDS_server.cfg b/configs/CMLDS_server.cfg
new file mode 100644
index 0000000..36f118c
--- /dev/null
+++ b/configs/CMLDS_server.cfg
@@ -0,0 +1,16 @@
+#
+# Common Mean Link Delay Service (CMLDS) example configuration for a
+# CMLDS Link Port, containing those attributes which differ from the
+# defaults.  See the file, default.cfg, for the complete list of
+# available options.
+#
+[global]
+clientOnly			1
+clockIdentity			C37D50.0000.000000
+free_running			1
+ignore_transport_specific	1
+transportSpecific		2
+uds_address			/var/run/cmlds_server
+
+[eth1]
+delay_mechanism	P2P
-- 
2.39.2

[Linuxptp-devel] [PATCH v3 5/6] Implement the COMMON_P2P delay mechanism.

[Richard C. <ric...@gm...>]

If a given port selects the COMMON_P2P delay mechanism, let it open a
local PTP Management channel to the Common Mean Link Delay Service
over a UNIX domain socket.

Signed-off-by: Richard Cochran <ric...@gm...>
---
 config.c            |   6 ++
 configs/default.cfg |   5 ++
 dm.h                |   3 +
 fd.h                |   1 +
 makefile            |   4 +-
 port.c              | 181 +++++++++++++++++++++++++++++++++++++++++---
 port_private.h      |   7 ++
 ptp4l.8             |  39 +++++++++-
 8 files changed, 233 insertions(+), 13 deletions(-)

diff --git a/config.c b/config.c
index fe65b76..8cf1620 100644
--- a/config.c
+++ b/config.c
@@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
 
 static struct config_enum delay_mech_enu[] = {
 	{ "Auto", DM_AUTO },
+	{ "COMMON_P2P", DM_COMMON_P2P },
 	{ "E2E",  DM_E2E },
 	{ "P2P",  DM_P2P },
 	{ "NONE", DM_NO_MECHANISM },
@@ -249,6 +250,11 @@ struct config_item config_tab[] = {
 	GLOB_ITEM_INT("clock_class_threshold", CLOCK_CLASS_THRESHOLD_DEFAULT, 6, CLOCK_CLASS_THRESHOLD_DEFAULT),
 	GLOB_ITEM_ENU("clock_servo", CLOCK_SERVO_PI, clock_servo_enu),
 	GLOB_ITEM_ENU("clock_type", CLOCK_TYPE_ORDINARY, clock_type_enu),
+	PORT_ITEM_STR("cmlds.client_address", "/var/run/cmlds_cleint"),
+	PORT_ITEM_INT("cmlds.domainNumber", 0, 0, 255),
+	PORT_ITEM_INT("cmlds.majorSdoId", 2, 0, 0x0F),
+	PORT_ITEM_INT("cmlds.port", 0, 0, UINT16_MAX),
+	PORT_ITEM_STR("cmlds.server_address", "/var/run/cmlds_server"),
 	GLOB_ITEM_ENU("dataset_comparison", DS_CMP_IEEE1588, dataset_comp_enu),
 	PORT_ITEM_INT("delayAsymmetry", 0, INT_MIN, INT_MAX),
 	PORT_ITEM_ENU("delay_filter", FILTER_MOVING_MEDIAN, delay_filter_enu),
diff --git a/configs/default.cfg b/configs/default.cfg
index 0c7661c..29a34fd 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -93,6 +93,11 @@ write_phase_mode	0
 #
 # Transport options
 #
+cmlds.client_address	/var/run/cmlds_cleint
+cmlds.domainNumber	0
+cmlds.majorSdoId	2
+cmlds.port		0
+cmlds.server_address	/var/run/cmlds_server
 transportSpecific	0x0
 ptp_dst_mac		01:1B:19:00:00:00
 p2p_dst_mac		01:80:C2:00:00:0E
diff --git a/dm.h b/dm.h
index 47bd847..80d1ce5 100644
--- a/dm.h
+++ b/dm.h
@@ -34,6 +34,9 @@ enum delay_mechanism {
 	/** Peer delay mechanism. */
 	DM_P2P,
 
+	/** Peer delay as measured by CMLDS. */
+	DM_COMMON_P2P,
+
 	/** No Delay Mechanism. */
 	DM_NO_MECHANISM = 0xFE,
 };
diff --git a/fd.h b/fd.h
index 16420d7..9a072ab 100644
--- a/fd.h
+++ b/fd.h
@@ -39,6 +39,7 @@ enum {
 	FD_SYNC_TX_TIMER,
 	FD_UNICAST_REQ_TIMER,
 	FD_UNICAST_SRV_TIMER,
+	FD_CMLDS,
 	FD_RTNL,
 	N_POLLFD,
 };
diff --git a/makefile b/makefile
index 7fc5f6f..e9c1ccc 100644
--- a/makefile
+++ b/makefile
@@ -30,8 +30,8 @@ TS2PHC	= ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_pps_source.o \
  ts2phc_nmea_pps_source.o ts2phc_phc_pps_source.o ts2phc_pps_sink.o ts2phc_pps_source.o
 OBJ	= bmc.o clock.o clockadj.o clockcheck.o config.o designated_fsm.o \
  e2e_tc.o fault.o $(FILTERS) fsm.o hash.o interface.o monitor.o msg.o phc.o \
- port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o rtnl.o $(SERVOS) \
- sk.o stats.o tc.o $(TRANSP) telecom.o tlv.o tsproc.o unicast_client.o \
+ pmc_common.o port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o rtnl.o \
+ $(SERVOS) sk.o stats.o tc.o $(TRANSP) telecom.o tlv.o tsproc.o unicast_client.o \
  unicast_fsm.o unicast_service.o util.o version.o
 
 OBJECTS	= $(OBJ) hwstamp_ctl.o nsm.o phc2sys.o phc_ctl.o pmc.o pmc_agent.o \
diff --git a/port.c b/port.c
index 23f021c..e2ebaeb 100644
--- a/port.c
+++ b/port.c
@@ -46,6 +46,8 @@
 #include "util.h"
 
 #define ANNOUNCE_SPAN 1
+#define CMLDS_SUBSCRIPTION_INTERVAL	60 /*seconds*/
+#define CMLDS_UPDATE_INTERVAL		(CMLDS_SUBSCRIPTION_INTERVAL / 2)
 
 enum syfu_event {
 	SYNC_MISMATCH,
@@ -965,7 +967,8 @@ static int port_management_fill_response(struct port *target,
 		ptp_text_copy(cd->userDescription, &desc->userDescription);
 		buf += sizeof(struct PTPText) + cd->userDescription->length;
 
-		if (target->delayMechanism == DM_P2P) {
+		if (target->delayMechanism == DM_P2P ||
+		    target->delayMechanism == DM_COMMON_P2P) {
 			memcpy(buf, profile_id_p2p, PROFILE_ID_LEN);
 		} else {
 			struct config *cfg = clock_config(target->clock);
@@ -1275,14 +1278,16 @@ int port_set_delay_tmo(struct port *p)
 	if (p->inhibit_delay_req) {
 		return 0;
 	}
-
-	if (p->delayMechanism == DM_P2P) {
+	switch (p->delayMechanism) {
+	case DM_COMMON_P2P:
+	case DM_P2P:
 		return set_tmo_log(p->fda.fd[FD_DELAY_TIMER], 1,
-			       p->logPdelayReqInterval);
-	} else {
-		return set_tmo_random(p->fda.fd[FD_DELAY_TIMER], 0, 2,
-				p->logMinDelayReqInterval);
+				   p->logPdelayReqInterval);
+	default:
+		break;
 	}
+	return set_tmo_random(p->fda.fd[FD_DELAY_TIMER], 0, 2,
+			      p->logMinDelayReqInterval);
 }
 
 static int port_set_manno_tmo(struct port *p)
@@ -1528,6 +1533,49 @@ static void port_syfufsm(struct port *p, enum syfu_event event,
 	}
 }
 
+static int port_cmlds_renew(struct port *p, time_t now)
+{
+	struct subscribe_events_np sen = {
+		.duration = CMLDS_SUBSCRIPTION_INTERVAL,
+	};
+	int err;
+
+	event_bitmask_set(sen.bitmask, NOTIFY_CMLDS, TRUE);
+	err = pmc_send_set_action(p->cmlds.pmc, MID_SUBSCRIBE_EVENTS_NP,
+				  &sen, sizeof(sen));
+	if (err) {
+		return err;
+	}
+	p->cmlds.last_renewal = now;
+	return 0;
+}
+
+static enum fsm_event port_cmlds_timeout(struct port *p)
+{
+	struct timespec now;
+	int err;
+
+	if (!p->cmlds.pmc) {
+		return EV_NONE;
+	}
+	clock_gettime(CLOCK_MONOTONIC, &now);
+	if (now.tv_sec - p->cmlds.last_renewal > CMLDS_UPDATE_INTERVAL) {
+		err = port_cmlds_renew(p, now.tv_sec);
+		if (err) {
+			return EV_FAULT_DETECTED;
+		}
+	}
+	p->cmlds.timer_count++;
+	if (p->cmlds.timer_count > p->allowedLostResponses) {
+		p->asCapable = NOT_CAPABLE;
+		err = port_cmlds_renew(p, now.tv_sec);
+		if (err) {
+			return EV_FAULT_DETECTED;
+		}
+	}
+	return EV_NONE;
+}
+
 static int port_pdelay_request(struct port *p)
 {
 	struct ptp_message *msg;
@@ -1871,6 +1919,33 @@ static void port_clear_fda(struct port *p, int count)
 		p->fda.fd[i] = -1;
 }
 
+static int port_cmlds_initialize(struct port *p)
+{
+	struct config *cfg = clock_config(p->clock);
+	const int zero_datalen = 1;
+	const UInteger8 hops = 0;
+	struct timespec now;
+
+	p->cmlds.port = config_get_int(cfg, p->name, "cmlds.port");
+	if (!p->cmlds.port) {
+		p->cmlds.port = portnum(p);
+	}
+	p->cmlds.pmc = pmc_create(cfg, TRANS_UDS,
+				  config_get_string(cfg, p->name, "cmlds.client_address"),
+				  config_get_string(cfg, p->name, "cmlds.server_address"),
+				  hops,
+				  config_get_int(cfg, p->name, "cmlds.domainNumber"),
+				  config_get_int(cfg, p->name, "cmlds.majorSdoId") << 4,
+				  zero_datalen);
+	if (!p->cmlds.pmc) {
+		return -1;
+	}
+	p->cmlds.timer_count = 0;
+	p->fda.fd[FD_CMLDS] = pmc_get_transport_fd(p->cmlds.pmc);
+	clock_gettime(CLOCK_MONOTONIC, &now);
+	return port_cmlds_renew(p, now.tv_sec);
+}
+
 void port_disable(struct port *p)
 {
 	int i;
@@ -1888,6 +1963,12 @@ void port_disable(struct port *p)
 		close(p->fda.fd[FD_FIRST_TIMER + i]);
 	}
 
+	if (p->cmlds.pmc) {
+		pmc_destroy(p->cmlds.pmc);
+		p->fda.fd[FD_CMLDS] = -1;
+		p->cmlds.pmc = NULL;
+	}
+
 	/* Keep rtnl socket to get link status info. */
 	port_clear_fda(p, FD_RTNL);
 	clock_fda_changed(p->clock);
@@ -1963,6 +2044,10 @@ int port_initialize(struct port *p)
 		goto no_tmo;
 	}
 
+	if (port_delay_mechanism(p) == DM_COMMON_P2P && port_cmlds_initialize(p)) {
+		goto no_tmo;
+	}
+
 	/* No need to open rtnl socket on UDS port. */
 	if (!port_is_uds(p)) {
 		/*
@@ -2104,6 +2189,65 @@ int process_announce(struct port *p, struct ptp_message *m)
 	return result;
 }
 
+static int process_cmlds(struct port *p)
+{
+	struct cmlds_info_np *cmlds;
+	struct management_tlv *mgt;
+	struct ptp_message *msg;
+	struct TLV *tlv;
+	int err = 0;
+
+	msg = pmc_recv(p->cmlds.pmc);
+	if (!msg) {
+		pr_err("%s: pmc_recv failed", p->log_name);
+		return -1;
+	}
+	if (msg_type(msg) != MANAGEMENT) {
+		pr_err("%s: pmc_recv bad message", p->log_name);
+		err = -1;
+		goto out;
+	}
+	tlv = (struct TLV *) msg->management.suffix;
+	if (tlv->type != TLV_MANAGEMENT) {
+		pr_err("%s: pmc_recv bad message", p->log_name);
+		err = -1;
+		goto out;
+	}
+	mgt = (struct management_tlv *) msg->management.suffix;
+	if (mgt->length == 2 && mgt->id != MID_NULL_MANAGEMENT) {
+		pr_err("%s: pmc_recv bad length", p->log_name);
+		goto out;
+	}
+
+	switch (mgt->id) {
+	case MID_CMLDS_INFO_NP:
+		if (msg->header.sourcePortIdentity.portNumber != p->cmlds.port) {
+			break;
+		}
+		cmlds = (struct cmlds_info_np *) mgt->data;
+		p->peer_delay = nanoseconds_to_tmv(cmlds->meanLinkDelay >> 16);
+		p->peerMeanPathDelay = cmlds->meanLinkDelay;
+		p->nrate.ratio = 1.0 + (double) cmlds->scaledNeighborRateRatio / POW2_41;
+		p->asCapable = cmlds->as_capable;
+		p->cmlds.timer_count = 0;
+		if (p->state == PS_UNCALIBRATED || p->state == PS_SLAVE) {
+			const tmv_t tx = tmv_zero();
+			clock_peer_delay(p->clock, p->peer_delay, tx, tx, p->nrate.ratio);
+		}
+		break;
+	case MID_SUBSCRIBE_EVENTS_NP:
+		break;
+	default:
+		pr_err("%s: pmc_recv bad mgt id 0x%x", p->log_name, mgt->id);
+		err = -1;
+		break;
+	}
+
+out:
+	msg_put(msg);
+	return err;
+}
+
 static int process_delay_req(struct port *p, struct ptp_message *m)
 {
 	struct ptp_message *msg;
@@ -2115,7 +2259,7 @@ static int process_delay_req(struct port *p, struct ptp_message *m)
 		return 0;
 	}
 
-	if (p->delayMechanism == DM_P2P) {
+	if (p->delayMechanism == DM_P2P || p->delayMechanism == DM_COMMON_P2P) {
 		pr_warning("%s: delay request on P2P port", p->log_name);
 		return 0;
 	}
@@ -2280,6 +2424,9 @@ int process_pdelay_req(struct port *p, struct ptp_message *m)
 		return -1;
 	}
 
+	if (p->delayMechanism == DM_COMMON_P2P) {
+		return 0;
+	}
 	if (p->delayMechanism == DM_E2E) {
 		pr_warning("%s: pdelay_req on E2E port", p->log_name);
 		return 0;
@@ -2475,6 +2622,9 @@ calc:
 
 int process_pdelay_resp(struct port *p, struct ptp_message *m)
 {
+	if (p->delayMechanism == DM_COMMON_P2P) {
+		return 0;
+	}
 	if (p->peer_delay_resp) {
                 if (!p->multiple_pdr_detected) {
                         pr_err("%s: multiple peer responses", p->log_name);
@@ -2750,10 +2900,14 @@ static void bc_dispatch(struct port *p, enum fsm_event event, int mdiff)
 		return;
 	}
 
-	if (p->delayMechanism == DM_P2P) {
+	switch (p->delayMechanism) {
+	case DM_COMMON_P2P:
+	case DM_P2P:
 		port_p2p_transition(p, p->state);
-	} else {
+		break;
+	default:
 		port_e2e_transition(p, p->state);
+		break;
 	}
 
 	if (p->jbod && p->state == PS_UNCALIBRATED && p->phc_index >= 0 ) {
@@ -2901,6 +3055,9 @@ static enum fsm_event bc_event(struct port *p, int fd_index)
 	case FD_DELAY_TIMER:
 		pr_debug("%s: delay timeout", p->log_name);
 		port_set_delay_tmo(p);
+		if (p->delayMechanism == DM_COMMON_P2P) {
+			return port_cmlds_timeout(p);
+		}
 		delay_req_prune(p);
 		p->service_stats.delay_timeout++;
 		if (port_delay_request(p)) {
@@ -2947,6 +3104,10 @@ static enum fsm_event bc_event(struct port *p, int fd_index)
 		p->service_stats.unicast_request_timeout++;
 		return unicast_client_timer(p) ? EV_FAULT_DETECTED : EV_NONE;
 
+	case FD_CMLDS:
+		pr_debug("%s: CMLDS push notification", p->log_name);
+		return process_cmlds(p) ? EV_FAULT_DETECTED : EV_NONE;
+
 	case FD_RTNL:
 		pr_debug("%s: received link status notification", p->log_name);
 		rtnl_link_status(fd, p->name, port_link_status, p);
diff --git a/port_private.h b/port_private.h
index 1072ad6..4f158cd 100644
--- a/port_private.h
+++ b/port_private.h
@@ -26,6 +26,7 @@
 #include "fsm.h"
 #include "monitor.h"
 #include "msg.h"
+#include "pmc_common.h"
 #include "power_profile.h"
 #include "tmv.h"
 #include "util.h"
@@ -164,6 +165,12 @@ struct port {
 	/* slave event monitoring */
 	struct monitor *slave_event_monitor;
 	bool unicast_state_dirty;
+	struct {
+		unsigned int timer_count;
+		time_t last_renewal;
+		struct pmc *pmc;
+		int port;
+	} cmlds;
 };
 
 #define portnum(p) (p->portIdentity.portNumber)
diff --git a/ptp4l.8 b/ptp4l.8
index 4cb9adb..87befcd 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -159,6 +159,41 @@ collection of clocks must be synchronized by an external program, for
 example phc2sys(8) in "automatic" mode.
 The default is 0 (disabled).
 
+.TP
+.B cmlds.client_address
+Specifies the source address for the UNIX domain socket that receives
+peer delay measurement when using the "COMMON_P2P" delay mechanism.
+The default is /var/run/cmlds_cleint.
+
+.TP
+.B cmlds.domainNumber
+Specifies the domainNumber message field for communications with the
+local Common Mean Link Delay Service, used with the "COMMON_P2P" delay
+mechanism.
+The default is 0.
+
+.TP
+.B cmlds.majorSdoId
+Specifies the transportSpecific message field for communications with
+the local Common Mean Link Delay Service, used with the "COMMON_P2P"
+delay mechanism.
+The default is 2.
+
+.TP
+.B cmlds.port
+Specifies the port number of local Common Mean Link Delay Service from
+which to accept delay measurements.  The range of valid port numbers
+is one to 65535, but the special value zero configures the port number
+of the port requesting the measurements.
+The default is 0, meaning the requesting port number.
+
+.TP
+.B
+cmlds.server_address
+Specifies the UNIX domain socket address of the local Common Mean Link
+Delay Service, for use with the "COMMON_P2P" delay mechanism.
+The default is /var/run/cmlds_server.
+
 .TP
 .B delayAsymmetry
 The time difference in nanoseconds of the transmit and receive
@@ -179,7 +214,9 @@ The default is 10.
 
 .TP
 .B delay_mechanism
-Select the delay mechanism. Possible values are E2E, P2P, NONE and Auto.
+
+Select the delay mechanism. Possible values are Auto, COMMON_P2P, E2E,
+P2P, and NONE.
 The default is E2E.
 
 .TP
-- 
2.39.2

[Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

[Richard C. <ric...@gm...>]

Signed-off-by: Richard Cochran <ric...@gm...>
---
 notification.h |  1 +
 pmc.c          |  6 ++++--
 pmc_common.c   | 14 ++++++++++----
 port.c         |  7 +++++++
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/notification.h b/notification.h
index c1a6395..7a8f641 100644
--- a/notification.h
+++ b/notification.h
@@ -45,6 +45,7 @@ enum notification {
 	NOTIFY_PORT_STATE,
 	NOTIFY_TIME_SYNC,
 	NOTIFY_PARENT_DATA_SET,
+	NOTIFY_CMLDS,
 };
 
 #endif
diff --git a/pmc.c b/pmc.c
index 12a6109..6cdd7b0 100644
--- a/pmc.c
+++ b/pmc.c
@@ -453,11 +453,13 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
 			IFMT "duration               %hu"
 			IFMT "NOTIFY_PORT_STATE      %s"
 			IFMT "NOTIFY_TIME_SYNC       %s"
-			IFMT "NOTIFY_PARENT_DATA_SET %s",
+			IFMT "NOTIFY_PARENT_DATA_SET %s"
+			IFMT "NOTIFY_CMLDS           %s",
 			sen->duration,
 			event_bitmask_get(sen->bitmask, NOTIFY_PORT_STATE) ? "on" : "off",
 			event_bitmask_get(sen->bitmask, NOTIFY_TIME_SYNC) ? "on" : "off",
-			event_bitmask_get(sen->bitmask, NOTIFY_PARENT_DATA_SET) ? "on" : "off");
+			event_bitmask_get(sen->bitmask, NOTIFY_PARENT_DATA_SET) ? "on" : "off",
+			event_bitmask_get(sen->bitmask, NOTIFY_CMLDS) ? "on" : "off");
 		break;
 	case MID_SYNCHRONIZATION_UNCERTAIN_NP:
 		mtd = (struct management_tlv_datum *) mgt->data;
diff --git a/pmc_common.c b/pmc_common.c
index 1d537f2..a549af5 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -180,6 +180,7 @@ static void do_set_action(struct pmc *pmc, int action, int index, char *str)
 	char onoff_port_state[4] = "off";
 	char onoff_time_status[4] = "off";
 	char onoff_parent_data_set[4] = "off";
+	char onoff_cmlds[4] = "off";
 	char display_name[11] = {0};
 	uint64_t jump;
 	uint8_t key;
@@ -306,13 +307,15 @@ static void do_set_action(struct pmc *pmc, int action, int index, char *str)
 			     "duration          %hu "
 			     "NOTIFY_PORT_STATE %3s "
 			     "NOTIFY_TIME_SYNC  %3s "
-			     "NOTIFY_PARENT_DATA_SET %3s ",
+			     "NOTIFY_PARENT_DATA_SET %3s "
+			     "NOTIFY_CMLDS %3s ",
 			     &sen.duration,
 			     onoff_port_state,
 			     onoff_time_status,
-			     onoff_parent_data_set);
-		if (cnt != 4) {
-			fprintf(stderr, "%s SET needs 4 values\n",
+			     onoff_parent_data_set,
+			     onoff_cmlds);
+		if (cnt != 5) {
+			fprintf(stderr, "%s SET needs 5 values\n",
 				idtab[index].name);
 			break;
 		}
@@ -326,6 +329,9 @@ static void do_set_action(struct pmc *pmc, int action, int index, char *str)
 			event_bitmask_set(sen.bitmask, NOTIFY_PARENT_DATA_SET,
 					  TRUE);
 		}
+		if (!strcasecmp(onoff_cmlds, "on")) {
+			event_bitmask_set(sen.bitmask, NOTIFY_CMLDS, TRUE);
+		}
 		pmc_send_set_action(pmc, code, &sen, sizeof(sen));
 		break;
 	case MID_SYNCHRONIZATION_UNCERTAIN_NP:
diff --git a/port.c b/port.c
index 2eca876..23f021c 100644
--- a/port.c
+++ b/port.c
@@ -748,6 +748,7 @@ capable:
 	if (p->asCapable == NOT_CAPABLE) {
 		pr_debug("%s: setting asCapable", p->log_name);
 		p->asCapable = AS_CAPABLE;
+		port_notify_event(p, NOTIFY_CMLDS);
 	}
 	return 1;
 
@@ -755,6 +756,7 @@ not_capable:
 	if (p->asCapable)
 		port_nrate_initialize(p);
 	p->asCapable = NOT_CAPABLE;
+	port_notify_event(p, NOTIFY_CMLDS);
 	return 0;
 }
 
@@ -2467,6 +2469,8 @@ calc:
 
 	msg_put(p->peer_delay_req);
 	p->peer_delay_req = NULL;
+
+	port_notify_event(p, NOTIFY_CMLDS);
 }
 
 int process_pdelay_resp(struct port *p, struct ptp_message *m)
@@ -3264,6 +3268,9 @@ void port_notify_event(struct port *p, enum notification event)
 	case NOTIFY_PORT_STATE:
 		id = MID_PORT_DATA_SET;
 		break;
+	case NOTIFY_CMLDS:
+		id = MID_CMLDS_INFO_NP;
+		break;
 	default:
 		return;
 	}
-- 
2.39.2

[Linuxptp-devel] [PATCH v3 3/6] Introduce the Common Mean Link Delay Information TLV.

[Richard C. <ric...@gm...>]

Add a new TLV to convey link delay measurements by the Common Mean
Link Delay Service (CMLDS) (as specified in IEEE 1588/16.6.3) over the
management interface.

Co-authored-by: Andrew Zaborowski <and...@in...>
Signed-off-by: Kishen Maloor <kis...@in...>
Signed-off-by: Richard Cochran <ric...@gm...>
---
 clock.c      |  1 -
 pmc.c        | 11 +++++++++++
 pmc_common.c |  1 +
 port.c       |  9 +++++++++
 port.h       |  2 ++
 tlv.c        | 16 ++++++++++++++++
 tlv.h        |  7 +++++++
 7 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index 6f7722c..c999c83 100644
--- a/clock.c
+++ b/clock.c
@@ -47,7 +47,6 @@
 #include "util.h"
 
 #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */
-#define POW2_41 ((double)(1ULL << 41))
 
 struct interface {
 	STAILQ_ENTRY(interface) list;
diff --git a/pmc.c b/pmc.c
index d18fea0..12a6109 100644
--- a/pmc.c
+++ b/pmc.c
@@ -169,6 +169,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
 	struct subscribe_events_np *sen;
 	struct port_properties_np *ppn;
 	struct port_hwclock_np *phn;
+	struct cmlds_info_np *cmlds;
 	struct timePropertiesDS *tp;
 	struct management_tlv *mgt;
 	struct time_status_np *tsn;
@@ -623,6 +624,16 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
 			pwr->networkTimeInaccuracy,
 			pwr->totalTimeInaccuracy);
 		break;
+	case MID_CMLDS_INFO_NP:
+		cmlds = (struct cmlds_info_np *) mgt->data;
+		fprintf(fp, "CMLDS_INFO_NP "
+			IFMT "meanLinkDelay           %" PRId64
+			IFMT "scaledNeighborRateRatio %" PRId32
+			IFMT "as_capable              %" PRIu32,
+			cmlds->meanLinkDelay >> 16,
+			cmlds->scaledNeighborRateRatio,
+			cmlds->as_capable);
+		break;
 	case MID_LOG_ANNOUNCE_INTERVAL:
 		mtd = (struct management_tlv_datum *) mgt->data;
 		fprintf(fp, "LOG_ANNOUNCE_INTERVAL "
diff --git a/pmc_common.c b/pmc_common.c
index fca16c6..1d537f2 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -156,6 +156,7 @@ struct management_id idtab[] = {
 	{ "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP, do_get_action },
 	{ "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action },
 	{ "POWER_PROFILE_SETTINGS_NP", MID_POWER_PROFILE_SETTINGS_NP, do_set_action },
+	{ "CMLDS_INFO_NP", MID_CMLDS_INFO_NP, do_get_action },
 };
 
 static void do_get_action(struct pmc *pmc, int action, int index, char *str)
diff --git a/port.c b/port.c
index b7fbfb1..2eca876 100644
--- a/port.c
+++ b/port.c
@@ -883,6 +883,7 @@ static int port_management_fill_response(struct port *target,
 	struct clock_description *desc;
 	struct port_properties_np *ppn;
 	struct port_hwclock_np *phn;
+	struct cmlds_info_np *cmlds;
 	struct management_tlv *tlv;
 	struct port_stats_np *psn;
 	struct foreign_clock *fc;
@@ -1125,6 +1126,14 @@ static int port_management_fill_response(struct port *target,
 		memcpy(pwr, &target->pwr, sizeof(*pwr));
 		datalen = sizeof(*pwr);
 		break;
+	case MID_CMLDS_INFO_NP:
+		cmlds = (struct cmlds_info_np *)tlv->data;
+		cmlds->meanLinkDelay = target->peerMeanPathDelay;
+		cmlds->scaledNeighborRateRatio =
+			(Integer32) (target->nrate.ratio * POW2_41 - POW2_41);
+		cmlds->as_capable = target->asCapable;
+		datalen = sizeof(*cmlds);
+		break;
 	default:
 		/* The caller should *not* respond to this message. */
 		tlv_extra_recycle(extra);
diff --git a/port.h b/port.h
index 57c8c2f..cc03859 100644
--- a/port.h
+++ b/port.h
@@ -26,6 +26,8 @@
 #include "notification.h"
 #include "transport.h"
 
+#define POW2_41 ((double)(1ULL << 41))
+
 /* forward declarations */
 struct interface;
 struct clock;
diff --git a/tlv.c b/tlv.c
index 9b82bd9..7cbd84d 100644
--- a/tlv.c
+++ b/tlv.c
@@ -176,6 +176,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
 	struct port_properties_np *ppn;
 	struct port_hwclock_np *phn;
 	struct timePropertiesDS *tp;
+	struct cmlds_info_np *cmlds;
 	struct time_status_np *tsn;
 	struct port_stats_np *psn;
 	int extra_len = 0, i, len;
@@ -481,6 +482,14 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
 		NTOHL(pwr->networkTimeInaccuracy);
 		NTOHL(pwr->totalTimeInaccuracy);
 		break;
+	case MID_CMLDS_INFO_NP:
+		if (data_len < sizeof(struct cmlds_info_np))
+			goto bad_length;
+		cmlds = (struct cmlds_info_np *)m->data;
+		net2host64_unaligned(&cmlds->meanLinkDelay);
+		NTOHL(cmlds->scaledNeighborRateRatio);
+		NTOHL(cmlds->as_capable);
+		break;
 	case MID_SAVE_IN_NON_VOLATILE_STORAGE:
 	case MID_RESET_NON_VOLATILE_STORAGE:
 	case MID_INITIALIZE:
@@ -514,6 +523,7 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
 	struct subscribe_events_np *sen;
 	struct port_properties_np *ppn;
 	struct port_hwclock_np *phn;
+	struct cmlds_info_np *cmlds;
 	struct timePropertiesDS *tp;
 	struct time_status_np *tsn;
 	struct port_stats_np *psn;
@@ -672,6 +682,12 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
 		HTONL(pwr->networkTimeInaccuracy);
 		HTONL(pwr->totalTimeInaccuracy);
 		break;
+	case MID_CMLDS_INFO_NP:
+		cmlds = (struct cmlds_info_np *)m->data;
+		host2net64_unaligned(&cmlds->meanLinkDelay);
+		HTONL(cmlds->scaledNeighborRateRatio);
+		HTONL(cmlds->as_capable);
+		break;
 	}
 }
 
diff --git a/tlv.h b/tlv.h
index 8b51ffd..d8875b3 100644
--- a/tlv.h
+++ b/tlv.h
@@ -129,6 +129,7 @@ enum management_action {
 #define MID_UNICAST_MASTER_TABLE_NP			0xC008
 #define MID_PORT_HWCLOCK_NP				0xC009
 #define MID_POWER_PROFILE_SETTINGS_NP			0xC00A
+#define MID_CMLDS_INFO_NP				0xC00B
 
 /* Management error ID values */
 #define MID_RESPONSE_TOO_BIG				0x0001
@@ -323,6 +324,12 @@ typedef struct Integer96 {
 	uint16_t fractional_nanoseconds;
 } PACKED ScaledNs;
 
+struct cmlds_info_np {
+	TimeInterval meanLinkDelay;
+	Integer32    scaledNeighborRateRatio;
+	uint32_t     as_capable;
+} PACKED;
+
 struct follow_up_info_tlv {
 	Enumeration16 type;
 	UInteger16    length;
-- 
2.39.2

[Linuxptp-devel] [PATCH v3 2/6] pmc/uds: Configure the remote server address using the interface API.

[Richard C. <ric...@gm...>]

The UDS uses a global setting from the configuration to set the remote
server address.  As a result, a program cannot act as both a UDS
client and server, because the source and destination addresses would
be identical.  Fix the issue by allowing the remote address to be
configurable.

Signed-off-by: Richard Cochran <ric...@gm...>
---
 pmc.c        | 6 ++++--
 pmc_agent.c  | 3 ++-
 pmc_common.c | 8 ++++----
 pmc_common.h | 6 +++---
 tz2alt.c     | 3 ++-
 uds.c        | 6 ++++--
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/pmc.c b/pmc.c
index 9faf790..d18fea0 100644
--- a/pmc.c
+++ b/pmc.c
@@ -823,8 +823,10 @@ int main(int argc, char *argv[])
 	print_set_syslog(1);
 	print_set_verbose(1);
 
-	pmc = pmc_create(cfg, transport_type, iface_name, boundary_hops,
-			 domain_number, transport_specific, zero_datalen);
+	pmc = pmc_create(cfg, transport_type, iface_name,
+			 config_get_string(cfg, NULL, "uds_address"),
+			 boundary_hops, domain_number, transport_specific,
+			 zero_datalen);
 	if (!pmc) {
 		fprintf(stderr, "failed to create pmc\n");
 		config_destroy(cfg);
diff --git a/pmc_agent.c b/pmc_agent.c
index 494c174..fec6dea 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -232,7 +232,8 @@ int run_pmc_wait_sync(struct pmc_agent *node, int timeout)
 int init_pmc_node(struct config *cfg, struct pmc_agent *node, const char *uds,
 		  pmc_node_recv_subscribed_t *recv_subscribed, void *context)
 {
-	node->pmc = pmc_create(cfg, TRANS_UDS, uds, 0,
+	node->pmc = pmc_create(cfg, TRANS_UDS, uds,
+			       config_get_string(cfg, NULL, "uds_address"), 0,
 			       config_get_int(cfg, NULL, "domainNumber"),
 			       config_get_int(cfg, NULL, "transportSpecific") << 4, 1);
 	if (!node->pmc) {
diff --git a/pmc_common.c b/pmc_common.c
index 5092c09..fca16c6 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -488,9 +488,9 @@ struct pmc {
 };
 
 struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
-		       const char *iface_name, UInteger8 boundary_hops,
-		       UInteger8 domain_number, UInteger8 transport_specific,
-		       int zero_datalen)
+		       const char *iface_name, const char *remote_address,
+		       UInteger8 boundary_hops, UInteger8 domain_number,
+		       UInteger8 transport_specific, int zero_datalen)
 {
 	struct pmc *pmc;
 	UInteger32 proc_id;
@@ -524,7 +524,7 @@ struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
 		goto failed;
 	}
 
-	pmc->iface = interface_create(iface_name, NULL);
+	pmc->iface = interface_create(iface_name, remote_address);
 	if (!pmc->iface) {
 		pr_err("failed to create interface");
 		goto failed;
diff --git a/pmc_common.h b/pmc_common.h
index 6fb2fae..355b2c0 100644
--- a/pmc_common.h
+++ b/pmc_common.h
@@ -29,9 +29,9 @@
 struct pmc;
 
 struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
-		       const char *iface_name, UInteger8 boundary_hops,
-		       UInteger8 domain_number, UInteger8 transport_specific,
-		       int zero_datalen);
+		       const char *iface_name, const char *remote_address,
+		       UInteger8 boundary_hops, UInteger8 domain_number,
+		       UInteger8 transport_specific, int zero_datalen);
 
 void pmc_destroy(struct pmc *pmc);
 
diff --git a/tz2alt.c b/tz2alt.c
index feb77a5..65b5835 100644
--- a/tz2alt.c
+++ b/tz2alt.c
@@ -181,7 +181,8 @@ static int update_ptp_serivce(struct tzinfo *tz, struct tzinfo *next)
 	struct pmc *pmc;
 	int err;
 
-	pmc = pmc_create(cfg, TRANS_UDS, uds_local, 0,
+	pmc = pmc_create(cfg, TRANS_UDS, uds_local,
+			 config_get_string(cfg, NULL, "uds_address"), 0,
 			 config_get_int(cfg, NULL, "domainNumber"),
 			 config_get_int(cfg, NULL, "transportSpecific") << 4, 1);
 	if (!pmc) {
diff --git a/uds.c b/uds.c
index 6d39dc8..fafb100 100644
--- a/uds.c
+++ b/uds.c
@@ -54,7 +54,7 @@ static int uds_open(struct transport *t, struct interface *iface, struct fdarray
 		    enum timestamp_type tt)
 {
 	char *uds_ro_path = config_get_string(t->cfg, NULL, "uds_ro_address");
-	char *uds_path = config_get_string(t->cfg, NULL, "uds_address");
+	const char *uds_path = interface_remote(iface);
 	struct uds *uds = container_of(t, struct uds, t);
 	const char *name = interface_name(iface);
 	const char* file_mode_cfg;
@@ -89,7 +89,9 @@ static int uds_open(struct transport *t, struct interface *iface, struct fdarray
 	/* For client use, pre load the server path. */
 	memset(&sa, 0, sizeof(sa));
 	sa.sun_family = AF_LOCAL;
-	strncpy(sa.sun_path, uds_path, sizeof(sa.sun_path) - 1);
+	if (uds_path) {
+		strncpy(sa.sun_path, uds_path, sizeof(sa.sun_path) - 1);
+	}
 	uds->address.sun = sa;
 	uds->address.len = sizeof(sa);
 
-- 
2.39.2