Re: [Linuxptp-devel] [PATCH 4/4] ts2phc: Add serial baudrate option

2021-07-19 Thread Richard Cochran
On Mon, Jul 19, 2021 at 11:00:40AM +, Geva, Erez wrote:
> This looks like a call to send the a patch with these options.
> 
> Richard, How do you think these options should be implemented?
> A single enumerator, separate integer for each?

Whichever is easier.  Maybe a single code like 8n1 parsed by sscanf
format "%d%c%d" ?

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCHv2 RFC 0/6] Support for dropping root privileges

2021-07-18 Thread Richard Cochran
On Tue, Jul 13, 2021 at 05:08:36PM +0200, Miroslav Lichvar wrote:
> In the v1 discussion there was a suggestion to not use libcap. I have
> not looked into that yet, but I can try it if this whole thing makes
> sense otherwise.

IIRC, after V1, I looked at avoiding libcap, but I came to the
conclusion that using libcap is the better way and that worth adding
the dependency.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] FW: [PATCH] Always send with a unique sequence count from uds. Only forward responses to UDS port with corresponding requests on the UDS port.

2021-07-18 Thread Richard Cochran
On Sat, Jul 03, 2021 at 03:59:24AM +, Eric Decker wrote:
> Subject: [PATCH] Always send with a unique sequence count from uds. Only 
> forward responses to UDS port with corresponding requests on the UDS port.

Please format the commit with:

1. A brief, one sentence subject line.

2. A commit message of that provides three pieces of information:

   1. The context of the issue/feature.
   2. The problem that needs fixing (or missing feature).
   3. How the proposed solution fixes the problem.

   (This means that the commit message is at least three complete English 
sentences)

> @@ -1404,17 +1405,83 @@ static void clock_forward_mgmt_msg(struct clock *c, 
> struct port *p, struct ptp_m  {
>   struct port *piter;
>   int pdulen = 0, msg_ready = 0;
> + static UInteger16 mgmt_seq_count = 0;
> + UInteger16 temp_seq_count = 0;
>  
> + struct uds_mgmt_req {
> + UInteger16 seqId;
> + Enumeration16 id;
> + };
> + #define MGMT_REQ_Q_SIZE 4
> + static struct uds_mgmt_req uds_req[MGMT_REQ_Q_SIZE] = {0};
> + static unsigned int uds_req_index = 0;
> + int forward_to_uds = 0;
> + struct management_tlv *mgt = (struct management_tlv *) 
> +msg->management.suffix;

Your mailer mangled that line ^^^

> + 
>   if (forwarding(c, p) && msg->management.boundaryHops) {
>   pdulen = msg->header.messageLength;
>   msg->management.boundaryHops--;
> +
> + if (p == c->uds_rw_port)
> + {

Please follow CODING_STYLE.org

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] config: Add workaround for glibc getopt_long().

2021-07-18 Thread Richard Cochran
On Tue, Jul 13, 2021 at 10:31:16AM +0200, Miroslav Lichvar wrote:
> getopt_long() in glibc allows shortened long option names, e.g.
> ptp4l --domain works as --domainNumber. When the match is ambiguous,
> e.g. --fault matches --fault_badpeernet_interval and
> --fault_reset_interval, it is supposed to return an error, but that
> works only if their struct option have different flags or vals.

I don't get it... even with your change, all of the 'vals' are zero.
There are not different from one and other.

> https://sourceware.org/bugzilla/show_bug.cgi?id=28081

The last comment says, "you need to use a different val".

But even with your patch, all the opts.val fields are zero.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v2] Increase the default tx_timestamp_timeout to 10

2021-07-18 Thread Richard Cochran
On Thu, Jul 08, 2021 at 12:59:30PM -0700, Jacob Keller wrote:
> The tx_timestamp_timeout configuration defines the number of
> milliseconds to wait for a Tx timestamp from the kernel stack. This
> delay is necessary as Tx timestamps are captured after a packet is sent
> and reported back via the socket error queue.
> 
> The current default is to poll for up to 1 millisecond. In practice, it
> turns out that this is not always enough time for hardware and software
> to capture the timestamp and report it back. Some hardware designs
> require reading timestamps over registers or other slow mechanisms.
> 
> This extra delay results in the timestamp not being sent back to
> userspace within the default 1 millisecond polling time. If that occurs
> the following can be seen from ptp4l:
> 
>   ptp4l[4756.840]: timed out while polling for tx timestamp
>   ptp4l[4756.840]: increasing tx_timestamp_timeout may correct this issue,
>but it is likely caused by a driver bug
>   ptp4l[4756.840]: port 1 (p2p1): send sync failed
>   ptp4l[4756.840]: port 1 (p2p1): MASTER to FAULTY on FAULT_DETECTED
>(FT_UNSPECIFIED)
> 
> This can confuse users because it implies this is a bug, when the
> correct solution in many cases is to just increase the timeout to
> a slightly higher value.
> 
> Since we know this is a problem for many drivers and hardware designs,
> lets increase the default timeout.
> 
> Note that a longer timeout should not affect setups which return the
> timestamp quickly. On modern kernels, the poll() call will return once
> the timestamp is reported back to the socket error queue. (On old
> kernels around the 3.x era the poll will sleep for the full duration
> before reporting the timestamp, but this is now quite an old kernel
> release).
> 
> Signed-off-by: Jacob Keller 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] lstab: update expiration to 28 December 2021

2021-07-18 Thread Richard Cochran
On Tue, Jun 29, 2021 at 11:47:21AM +0800, Yangbo Lu wrote:
> Bring the built in leap second table up to date through IERS Bulletin C59.

The bulletin is C62, not C59.

See:

   https://datacenter.iers.org/data/html/bulletinc-062.html

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 4/4] ts2phc: Add serial baudrate option

2021-07-18 Thread Richard Cochran
On Sun, Jul 18, 2021 at 07:09:54PM -0700, Richard Cochran wrote:
> Otherwise, the patch is fine.  I'll fix the order myself.

Also, I set a minimum of 300.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 4/4] ts2phc: Add serial baudrate option

2021-07-18 Thread Richard Cochran
On Fri, May 14, 2021 at 10:16:02AM -0700, Hal Murray wrote:
> 
> > I did think of that, yet of the many serial devices I have used they have 
> > all
> > been running with the setting 8/N/1 and no HW flow control.
> 
> The HP Z3801A is 19200 baud, 7 data bits, odd parity

Looks like it is configurable on that device?

8n1 is almost universally used.

We can add a second option if people really are stuck with
non-configurable equipment.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 4/4] ts2phc: Add serial baudrate option

2021-07-18 Thread Richard Cochran
On Fri, May 14, 2021 at 01:33:44PM +0200, Lars Munch wrote:
> Add serial baudrate configuration option. Default to 9600 bps.
> 
> Signed-off-by: Lars Munch 
> ---
>  config.c |  1 +
>  ts2phc.8 | 23 ---
>  ts2phc_nmea_master.c | 10 +-
>  3 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/config.c b/config.c
> index 4472d3d..a42a57f 100644
> --- a/config.c
> +++ b/config.c
> @@ -319,6 +319,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_STR("ts2phc.nmea_remote_host", ""),
>   GLOB_ITEM_STR("ts2phc.nmea_remote_port", ""),
>   GLOB_ITEM_STR("ts2phc.nmea_serialport", "/dev/ttyS0"),
> + PORT_ITEM_INT("ts2phc.nmea_baudrate", 9600, 0, INT_MAX),

In the future, please preserve alphabetical order of the options
strings.

Otherwise, the patch is fine.  I'll fix the order myself.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] ts2phc: Close socket on peer shutdown

2021-07-18 Thread Richard Cochran
On Fri, May 14, 2021 at 01:33:42PM +0200, Lars Munch wrote:
> Read returns 0 (the traditional "end-of-file" return) when a remote
> peer performs an orderly shutdown. Hence, ts2phc needs to close the
> socket and try to establish a new connection.
> 
> Signed-off-by: Lars Munch 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 3/4] ts2phc: Update leapfile documentation

2021-07-18 Thread Richard Cochran
On Fri, May 14, 2021 at 01:33:43PM +0200, Lars Munch wrote:
> Update leapfile documentation to note the file will be
> reloaded if modified.
> 
> Signed-off-by: Lars Munch 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/4] ts2phc: Fix uninitialized variable in nmea_scan_rmc

2021-07-18 Thread Richard Cochran
On Fri, May 14, 2021 at 01:33:41PM +0200, Lars Munch wrote:
> tm_isdst needs to be initialized to make sure mktime does not fail
> on recent versions of glibc
> 
> See:
> https://bugzilla.redhat.com/show_bug.cgi?id=1653340
> https://sourceware.org/bugzilla/show_bug.cgi?id=24630
> 
> Signed-off-by: Lars Munch 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Richard Cochran
On Wed, Jul 14, 2021 at 11:20:00AM +, Keller, Jacob E wrote:

