linuxptp-devel — Development channel for the linuxptp project

Re: [Linuxptp-devel] [PATCH] lstab: Update leapfile validity

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

Hi,

The leap file from IETF was removed.
Seems ftp.nist.gov is no longer available.
And I could not find "leap-seconds.list" on nist.gov
I did find the information in web page:
https://www.nist.gov/pml/time-and-frequency-division/time-realization/leap-seconds

Perhaps we should remove these obsolete URLs?

Erez



On Tue, 21 Nov 2023 at 09:35, Maciek Machnikowski <ma...@ma...>
wrote:

> No leap seconds will happen till 28 June 2024. Update leapfile validity
> accordingly.
> Add a new source of leapsecond file from the IERS.
>
> Signed-off-by: Maciek Machnikowski <ma...@ma...>
> ---
>  lstab.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lstab.c b/lstab.c
> index df8cce0..2aa87be 100644
> --- a/lstab.c
> +++ b/lstab.c
> @@ -27,6 +27,8 @@
>   *
>   * ftp://ftp.nist.gov/pub/time/leap-seconds.list
>   *
> + * https://hpiers.obspm.fr/iers/bul/bulc/ntp/leap-seconds.list
> + *
>   * When updating this table, do not forget to set N_HISTORICAL_LEAPS
>   * and the expiration date.
>   */
> @@ -51,7 +53,7 @@ struct lstab {
>         int length;
>  };
>
> -static const uint64_t expiration_date_ntp = 3912710400ULL; /* 28 December
> 2023 */
> +static const uint64_t expiration_date_ntp = 3928521600ULL; /* 24 June
> 2024 */
>
>  static const uint64_t offset_table[N_LEAPS * 2] = {
>         2272060800ULL,  10,     /* 1 Jan 1972 */
> --
> 2.30.2
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

[Linuxptp-devel] [PATCH] lstab: Update leapfile validity

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

No leap seconds will happen till 28 June 2024. Update leapfile validity
accordingly.
Add a new source of leapsecond file from the IERS.

Signed-off-by: Maciek Machnikowski <ma...@ma...>
---
 lstab.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lstab.c b/lstab.c
index df8cce0..2aa87be 100644
--- a/lstab.c
+++ b/lstab.c
@@ -27,6 +27,8 @@
  *
  * ftp://ftp.nist.gov/pub/time/leap-seconds.list
  *
+ * https://hpiers.obspm.fr/iers/bul/bulc/ntp/leap-seconds.list
+ *
  * When updating this table, do not forget to set N_HISTORICAL_LEAPS
  * and the expiration date.
  */
@@ -51,7 +53,7 @@ struct lstab {
 	int length;
 };
 
-static const uint64_t expiration_date_ntp = 3912710400ULL; /* 28 December 2023 */
+static const uint64_t expiration_date_ntp = 3928521600ULL; /* 24 June 2024 */
 
 static const uint64_t offset_table[N_LEAPS * 2] = {
 	2272060800ULL,	10,	/* 1 Jan 1972 */
-- 
2.30.2

Re: [Linuxptp-devel] [PATCH] Add support for DELAY_REQ and SYNC packets rx_filter

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

On Tue, Nov 14, 2023 at 06:50:26PM +0300, IlorDash wrote:
> Honestly, I don't know, is it necessary to dynamically switch RX filters,
> If the device supports EVENT RX filters, because with EVENT RX filters it
> can handle all PTP messages. Maybe you could suggest appropriate behaviour
> for utility in that case?

Selectively timestamping only sync messages can be useful in networks
with very large numbers of clients sharing the same communication
path, so they don't need to timestamp each others delay requests and
possibly miss the timestamp of server's sync message.

If the hardware supports the type-specific filters, I think ptp4l
should be able to set it.

-- 
Miroslav Lichvar

Re: [Linuxptp-devel] port immediately enters faulty state

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

On Fri, Nov 17, 2023 at 03:29:16PM -0800, Cliff Spradlin via Linuxptp-devel wrote:
> I'm investigating why a port immediately enters a faulty state on
> startup (and then self-recovers).
> 
> It looks like there might be some buggy rtnl logic.

This sounds familiar... wasn't there a bug in this area?
Are you on the latest sha?

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Fri, Nov 17, 2023 at 08:27:43PM -0800, Richard Cochran wrote:
> How does this requirement improve synchronization?
> 
> What benefit does it bring to users of the PTP?

rhetorical questions  :^(

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Fri, Nov 17, 2023 at 01:51:18PM +0100, Andrew Zaborowski wrote:

> It is exposed on the wire in the Pdelay messages.  Compliance tests
> look at this.  They also simulate a few hypothetical scenarios like a
> domain 0 PTP port trying to communicate with a CMLDS link port since
> 1588 talks about this.

How does this requirement improve synchronization?

What benefit does it bring to users of the PTP?

Thanks,
Richard

[Linuxptp-devel] port immediately enters faulty state

[Cliff S. <csp...@go...>]

I'm investigating why a port immediately enters a faulty state on
startup (and then self-recovers).

It looks like there might be some buggy rtnl logic.

In rtnl_link_status(), the following code is used to pass info to callers:

------
int slave_index = -1;
...
if (tb[IFLA_LINKINFO])
  slave_index = rtnl_linkinfo_parse(index, tb[IFLA_LINKINFO]);
...
cb(ctx, link_up, slave_index);
-----------

Note that if slave_index is not set, then it will default to -1.

The problem is that in the callback port_link_status(), the passed
index is used in to check if the ethernet label has changed:

----
void port_link_status(void *ctx, int linkup, int ts_index) {
if (if_indextoname(ts_index, ts_label) && strcmp(old_ts_label, ts_label))
  p->link_status |= TS_LABEL_CHANGED;
-----

The "weird" thing is that if_indextoname() succeeds even and returns
an empty string when -1 is passed, whereas it seems like it should
return NULLPTR. Still, we should be passing the regular if index.

The effect of this is that TS_LABEL_CHANGED is set immediately and
that causes the port to go into a FAULTY state.

I think the fix here is to set slave_index to index if IFLA_LINKINFO
is not set. That said, I'm not really familiar with RTNL logic so I'm
not sure what this all means or why that is not set in this case. The
kernel version is Debian 6.5.6-1.

-cliff

Re: [Linuxptp-devel] linuxptp doesn't detect HW timestamp capability of BCM57416

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

On Fri, 17 Nov 2023 at 16:24, Pham, Calvin via Linuxptp-devel <
lin...@li...> wrote:

> Hi Richard,
>
> I am running Oracle Linux 9.1 in my server which is equipped with
> BCM57416. This Broadcom NIC supports IEEE-1588. But when I check if it
> supports HW Timestamp, it’s not there.
>
>
>
> [root@COTS-DL380Gen11-39 ~]# ethtool -T eth0
>
> Time stamping parameters for eth0:
>
> Capabilities:
>
>         software-transmit
>
>         software-receive
>
>         software-system-clock
>
> *PTP Hardware Clock: none*
>

You need PHC (PTP hardware clock) on the NIC, to use PTP.
It could be a driver issue.
Looking on Broadcom page I see no reference to PTP or IEEE 1588.
https://www.broadcom.com/products/ethernet-connectivity/network-adapters/bcm57416-1gbase-t-ic
I can not tell you if the NIC supports PTP or not.

Anyhow this mailing list is for linuxptp and not for Broadcom's NIC.

You should try in forums for Broadcom NICs.
Or Linux kernel modules forums.

Erez


Hardware Transmit Timestamp Modes: none
>
> Hardware Receive Filter Modes: none
>
>
>
> The kernel version is 5.14.0-284.30.0.1.el9_2.x86_64.
>
> The driver is bnxt_en and I cannot find its version.
>
> I cannot find the version of ptp module cause it is built-in, and I am
> using linuxptp-2.0.
>
>
>
> Can you give some help?
>
>
>
> Thanks!
>
>
>
> Calvin P.
>
>
>
>
>
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

[Linuxptp-devel] linuxptp doesn't detect HW timestamp capability of BCM57416

[Pham, C. <Cal...@ne...>]

Hi Richard,
I am running Oracle Linux 9.1 in my server which is equipped with BCM57416. This Broadcom NIC supports IEEE-1588. But when I check if it supports HW Timestamp, it's not there.

[root@COTS-DL380Gen11-39 ~]# ethtool -T eth0
Time stamping parameters for eth0:
Capabilities:
        software-transmit
        software-receive
        software-system-clock
PTP Hardware Clock: none
Hardware Transmit Timestamp Modes: none
Hardware Receive Filter Modes: none

The kernel version is 5.14.0-284.30.0.1.el9_2.x86_64.
The driver is bnxt_en and I cannot find its version.
I cannot find the version of ptp module cause it is built-in, and I am using linuxptp-2.0.

Can you give some help?

Thanks!

Calvin P.

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Fri, 17 Nov 2023 at 05:50, Richard Cochran <ric...@gm...> wrote:
> On Fri, Nov 17, 2023 at 01:41:19AM +0100, Andrew Zaborowski wrote:
> > Do you want to require the user to enforce that the port numbering is
> > the same between the ptp4l processes?
>
> No.

(I meant: do you want to require that the user takes care to maintain
the order of interfaces on the command line / in the config file,
which I think you do -- Ok)

>
> > In our use case spec compliance
> > is the priority, including the unique clockIdentity for CMLDS
> > requirement, so we'd definitely need to be running a separate ptp4l
> > process for CMLDS in this schema.
>
> The clockIdentity is not exposed, so what do you mean by "compliance"?

It is exposed on the wire in the Pdelay messages.  Compliance tests
look at this.  They also simulate a few hypothetical scenarios like a
domain 0 PTP port trying to communicate with a CMLDS link port since
1588 talks about this.

Best regards

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Fri, Nov 17, 2023 at 01:41:19AM +0100, Andrew Zaborowski wrote:
> On Thu, 16 Nov 2023 at 23:46, Richard Cochran <ric...@gm...> wrote:
> > No, I mean the PTP port number.  These are taken from the order of the
> > interfaces on the command line and in the configuration file.
> 
> Won't this be the same as the UDS message's sourcePortIdentity.portNumber?

Ah, yes, you are right.  Even simpler!

> Do you want to require the user to enforce that the port numbering is
> the same between the ptp4l processes?

No.

> In our use case spec compliance
> is the priority, including the unique clockIdentity for CMLDS
> requirement, so we'd definitely need to be running a separate ptp4l
> process for CMLDS in this schema.

The clockIdentity is not exposed, so what do you mean by "compliance"?

> I'm asking because the port
> numbering thing is unobvious and the user effort to configure a
> compliant setup with ptp4l is already very high.

That is just par for the course.  I'm not the one making up tons of
crazy new options.

The strength of linuxptp is modularity and fine grained, configurable
options.  This means that a) ptp4l works even with equipment that
fails to follow the standards and/or profiles, and b) ptp4l can be
really hard to configure.

> So one solution would be to let the user configure a custom portNumber
> under port settings.

Port numbers start with 1 and increase in the order of the configured
interfaces.  If it really is required for CMLDS ports to have
*different* numbers than the normal ports, we can add a "port index
offset" option that would start the numbering at some given value.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Thu, 16 Nov 2023 at 23:46, Richard Cochran <ric...@gm...> wrote:
> On Thu, Nov 16, 2023 at 11:11:50PM +0100, Andrew Zaborowski wrote:
> > The two timestamps are passed to clock_peer_delay() by the receiving
> > port and stored in c->tsproc.  Then they're accessed by
> > get_raw_delay() which is used in the filter logic.  I'm not sure how
> > much value that has, we can possibly pass 0s to clock_peer_delay().
>
> If the client uses CMLDS, then it doesn't need tsproc logic at all.
> It simply take the delay value from the CMLDS server.

Make sense, the CMLDS would already be filtering the timestamps once.

>
> > Regarding "source_port_index", I assume that would contain the ifindex
> > of the interface?  With virtual clocks I believe the PHC indices may
> > differ between the PTP ports on one physical port ("link port").
>
> No, I mean the PTP port number.  These are taken from the order of the
> interfaces on the command line and in the configuration file.

Won't this be the same as the UDS message's sourcePortIdentity.portNumber?

Do you want to require the user to enforce that the port numbering is
the same between the ptp4l processes?  In our use case spec compliance
is the priority, including the unique clockIdentity for CMLDS
requirement, so we'd definitely need to be running a separate ptp4l
process for CMLDS in this schema.  I'm asking because the port
numbering thing is unobvious and the user effort to configure a
compliant setup with ptp4l is already very high.

So one solution would be to let the user configure a custom portNumber
under port settings.  My colleague even had a local change to allow
passing -i <ifname>,<portNumber>.  This would also remove the
(possibly unobvious) requirement to run all ports sharing the
clockIdentity as one ptp4l process.

Another option would be to pass the ifindex as I thought you were suggesting.

Best regards

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Thu, Nov 16, 2023 at 11:11:50PM +0100, Andrew Zaborowski wrote:

> The two timestamps are passed to clock_peer_delay() by the receiving
> port and stored in c->tsproc.  Then they're accessed by
> get_raw_delay() which is used in the filter logic.  I'm not sure how
> much value that has, we can possibly pass 0s to clock_peer_delay().

If the client uses CMLDS, then it doesn't need tsproc logic at all.
It simply take the delay value from the CMLDS server.
 
> Regarding "source_port_index", I assume that would contain the ifindex
> of the interface?  With virtual clocks I believe the PHC indices may
> differ between the PTP ports on one physical port ("link port").

No, I mean the PTP port number.  These are taken from the order of the
interfaces on the command line and in the configuration file.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Thu, 16 Nov 2023 at 05:27, Richard Cochran <ric...@gm...> wrote:
> On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote:
> > @@ -1129,6 +1130,27 @@ 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;
> > +             /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid because
> > +              * we have no extra field to convey that separately.
> > +              */
> > +             cmlds->serviceMeasurementValid =
> > +                     target->peer_portid_valid && !target->pdr_missing &&
> > +                     !target->multiple_pdr_detected &&
> > +                     target->nrate.ratio_valid;
> > +             cmlds->meanLinkDelay = target->peerMeanPathDelay;
> > +             cmlds->scaledNeighborRateRatio =
> > +                     (Integer32) (target->nrate.ratio * POW2_41 - POW2_41);
>
> > +             /* 16.6.3.2: "Upon receipt of a request for information, the
> > +              * Common Mean Link Delay Service may in addition return the
> > +              * raw measurement data gathered by the service for use in
> > +              * estimating the <meanLinkDelay> and <neighborRateRatio>."
> > +              */
> > +             cmlds->egress_ts = tmv_to_nanoseconds(target->peer_delay_t1);
> > +             cmlds->rx_ts = tmv_to_nanoseconds(target->peer_delay_t2);
>
> Please drop these two fields.  They don't provide any benefit.  If the
> clients don't trust the CMLDS values, then they are free to measure
> the p2p delay themselves!

The two timestamps are passed to clock_peer_delay() by the receiving
port and stored in c->tsproc.  Then they're accessed by
get_raw_delay() which is used in the filter logic.  I'm not sure how
much value that has, we can possibly pass 0s to clock_peer_delay().

Regarding "source_port_index", I assume that would contain the ifindex
of the interface?  With virtual clocks I believe the PHC indices may
differ between the PTP ports on one physical port ("link port").

Best regards

Re: [Linuxptp-devel] [PATCH] Problem: Under GNSS error condition/fluctuation, ts2phc updating 1sec time into PHC resulting in wrong time.

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

On Thu, Nov 16, 2023 at 10:42:33AM +0000, ramesh t wrote:

> Under GNSS fluctuation condition due bad weather or improper
> location of GNSS antenna, observed ts2phc process updates 1 sec
> difference into the physical hard clock (PHC) of the NIC.  This 1
> sec jump is seen momentarily as captured below.

What is the root cause of the one second offset?

You need to identify the problem.  This issue sounds like a bug in the
GNSS radio firmware.  It does not seem like a bug in linuxptp.

I don't take patches that work around buggy third party equipment!

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH] Problem: Under GNSS error condition/fluctuation, ts2phc updating 1sec time into PHC resulting in wrong time.

[ramesh t <ram...@ya...>]

Thanks Richard for the feedback.
Please find update comments with code changes.

>From a8e9b8ade110ff9eb9acc2bb28c6e3d1db136c29 Mon Sep 17 00:00:00 2001
From: Ramesh Gowda <ram...@ma...>
Date: Thu, 12 Oct 2023 15:26:27 +0530
Subject: [PATCH] GNSS fluctuation resulting in wrong update of NIC PHC time.


Context:
Under GNSS fluctuation condition due bad weather or improper location of GNSS antenna, observed ts2phc process updates 1 sec difference into the physical hard clock (PHC) of the NIC.
This 1 sec jump is seen momentarily as captured below.


    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.611] nmea delay: 89943030 ns
    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.611] enp81s0f0 extts index 0 at 1678461497.000000000 corr 0 src 1678461498.844774711 diff -1000000000
    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.611] enp81s0f0 master offset -1000000000 s2 freq -100000000
    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.612] nmea delay: 89943030 ns
    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.612] enp138s0f0 extts index 0 at 1678461497.000000000 corr 0 src 1678461498.845574965 diff -1000000000
    Mar 10 15:17:40 sno-cluster-nrnl04-master1 ts2phc: [161756.612] enp138s0f0 master offset -1000000000 s2 freq -100000000


Problem:
ts2phc process will update wrong timing into PHC, this time is periodically copied into system timing using phc2sys process.
This will result in sudden jump of time in system clock impacting others processes which are tightly bound to system timing.


Solution:
With ts2phc in SERVO_LOCKED state, time difference between PHC and GPS would be in +-1ns.
But under problem condition, the time difference will be +-1 sec.
To prevent wrong update of time, defined a new configurable variable called skip count.
With SERVO_LOCKED state and time diff is more than 500 msecond, ts2phc will skip update of PHC for configured skip count value.
Default skip count value would be set 120 (2 minutes).
Time difference stays more than 500 msecond for continuous intervals more than skip count value, ts2phc will update the PHC with time difference value.
---
 config.c       |  1 +
 ts2phc.c       |  3 +++
 ts2phc_slave.c | 23 +++++++++++++++++++++++
 3 files changed, 27 insertions(+)


diff --git a/config.c b/config.c
index d237de9..2a9ee6b 100644
--- a/config.c
+++ b/config.c
@@ -331,6 +331,7 @@ struct config_item config_tab[] = {
     GLOB_ITEM_INT("utc_offset", CURRENT_UTC_OFFSET, 0, INT_MAX),
     GLOB_ITEM_INT("verbose", 0, 0, 1),
     GLOB_ITEM_INT("write_phase_mode", 0, 0, 1),
+    PORT_ITEM_INT("max_phc_update_skip_cnt", 120, 0, 14400),
 };
 
 static struct unicast_master_table *current_uc_mtab;
