PTP one step sync packets cannot have CSUM padding and insertion in
SW since time stamp is inserted on the fly by HW.
In addition, ptp4l version 3.0 and above report an error when skb
timestamps are reported for packets that not processed for TX TS
after transmission.
Add a helper to identify PTP one step sync and fix the above two
errors. Add a common mask for PTP header flag field "twoStepflag".
Also reset ptp OSS bit when one step is not selected.
Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
Fixes: 653e92a9175e ("net: macb: add support for padding and fcs computation")
Signed-off-by: Harini Katakam <[email protected]>
Reviewed-by: Radhey Shyam Pandey <[email protected]>
---
v3:
- Rebase on net branch
- Squash both commits on macb driver and ptp_classify.h
v2:
- Separate fix for net branch
(Split from "Macb PTP updates" series
https://lore.kernel.org/netdev/[email protected]/T/)
- Fix include order
- ptp_oss -> ptp_one_step_sync
- Remove inline and add "likely" on SKB_HWTSTAMP check in ptp helper
- Dont split gem_ptp_do_tx_tstamp from if condition as order of
evaluation will take care of intent
- Remove redundant comments in macb_pad_and_fcs
- Add PTP flag to ptp_classify header for common use
(Dint add Richard's ACK as the patch changed and there's a minor addition
in ptp_classify.h as well)
v1 Notes:
-> Added the macb pad and fcs fixes tag because strictly speaking the PTP support
patch precedes the fcs patch in timeline.
-> FYI, the error observed with setting HW TX timestamp for one step sync packets:
ptp4l[405.292]: port 1: unexpected socket error
drivers/net/ethernet/cadence/macb_main.c | 40 +++++++++++++++++++++---
drivers/net/ethernet/cadence/macb_ptp.c | 4 ++-
include/linux/ptp_classify.h | 3 ++
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 61284baa0496..3a1b5ac48ca5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -36,6 +36,7 @@
#include <linux/iopoll.h>
#include <linux/phy/phy.h>
#include <linux/pm_runtime.h>
+#include <linux/ptp_classify.h>
#include <linux/reset.h>
#include "macb.h"
@@ -1124,6 +1125,36 @@ static void macb_tx_error_task(struct work_struct *work)
spin_unlock_irqrestore(&bp->lock, flags);
}
+static bool ptp_one_step_sync(struct sk_buff *skb)
+{
+ struct ptp_header *hdr;
+ unsigned int ptp_class;
+ u8 msgtype;
+
+ /* No need to parse packet if PTP TS is not involved */
+ if (likely(!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)))
+ goto not_oss;
+
+ /* Identify and return whether PTP one step sync is being processed */
+ ptp_class = ptp_classify_raw(skb);
+ if (ptp_class == PTP_CLASS_NONE)
+ goto not_oss;
+
+ hdr = ptp_parse_header(skb, ptp_class);
+ if (!hdr)
+ goto not_oss;
+
+ if (hdr->flag_field[0] & PTP_FLAG_TWOSTEP)
+ goto not_oss;
+
+ msgtype = ptp_get_msgtype(hdr, ptp_class);
+ if (msgtype == PTP_MSGTYPE_SYNC)
+ return true;
+
+not_oss:
+ return false;
+}
+
static void macb_tx_interrupt(struct macb_queue *queue)
{
unsigned int tail;
@@ -1168,8 +1199,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
/* First, update TX stats if needed */
if (skb) {
- if (unlikely(skb_shinfo(skb)->tx_flags &
- SKBTX_HW_TSTAMP) &&
+ if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+ !ptp_one_step_sync(skb) &&
gem_ptp_do_txstamp(queue, skb, desc) == 0) {
/* skb now belongs to timestamp buffer
* and will be removed later
@@ -1999,7 +2030,8 @@ static unsigned int macb_tx_map(struct macb *bp,
ctrl |= MACB_BF(TX_LSO, lso_ctrl);
ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
if ((bp->dev->features & NETIF_F_HW_CSUM) &&
- skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl)
+ skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl &&
+ !ptp_one_step_sync(skb))
ctrl |= MACB_BIT(TX_NOCRC);
} else
/* Only set MSS/MFS on payload descriptors
@@ -2097,7 +2129,7 @@ static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
if (!(ndev->features & NETIF_F_HW_CSUM) ||
!((*skb)->ip_summed != CHECKSUM_PARTIAL) ||
- skb_shinfo(*skb)->gso_size) /* Not available for GSO */
+ skb_shinfo(*skb)->gso_size || ptp_one_step_sync(*skb))
return 0;
if (padlen <= 0) {
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index fb6b27f46b15..9559c16078f9 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -470,8 +470,10 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
case HWTSTAMP_TX_ONESTEP_SYNC:
if (gem_ptp_set_one_step_sync(bp, 1) != 0)
return -ERANGE;
- fallthrough;
+ tx_bd_control = TSTAMP_ALL_FRAMES;
+ break;
case HWTSTAMP_TX_ON:
+ gem_ptp_set_one_step_sync(bp, 0);
tx_bd_control = TSTAMP_ALL_FRAMES;
break;
default:
diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
index fefa7790dc46..2b6ea36ad162 100644
--- a/include/linux/ptp_classify.h
+++ b/include/linux/ptp_classify.h
@@ -43,6 +43,9 @@
#define OFF_PTP_SOURCE_UUID 22 /* PTPv1 only */
#define OFF_PTP_SEQUENCE_ID 30
+/* PTP header flag fields */
+#define PTP_FLAG_TWOSTEP BIT(1)
+
/* Below defines should actually be removed at some point in time. */
#define IP6_HLEN 40
#define UDP_HLEN 8
--
2.17.1
On Wed, 18 May 2022 22:37:56 +0530 Harini Katakam wrote:
> PTP one step sync packets cannot have CSUM padding and insertion in
> SW since time stamp is inserted on the fly by HW.
> In addition, ptp4l version 3.0 and above report an error when skb
> timestamps are reported for packets that not processed for TX TS
> after transmission.
> Add a helper to identify PTP one step sync and fix the above two
> errors. Add a common mask for PTP header flag field "twoStepflag".
> Also reset ptp OSS bit when one step is not selected.
>
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Fixes: 653e92a9175e ("net: macb: add support for padding and fcs computation")
> Signed-off-by: Harini Katakam <[email protected]>
> Reviewed-by: Radhey Shyam Pandey <[email protected]>
Acked-by: Jakub Kicinski <[email protected]>
On Wed, 2022-05-18 at 22:37 +0530, Harini Katakam wrote:
> PTP one step sync packets cannot have CSUM padding and insertion in
> SW since time stamp is inserted on the fly by HW.
> In addition, ptp4l version 3.0 and above report an error when skb
> timestamps are reported for packets that not processed for TX TS
> after transmission.
> Add a helper to identify PTP one step sync and fix the above two
> errors. Add a common mask for PTP header flag field "twoStepflag".
> Also reset ptp OSS bit when one step is not selected.
>
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Fixes: 653e92a9175e ("net: macb: add support for padding and fcs computation")
> Signed-off-by: Harini Katakam <[email protected]>
> Reviewed-by: Radhey Shyam Pandey <[email protected]>
I'm sorry, but I cut the -net PR to Linus too early for this, so the
fix will have to wait for a little more (no need to repost!) and even
more pause will be required for the net-next follow-up.
Sorry for the inconvenince,
Paolo
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Wed, 18 May 2022 22:37:56 +0530 you wrote:
> PTP one step sync packets cannot have CSUM padding and insertion in
> SW since time stamp is inserted on the fly by HW.
> In addition, ptp4l version 3.0 and above report an error when skb
> timestamps are reported for packets that not processed for TX TS
> after transmission.
> Add a helper to identify PTP one step sync and fix the above two
> errors. Add a common mask for PTP header flag field "twoStepflag".
> Also reset ptp OSS bit when one step is not selected.
>
> [...]
Here is the summary with links:
- [net,v3] net: macb: Fix PTP one step sync support
https://git.kernel.org/netdev/net/c/5cebb40bc955
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
> -----Original Message-----
> From: Paolo Abeni <[email protected]>
> Sent: Thursday, May 19, 2022 11:48 PM
> To: Harini Katakam <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Michal Simek
> <[email protected]>; [email protected]; Radhey Shyam
> Pandey <[email protected]>
> Subject: Re: [PATCH net v3] net: macb: Fix PTP one step sync support
>
> On Wed, 2022-05-18 at 22:37 +0530, Harini Katakam wrote:
> > PTP one step sync packets cannot have CSUM padding and insertion in SW
> > since time stamp is inserted on the fly by HW.
> > In addition, ptp4l version 3.0 and above report an error when skb
> > timestamps are reported for packets that not processed for TX TS after
> > transmission.
> > Add a helper to identify PTP one step sync and fix the above two
> > errors. Add a common mask for PTP header flag field "twoStepflag".
> > Also reset ptp OSS bit when one step is not selected.
> >
> > Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> > Fixes: 653e92a9175e ("net: macb: add support for padding and fcs
> > computation")
> > Signed-off-by: Harini Katakam <[email protected]>
> > Reviewed-by: Radhey Shyam Pandey <[email protected]>
>
> I'm sorry, but I cut the -net PR to Linus too early for this, so the fix will have to
> wait for a little more (no need to repost!) and even more pause will be
> required for the net-next follow-up.
>
> Sorry for the inconvenince,
Thanks Paolo. No problem, will wait and send net-next follow up.
Regards,
Harini