> I think for Tx the challenges are higher: the timestamp is taken
> after we've filled in the descriptor and sent the frame. The only
> place it could reasonably be stored again is the descriptor
> writeback (since we don't get completion messages).

Right, the would be the place to do it.

> If I remember correctly, the challenge here is that in a traditional
> ring model the writeback is completed much earlier than the
> timestamp so we potentially delay cleanup of other packets by
> waiting to insert the timestamp into the writeback.

If *every* frame gets a time stamp, then their write-backs would all
be delayed by the same amount.  Hence no clean up operations would be
"delayed".  They would all take the same amount of time.

The only cost would be in space to keep the data for the write-back
around until the time stamp becomes available.  Paying the price of
the little extra memory is well worth it, as it simplifies the time
stamping logic and removes every class of problem related to time
stamp delivery.

IOW, KISS!

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Richard Cochran
On Wed, Jul 14, 2021 at 11:20:23AM +, Keller, Jacob E wrote:
> What about at least checking for the case where a timestamp was never
> started? Drivers are supposed to set a flag in the SKB when they start a
> timestamp (and not set it if they can't start it).

How could that happen?

Putting aside egregious driver bugs, it is hard for me to imagine a
use case that would cause this failure mode.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Richard Cochran
On Mon, Jul 12, 2021 at 03:02:50PM +, Keller, Jacob E wrote:

> Right. Though.. running something like ptp4l on the same connection
> could be problematic if the applications aren't working together
> because most hardware supports a single request at once,

I wouldn't say "most".  Surely some HW is like that, but I never
counted.  I always hoped that HW designers would get a clue a simply
provide a time stamp for each and every frame, Tx and Rx, in the
buffer descriptors.  No filters, no parsers, just a big on/off switch.
It would be way easier to implement in HW, and it would solve all of
these sorts of problems.

> so if both
> applications send a request at the same time one of them will
> fail. They would need to either ensure they're off-sync or be
> communicating to each other about when its ok to timestamp request
> somehow.

Oh, that will make the users of the new PHC vclock thing happy!  I can
already hear the complaints and bug reports here on our lists...

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Richard Cochran
On Mon, Jul 12, 2021 at 05:02:58PM -0700, Vinicius Costa Gomes wrote:
> Speaking of future improvements, wouldn't it be easier if the
> kernel/driver was able to notify userspace that a timestamping request
> wasn't able to be serviced?

It would fall to the drivers to implement that correctly.  I doubt the
situation would improve.  We'd only end up chasing another class of
bugs.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Richard Cochran
On Thu, Jul 08, 2021 at 07:15:17PM +, Machnikowski, Maciej wrote:
> Can it be a half of the packet rate?

No!

> Or there is any reason to make a specific tighter
> limit to it?

See the discussion of the effect of computational delay on stability
in John Eidson's "Measurement, Control, and Communication Using IEEE
1588" section "5.2 Clock Servo Design", especially the analysis
starting on page 153.

   https://www.springer.com/gp/book/9781846282508

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Richard Cochran
On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for the timeout, or continue processing after the timestamp is 
> > received?
> 
> The poll() call is waiting for the descriptor, so it should return as
> soon as the timestamp is ready. The option sets the maximum time it
> waits.

But on kernels older than (mumble) (3.5?) the code will sleep the
entire period, then wake to read the time stamp.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-07 Thread Richard Cochran
On Wed, Jul 07, 2021 at 05:02:21PM -0700, Jacob Keller wrote:
> diff --git a/config.c b/config.c
> index 4472d3d9d6f9..f33f177c696a 100644
> --- a/config.c
> +++ b/config.c
> @@ -323,7 +323,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_INT("ts2phc.pulsewidth", 5, 100, 99900),
>   PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
>   GLOB_ITEM_INT("twoStepFlag", 1, 0, 1),
> - GLOB_ITEM_INT("tx_timestamp_timeout", 1, 1, INT_MAX),
> + GLOB_ITEM_INT("tx_timestamp_timeout", 5, 1, INT_MAX),

Let's make it 10 ms.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] tx_timestamp_timeout default

2021-07-07 Thread Richard Cochran
On Wed, Jul 07, 2021 at 11:46:16PM +, Keller, Jacob E wrote:

> Either way, I found that whether I used a kthread or not I was
> unable to avoid the timeout issue with ice hardware because the
> delay is caused by the method we must use to access the Tx
> timestamps :( We get into the kthread function within a few hundred
> usec or less, and then the firmware read takes a long time,
> sometimes over 2 milliseconds.

Yes, but when using "work" that 2 ms can easily become 200 ms or more
on a busy RT system.  The work is sched_other, and as such it is
effectively running at the lowest priority WRT the sched_fifo tasks.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] tx_timestamp_timeout default

2021-07-07 Thread Richard Cochran
On Wed, Jul 07, 2021 at 10:22:59PM +, Keller, Jacob E wrote:
> I am wondering if there would be support for either (a) increasing
> the default timeout, or (b) adding something to the PTP get
> capabilities for indicating a sort of default timeout for the
> device, and if it's not set in the config then the default timeout
> is used?

I wouldn't mind increasing the default.

I doubt capabilities would help, because much depends on the system
being used.  We really should replace "work" with the kthread in the
drivers, and then tell people to give the kthreads sched_fifo using
chrt.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Support in Annex P of IEEE 1588-2019 - linuxptp

2021-07-06 Thread Richard Cochran
On Sun, Jul 04, 2021 at 08:45:05AM +, Ariel Almog wrote:
> We are mostly interested in Annex P implementation and in particular
> authentication TLV in high priority and ipsec.
> 
> Can you share some information on current status, demand, and future plans?

I took a close look at the new security features, and I did develop an
idea of both how to implement them and the effort involved.

However, I have no immediate plans to work on this.  Perhaps somebody
else does...

In any case, before there can be any talk of implementaion, there is a
big open question on the 16.14 security mechanism.  There are two
flavors:

1. Immediate security processing

2. Delayed security processing (16.14.3.6 optional disclosedKey)

#1 makes sense, but #2 does not make any sense at all.  At least, I
can't see how to use #2 in any practical way.

I know how to implement #1, but I have doubts about #2.  I think #2,
as described in the standard, is totally useless.

In the example in the standard, the disclosedKey arrives once per day.
The question is, what does a client do when the disclosedKey
invalidates the previously received messages?  AFAICT, the client
would be hopelessly lost.  After all, you cannot simply "undo" 24
hours worth of synchronization.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] linuxptp: Fixes published for CVE-2021-3570 and CVE-2021-3571

2021-07-05 Thread Richard Cochran
Dear list,

Now that the embargo period has expired, I published fixes for:

   CVE-2021-3570 linuxptp: missing length check of forwarded messages
   CVE-2021-3571 linuxptp: wrong length of one-step follow-up in transparent 
clock

The fixes have been published to SourceForge and to GitHub:

   https://sourceforge.net/projects/linuxptp
   https://github.com/richardcochran/linuxptp

The tags with the fixes are as follows:

   v1.5.1
   v1.6.1
   v1.7.1
   v1.8.1
   v1.9.3
   v2.0.1
   v3.1.1

In addition, the head of the master branch (soon to be version 3.2)
also includes the fixes.

Although it is possible to apply the fix to versions 1.2, 1.3, and
1.4, those versions are obsolete and do not pass our CI tests.  For
this reason I decided to withdraw them instead.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/2] gPTP profile interoperability problems

2021-06-21 Thread Richard Cochran
On Mon, Jun 14, 2021 at 12:03:45PM +, Miklas, Marcin via Linuxptp-devel 
wrote:

> gPTP requires that PTP_TIMESCALE flag is set in messages. I noticed that
> PDelayReq, PDelayResp, PDelayRespFollowUp, Sync, FollowUp and Signaling all 
> have
> that flag set to 0. One of the bridges just ignores communication with ptp4l
> because of that.
> 
> The fault is on both sides, because gPTP specification says that PTP_TIMESCALE
> should be set to 1 and ignored on reception.

No, the fault is not on both sides.

> Since fixing linuxptp is easier I created a patch which should fix that 
> problem.

Adding a hack into linuxptp is easier than fixing the actual problem
in the other implementation, but that is a poor excuse.

> We can see that ptp4l slave is requesting gPTP master to use master's initial
> sync interval.  This might be different than slave's initial sync interval. If
> this is a case the signallings will be send over and over again.

No, in a correctly configured gPTP network, all of the nodes have the
same settings.

> Slave's own
> initalLogSyncInterval should be used here.
> 
> Current version is causing other problems with OpenAvnu's daemon_cl.

So fix daemon_cl.

NAK to both of these patches.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Intel 210 to Intel 255

2021-06-07 Thread Richard Cochran
On Mon, Jun 07, 2021 at 02:01:28PM +, Geva, Erez wrote:

> Jun  7 15:44:07 ipc01 ptp4l: [673.869] timed out while polling for tx 
> timestamp
> Jun  7 15:44:07 ipc01 ptp4l: [673.869] increasing tx_timestamp_timeout may 
> correct this issue, but it is likely caused by a driver bug

Try  --tx_timestamp_timeout=100

HTH,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v2 3/3] port: Don't check timestamps from non-slave ports.

2021-05-28 Thread Richard Cochran
On Fri, May 28, 2021 at 09:26:07AM +, Amar Subramanyam via Linuxptp-devel 
wrote:
> We could see that ptp4l already has RTNL functionality and hence this 
> port_renew_transport() seems to be unnecessary redundancy.

The link notifications from the kernel are best effort and are not
guaranteed.  So renewing the transport is the safe thing to do.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/2] Log optimization for ptp4l in jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-27 Thread Richard Cochran
On Wed, May 26, 2021 at 12:24:07PM +0300, Amar Subramanyam via Linuxptp-devel 
wrote:
> The port other than SLAVE (LISTENING port) prints an error
> "port 1: master state recommended in slave only mode
> ptp4l[1205469.356]: port 1: defaultDS.priority1 probably misconfigured"
> for every ANNOUNCE RECEIPT Timeout.
> 
> This log is printed when the event EV_RS_MASTER is thrown
> in clientOnly mode. But single port clientOnly mode will never
> hit this event instead EV_RS_GRAND_MASTER will be hit.
> EV_RS_MASTER is thrown when clientOnly=1 and boundary_clock_jbod=1
> which results in continuous printing. So EV_RS_MASTER check when
> clientOnly=1 to print this error can be avoided.
> 
> Signed-off-by: Amar Subramanyam 
> Signed-off-by: Karthikkumar Valoor 
> Signed-off-by: Ramana Reddy 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/2] Log optimization for ptp4l in jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-27 Thread Richard Cochran
On Wed, May 26, 2021 at 12:24:06PM +0300, Amar Subramanyam via Linuxptp-devel 
wrote:
> The LISTENING port prints continuously
> "selected best master clock 00..03
> updating UTC offset to 37"
> 
> We limited the log such that now it prints only when there is a
> change in the best-master clock.
> 
> Signed-off-by: Amar Subramanyam 
> Signed-off-by: Karthikkumar Valoor 
> Signed-off-by: Ramana Reddy 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and clock

2021-05-27 Thread Richard Cochran
On Tue, May 25, 2021 at 08:28:34PM +, Tim Martin wrote:
> Currently there is no way to programmatically access statistics about
> the clock frequency offset, time delay, or time offset (collectively,
> the "clock_stats" metrics), except for parsing the ptp4l logs.  One
> option for time offset would be to poll TLV_TIME_STATUS_NP in regular
> intervals from a custom client, but that has the disadvantage of either
> very high poll rates or missed updates which would affect the statistics.

Not if you use the push method.

commit 6d7c090706e76af334185ffcec9cc56d0570e215
Author: Juergen Werner 
Date:   Wed Jan 20 20:11:34 2021 +0100

Implement push notification for TIME_STATUS_NP

Subscribers to NOTIFY_TIME_SYNC will be notified on every clock
synchronization.


Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v3 1/1] Add master only management TLV

2021-05-25 Thread Richard Cochran
On Mon, May 24, 2021 at 01:24:10AM +0200, Erez Geva wrote:
> - Add support in the pmc tool
> - Add the TLV in port module.
> 
> Signed-off-by: Erez Geva 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/2] Log optimization for ptp4l in jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-24 Thread Richard Cochran
On Mon, May 24, 2021 at 11:24:47AM +0200, Miroslav Lichvar wrote:
> Is there something else that is not already covered by the
> clock_update_time_properties() call from port.c?

Let's see...

void clock_update_time_properties(struct clock *c, struct 
timePropertiesDS tds)
{
c->tds = tds;
}

That is just the TDS.  The other method has more...

static void clock_update_slave(struct clock *c)
c->cur.stepsRemoved= 1 + 
c->best->dataset.stepsRemoved;

This could change if the network topology changes.

pds->parentPortIdentity= c->best->dataset.sender;

Ditto.

pds->grandmasterIdentity   = 
msg->announce.grandmasterIdentity;

I guess this won't change, but

pds->grandmasterClockQuality   = 
msg->announce.grandmasterClockQuality;
pds->grandmasterPriority1  = 
msg->announce.grandmasterPriority1;
pds->grandmasterPriority2  = 
msg->announce.grandmasterPriority2;

these three could change, even if the GM remains the same.

c->tds.currentUtcOffset= msg->announce.currentUtcOffset;
c->tds.flags   = msg->header.flagField[1];
c->tds.timeSource  = msg->announce.timeSource;

if ((c->tds.flags & UTC_OFF_VALID && c->tds.flags & 
TIME_TRACEABLE) ||
(c->tds.currentUtcOffset > c->utc_offset)) {
pr_info("updating UTC offset to %d", 
c->tds.currentUtcOffset);
c->utc_offset = c->tds.currentUtcOffset;
This update only happens here and not in clock_update_time_properties().
}

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v2 1/1] Add master only management TLV

2021-05-23 Thread Richard Cochran
On Mon, Apr 26, 2021 at 11:18:21AM +0200, Erez Geva wrote:

> diff --git a/pmc_common.c b/pmc_common.c
> index c5cd992..2ab32d4 100644
> --- a/pmc_common.c
> +++ b/pmc_common.c
> @@ -106,6 +106,7 @@ struct management_id idtab[] = {
>   { "ALTERNATE_TIME_OFFSET_PROPERTIES", 
> TLV_ALTERNATE_TIME_OFFSET_PROPERTIES, not_supported },

TLV_ALTERNATE_TIME_OFFSET_PROPERTIES0x2021

>   { "TRANSPARENT_CLOCK_DEFAULT_DATA_SET", 
> TLV_TRANSPARENT_CLOCK_DEFAULT_DATA_SET, not_supported },

TLV_ALTERNATE_TIME_OFFSET_PROPERTIES0x2021

>   { "PRIMARY_DOMAIN", TLV_PRIMARY_DOMAIN, not_supported },

TLV_PRIMARY_DOMAIN  0x4002

> + { "MASTER_ONLY", TLV_MASTER_ONLY, do_get_action },

TLV_MASTER_ONLY 0x3001  out of order!

>   { "TIME_STATUS_NP", TLV_TIME_STATUS_NP, do_get_action },

TLV_TIME_STATUS_NP  0xC000

>   { "GRANDMASTER_SETTINGS_NP", TLV_GRANDMASTER_SETTINGS_NP, do_set_action 
> },

TLV_GRANDMASTER_SETTINGS_NP 0xC001

>   { "SUBSCRIBE_EVENTS_NP", TLV_SUBSCRIBE_EVENTS_NP, do_set_action },

TLV_SUBSCRIBE_EVENTS_NP 0xC003


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v4] Rework twoStepFlag in order to handle one step on port basis.

2021-05-23 Thread Richard Cochran
On Thu, Apr 22, 2021 at 09:18:33AM +0200, Luigi 'Comio' Mantellini wrote:
> With this patch we introduce the twoStepFlag evaluation at port level.

This isn't going to work.  The two step flag is an element of a clock,
not port, data set.

8.2.1.2.1 defaultDS.twoStepFlag

The value of defaultDS.twoStepFlag shall be TRUE if the clock is a
two-step clock; otherwise, the value shall be FALSE.

Sorry,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Set domainNumber for telecom examples

2021-05-23 Thread Richard Cochran
On Wed, Apr 21, 2021 at 07:41:48PM +0200, Lars Munch wrote:
> Set the default domain numbers according to the ITU-T standards
> 
> Signed-off-by: Lars Munch 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Handle step_window at port level.

2021-05-23 Thread Richard Cochran
On Sun, May 23, 2021 at 09:31:39AM -0700, Richard Cochran wrote:
> On Mon, Apr 19, 2021 at 06:11:34PM +0200, Luigi 'Comio' Mantellini wrote:
> > From: Luigi Mantellini 
> > 
> > The step_window functionality should be defined at the port level because
> > we cannot assume that different ports have the same sync message rate.

And BTW, when submitting v2, v3, ... please always include a brief
Change Log explaining what has changed.  That helps the reviewers keep
track and makes their job easier.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Handle step_window at port level.

2021-05-23 Thread Richard Cochran
On Mon, Apr 19, 2021 at 06:11:34PM +0200, Luigi 'Comio' Mantellini wrote:
> From: Luigi Mantellini 
> 
> The step_window functionality should be defined at the port level because
> we cannot assume that different ports have the same sync message rate.
> ---
>  clock.c| 13 +
>  clock.h|  5 +++--
>  config.c   |  2 +-
>  port.c | 18 +-
>  port.h |  7 +++
>  port_private.h |  1 +
>  6 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/clock.c b/clock.c
> index e545a9b..6072ea0 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -710,6 +710,9 @@ static void clock_update_slave(struct clock *c)
>   pr_info("updating UTC offset to %d", c->tds.currentUtcOffset);
>   c->utc_offset = c->tds.currentUtcOffset;
>   }
> +
> + // Port changed
> + c->step_window_counter = 0;
>  }
>  
>  static int clock_utc_correct(struct clock *c, tmv_t ingress)
> @@ -1103,7 +1106,8 @@ struct clock *clock_create(enum clock_type type, struct 
> config *config,
>   c->kernel_leap = config_get_int(config, NULL, "kernel_leap");
>   c->utc_offset = config_get_int(config, NULL, "utc_offset");
>   c->time_source = config_get_int(config, NULL, "timeSource");
> - c->step_window = config_get_int(config, NULL, "step_window");
> + c->step_window = 0;
> + c->step_window_counter = 0;

If you really want to move the step window into the port, then delete
these fields from the clock data structure.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/2] Log optimization for ptp4l in jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-23 Thread Richard Cochran
On Sat, May 22, 2021 at 05:09:07PM +0300, Amar Subramanyam via Linuxptp-devel 
wrote:
> The LISTENING port prints continuously
> "selected best master clock 00..03
> updating UTC offset to 37"
> 
> We limited the log such that now it prints only when there is a
> change in the best-master clock.
> 
> Signed-off-by: Amar Subramanyam 
> Signed-off-by: Karthikkumar Valoor 
> Signed-off-by: Ramana Reddy 
> ---
>  clock.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/clock.c b/clock.c
> index e545a9b..2a95d79 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1939,14 +1939,6 @@ static void handle_state_decision_event(struct clock 
> *c)
>   best_id = c->dds.clockIdentity;
>   }
>  
> - if (cid_eq(_id, >dds.clockIdentity)) {
> - pr_notice("selected local clock %s as best master",
> -   cid2str(_id));
> - } else {
> - pr_notice("selected best master clock %s",
> -   cid2str(_id));
> - }
> -
>   if (!cid_eq(_id, >best_id)) {
>   clock_freq_est_reset(c);
>   tsproc_reset(c->tsproc, 1);
> @@ -1957,6 +1949,13 @@ static void handle_state_decision_event(struct clock 
> *c)
>   c->master_local_rr = 1.0;
>   c->nrr = 1.0;
>   fresh_best = 1;
> + if (cid_eq(_id, >dds.clockIdentity)) {
> + pr_notice("selected local clock %s as best master",
> + cid2str(_id));
> + } else {
> + pr_notice("selected best master clock %s",
> + cid2str(_id));
> + }

This is a nice improvement as it prevent log spam in common
situations.

>   }
>  
>   c->best = best;
> @@ -1983,7 +1982,9 @@ static void handle_state_decision_event(struct clock *c)
>   event = EV_RS_PASSIVE;
>   break;
>   case PS_SLAVE:
> - clock_update_slave(c);
> + if (fresh_best) {
> + clock_update_slave(c);
> + }

This is not correct.  Elements of the upstream data sets might have
changed, and the port needs to stay up to date.

>   event = EV_RS_SLAVE;
>   break;
>   default:
> -- 
> 1.8.3.1

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] gpsd support in ts2phc

2021-05-14 Thread Richard Cochran
On Thu, May 13, 2021 at 07:31:27PM +0200, Lars Munch wrote:
> On Sat, May 8, 2021 at 8:47 PM Richard Cochran  
> wrote:
> >
> > On Sat, May 08, 2021 at 02:57:30PM +0200, Lars Munch wrote:
> >
> > > 1. Use the time provided by gpsd to ntpshm. This can be implemented 
> > > without
> > > dependencies to gpsd.
> >
> > Doesn't sound aweful, but not great either.
> 
> I kind of like the simplicity of this solution. No need for extra
> daemons to pipe the
> data from gpsd to ts2phc. The thing I do not like is the polling that
> it would have to
> do, but that's only a few times per second. Anything else that I have
> missed which
> is awful?
> 
> Does this mean I should not waste my time implementing this as you can already
> now say it will not be accepted?

There are two design questions that need answering.

1. Will this work reliably?  That is, will the ntpshm (almost) always
   have correct time?  Or will this become a constant source of
   questions on the list to the tune of, "I used linuxptp as a GPS GM
   and it does not work ..."  I do NOT want to become a support
   channel for gpsd.

2. How can the GM logic determine whether the time in the ntpshm is
   verified to be globally correct or not?

With the present solution using RMC, we have a simple flag (field 2 is
either "A" or "V") that signals the validity.  It is practically
impossible to determine generically whether the reading from a
particular GNSS radio is valid.  For this reason we push the
responsibility to an external program.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Sync issues observed when ptp4l is ran with jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-12 Thread Richard Cochran
On Wed, May 12, 2021 at 10:28:34AM +0200, Miroslav Lichvar wrote:
> I think the fix should be one of the following:
> - disable clock check in jbod mode (it cannot work reliably as it is)
> - limit the check to timestamps from the synchronized port

This sounds like the best choice to me.

> - have a separate clock check instance for each clock, checking only
>   its own timestamps

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Sync issues observed when ptp4l is ran with jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-12 Thread Richard Cochran
On Wed, May 12, 2021 at 10:57:24AM +, Amar Subramanyam wrote:
> Should we rate limit log (a) as it will be printed whenever BMCA is triggered 
> and avoid log (b) when boundary_clock_jbod=1 and clientOnly=1

Sounds reasonable to me.

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Sync issues observed when ptp4l is ran with jbod and client only mode (clientOnly=1 and boundary_clock_jbod=1)

2021-05-11 Thread Richard Cochran
On Tue, May 11, 2021 at 02:16:04PM +, Amar Subramanyam via Linuxptp-devel 
wrote:

> What happens here exactly is, due to the continuous triggering of
> BMCA, the value of mono_interval (the interval between two
> successive calls of clock_check_sample ()) gets increased and
> SYNCRONIZATION FAULT occurs with the default value of
> sanity_freq_limit (which is 2).

Ah!  Now we are getting somewhere!

This is a bug in the sanity check.  I've seen it, too.  Let's fix the bug.

> Modifying the configuration with --sanity_freq_limit=0 will prevent
> the FAULT from occurring , but it will not address the root cause of
> BMCA getting triggered continuously even though there is no change
> in successive announce messages in the port.

No, running the BMCA is not the root cause.  It is perfectly harmless
to run the BMCA with the same inputs and get the same result.  In
fact, the 1588 standard specifies doing exactly this, over and over
again.

9.2.6.8 STATE_DECISION_EVENT

...

The STATE_DECISION_EVENT shall:

-  Logically occur simultaneously on all ports of a clock
-  Occur at least once per Announce message transmission interval

IEEE Std 1588-2008, page 81

The linuxptp implement does not follow this directive all the time,
because the BMCA has no time component.  If the inputs have not
changed, then the output remains the same.

But you see that re-running the BMCA is harmless and not a bug.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] gpsd support in ts2phc

2021-05-08 Thread Richard Cochran
On Sat, May 08, 2021 at 02:57:30PM +0200, Lars Munch wrote:

> 1. Use the time provided by gpsd to ntpshm. This can be implemented without
> dependencies to gpsd.

Doesn't sound aweful, but not great either.

> 2. Use libgpsd to get the time, but that adds a dependency to gpsd

No, I won't use that library in linuxptp.  Before implementing the
current solution in nmea.c, I tried really hard to use gpsd, hoping
that it would "just work" because of the multiple different nmea and
binary protocols that it has.  That seemed like a compelling feature.
However, I couldn't get it to work with either a commodity gps or a
high end, survey quality gps.
 
> I might also be able to use gpspipe and socat to get the NMEA sentences
> from gpsd to ts2phc, but that is not very elegant.

This is best approach IMO.  You can wrap gpspipe|socat in a systemd
job to get automatic restarting and logging and so on.  If the
gpspipe|socat combination is too flaky to keep running, then writing a
little C or python program to read the UART and publish two sockets is
pretty easy.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Announce message generates SDE on MasterOnly port

2021-05-04 Thread Richard Cochran
On Tue, May 04, 2021 at 02:24:07PM +0200, Luigi 'Comio' Mantellini wrote:
> As side note, In my testing scenario the master-only ports receive
> announce messages and when the slave port loses the signal the node
> evolves in grand-master flooding the log with "assuming the grand
> master role" message. Not a critical issue, of course... but in this
> case we lost time with logging and blocking the main loop.

Right, there are a few configurations that result in log spam.  It
makes sense to inhibit and/or rate limit the spam, but without hurting
the "normal" logging.

However, it doesn't make sense to fill the code with random tests and
logic for bizarre or seldom used configurations.

Many of the options from the so-called PTP profiles are poorly
designed, but we try to support them as best we can, without gross
hackery.
 
> My 2Eurocents.

Oh, those are worth 2.44 of my cents!

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Announce message generates SDE on MasterOnly port

2021-05-04 Thread Richard Cochran
On Mon, May 03, 2021 at 06:52:02PM +0200, Luigi 'Comio' Mantellini wrote:
> I noticed that the Announce messages received on MasterOnly ports
> generate a SDE condition in bc_event().

and so what?  What is the problem?

> I think that we can return
> EV_NONE when the port is a master_only (or we can skip the
> process_annonce() at all). Is it correct?

No thanks, I don't want special cases sprinkled all over.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Port down and sde

2021-05-04 Thread Richard Cochran
On Mon, May 03, 2021 at 06:45:56PM +0200, Luigi 'Comio' Mantellini wrote:
> /*
> * A port going down can affect the BMCA result.
> * Force a state decision event.
> */
> if (p->link_status & LINK_DOWN)
> clock_set_sde(p->clock, 1);
> 
> I think that should be removed

Why?  What problem does it cause?

> because the calling code look like this:
> 
> case FD_RTNL:
> pr_debug("port %hu: received link status notification", portnum(p));
> transport_rtnl_link_status(p->trp, fd, p->name, port_link_status, p);
> if (p->link_status == (LINK_UP | LINK_STATE_CHANGED))
> return EV_FAULT_CLEARED;
> else if ((p->link_status == (LINK_DOWN | LINK_STATE_CHANGED)) ||
>  (p->link_status & TS_LABEL_CHANGED))
> return EV_FAULT_DETECTED;
> else
> return EV_NONE;
> 
> raising a EV_FAULT_DETECT on LINK_DOWN

So the SDE gets triggered twice?  Not really a problem.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Planning release 3.2

2021-05-04 Thread Richard Cochran
On Mon, May 03, 2021 at 05:09:09PM +0200, Michael Walle wrote:
> did I miss something or wasn't there a 3.2 release?

You didn't miss anything.  I have been delayed in pushing out the 3.2
release.

Sorry,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [Linuxptp-users] phc2sys jump into huge value

2021-04-29 Thread Richard Cochran
On Thu, Apr 29, 2021 at 03:04:07PM +, ramesh t via Linuxptp-users wrote:

> Did time profiling using time ticks. Under the problem condition,
> observing clock_gettime of interface connected to BC is taking more
> time ticks. This results in phc offset jumping to 4 digit value
> momentarily. Also i'm not sure if reading across numa is triggering
> this issue.  Please suggest a way to fix in the phc2sys offset
> code?1) Should we prevent update on phc2sys value if there is
> momentarily jump?

You can use 'chrt' to give phc2sys scheduling priority so that it will
not be preempted.

HTH,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] To support Ordinary Clock-Subordinate/Slave just a bunch of devices(jbod) feature.

2021-04-27 Thread Richard Cochran
On Tue, Apr 27, 2021 at 03:59:05PM +, Ramana Reddy wrote:

>  As I said, running independent clients defeats the purpose of BMC 
> algorithm and breaks the ITU-T G.8275.2
> Spec compliance. The BMC algorithm should be run locally on all ports of 
> every ordinary and boundary clock in a domain. Since it 
> runs continuously, it continually readapts to changes in the network or the 
> clocks. Pls check section 9.3 in IEEE1588-2008
> Spec for details on BMC algorithm. Also refer to Section 6.7 of A-BMCA 
> requirements of ITU-T G.8275.2 spec.

Did you even read ITU-T G.8275.2 Section 6.7 ?

It makes no mention of slaveOnly, but rather masterOnly and
localPriority.

You have not clearly identified a problem with linuxptp WRT G.8275.2.
If there is a problem, please state it.

> >  I believe Ordinary Clock can have multiple ports

No, your belief is incorrect.  The number of ports differentiates OC
from BC.  In fact, that is the only difference.

>  Pls refer my earlier reply for why we need to run single ptp4l 
> instance with multiple ports in OC mode.

You _seem_ to be proposing a new and different state machine, not
specified by 1588 or G.8275.2.  If you want to do that, fine, but
please develop some kind of design document that explains both the
motivation and the theory of operation.

Then, you can provide the implementation in the modular way that does
not deface the linuxptp architecture.

Hint:
p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] To support Ordinary Clock-Subordinate/Slave just a bunch of devices(jbod) feature.

2021-04-27 Thread Richard Cochran
On Tue, Apr 27, 2021 at 05:28:46PM +0200, Miroslav Lichvar wrote:
> An advantage of having multiple independent clients is that you can
> better detect failures in synchronization and avoid corrupting the
> other clocks.

Right.  You can query the ptp4l instances (for example via pmc) and
then pick the one to use (for CLOCK_REALTIME or whatever) based on the
reported PARENT_DATA_SET.

> The specification defines an ordinary clock as "A PTP Instance that
> has a single PTP Port in its domain and maintains the timescale used
> in the domain."
> 
> If you run ptp4l with multiple specified interfaces, it cannot be an
> ordinary clock. The boundary_clock_jbod and clientOnly options don't
> matter here.

+1
 
> I read the original patch+report again and it's not clear to me why
> you need the port to be in the passive state.
> 
> I tried --boundary_clock_jbod=1 --clientOnly=1 with two interfaces and
> it seems to be switching them between the LISTENING and
> UNCALIBRATED/SLAVE states as expected.

Yeah, that is what I saw.

Thanks,
Richard




___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/1] Add master only management TLV

2021-04-25 Thread Richard Cochran
On Thu, Apr 22, 2021 at 03:04:17PM +0200, Erez Geva wrote:

> diff --git a/pmc.c b/pmc.c
> index a767c8a..00d6014 100644
> --- a/pmc.c
> +++ b/pmc.c
> @@ -335,6 +335,11 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
>   fprintf(fp, "TIMESCALE_PROPERTIES "
>   IFMT "ptpTimescale %d", mtd->val & PTP_TIMESCALE ? 1 : 
> 0);
>   break;
> + case TLV_MASTER_ONLY:
> + mtd = (struct management_tlv_datum *) mgt->data;
> + fprintf(fp, "MASTER_ONLY "
> + IFMT "masterOnly %d", mtd->val);
> + break;

These switch/case tables are in numeric order.

Could you please put this entry between TLV_VERSION_NUMBER (0x200C)
and TLV_DELAY_MECHANISM (0x6000)?

Ditto for the other tables.

>   case TLV_TIME_STATUS_NP:
>   tsn = (struct time_status_np *) mgt->data;
>   fprintf(fp, "TIME_STATUS_NP "

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/1] Add master only management TLV

2021-04-25 Thread Richard Cochran
On Thu, Apr 22, 2021 at 09:40:46AM -0700, Jacob Keller wrote:
> Most of these do sound like great features to have, but many would
> require significant architecture changes. As LinuxPTP is an open source
> project, you (or your customers) are free to work on such improvements
> and contribute them back.

If wishes were horses, beggars would ride.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/1] Add master only management TLV

2021-04-22 Thread Richard Cochran
On Thu, Apr 22, 2021 at 01:35:11PM +, Geva, Erez wrote:
> You need to verify the ptp4l can dynamically change the master only flag.

+1

> As the 'slave only' flag does not allow set, I skip it for now.

Right, and it wouldn't work if added naively.

> Only priorities 1 and  2 can be changed at the moment.

They do work dynamically.
 
> However as the 'master only' is a port action and not clock action as 'slave 
> only', it might be easier to implement and verify.

Right, many of the SET actions would require re-initializing the
stack, but this isn't trivial to implement.  I tried adding more SET
actions some time ago, but I soon ran into corner cases in the code
that spoiled everything.  It is one of the weaknesses of linuxptp.

As practical matter, in my own projects I simply re-start the start
when the configuration changes.  Although not ideal, that is easier
than re-working linuxptp to properly handle each dynamic variable.

Sorry,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Fix uninitialized variable in nmea_scan_rmc

2021-04-21 Thread Richard Cochran
On Wed, Apr 21, 2021 at 07:28:59PM +0200, Lars Munch wrote:
> My tests show (with recent glibc version) that with TZ=UTC mktime will
> return -1 for tm_isdst=1. For tm_isdst=0 and tm_isdst=-1 mktime returns the
> same value.
> 
> IMHO, tm_isdst=0 makes the most sense for TZ=UTC as UTC does not support
> DST.

Okay, sounds good.  Thanks for following up.

Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] RFC: add port.twoStepFlag option in order to handle one step on port basis.

2021-04-21 Thread Richard Cochran
On Wed, Apr 21, 2021 at 07:16:29PM +0200, luigi.mantell...@gmail.com wrote:
> Sorry I don't understand.
> 
> Are you suggest to use twoStepFlag in this way:
> 
> PORT_ITEM_INT("twoStepFlag", 0, 0, 1)

Yes, sorry, that is what I meant.

> Then ptpt.conf should look like this:
> 
> [global]
> twoStepFlag 1
> 
> [eth1]
> twoStepFlag 0
> 
> [eth2]
> twoStepFlag 1
> 
> [eth3]
> # twoStepFlag ND -> inherit the [globa]/twoStepFlag

Right.

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] RFC: add port.twoStepFlag option in order to handle one step on port basis.

