[Linuxptp-devel] [PATCH v2 1/1] msg: append TLV onto all PTP event messages
According to IEEE Std 1588-2019 Chapter 6.4 PTP message classes, all PTP messages can be extended by means of a standard Type, Length, Value (TLV) extension mechanism. Avnu conformance tests for 802.1AS-2020 do validate whether the device can properly ignore unknown TLVs in received PTP messages. With the present code, we observed "bad message" error when the device receives Sync, Pdelay_Req and Pdelay_Resp event messages with unknown TLV appended. This error is due to PTP event messages are not enabled/allowed to carry TLV yet. To fix the problem, we have to append TLV onto the PTP event messages. v2: Fix commit message Signed-off-by: Tan Tee Min --- msg.c | 6 +++--- msg.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/msg.c b/msg.c index c2d358b..ab841f0 100644 --- a/msg.c +++ b/msg.c @@ -100,13 +100,13 @@ static uint8_t *msg_suffix(struct ptp_message *m) { switch (msg_type(m)) { case SYNC: - return NULL; + return m->sync.suffix; case DELAY_REQ: return m->delay_req.suffix; case PDELAY_REQ: - return NULL; + return m->pdelay_req.suffix; case PDELAY_RESP: - return NULL; + return m->pdelay_resp.suffix; case FOLLOW_UP: return m->follow_up.suffix; case DELAY_RESP: diff --git a/msg.h b/msg.h index b7423ee..e3744dc 100644 --- a/msg.h +++ b/msg.h @@ -124,6 +124,7 @@ struct announce_msg { struct sync_msg { struct ptp_header hdr; struct TimestamporiginTimestamp; + uint8_t suffix[0]; } PACKED; struct delay_req_msg { @@ -149,12 +150,14 @@ struct pdelay_req_msg { struct ptp_header hdr; struct TimestamporiginTimestamp; struct PortIdentity reserved; + uint8_t suffix[0]; } PACKED; struct pdelay_resp_msg { struct ptp_header hdr; struct TimestamprequestReceiptTimestamp; struct PortIdentity requestingPortIdentity; + uint8_t suffix[0]; } PACKED; struct pdelay_resp_fup_msg { -- 1.9.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 1/1] msg: append TLV onto all PTP event messages
On Mon, May 08, 2023 at 11:52:01AM -0700, Richard Cochran wrote: > On Mon, May 08, 2023 at 05:46:30PM +0800, Tan Tee Min wrote: > > > We are working towards the Avnu conformance tests for 802.1AS-2020, and > > actually Avnu do validate whether the device can properly ignore unknown > > TLVs in received PTP messages. > > > > Without this implementation, we observe "bad message" error when the device > > receives Sync, Pdelay_Req and Pdelay_Resp event messages with unknown TLV > > appended. > > This is important information. You need to put that into your commit > log message, as it explains the reason why your change is needed. > > A proper commit log message contains three elements: > > 1. Context > 2. Problem > 3. Solution > > Your log message only gave the Context. > It failed to state the problem or the solution. > Noted. I apologize for my poor commit message. Let me rewrite the commit message and resubmit as v2. Thanks, Tee Min ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 1/1] msg: append TLV onto all PTP event messages
On Mon, May 08, 2023 at 05:46:30PM +0800, Tan Tee Min wrote: > We are working towards the Avnu conformance tests for 802.1AS-2020, and > actually Avnu do validate whether the device can properly ignore unknown > TLVs in received PTP messages. > > Without this implementation, we observe "bad message" error when the device > receives Sync, Pdelay_Req and Pdelay_Resp event messages with unknown TLV > appended. This is important information. You need to put that into your commit log message, as it explains the reason why your change is needed. A proper commit log message contains three elements: 1. Context 2. Problem 3. Solution Your log message only gave the Context. It failed to state the problem or the solution. 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] msg: append TLV onto all PTP event messages
On Sun, Apr 30, 2023 at 11:59:51AM -0700, Richard Cochran wrote: > On Wed, Mar 01, 2023 at 01:04:34PM +0800, Tan Tee Min wrote: > > According to IEEE Std 1588-2019 Chapter 6.4 PTP message classes, > > all PTP messages can be extended by means of a standard > > Type, Length, Value (TLV) extension mechanism. > > Since the present code does not append any TLVs on such message types, > this change is not needed. The patch will become relevant if/when the > use case is implemented, not before. We are working towards the Avnu conformance tests for 802.1AS-2020, and actually Avnu do validate whether the device can properly ignore unknown TLVs in received PTP messages. Without this implementation, we observe "bad message" error when the device receives Sync, Pdelay_Req and Pdelay_Resp event messages with unknown TLV appended. Thanks, Tee Min ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] Bug caused by commit afeabf3 "ptp4l: add VLAN over bond support" on kernel 4.9
Hi Hagbin, I just back from holiday. Sorry I forgot to check the errno returned by ioctl. I saw Martin has send the fix for this issue. It would be good if you could check the fix does not break your use-case. Also, if you had any idea how to check for the VLAN over bond support without using SIOCGHWTSTAMP (which is only optional), it would be great. With the current patch, any NIC without SIOCGHWTSTAMP support will not report VLAN over bond support (but that was the case in your original patch before, too). Thanks, Martin smime.p7s Description: Elektronicky podpis S/MIME ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] Fix detection of VLAN over bond support in case the driver does not support SIOCGHWTSTAMP ioctl.
On Tue, May 2, 2023 at 10:56 PM Martin Pecka wrote: > > Fixes issue "Bug caused by commit afeabf3 "ptp4l: add VLAN over bond support" > on kernel 4.9" reported on linuxptp-devel. > > SIOCGHWTSTAMP ioctl is optional according to Linux timestamping.txt document, > but the code failed to set any HW timestamping when the ioctl was actually > not supported. For now, VLAN over bond support requires that the driver > supports SIOCGHWTSTAMP ioctl (but this limitation can be removed in the > future). > > Signed-off-by: Martin Pecka Acked-by: Hangbin Liu > --- > sk.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/sk.c b/sk.c > index 6a9e5b8..a72aca3 100644 > --- a/sk.c > +++ b/sk.c > @@ -66,10 +66,21 @@ static int hwts_init(int fd, const char *device, int > rx_filter, > > init_ifreq(&ifreq, &cfg, device); > > + /* Test if VLAN over bond is supported. */ > cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX; > - /* Fall back without flag if user run new build on old kernel */ > - if (ioctl(fd, SIOCGHWTSTAMP, &ifreq) == -EINVAL) > - init_ifreq(&ifreq, &cfg, device); > + err = ioctl(fd, SIOCGHWTSTAMP, &ifreq); > + if (err < 0) { > + /* > +* Fall back without flag if user runs new build on old kernel > +* or if driver does not support SIOCGHWTSTAMP ioctl. > +*/ > + if (errno == EINVAL || errno == EOPNOTSUPP) { > + init_ifreq(&ifreq, &cfg, device); > + } else { > + pr_err("ioctl SIOCGHWTSTAMP failed: %m"); > + return err; > + } > + } > > switch (sk_hwts_filter_mode) { > case HWTS_FILTER_CHECK: > -- > 2.17.1 > > > > ___ > Linuxptp-devel mailing list > Linuxptp-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] Bug caused by commit afeabf3 "ptp4l: add VLAN over bond support" on kernel 4.9
Hi Richard, Martin, I just back from holiday. Sorry I forgot to check the errno returned by ioctl. I saw Martin has send the fix for this issue. Thanks Hangbin On Mon, May 01, 2023 at 09:08:30PM -0700, Richard Cochran wrote: > On Mon, May 01, 2023 at 06:29:59PM +0200, Martin Pecka wrote: > > > > A driver which supports hardware time stamping must support the > > > SIOCSHWTSTAMP ioctl and update the supplied struct hwtstamp_config with > > > the actual values as described in the section on SIOCSHWTSTAMP. It > > > should also support SIOCGHWTSTAMP. > > The support for SIOCGHWTSTAMP is optional. > > > > So it seems to me wrong to test for the bonded PHC support using this ioctl, > > which is only optional. > > +1 > > > 1. Check errno not only for EINVAL, but also for EOPNOTSUPP. This would > > solve the issue for me (and Jetsons in general), but would probably leave > > some users with bonded PHCs whose drivers do not support SIOCGHWTSTAMP > > without the possibility to use the bonded PHC. > > +1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel