[Linuxptp-devel] [PATCH v2 1/1] msg: append TLV onto all PTP event messages

2023-05-08 Thread Tan Tee Min
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

2023-05-08 Thread Tan Tee Min
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

2023-05-08 Thread Richard Cochran
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

2023-05-08 Thread Tan Tee Min
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

2023-05-08 Thread Martin Pecka

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.

2023-05-08 Thread Hangbin Liu
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

2023-05-08 Thread Hangbin Liu
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