2021-04-21 Thread Richard Cochran
On Tue, Apr 20, 2021 at 12:35:08PM +0200, Luigi 'Comio' Mantellini wrote:
> port.twoStepFlag.
> 
> When -1, inherit the global twoStepFlag value, otherwise enable two-step mode
> for sync messages on port basis. One-step mode can be used only with hardware
> time stamping. The default is -1 (as global twoStepFlag value).
> ---
>  config.c |  1 +
>  port.c   | 40 +++-
>  ptp4l.8  |  4 
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/config.c b/config.c
> index 4472d3d..13dcea8 100644
> --- a/config.c
> +++ b/config.c
> @@ -291,6 +291,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_DBL("pi_proportional_exponent", -0.3, -DBL_MAX, DBL_MAX),
>   GLOB_ITEM_DBL("pi_proportional_norm_max", 0.7, DBL_MIN, 1.0),
>   GLOB_ITEM_DBL("pi_proportional_scale", 0.0, 0.0, DBL_MAX),
> + PORT_ITEM_INT("port.twoStepFlag", -1, -1, 1),

Why not just make twoStepFlag into GLOB_ITEM_INT ?

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] To support Ordinary Clock-Subordinate/Slave just a bunch of devices(jbod) feature.

2021-04-21 Thread Richard Cochran
On Thu, Apr 15, 2021 at 11:16:00AM +0300, Amar Subramanyam via Linuxptp-devel 
wrote:
> This change brings slave clock jbod feature which allows ptp4l to work as a
> Ordinary Subordinate/Slave clock using "just a bunch of devices" that are not
> synchronized to each other but a collection of clocks synchronized by an 
> external
> source.The best master will be decided by BMCA among the collection of the 
> clocks.
> This feature gets enabled by ptp4l config option: slave_clock_jbod and
> by default it is disabled.

This patch is unacceptable for two reasons...

>  bmc.c|  4 
>  clock.c  | 14 ++
>  clock.h  |  7 +++
>  config.c |  1 +
>  designated_fsm.c |  4 ++--
>  designated_fsm.h |  6 --
>  fsm.c| 51 ++-

1. Your are changing the core state machines from IEEE 1588.  We don't
   deviate from the standard, especially not from the critical foundation.
   If we do deviate, then only on minor points and only with a very
   strong reason.  However, you have given no justification at all for
   this change.

2. You hack your feature all over the place, like:

   if (my_special_flag) { do_my_special_thing(); }

   This a both red flag and a code smell.  Changes must respect the
   architecture.  If the architecture needs changes, then you must
   change it, step by step.

But you haven't even identified a problem.  clientOnly and
boundary_clock_jbod work just fine together:

./ptp4l -m -q --clientOnly=1 --boundary_clock_jbod=1 -i eth6 -i eth4 -i eth3
ptp4l[94622.316]: selected /dev/ptp1 as PTP clock
ptp4l[94622.316]: port 2 (eth4): just a bunch of devices
ptp4l[94622.316]: port 3 (eth3): just a bunch of devices
ptp4l[94622.318]: port 1 (eth6): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.319]: port 2 (eth4): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.320]: port 3 (eth3): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.320]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on 
INIT_COMPLETE
ptp4l[94622.320]: port 0 (/var/run/ptp4lro): INITIALIZING to LISTENING on 
INIT_COMPLETE
ptp4l[94624.185]: port 1 (eth6): new foreign master 00e00c.fffe.bce560-1
ptp4l[94628.224]: selected best master clock 00e00c.fffe.bce560
ptp4l[94628.224]: port 1 (eth6): LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[94628.825]: selected best master clock 00e00c.fffe.bce560
ptp4l[94629.136]: selected best master clock 00e00c.fffe.bce560
ptp4l[94629.767]: master offset 1619024566521062408 s0 freq  +0 path delay  
   50273
ptp4l[94630.787]: master offset 1619024566520947488 s1 freq -112695 path delay  
   50273
ptp4l[94631.806]: master offset  -5796 s2 freq -118491 path delay 50273
ptp4l[94631.806]: port 1 (eth6): UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[94632.826]: master offset  50923 s2 freq  -63510 path delay  -555
ptp4l[94633.846]: master offset899 s2 freq  -98258 path delay  -555
ptp4l[94634.866]: master offset -15796 s2 freq -114683 path delay  1392
ptp4l[94635.885]: master offset -12462 s2 freq -116088 path delay27
ptp4l[94636.058]: selected best master clock 00e00c.fffe.bce560
ptp4l[94636.130]: selected best master clock 00e00c.fffe.bce560
ptp4l[94636.905]: master offset  -8424 s2 freq -115788 path delay  -555
ptp4l[94637.925]: master offset  -5263 s2 freq -115154 path delay  -555
ptp4l[94638.945]: master offset  -2474 s2 freq -113944 path delay  -762
ptp4l[94639.965]: master offset  -1157 s2 freq -113370 path delay  -762
ptp4l[94640.984]: master offset   -532 s2 freq -113092 path delay  -658
ptp4l[94642.004]: master offset -7 s2 freq -112726 path delay  -658

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] [RFC] Add CMake support

2021-04-21 Thread Richard Cochran
On Wed, Apr 21, 2021 at 03:52:09PM +0200, Luigi 'Comio' Mantellini wrote:

> I marked it as "RFC" because this patch doesn't add anything and doesn't
> resolve any bugs. It is just a product of my work that I prefer to share
> with all.

Thanks for sharing, but I won't apply this patch, because I don't have
the free time to maintain two build systems.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Fix uninitialized variable in nmea_scan_rmc

2021-04-21 Thread Richard Cochran
On Wed, Apr 21, 2021 at 11:51:55AM +, Geva, Erez wrote:
> Looks like the man page is not accurate.
> Looking in 
> https://pubs.opengroup.org/onlinepubs/009695399/functions/mktime.html

Okay, I see now, in the man page we read:

   The value specified in the tm_isdst field informs mktime() whether  or
   not  daylight  saving time (DST) is in effect for the time supplied in
   the tm structure: a positive value means DST is in effect; zero  means
   that  DST  is  not in effect; and a negative value means that mktime()
   should (use timezone information and system databases to)  attempt  to
   determine whether DST is in effect at the specified time.

So the only input values are -1, 0, and 1.

Can anybody tell me what glibc will do with TZ=UTC ?

IOW, should tm_isdst be passed as 0 or -1 ?

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Fix uninitialized variable in nmea_scan_rmc

2021-04-20 Thread Richard Cochran
On Tue, Apr 20, 2021 at 11:44:06AM +0200, Lars Munch wrote:
> tm_isdst needs to be initialized to make sure mktime does not fail
> or calculates the wrong time.

No, take a look at the mktime(3) man page.  There you will read the
following.

   The  mktime() function modifies the fields of the tm structure as fol‐
   lows: tm_wday and tm_yday are set to values determined from  the  con‐
   tents  of  the  other  fields;  if structure members are outside their
   valid interval, they will be normalized (so that, for example, 40  Oc‐
   tober  is changed into 9 November); tm_isdst is set (regardless of its
   initial value) to a positive value or to 0, respectively, to  indicate
   whether DST is or is not in effect at the specified time.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] This code changes brings in the ability to program the acceptable clockClass threshold beyond which device will move to holdover/free-run. Default clockClass threshold is

2021-04-17 Thread Richard Cochran
On Mon, Feb 08, 2021 at 11:34:25AM +, Karthikkumar V via Linuxptp-devel 
wrote:

> diff --git a/clock.c b/clock.c
> index a34737a..11b94f5 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -132,6 +132,7 @@ struct clock {
>   struct interface *udsif;
>   LIST_HEAD(clock_subscribers_head, clock_subscriber) subscribers;
>   struct monitor *slave_event_monitor;
> + UInteger8 clock_class_threshold;

Rather than pasting this at the end, please group this with other,
related fields.

>  };
>  
>  struct clock the_clock;

> @@ -1656,6 +1658,14 @@ UInteger8 clock_max_steps_removed(struct clock *c)
>   return c->max_steps_removed;
>  }
>  
> +UInteger8 clock_get_clock_class_threshold(struct clock *c)
> +{
> + if(c != NULL){

Observe coding style.

> + return c->clock_class_threshold;
> + }
> + return CLOCK_CLASS_THRESHOLD_DEFAULT; /* Return Default Value  */

Comment is redundant.

> +}
> +
>  UInteger16 clock_steps_removed(struct clock *c)
>  {
>   return c->cur.stepsRemoved;

> diff --git a/config.c b/config.c
> index 4095b33..41735d3 100644
> --- a/config.c
> +++ b/config.c
> @@ -332,6 +332,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),
> + GLOB_ITEM_INT("clockClassThreshold", CLOCK_CLASS_THRESHOLD_DEFAULT, 6, 
> CLOCK_CLASS_THRESHOLD_DEFAULT),

Preserve alphabetical order.

>  };
>  
>  static struct unicast_master_table *current_uc_mtab;

> diff --git a/port.c b/port.c
> index f44d239..ae2a00e 100644
> --- a/port.c
> +++ b/port.c
> @@ -1861,6 +1861,11 @@ int process_announce(struct port *p, struct 
> ptp_message *m)
>   return result;
>   }
>  
> + /* If the clock class is greater than clock_class_threshold , ignore 
> this master */
> + if(m->announce.grandmasterClockQuality.clockClass > 
> clock_get_clock_class_threshold(p->clock)){

Coding style!

> + return result;
> + }
> +
>   switch (p->state) {
>   case PS_INITIALIZING:
>   case PS_FAULTY:

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/1] Fix SLAVE_ONLY TLV

2021-04-17 Thread Richard Cochran
On Tue, Mar 30, 2021 at 01:26:19AM +0200, Erez Geva wrote:
> According to IEEE 1588 The slave only flag in the SLAVE_ONLY TLV
>  is bit 0 and not bit 1 as in the DEFAULT_DATA_SET TLV.
> 
> In pmc we can simply print the value.
> According to IEEE 1588 the other bits are zero.
> When pmc query old ptp4l it will print 2 instead of 1.
> 
> Signed-off-by: Erez Geva 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] SyncE support

2021-03-30 Thread Richard Cochran
On Tue, Mar 30, 2021 at 11:40:42AM -0700, Jacob Keller wrote:
> On 3/22/2021 8:40 AM, Miroslav Lichvar wrote:
> > FWIW, some onboard NICs supported by the e1000e driver can
> > "cross-timestamp" using the Always Running Timer (ART), which should
> > avoid the asymmetry of PCIe. I have not seen any detailed description
> > of how it actually works.
> >
> 
> I'm not sure in the e1000e case but in general this is done via PCIe
> Precision Time Measurement extension which allows an end-point device
> that supports the function to setup a transaction that captures the
> device clock and ART value near-simultaneously.
> 
> I think the e1000e support might be a variation of this which pre-dated
> the PTM standard and works when the device is a LAN-on-motherboard.

My understanding was that the e1000e did in fact implement the
newfangled PCIe thingy.  But I could be wrong.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/1] Fix SLAVE_ONLY TLV

2021-03-28 Thread Richard Cochran
On Mon, Mar 22, 2021 at 04:31:43PM +0100, Erez Geva wrote:
> According to IEEE 1588 The slave only flag in the SLAVE_ONLY TLV
>  is bit 0 and not bit 1 as in the DEFAULT_DATA_SET TLV.
> 
> To retain backward compatibility and as bit 1 in SLAVE_ONLY
>  is not used anyway. Read and set both bits with same value.

This seems really not very important to keep compatibility with older
buggy versions.  Let's just fix the code to set the correct bit.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/1] Ensure TLV_PORT_STATS_NP statistics uses little endian.

