RE: [PATCH net-next] net: add a helper to avoid issues with HW TX timestamping and SO_TXTIME

-----Original Message-----
From: Vladimir Oltean <olteanv@gmail.com>
Sent: 2021年3月10日 22:51
To: Jakub Kicinski <kuba@kernel.org>; netdev@vger.kernel.org; Po Liu
[off-list ref]; Claudiu Manoil [off-list ref]
Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com>; Richard Cochran
[off-list ref]
Subject: [PATCH net-next] net: add a helper to avoid issues with HW TX
timestamping and SO_TXTIME

Caution: EXT Email

As explained in commit 29d98f54a4fe ("net: enetc: allow hardware
timestamping on TX queues with tc-etf enabled"), hardware TX timestamping
requires an skb with skb->tstamp = 0. When a packet is sent with SO_TXTIME,
the skb->skb_mstamp_ns corrupts the value of skb->tstamp, so the drivers need
to explicitly reset skb->tstamp to zero after consuming the TX time.

Create a helper named skb_txtime_consumed() which does just that. All drivers
which offload TC_SETUP_QDISC_ETF should implement it, and it would make it
easier to assess during review whether they do the right thing in order to be
compatible with hardware timestamping or not.

Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 8 ++------
 drivers/net/ethernet/intel/igb/igb_main.c    | 2 +-
 drivers/net/ethernet/intel/igc/igc_main.c    | 2 +-
 include/net/pkt_sched.h                      | 9 +++++++++
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c

b/drivers/net/ethernet/freescale/enetc/enetc.c
index e85dfccb9ed1..5a54976e6a28 100644

--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c

@@ -5,6 +5,7 @@
 #include <linux/tcp.h>
 #include <linux/udp.h>
 #include <linux/vmalloc.h>
+#include <net/pkt_sched.h>

 /* ENETC overhead: optional extension BD + 1 BD gap */
 #define ENETC_TXBDS_NEEDED(val)        ((val) + 2)

@@ -293,12 +294,7 @@ static void enetc_tstamp_tx(struct sk_buff *skb, u64

tstamp)
        if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) {
                memset(&shhwtstamps, 0, sizeof(shhwtstamps));
                shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
-               /* Ensure skb_mstamp_ns, which might have been populated with
-                * the txtime, is not mistaken for a software timestamp,
-                * because this will prevent the dispatch of our hardware
-                * timestamp to the socket.
-                */
-               skb->tstamp = ktime_set(0, 0);
+               skb_txtime_consumed(skb);
                skb_tstamp_tx(skb, &shhwtstamps);
        }
 }

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c

b/drivers/net/ethernet/intel/igb/igb_main.c
index 878b31d534ec..369533feb4f2 100644

--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c

@@ -5856,7 +5856,7 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring,
         */
        if (tx_ring->launchtime_enable) {
                ts = ktime_to_timespec64(first->skb->tstamp);
-               first->skb->tstamp = ktime_set(0, 0);
+               skb_txtime_consumed(first->skb);
                context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32);
        } else {
                context_desc->seqnum_seed = 0; diff --git

a/drivers/net/ethernet/intel/igc/igc_main.c
b/drivers/net/ethernet/intel/igc/igc_main.c
index 7ac9597ddb84..059ffcfb0bda 100644

--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c

@@ -941,7 +941,7 @@ static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
                struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
                ktime_t txtime = first->skb->tstamp;

-               first->skb->tstamp = ktime_set(0, 0);
+               skb_txtime_consumed(first->skb);
                context_desc->launch_time = igc_tx_launchtime(adapter,
                                                              txtime);
        } else {

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h index

15b1b30f454e..f5c1bee0cd6a 100644

--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h

@@ -188,4 +188,13 @@ struct tc_taprio_qopt_offload

*taprio_offload_get(struct tc_taprio_qopt_offload
                                                  *offload);  void taprio_offload_free(struct
tc_taprio_qopt_offload *offload);

+/* Ensure skb_mstamp_ns, which might have been populated with the
+txtime, is
+ * not mistaken for a software timestamp, because this will otherwise
+prevent
+ * the dispatch of hardware timestamps to the socket.
+ */
+static inline void skb_txtime_consumed(struct sk_buff *skb) {
+       skb->tstamp = ktime_set(0, 0);
+}
+
 #endif
--
2.25.1