diff --git a/ts2phc.c b/ts2phc.c
index 2342858..5687c9b 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -15,6 +15,8 @@
 #include "ts2phc_slave.h"
 #include "version.h"
 
+int max_phc_update_skip_count;
+
 struct interface {
     STAILQ_ENTRY(interface) list;
 };
@@ -146,6 +148,7 @@ int main(int argc, char *argv[])
     print_set_verbose(config_get_int(cfg, NULL, "verbose"));
     print_set_syslog(config_get_int(cfg, NULL, "use_syslog"));
     print_set_level(config_get_int(cfg, NULL, "logging_level"));
+    max_phc_update_skip_count = config_get_int(cfg, NULL, "max_phc_update_skip_cnt");
 
     STAILQ_FOREACH(iface, &cfg->interfaces, list) {
         if (1 == config_get_int(cfg, interface_name(iface), "ts2phc.master")) {
diff --git a/ts2phc_slave.c b/ts2phc_slave.c
index 749efe5..6af1aeb 100644
--- a/ts2phc_slave.c
+++ b/ts2phc_slave.c
@@ -29,6 +29,8 @@
 #define SAMPLE_WEIGHT        1.0
 #define SERVO_SYNC_INTERVAL    1.0
 
+extern int max_phc_update_skip_count;
+
 struct ts2phc_slave {
     char *name;
     STAILQ_ENTRY(ts2phc_slave) list;
@@ -42,6 +44,8 @@ struct ts2phc_slave {
     clockid_t clk;
     int no_adj;
     int fd;
+    int max_offset_skip_count;
+    int current_offset_skip_count;
 };
 
 struct ts2phc_slave_array {
@@ -219,6 +223,10 @@ static struct ts2phc_slave *ts2phc_slave_create(struct config *cfg, const char *
         goto no_ext_ts;
     }
 
+    slave->max_offset_skip_count = max_phc_update_skip_count;
+    slave->current_offset_skip_count = 0;
+    pr_debug("PHC slave %s has skip cnt %d", device, slave->max_offset_skip_count);
+
     return slave;
 no_ext_ts:
 no_pin_func:
@@ -278,6 +286,17 @@ static int ts2phc_slave_event(struct ts2phc_slave *slave,
     adj = servo_sample(slave->servo, offset, extts_ts,
                SAMPLE_WEIGHT, &slave->state);
 
+    if ((slave->state == SERVO_LOCKED) || (slave->state == SERVO_LOCKED_STABLE)) {
+        if (llabs(offset) >= NS_PER_SEC / 2) {
+            if (slave->current_offset_skip_count < slave->max_offset_skip_count) {
+                pr_debug("Skip current PHC update %s offset %10" PRId64 " s%d freq %+7.0f",
+                    slave->name, offset, slave->state, adj);
+                slave->current_offset_skip_count++;
+                return 0;
+            }
+        }
+    }
+
     pr_debug("%s master offset %10" PRId64 " s%d freq %+7.0f",
          slave->name, offset, slave->state, adj);
 
@@ -290,6 +309,10 @@ static int ts2phc_slave_event(struct ts2phc_slave *slave,
         break;
     case SERVO_LOCKED:
     case SERVO_LOCKED_STABLE:
+        if (llabs(offset) < 500) {
+            slave->current_offset_skip_count = 0;
+        }
+
         clockadj_set_freq(slave->clk, -adj);
         break;
     }
-- 
2.20.1





On Friday, November 10, 2023 at 11:46:05 AM GMT+5:30, Richard Cochran <ric...@gm...> wrote: 





On Thu, Nov 09, 2023 at 06:07:57PM +0000, ramesh t via Linuxptp-devel wrote:


> Update Code review request. Can you please provide comments?


You have not identified any problem, and so I cannot comment.

A proper commit message has three elements:

1. context
2. problem
3. solution

That means writing at least three complete, coherent sentences.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v4 0/4] add get_pins_cfg command to display pin functions

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

On Mon, Jun 26, 2023 at 12:03:29PM -0700, Jacob Keller wrote:
> Extend phc_ctl with a new get_pins_cfg command which calls the
> PTP_PIN_GETFUNC ioctl on a PHC clock and prints the pin configuration for
> each pin.
> 
> Changes since v3:
> * remove use of PTP_PIN_SETFUNC2 and PTP_PIN_GETFUNC2 at Erez's request
> * avoid converting PTP_PF_NONE in pin_func_string()
> * check clkid >= 0 instead of clkid == CLOCK_REALTIME
> * add a patch to do the same clkid check in do_caps()

Jacob,

Sorry for the delay.  I wanted to merge this series, but there is a
conflict.  Can you fix it up in a v5?

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Wed, Nov 15, 2023 at 09:45:52PM -0800, Richard Cochran wrote:
> On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote:
> 
> > @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv {
> >         UInteger16    numberOfBitsAfterTimestamp;
> >  } PACKED;
> >  
> > +struct cmlds_info_np {
> > +	Integer8     serviceMeasurementValid;
> > +	TimeInterval meanLinkDelay;
> > +	Integer32    scaledNeighborRateRatio;
> 
> I think you need one more field, like "source_port_index"
> 
> If a client subscribes to TLVs from multiple ports, then it needs a
> way to tell which is which.

Or maybe the subscription will cause the CMLDS server to forward all
p2p delay measurements from all p2p ports.

Still the TLVs needs to include the port index.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 4/9] Update the PdelayReq/Res flows for CMLDS Link Ports

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

On Wed, Nov 15, 2023 at 08:58:58PM -0800, Richard Cochran wrote:
> The text of 1588 strongly suggests that the CMLDS service be stand
> alone daemon.  However, we can provide the same functionality without
> the extra complexity, by simply letting ptp4l serve the measured peer
> delay to any local client.

And, if you _really_ want to have a stand alone CMLDS server with its
own clockIdentity etc, then that is easy:

ptp4l \
	--clientOnly=1 \
	--clockIdentity=100000.2000.300000 \
	--domainNumber=0 \
	--free_running=1 \
	--delay_mechanism=P2P \
	-i eth0

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation

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

On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote:

> @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv {
>         UInteger16    numberOfBitsAfterTimestamp;
>  } PACKED;
>  
> +struct cmlds_info_np {
> +	Integer8     serviceMeasurementValid;
> +	TimeInterval meanLinkDelay;
> +	Integer32    scaledNeighborRateRatio;

I think you need one more field, like "source_port_index"

If a client subscribes to TLVs from multiple ports, then it needs a
way to tell which is which.

> +	Integer64    egress_ts;
> +	Integer64    rx_ts;
> +} PACKED;

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 6/9] Add plumbing for interacting with the CMLDS

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

On Wed, Nov 15, 2023 at 09:05:45PM -0800, Richard Cochran wrote:

> I'd like to have this:
> 
> 1. provide the new TLV via the PUSH mechanism.
> 
> 2. let clients using DM_COMMON_P2P subscribe to the new TLV.
> 
> 3. let the client update their peer delay values based on an incoming TLV.

For clients to do this, maybe the cleanest way is to add a new UDS
client FD into the 'struct fdarray' (see fd.h)

Thanks,
Richard

Re: [Linuxptp-devel] CMLDS Patches

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

On Mon, Aug 21, 2023 at 10:44:27AM +0200, Walfred Tedeschi wrote:
> On 11.08.23 19:13, Richard Cochran wrote:
> > The CMLDS series will require careful review.  My superficial review
> > gave me the impression that the approach was too complex.
> Do you have an indication on how it could be simplified?

I commented on the v2 patch series just now.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 4/9] Update the PdelayReq/Res flows for CMLDS Link Ports

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

On Mon, May 15, 2023 at 06:26:07PM -0400, Kishen Maloor wrote:

> For ports that are configured with "run_cmlds=1", i.e. CMLDS Link
> Ports, this change updates port_pdelay_request(), process_pdelay_req() and
> port_peer_delay() to utilize the CMLDS sdoid/domainNumber/portId
> along code paths where necessary.

We already have options to configure those attributes.  The use can
simply configure the port that will do the p2p measurement
accordingly.

> It also ensures that
> neighborRateRatio is calculated on CMLDS Link Ports and is available
> to users of MID_CMLDS_INFO_NP. Further, process_pdelay_request()
> enforces two-step Pdelay when responding as CMLDS.

Again, we already have a way to configure this.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 9/9] Make allowedLostResponses configurable

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

On Mon, May 15, 2023 at 06:26:12PM -0400, Kishen Maloor wrote:
> This change adds 'allowedLostResponses' as a per-port parameter
> with a default value of 3 (per IEEE 802.1AS-2011, clause 11.5.3).
> (Note that this matches the value of the previously #define'd
> ALLOWED_LOST_RESPONSES, so this change does not alter any prevailing
> behavior)

This is not really in scope for the CMDLS feature.

Did you see this recent post?

     08.Nov'23 Chwee-Lin Choon      [Linuxptp-devel] [PATCH 0/2] Enhanced Handling of Multiple Pdelay Responses
     08.Nov'23 Chwee-Lin Choon      ├─>[Linuxptp-devel] [PATCH 1/2] Make allowedLostResponses configurable
     08.Nov'23 Chwee-Lin Choon      └─>[Linuxptp-devel] [PATCH 2/2] port: Fix multiple pdelay response handling

Please review it, as it also implements allowedLostResponses.

Thanks,
Richard

Re: [Linuxptp-devel] [RFC PATCH v2 7/9] Implement the COMMON_P2P delay mechanism

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

On Mon, May 15, 2023 at 06:26:10PM -0400, Kishen Maloor wrote:
> From: Andrew Zaborowski <and...@in...>
> 
> This change implements the COMMON_P2P DM by issuing a request
> to the CMLDS for CommonMeanLinkDelayInformation upon expiry of
> the delay timer and handles the response to assimilate the
> received meanPathDelay and NRR.

Cleints using COMMON_P2P don't need to do anything in the delay timer.

Just use the latest pushed TLV from the server.

Thanks,
Richard