2021-03-28 Thread Richard Cochran
On Tue, Mar 23, 2021 at 11:37:39PM +0100, Erez Geva wrote:
> As machine byte order may vary.
> Ensure TLV_PORT_STATS_NP statistics use defined order.
> 
> As most of us use little endian hardware and
>  to retain backward compatible with most of us,
>  we decide to use little endian for the statistics.
> All other TLVs messages remain in network order.
> 
> Signed-off-by: Erez Geva 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Unable to have UNCALIBRAT to SLAVE transition

2021-03-28 Thread Richard Cochran
On Wed, Mar 24, 2021 at 02:10:59PM +0100, Luigi 'Comio' Mantellini wrote:
> Yes, it works for me.

I am going to apply a fix.  May I add this tag into the git commit to
give you credit?

Reported-by: Luigi 'Comio' Mantellini 


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Unable to have UNCALIBRAT to SLAVE transition

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 12:38:58PM +0100, Luigi 'Comio' Mantellini wrote:
> Inside port_synchronize() I noticed this:
> 
> case SERVO_LOCKED:
> port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
> break;
> case SERVO_LOCKED_STABLE:
> message_interval_request(p, last_state, sync_interval);
> break;
> 
> Supposing to have the port X as SLAVE with "SERVO_LOCKED_STABLE" state,
> after a while I disable the ingoing traffic (SYNC/ANNUNCE) to the port X,
> switching to another port Y still keeping the SERVO_LOCKED_STABLE
> condition. The check_offset_threshold(), called by sample_sample(), should
> always return "1' (true) because the s->curr_offset_values == 0  (and it is
> fixed to servo_num_offset_values only in SERV_UNLOCKED and SERVO_JUMP
> conditions).
> 
> In these conditions the SERVO_LOCKED will not happen and the
> port_dispatach(p, EV_MASTER_CLOCK_SELECTED, 0) should never be called,
> resulting in a forever "UNCALIBRATED" condition.

Does changing port_synchronize() to this fix the issue for you?

case SERVO_LOCKED_STABLE:
message_interval_request(p, last_state, sync_interval);
+   port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
break;

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/1] Ensure TLV_PORT_STATS_NP statistics uses little endian.

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 04:58:09PM +0100, Erez Geva wrote:

> diff --git a/tlv.c b/tlv.c
> index 98ef6e1..6e919e6 100644
> --- a/tlv.c
> +++ b/tlv.c
> @@ -324,6 +324,10 @@ static int mgt_post_recv(struct management_tlv *m, 
> uint16_t data_len,
>   psn = (struct port_stats_np *)m->data;
>   psn->portIdentity.portNumber =
>   ntohs(psn->portIdentity.portNumber);
> + for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
> + psn->stats.rxMsgType[i] = 
> le64_to_cpu(psn->stats.rxMsgType[i]);
> + psn->stats.txMsgType[i] = 
> le64_to_cpu(psn->stats.txMsgType[i]);
> + }

Thanks for taking this approach.  I'm getting build errors.  Could you
please take a look?

/home/richard/git/linuxptp/tlv.c: In function ‘mgt_post_recv’:
/home/richard/git/linuxptp/tlv.c:327:8: error: ‘i’ undeclared (first use in 
this function)
   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
^
/home/richard/git/linuxptp/tlv.c:327:8: note: each undeclared identifier is 
reported only once for each function it appears in
/home/richard/git/linuxptp/tlv.c:328:30: error: implicit declaration of 
function ‘le64_to_cpu’; did you mean ‘le64toh’? 
[-Werror=implicit-function-declaration]
psn->stats.rxMsgType[i] = le64_to_cpu(psn->stats.rxMsgType[i]);
  ^~~
  le64toh
/home/richard/git/linuxptp/tlv.c: In function ‘mgt_pre_send’:
/home/richard/git/linuxptp/tlv.c:443:8: error: ‘i’ undeclared (first use in 
this function)
   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
^
/home/richard/git/linuxptp/tlv.c:444:30: error: implicit declaration of 
function ‘cpu_to_le64’; did you mean ‘htole64’? 
[-Werror=implicit-function-declaration]
psn->stats.rxMsgType[i] = cpu_to_le64(psn->stats.rxMsgType[i]);
  ^~~
  htole64

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Revert "phc2sys: Expand the validation of the PPS mode."

2021-03-23 Thread Richard Cochran
On Mon, Mar 22, 2021 at 05:23:34PM +0100, Miroslav Lichvar wrote:
> Allow the -s option to be used together with the -d option again. The
> PHC is used in the PPS mode to fix the offset by an integer number of
> seconds to get the system clock close to the PHC.
> 
> This reverts commit 228325c1bda4.

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCHv2] Avoid undefined integer operations.

2021-03-23 Thread Richard Cochran
On Mon, Mar 22, 2021 at 05:04:04PM +0100, Miroslav Lichvar wrote:
> This fixes errors reported by the -fsanitize=undefined sanitizer.
> 
> Before accepting the message interval value from a sync message, check
> if it is between -10 and 22, same as required for the delay request
> interval.
> 
> In the calculation of fest/stats/nrate max_count use unsigned 1 to avoid
> an invalid shift by 31.
> 
> In tmv.h operations cast values to uint64_t to avoid signed overflows
> and a left-shift of a negative value.
> 
> Signed-off-by: Miroslav Lichvar 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] pmc: Fix printed totalCorrectionField.

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 11:46:58AM +0100, Miroslav Lichvar wrote:
> The value needs to be shifted to right to get nanoseconds.
> 
> Signed-off-by: Miroslav Lichvar 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Quation regarding the new 'uds_ro_address'

2021-03-23 Thread Richard Cochran
On Tue, Mar 16, 2021 at 12:38:38PM +, Geva, Erez wrote:
> Can we set the read only file with a group, so we can run quaries 
> without root?

You can change the permissions to whatever you want.

> How about adding a "group" configuration for the uds_ro_address?

I don't think we need yet another config. option for this.  After all,
it is trivial for the sysadmin to script the desired permissions.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] FYI: a summary scribble on my i210 SDP / PPS-in + 25 MHz ref etc sprees

2021-03-22 Thread Richard Cochran
On Mon, Mar 22, 2021 at 06:35:48PM +0100, Frantisek Rysanek wrote:
> Dear everybody,
> 
> I've posted some of my source code before,
> based on Richard Cochran's synbc.c .

FYI, the new ts2phc program does everything that synbc did, and more.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Unable to have UNCALIBRAT to SLAVE transition

2021-03-14 Thread Richard Cochran
On Sun, Mar 14, 2021 at 10:30:31AM +0100, Luigi 'Comio' Mantellini wrote:
> The failures are part of the test and after the HW restoring I'm pretty
> sure that the protocol waltzer is running fine. I noticed that the ptp4l
> shows master offset and delay summaries. In order to have offset and delay
> values, the Sync/DelayReq/DelayResp should be correctly exchanged, am I
> right?

Right.

> Another observation is that Killing and restarting again the ptp4l I reach
> the SLAVE state without servo jump.

Probably because the offset is below the threshold.

> Just now I placed the servo_reset() inside handle_state_decision_event()
> when we have a fresh new best master, after the clock_freq_est_reset()
> method.

Don't do that.  That spoils your synchronization for nothing.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCHv2] Avoid unaligned pointers to packed members.

2021-03-13 Thread Richard Cochran
On Wed, Mar 10, 2021 at 05:05:55PM +0100, Miroslav Lichvar wrote:
> This fixes "taking address of packed member ... may result in an
> unaligned pointer value [-Waddress-of-packed-member]" warnings from gcc.
> 
> Signed-off-by: Miroslav Lichvar 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/2] Ensure TLV_PORT_STATS_NP statistics uses little endian

2021-03-13 Thread Richard Cochran
On Thu, Mar 11, 2021 at 01:02:36PM +0100, Erez Geva wrote:

> @@ -469,26 +480,26 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
>   IFMT "tx_Signaling  %" PRIu64
>   IFMT "tx_Management %" PRIu64,
>   pid2str(>portIdentity),
> - pcp->stats.rxMsgType[SYNC],
> - pcp->stats.rxMsgType[DELAY_REQ],
> - pcp->stats.rxMsgType[PDELAY_REQ],
> - pcp->stats.rxMsgType[PDELAY_RESP],
> - pcp->stats.rxMsgType[FOLLOW_UP],
> - pcp->stats.rxMsgType[DELAY_RESP],
> - pcp->stats.rxMsgType[PDELAY_RESP_FOLLOW_UP],
> - pcp->stats.rxMsgType[ANNOUNCE],
> - pcp->stats.rxMsgType[SIGNALING],
> - pcp->stats.rxMsgType[MANAGEMENT],
> - pcp->stats.txMsgType[SYNC],
> - pcp->stats.txMsgType[DELAY_REQ],
> - pcp->stats.txMsgType[PDELAY_REQ],
> - pcp->stats.txMsgType[PDELAY_RESP],
> - pcp->stats.txMsgType[FOLLOW_UP],
> - pcp->stats.txMsgType[DELAY_RESP],
> - pcp->stats.txMsgType[PDELAY_RESP_FOLLOW_UP],
> - pcp->stats.txMsgType[ANNOUNCE],
> - pcp->stats.txMsgType[SIGNALING],
> - pcp->stats.txMsgType[MANAGEMENT]);
> + getStat(pcp, true, SYNC),

All of the code for converting from network to host byte order lives
in the messaging layer.  The call chain is:

msg_post_recv:
 suffix_post_recv:
  tlv_post_recv:
mgt_post_recv:

and then, in tlv.c,

switch (id) {
...
case TLV_PORT_STATS_NP:
...
}

That is the place to call le64_to_cpu() on the table of statistics.
In this way, every user of the message receive path always will have
the message in native byte order.

