[PATCH net v2 0/2] Macb PTP one step fix

[PATCH net v2 0/2] Macb PTP one step fix

[Harini Katakam]

Split from "Macb PTP updates" series
https://lore.kernel.org/netdev/20220517135525.GC3344@hoboy.vegasvil.org/T/
- Fix Macb PTP one step.
- Add common twoStepflag field definition.

Harini Katakam (2):
  include: ptp: Add common two step flag mask
  net: macb: Fix PTP one step sync support

 drivers/net/ethernet/cadence/macb_main.c | 41 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb_ptp.c  |  4 ++-
 include/linux/ptp_classify.h             |  3 ++
 3 files changed, 43 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH net v2 1/2] include: ptp: Add common two step flag mask

[Harini Katakam]

Add mask for PTP header flag field "twoStepflag".

Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
---
 include/linux/ptp_classify.h | 3 +++
 1 file changed, 3 insertions(+)

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

[PATCH net v2 2/2] net: macb: Fix PTP one step sync support

[Harini Katakam]

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.
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 <harini.katakam@xilinx.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
v2:
- Separate fix for net branch
- 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 | 41 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb_ptp.c  |  4 ++-
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e993616308f8..d6e71f03e73b 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"
 
@@ -1122,6 +1123,36 @@ static void macb_tx_error_task(struct work_struct *work)
 	napi_enable(&queue->napi_tx);
 }
 
+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 int macb_tx_complete(struct macb_queue *queue, int budget)
 {
 	struct macb *bp = queue->bp;
@@ -1158,8 +1189,8 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 
 			/* 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
@@ -2063,7 +2094,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
@@ -2159,9 +2191,10 @@ static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
 	struct sk_buff *nskb;
 	u32 fcs;
 
+	/* Not available for GSO and PTP one step sync */
 	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:
-- 
2.17.1

Re: [PATCH net v2 0/2] Macb PTP one step fix

[Jakub Kicinski]

On Wed, 18 May 2022 17:03:08 +0530 Harini Katakam wrote:
> Split from "Macb PTP updates" series
> https://lore.kernel.org/netdev/20220517135525.GC3344@hoboy.vegasvil.org/T/
> - Fix Macb PTP one step.
> - Add common twoStepflag field definition.

The series does not apply cleanly to net:

  Applying: include: ptp: Add common two step flag mask
  Applying: net: macb: Fix PTP one step sync support
  Using index info to reconstruct a base tree...
  M	drivers/net/ethernet/cadence/macb_main.c
  Falling back to patching base and 3-way merge...
  Auto-merging drivers/net/ethernet/cadence/macb_main.c
  CONFLICT (content): Merge conflict in drivers/net/ethernet/cadence/macb_main.c
  error: Failed to merge in the changes.
  hint: Use 'git am --show-current-patch=diff' to see the failed patch
  Patch failed at 0002 net: macb: Fix PTP one step sync support


You need to rebase on top of this:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/

Also you can merge the patches together, no need to make the header
change separate.