> + getStat(pcp, true, DELAY_REQ),
> + getStat(pcp, true, PDELAY_REQ),
> + getStat(pcp, true, PDELAY_RESP),
> + getStat(pcp, true, FOLLOW_UP),
> + getStat(pcp, true, DELAY_RESP),
> + getStat(pcp, true, PDELAY_RESP_FOLLOW_UP),
> + getStat(pcp, true, ANNOUNCE),
> + getStat(pcp, true, SIGNALING),
> + getStat(pcp, true, MANAGEMENT),
> + getStat(pcp, false, SYNC),
> + getStat(pcp, false, DELAY_REQ),
> + getStat(pcp, false, PDELAY_REQ),
> + getStat(pcp, false, PDELAY_RESP),
> + getStat(pcp, false, FOLLOW_UP),
> + getStat(pcp, false, DELAY_RESP),
> + getStat(pcp, false, PDELAY_RESP_FOLLOW_UP),
> + getStat(pcp, false, ANNOUNCE),
> + getStat(pcp, false, SIGNALING),
> + getStat(pcp, false, MANAGEMENT));
>   break;
>   case TLV_LOG_ANNOUNCE_INTERVAL:
>   mtd = (struct management_tlv_datum *) mgt->data;
> diff --git a/port.c b/port.c
> index cefe780..7f45bf3 100644
> --- a/port.c
> +++ b/port.c
> @@ -806,6 +806,7 @@ static int port_management_fill_response(struct port 
> *target,
>   uint16_t u16;
>   uint8_t *buf;
>   int datalen;
> + int i;
>  
>   extra = tlv_extra_alloc();
>   if (!extra) {
> @@ -958,7 +959,10 @@ static int port_management_fill_response(struct port 
> *target,
>   case TLV_PORT_STATS_NP:
>   psn = (struct port_stats_np *)tlv->data;
>   psn->portIdentity = target->portIdentity;
> - psn->stats = target->stats;
> + for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
> + psn->stats.rxMsgType[i] = 
> htole64(target->stats.rxMsgType[i]);
> + psn->stats.txMsgType[i] = 
> htole64(target->stats.txMsgType[i]);

Similarly, this belongs in mgt_pre_send().

> + }
>   datalen = sizeof(*psn);
>   break;
>   default:
> -- 
> 2.20.1

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/2] Add host order to network order of 64 bits

2021-03-13 Thread Richard Cochran
On Thu, Mar 11, 2021 at 01:02:35PM +0100, Erez Geva wrote:
> @@ -455,4 +456,26 @@ void parray_extend(void ***a, ...);
>   */
>  int rate_limited(int interval, time_t *last);
>  
> +/**
> + * Swap host order to network order of 64 bits unsigned integer.
> + *
> + * @param val   value to swap.
> + * @return  swaped value.
> + */
> +static inline uint64_t htonll(uint64_t val)
> +{
> + return htobe64(val);
> +}

The second patch in this series doesn't use htonll or ntohll, and so
you can drop this patch.

Thanks,
Richard

> +
> +/**
> + * Swap network order to host order of 64 bits unsigned integer.
> + *
> + * @param val   value to swap.
> + * @return  swaped value.
> + */
> +static inline uint64_t ntohll(uint64_t val)
> +{
> + return be64toh(val);
> +}
> +
>  #endif
> -- 
> 2.20.1


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Unable to have UNCALIBRAT to SLAVE transition

2021-03-12 Thread Richard Cochran
On Fri, Mar 12, 2021 at 10:03:39AM +0100, luigi.mantell...@gmail.com wrote:

> The issue that I'm facing is the following:
>  - After a fault, the port lost the SLAVE role (correct) passing to
> MASTER state (cortect)
>  - After this transition, and after restoring the working condition I'm
> unable to move from UNCALIBRATED state to SLAVE state.

The main reason for getting stuck in UNCALIBRATED is that the port is
not able to complete a delay request/response exchange.

> My suspect is a wrong servo condition and my idea is to add a
> servo_reset() into clock_update_slave() function.
> 
> Is it a good idea or a comlete non-sense?

That doesn't make any sense to me.  You need to find the root cause
and fix it.

HTH,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] port: Accept signalling messages with OpenAVnu/gPTP wildcard, too

2021-03-12 Thread Richard Cochran
On Fri, Mar 12, 2021 at 01:25:21PM +, Wischer Timo (XC-CI1/EPC1-E) wrote:
> Hello Richard,
> 
> at least for me the specification is quite confusing at this point.
> 
> Chapter "10.5.4.1 General Signaling message specifications" of 
> IEEE8021AS-2011 describes
> targetPortIdentity with a size of 10 octets.

Look at the 2020 edition.  There it is crystal clear.

10.6.4.2 Signaling message field specifications

10.6.4.2.1 targetPortIdentity (PortIdentity)

The value of targetPortIdentity.clock identity is all ones, i.e.,
0x. The value of targetPortIdentity.portNumber is
all ones, i.e., 0x.

-- IEEE Std 802.1AS-2020, page 150

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 1/2] Explicit length byte order swap functions.

2021-03-10 Thread Richard Cochran
On Wed, Mar 10, 2021 at 11:17:48PM +0100, Erez Geva wrote:
> Replace byte order with explicit length.
> 
> Add function for byte order for 64 bits.
> 
> Signed-off-by: Erez Geva 
> ---
>  clock.c |   4 +-
>  msg.c   |  51 ++---
>  nsm.c   |   2 +-
>  port.c  |   5 +-
>  raw.c   |  10 +--
>  tc.c|   8 +-
>  tlv.c   | 207 ++--
>  transport.c |   9 ++-
>  udp.c   |   6 +-
>  udp6.c  |   4 +-
>  util.h  |  67 +
>  11 files changed, 221 insertions(+), 152 deletions(-)
> 
> diff --git a/clock.c b/clock.c
> index 7005636..5b3b4d0 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -255,12 +255,12 @@ void clock_send_notification(struct clock *c, struct 
> ptp_message *msg,
>   if (!event_bitmask_get(s->events, event))
>   continue;
>   /* send event */
> - msg->header.sequenceId = htons(s->sequenceId);
> + msg->header.sequenceId = hton16(s->sequenceId);

I really don't see any improvement here.

Sorry,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Avoid unaligned pointers to packed members.

2021-03-10 Thread Richard Cochran
On Wed, Mar 10, 2021 at 04:26:18PM +0100, Miroslav Lichvar wrote:

> Like this?
> 
> struct message_storage {
> unsigned char reserved[MSG_HEADROOM];
>   struct ptp_message msg __attribute__ ((aligned (8)));
> };

Yes, assuming that makes gcc happy again.

> It was meant as a protection against modification of the structure
> that would leap to padding. It would normally be optimized away. But
> if that is not a problem, I'll drop it.

Please drop...  I'll keep an eye on message_storage ;^)

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Avoid unaligned pointers to packed members.

2021-03-10 Thread Richard Cochran
On Wed, Mar 10, 2021 at 03:25:18PM +0100, Miroslav Lichvar wrote:
> @@ -37,7 +38,7 @@ int assume_two_step = 0;
>  struct message_storage {
>   unsigned char reserved[MSG_HEADROOM];
>   struct ptp_message msg;
> -} PACKED;
> +};

Is there a better way?  How about dropping PACKED but adding an
alignment attribute on 'msg'?

We don't about extra padding between 'reserved' and 'msg'.


> @@ -267,6 +268,10 @@ struct ptp_message *msg_allocate(void)
>   pool_stats.total++;
>   pool_debug("allocate", m);
>   }
> + if (sizeof(s->reserved) + sizeof(s->msg) != sizeof(*s)) {
> + pr_err("unexpected alignment");
> + exit(1);
> + }

This is clunky.  If we really _must_ have it, then it should be a one
time check (msg_init()?) and not every time a message is allocated.
But I would prefer avoiding this altogether.

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] port: Accept signalling messages with OpenAVnu/gPTP wildcard, too

2021-03-10 Thread Richard Cochran
On Wed, Mar 10, 2021 at 09:47:18AM +0100, Timo Wischer via Linuxptp-devel wrote:
> +const struct PortIdentity wildcard_pid2 = {
> + .clockIdentity = {
> + {0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> + },
> + .portNumber = 0x,

Where does this wildcard come from?

(I don't recall seeing it in 802.1AS)

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] PORT_STATS_NP management TLV message

2021-03-10 Thread Richard Cochran
On Tue, Mar 09, 2021 at 10:28:48PM +, Geva, Erez wrote:
> However with the current code the endian depends on machine.
> So a linuxptp daemon that runs on a big endian (there are yet some) will send 
> the message in big endian.

Yeah, so at the very minimum, we should:

1. document the oversight in the source code, and

2. add le64_to_cpu/cpu_to_le64 in the post_recv/pre_send paths.

Thanks,
Richard





___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] PORT_STATS_NP management TLV message

2021-03-10 Thread Richard Cochran
On Tue, Mar 09, 2021 at 11:34:01PM +, Keller, Jacob E wrote:
> Can this message be sent or processed over the network? Or do the _NP 
> messages always get restricted to the local socket only?

Network, too.


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] PORT_STATS_NP management TLV message

2021-03-09 Thread Richard Cochran
On Tue, Mar 09, 2021 at 01:59:33PM +, Geva, Erez wrote:
> As I add the PORT_STATS_NP message,
>  I notice that the statistics is stored in unsigned integer 64 bits values, 
> but using host order instead of network order.

Oops, that was an oversight.
 
> As I know you follow the IEEE standard,
> It would be nice if you could elaborate on this message.

This message is a custom message, and it is not covered by IEEE 1588
or any other standard.
 
> Can we fix it and rebase it to network order?

Unfortunately, simply "fixing" it unconditionally will break existing
deployments.

> May be add a flag for backward compatible.

If we go with that, then the default value of "compatible" will have
to be "true".

> What do you think?

Maybe it is better to simply leave it as is.  I myself don't use this
TLV.  Since the pmc tool prints the correct values, probably nobody
would insist on having big endian in the on-the-wire format.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] Planning release 3.2

2021-03-06 Thread Richard Cochran
Dear linuxptp developers and users,

The time has come for another release.  Starting today, a "code
freeze" is in effect, where only bug fixes will be applied.  In one
week's time I'll release v3.2 of the software.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] phc_ctl: Fix incorrect memset in do_cmp()

2021-03-06 Thread Richard Cochran
On Fri, Mar 05, 2021 at 01:21:07PM +0800, Wong Vee Khee wrote:
> Both 'rta' and 'rtb' are not properly initialized to zero.
> 
> Fixed this by assigning to correct argument to memset calls.
> 
> Fixes: bdb6a35883b0 ("linuxptp: add phc_ctl program to help debug PHC 
> devices")
> Signed-off-by: Wong Vee Khee 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [v4] Bump to IEEE 1588-2019 version

2021-03-04 Thread Richard Cochran
On Tue, Mar 02, 2021 at 02:13:02PM +0800, Yangbo Lu wrote:
> IEEE 1588-2019 specified new UInteger4 type minorVersionPTP field
> in header, and minorVersionNumber data in portDS. It has the value
> 1 for IEEE 1588-2019, and has the value 0 for IEEE 1588-2008.
> However missing minorVersionNumber definition in PORT_DATA_SET and
> VERSION_NUMBER management TLVs was an oversight in this standard.
> 
> This patch is to bump to IEEE 1588-2019 version directly in message,
> considering v2.1 and even future v2.x are all backward compatible.
> For PORT_DATA_SET and VERSION_NUMBER TLVs, keep using only
> versionNumber (major version) per current active IEEE 1588-2019
> standard regardless.
> 
> Signed-off-by: Yangbo Lu 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Make step_window configurable at port level

2021-03-03 Thread Richard Cochran
On Tue, Mar 02, 2021 at 12:02:58PM +0100, luigi.mantell...@gmail.com wrote:
> In my setup I have ports with different sync frequency. Making
> step_window at port level I can configure a fine grained TS propagation
> time on each port.

This makes sense, since logSyncInterval, etc, are per-port options.

> @@ -1787,7 +1787,7 @@ static void clock_synchronize_locked(struct clock
> *c, double adj)
>   }
>  }
>  
> -enum servo_state clock_synchronize(struct clock *c, tmv_t ingress,
> tmv_t origin)
> +enum servo_state clock_synchronize(struct clock *c, struct port *p,
> tmv_t ingress, tmv_t origin)
>  {
>   enum servo_state state = SERVO_UNLOCKED;
>   double adj, weight;
> @@ -1845,7 +1845,7 @@ enum servo_state clock_synchronize(struct clock
> *c, tmv_t ingress, tmv_t origin)
>   -tmv_to_nanoseconds(c-
> >master_offset));

Your mailer is ruining your patches.  Looks like you are using
Evolution.  Maybe this will help?


https://github.com/torvalds/linux/blob/master/Documentation/process/email-clients.rst#evolution-gui

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Make portNumber configurable

2021-03-03 Thread Richard Cochran
On Tue, Mar 02, 2021 at 11:54:31AM +0100, luigi.mantell...@gmail.com wrote:
> This patch borns from my requirements and it permits to have a
> configurable portNumber.

Why is this useful?

Please explain the requirements and state the use case.

> @@ -826,12 +826,22 @@ static int clock_add_port(struct clock *c, const
> char *phc_device,
> struct interface *iface)
>  {
>   struct port *p, *piter, *lastp = NULL;
> + int port_number = config_get_int(c->config,
> interface_name(iface), "portNumber");

Your mailer is mangling the patch.

>  
>   if (clock_resize_pollfd(c, c->nports + 2)) {
>   return -1;
>   }
> +
> + if (port_number) {
> + c->last_port_number = (c->last_port_number >
> port_number ?
> +c->last_port_number :
> port_number) + 1;
> + } else {
> + port_number = ++c->last_port_number;
> + }
> +

This logic won't work correctly.  The standard is quite explicit WRT
port numbering:

The value of the portNumber for a port on a PTP node supporting a
single PTP port shall be 1. The values of the port numbers for the
N ports on a PTP node supporting N PTP ports shall be 1, 2, ...N,
respectively.

Thus if you want configurable port numbers, then you need to ensure
that the configured numbering adheres to the scheme 1, 2, ... N.

> @@ -291,6 +291,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_DBL("pi_proportional_exponent", -0.3, -DBL_MAX,
> DBL_MAX),
>   GLOB_ITEM_DBL("pi_proportional_norm_max", 0.7, DBL_MIN, 1.0),
>   GLOB_ITEM_DBL("pi_proportional_scale", 0.0, 0.0, DBL_MAX),
> + GLOB_ITEM_INT("portNumber", 0, 0, UINT16_MAX),

Zero and 0x are not valid port numbers.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [v3] Bump to IEEE 1588-2019 version

2021-03-01 Thread Richard Cochran
Sorry for the churn, but now I understand how the minor version is
not part of the PORT_DATA_SET or of the VERSION_NUMBER TLV.  Even
though this might have been an oversight, still, as written, the
standard is crystal clear on this point.

On Mon, Mar 01, 2021 at 11:40:30AM +0800, Yangbo Lu wrote:

> diff --git a/pmc.c b/pmc.c
> index 3678800..ea2af3f 100644
> --- a/pmc.c
> +++ b/pmc.c
> @@ -413,12 +413,14 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
>   IFMT "logSyncInterval %hhd"
>   IFMT "delayMechanism  %hhu"
>   IFMT "logMinPdelayReqInterval %hhd"
> - IFMT "versionNumber   %hhu",
> + IFMT "versionNumber   %u.%u",

Please drop the .%u here, and ...

>   pid2str(>portIdentity), ps_str[p->portState],
>   p->logMinDelayReqInterval, p->peerMeanPathDelay >> 16,
>   p->logAnnounceInterval, p->announceReceiptTimeout,
>   p->logSyncInterval, p->delayMechanism,
> - p->logMinPdelayReqInterval, p->versionNumber);
> + p->logMinPdelayReqInterval,
> + p->versionNumber & MAJOR_VERSION_MASK,

keep this part, just in case some other stacks do include the minor
version in the reserved nibble.

> + p->versionNumber >> 4);

And drop this.

Please also update the code that sends PORT_DATA_SET and
VERSION_NUMBER so that it applies the mask:

p->versionNumber & MAJOR_VERSION_MASK

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] ptp4l: version preparation for IEEE 1588-2019

2021-03-01 Thread Richard Cochran
On Thu, Feb 25, 2021 at 12:31:51AM +, Rodney Cummings wrote:

> Yangbo asked:
> > It seems IEEE 1588-2019 specified minorVersionNumber in portDS, 
> > but not in PORT_DATA_SET management TLV data field. It's confusing.

Oh, now I see what yo meant.  Both PORT_DATA_SET and VERSION_NUMBER
include only the major version.  The upper nibble is still reversed in
1588 v2.1.

> That was an oversight. I'll submit a maintenance request to add it.
> It'll happen at the blazing fast speeds of IEEE SA process (sarcasm), but 
> it'll happen eventually.

Right, and so the first legitimate non-zero value for the upper nibble
will be "2" starting with 1588 v2.2.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Clock Class Threshold Feature addition for PTP4L

2021-02-26 Thread Richard Cochran
On Fri, Feb 26, 2021 at 06:54:07AM +, Karthikkumar V wrote:
> This code changes brings in the ability to program the acceptable
> clockClass threshold beyond which device will move to holdover/free-run.
> Default clockClass threshold is 248.
> Example Use-Case:
> This is needed in the cases where T-SC/T-BC Slave might want to listen
> only on PRC clockCLass and anything beyond that might not be acceptible
> and would want to go to holdover (with SyncE backup or internal oscillator).
> 
> Signed-off-by: Karthikkumar V 
> Signed-off-by: Ramana Reddy 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [v2] msg: bump to IEEE 1588-2019 version

2021-02-26 Thread Richard Cochran
On Fri, Feb 26, 2021 at 02:42:40AM +, Y.b. Lu wrote:
> My fault. I Just forgot the message printed. How about,
> 
> versionNumber   2
> minorVersionNumber  1
> 
> This may match field definition in standard. Considering it's only message 
> printed, I think either is ok.
> What do you think?

I prefer keeping the same number of lines and words in the ascii
output, like this:

versionNumber 2 (older pmc)
versionNumber 2.1   (new pmc)

Why?

Because that makes life easier for people who have scripts that parse
the ascii output.  The needed changes are smaller.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Performance Monitoring Statistics Architectural Question

2021-02-25 Thread Richard Cochran
On Thu, Feb 25, 2021 at 12:25:57PM -0800, Kien Tran wrote:

> I notice these statistics are per port and do not have values such as
> master-slave delay or slave-master delay as described in the 1588-2019
> Annex J. Do you recommend me adding a new tag for these per instance
> statistics or update this current tag (TLV_PORT_STATS_NP) with those
> statistics that are missing?

The delay and offset numbers are readily available using other
management queries, like CURRENT_DATA_SET.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [v2] msg: bump to IEEE 1588-2019 version

2021-02-25 Thread Richard Cochran
On Thu, Feb 25, 2021 at 03:20:41PM +0800, Yangbo Lu wrote:
> IEEE 1588-2019 specified new UInteger4 type minorVersionPTP field
> in header, and minorVersionNumber data in portDS. It has the value
> 1 for IEEE 1588-2019, and has the value 0 for IEEE 1588-2008.
> 
> This patch is to bump to IEEE 1588-2019 version directly in message,
> considering v2.1 and even future v2.x are all backward compatible.
> 
> Signed-off-by: Yangbo Lu 
> ---
> Changes for v2:
>   - Made v2.1 as macros.

I really like this change.  It is short and sweet.

However, now the test suite fails on 20-pmc.

checking pmc output:BAD
sending: GET PORT_DATA_SET
123456.fffe.780102-1 seq 0 RESPONSE MANAGEMENT PORT_DATA_SET 
portIdentity123456.fffe.780102-1
portState   SLAVE
logMinDelayReqInterval  0
peerMeanPathDelay   0
logAnnounceInterval 1
announceReceiptTimeout  3
logSyncInterval 0
delayMechanism  1
logMinPdelayReqInterval 0
versionNumber   18

I think we should have pmc print versionNumber 2.1 here (format %u.%u)
and then ask Miroslav to adapt the test suite...

Miroslav, I'm thinking the way to handle this in the test suite is to
accept both versionNumber 2 and versionNumber 2.1.

Thoughts?

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/2] Clock Class Threshold Feature addition for PTP4L

2021-02-25 Thread Richard Cochran
On Thu, Feb 25, 2021 at 06:31:09AM +, Karthikkumar V wrote:

> diff --git a/clock.c b/clock.c
> index 6dd4661..0879e9b 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -136,7 +136,7 @@ struct clock {
>   struct monitor *slave_event_monitor;
>   int step_window_counter;
>   int step_window;
> -  UInteger8 clock_class_threshold;
> + UInteger8 clock_class_threshold;

Please put this field just before max_steps_removed in the struct.

Also, can you squash this commit the first one together?

(git rebase -i)

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] Performance Monitoring Statistics Architectural Question

2021-02-25 Thread Richard Cochran
On Wed, Feb 24, 2021 at 02:31:21PM -0800, Kien Tran wrote:

> Another way I was thinking was to create a circular buffer to contain the
> 99 sets of data points and updating the pointer of the current head (every
> 15 minutes) to the last entry in the circular buffer.
> 
> Please let me know what you think, or if this is not the right place to be
> asking these questions.

We already have TLV_PORT_STATS_NP.  Just use that.  If you _really_
want circular buffers, memory allocation, etc, please write your own
monitor program that reads TLV_PORT_STATS_NP and converts the
statistics into whatever form you like.

But on this topic, the linuxptp approach is K.I.S.S.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] sk: Don't return error for zero-length messages.

2021-02-24 Thread Richard Cochran
On Tue, Feb 23, 2021 at 11:01:43AM +0100, Miroslav Lichvar wrote:
> The recvmsg() call can return zero for a zero-length UDP message, which
> should be handled as a bad message and not a fault of the port. This was
> addressed in commit 6b61ba29c78e ("Avoid fault when receiving zero
> length packets"), but later regressed in commit a6e0b83bd503
> ("sk: Convey transmit path errors to the caller.").
> 
> Signed-off-by: Miroslav Lichvar 
> Fixes: a6e0b83bd503 ("sk: Convey transmit path errors to the caller.")

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] ptp4l: version preparation for IEEE 1588-2019

2021-02-24 Thread Richard Cochran
On Tue, Feb 23, 2021 at 03:40:38AM +, Y.b. Lu wrote:
> Considering only 1588, v2.1 is backward compatible.

Yes.

> Regarding to many profiles, I only know 802.1AS... One thing I'm
> unsure is, if a profile is based on a specific 1588 version, does
> the message must use the corresponding version in header?

I think that if you implement an optional v2.1 feature (like the
security extension) then you can and should advertise v2.1 in the
header.  We already have some v2.1 optional stuff, and so we can bump
up the version number.  (I've just been too lazy to do that myself,
and so I'm glad you are doing it!)

> Should the message header use version v2 for AS-2011 profile, and use v2.1 
> for AS-2020 profile?

No, I don't think the minor version (the X in 2.X) conveys any
actionable information to the PTP network at run time.  There are no
practical standardized constraints on the use of profiles.  Sadly, It
is up to the administrator to get the settings right.

> Another thing I'm unsure is, whether new version of a profile is also 
> backward compatible. I hope yes.

Yes.  All 2.x versions are compatible, according to 1588.
 
> So, may I have your suggestion on how to move ptp4l to v2.1? Do we need to 
> implement something like deciding 1588 version per profile in program?

Please, just make the v2.1 as macros, fill out the minor field in the
frames, and forget about dynamic version changes at run time.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


  1   2   3   4   5   6   7   8   9   10   >