Optimization could be done on dsa_skb_tx_timestamp(), and dsa device
drivers should adapt to it.

- Check SKBTX_HW_TSTAMP request flag at the very beginning, instead of in
  port_txtstamp, so that most skbs not requiring tx timestamp just return.

- No longer to identify PTP packets, and limit tx timestamping only for PTP
  packets. If device driver likes, let device driver do.

- It is a waste to clone skb directly in dsa_skb_tx_timestamp().
  For one-step timestamping, a clone is not needed. For any failure of
  port_txtstamp (this may usually happen), the skb clone has to be freed.
  So put skb cloning into port_txtstamp where it really needs.

Signed-off-by: Yangbo Lu <yangbo...@nxp.com>
---
 Documentation/networking/timestamping.rst     |  7 +++++--
 .../net/dsa/hirschmann/hellcreek_hwtstamp.c   | 20 ++++++++++++------
 .../net/dsa/hirschmann/hellcreek_hwtstamp.h   |  2 +-
 drivers/net/dsa/mv88e6xxx/hwtstamp.c          | 21 +++++++++++++------
 drivers/net/dsa/mv88e6xxx/hwtstamp.h          |  6 +++---
 drivers/net/dsa/ocelot/felix.c                | 11 ++++++----
 drivers/net/dsa/sja1105/sja1105_ptp.c         |  6 +++++-
 drivers/net/dsa/sja1105/sja1105_ptp.h         |  2 +-
 include/net/dsa.h                             |  2 +-
 net/dsa/slave.c                               | 20 +++++-------------
 10 files changed, 57 insertions(+), 40 deletions(-)

diff --git a/Documentation/networking/timestamping.rst 
b/Documentation/networking/timestamping.rst
index f682e88fa87e..7f04a699a5d1 100644
--- a/Documentation/networking/timestamping.rst
+++ b/Documentation/networking/timestamping.rst
@@ -635,8 +635,8 @@ in generic code: a BPF classifier (``ptp_classify_raw``) is 
used to identify
 PTP event messages (any other packets, including PTP general messages, are not
 timestamped), and provides two hooks to drivers:
 
-- ``.port_txtstamp()``: The driver is passed a clone of the timestampable skb
-  to be transmitted, before actually transmitting it. Typically, a switch will
+- ``.port_txtstamp()``: A clone of the timestampable skb to be transmitted
+  is needed, before actually transmitting it. Typically, a switch will
   have a PTP TX timestamp register (or sometimes a FIFO) where the timestamp
   becomes available. There may be an IRQ that is raised upon this timestamp's
   availability, or the driver might have to poll after invoking
@@ -645,6 +645,9 @@ timestamped), and provides two hooks to drivers:
   later use (when the timestamp becomes available). Each skb is annotated with
   a pointer to its clone, in ``DSA_SKB_CB(skb)->clone``, to ease the driver's
   job of keeping track of which clone belongs to which skb.
+  But one-step timestamping request is handled differently with above two-step
+  timestamping. The skb clone is no longer needed since hardware will insert
+  TX time information on packet during egress.
 
 - ``.port_rxtstamp()``: The original (and only) timestampable skb is provided
   to the driver, for it to annotate it with a timestamp, if that is immediately
diff --git a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c 
b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
index 69dd9a2e8bb6..2ff4b7c08b72 100644
--- a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
+++ b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
@@ -374,31 +374,39 @@ long hellcreek_hwtstamp_work(struct ptp_clock_info *ptp)
 }
 
 bool hellcreek_port_txtstamp(struct dsa_switch *ds, int port,
-                            struct sk_buff *clone, unsigned int type)
+                            struct sk_buff *skb, struct sk_buff **clone)
 {
        struct hellcreek *hellcreek = ds->priv;
        struct hellcreek_port_hwtstamp *ps;
        struct ptp_header *hdr;
+       unsigned int type;
 
        ps = &hellcreek->ports[port].port_hwtstamp;
 
-       /* Check if the driver is expected to do HW timestamping */
-       if (!(skb_shinfo(clone)->tx_flags & SKBTX_HW_TSTAMP))
+       type = ptp_classify_raw(skb);
+       if (type == PTP_CLASS_NONE)
                return false;
 
        /* Make sure the message is a PTP message that needs to be timestamped
         * and the interaction with the HW timestamping is enabled. If not, stop
         * here
         */
-       hdr = hellcreek_should_tstamp(hellcreek, port, clone, type);
+       hdr = hellcreek_should_tstamp(hellcreek, port, skb, type);
        if (!hdr)
                return false;
 
+       *clone = skb_clone_sk(skb);
+       if (!(*clone))
+               return false;
+
        if (test_and_set_bit_lock(HELLCREEK_HWTSTAMP_TX_IN_PROGRESS,
-                                 &ps->state))
+                                 &ps->state)) {
+               kfree_skb(*clone);
+               *clone = NULL;
                return false;
+       }
 
-       ps->tx_skb = clone;
+       ps->tx_skb = *clone;
 
        /* store the number of ticks occurred since system start-up till this
         * moment
diff --git a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h 
b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h
index c0745ffa1ebb..58cc96642076 100644
--- a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h
+++ b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h
@@ -45,7 +45,7 @@ int hellcreek_port_hwtstamp_get(struct dsa_switch *ds, int 
port,
 bool hellcreek_port_rxtstamp(struct dsa_switch *ds, int port,
                             struct sk_buff *clone, unsigned int type);
 bool hellcreek_port_txtstamp(struct dsa_switch *ds, int port,
-                            struct sk_buff *clone, unsigned int type);
+                            struct sk_buff *skb, struct sk_buff **clone);
 
 int hellcreek_get_ts_info(struct dsa_switch *ds, int port,
                          struct ethtool_ts_info *info);
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c 
b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 094d17a1d037..280a95962861 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -469,24 +469,33 @@ long mv88e6xxx_hwtstamp_work(struct ptp_clock_info *ptp)
 }
 
 bool mv88e6xxx_port_txtstamp(struct dsa_switch *ds, int port,
-                            struct sk_buff *clone, unsigned int type)
+                            struct sk_buff *skb, struct sk_buff **clone)
 {
        struct mv88e6xxx_chip *chip = ds->priv;
        struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
        struct ptp_header *hdr;
+       unsigned int type;
 
-       if (!(skb_shinfo(clone)->tx_flags & SKBTX_HW_TSTAMP))
-               return false;
+       type = ptp_classify_raw(skb);
+       if (type == PTP_CLASS_NONE)
+               return false
 
-       hdr = mv88e6xxx_should_tstamp(chip, port, clone, type);
+       hdr = mv88e6xxx_should_tstamp(chip, port, skb, type);
        if (!hdr)
                return false;
 
+       *clone = skb_clone_sk(skb);
+       if (!(*clone))
+               return false;
+
        if (test_and_set_bit_lock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,
-                                 &ps->state))
+                                 &ps->state)) {
+               kfree_skb(*clone);
+               *clone = NULL;
                return false;
+       }
 
-       ps->tx_skb = clone;
+       ps->tx_skb = *clone;
        ps->tx_tstamp_start = jiffies;
        ps->tx_seq_id = be16_to_cpu(hdr->sequence_id);
 
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.h 
b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
index 9da9f197ba02..da2b253334d0 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.h
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
@@ -118,7 +118,7 @@ int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, int 
port,
 bool mv88e6xxx_port_rxtstamp(struct dsa_switch *ds, int port,
                             struct sk_buff *clone, unsigned int type);
 bool mv88e6xxx_port_txtstamp(struct dsa_switch *ds, int port,
-                            struct sk_buff *clone, unsigned int type);
+                            struct sk_buff *skb, struct sk_buff **clone);
 
 int mv88e6xxx_get_ts_info(struct dsa_switch *ds, int port,
                          struct ethtool_ts_info *info);
@@ -152,8 +152,8 @@ static inline bool mv88e6xxx_port_rxtstamp(struct 
dsa_switch *ds, int port,
 }
 
 static inline bool mv88e6xxx_port_txtstamp(struct dsa_switch *ds, int port,
-                                          struct sk_buff *clone,
-                                          unsigned int type)
+                                          struct sk_buff *skb,
+                                          struct sk_buff **clone)
 {
        return false;
 }
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 6b5442be0230..cdec2f5e271c 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1396,14 +1396,17 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int 
port,
 }
 
 static bool felix_txtstamp(struct dsa_switch *ds, int port,
-                          struct sk_buff *clone, unsigned int type)
+                          struct sk_buff *skb, struct sk_buff **clone)
 {
        struct ocelot *ocelot = ds->priv;
        struct ocelot_port *ocelot_port = ocelot->ports[port];
 
-       if (ocelot->ptp && (skb_shinfo(clone)->tx_flags & SKBTX_HW_TSTAMP) &&
-           ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
-               ocelot_port_add_txtstamp_skb(ocelot, port, clone);
+       if (ocelot->ptp && ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
+               *clone = skb_clone_sk(skb);
+               if (!(*clone))
+                       return false;
+
+               ocelot_port_add_txtstamp_skb(ocelot, port, *clone);
                return true;
        }
 
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c 
b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 1b90570b257b..6a1f854a8c33 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -436,7 +436,7 @@ bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
  * callback, where we will timestamp it synchronously.
  */
 bool sja1105_port_txtstamp(struct dsa_switch *ds, int port,
-                          struct sk_buff *skb, unsigned int type)
+                          struct sk_buff *skb, struct sk_buff **clone)
 {
        struct sja1105_private *priv = ds->priv;
        struct sja1105_port *sp = &priv->ports[port];
@@ -444,6 +444,10 @@ bool sja1105_port_txtstamp(struct dsa_switch *ds, int port,
        if (!sp->hwts_tx_en)
                return false;
 
+       *clone = skb_clone_sk(skb);
+       if (!(*clone))
+               return false;
+
        return true;
 }
 
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.h 
b/drivers/net/dsa/sja1105/sja1105_ptp.h
index 3daa33e98e77..ab80b73219cb 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.h
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.h
@@ -105,7 +105,7 @@ bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
                           struct sk_buff *skb, unsigned int type);
 
 bool sja1105_port_txtstamp(struct dsa_switch *ds, int port,
-                          struct sk_buff *skb, unsigned int type);
+                          struct sk_buff *skb, struct sk_buff **clone);
 
 int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 1259b0f40684..c8415c324e27 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -734,7 +734,7 @@ struct dsa_switch_ops {
        int     (*port_hwtstamp_set)(struct dsa_switch *ds, int port,
                                     struct ifreq *ifr);
        bool    (*port_txtstamp)(struct dsa_switch *ds, int port,
-                                struct sk_buff *clone, unsigned int type);
+                                struct sk_buff *skb, struct sk_buff **clone);
        bool    (*port_rxtstamp)(struct dsa_switch *ds, int port,
                                 struct sk_buff *skb, unsigned int type);
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9300cb66e500..5b746a903ef4 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -19,7 +19,6 @@
 #include <linux/if_bridge.h>
 #include <linux/if_hsr.h>
 #include <linux/netpoll.h>
-#include <linux/ptp_classify.h>
 
 #include "dsa_priv.h"
 
@@ -555,26 +554,19 @@ static void dsa_skb_tx_timestamp(struct dsa_slave_priv *p,
                                 struct sk_buff *skb)
 {
        struct dsa_switch *ds = p->dp->ds;
-       struct sk_buff *clone;
-       unsigned int type;
+       struct sk_buff *clone = NULL;
 
-       type = ptp_classify_raw(skb);
-       if (type == PTP_CLASS_NONE)
+       if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
                return;
 
        if (!ds->ops->port_txtstamp)
                return;
 
-       clone = skb_clone_sk(skb);
-       if (!clone)
+       if (!ds->ops->port_txtstamp(ds, p->dp->index, skb, &clone))
                return;
 
-       if (ds->ops->port_txtstamp(ds, p->dp->index, clone, type)) {
+       if (clone)
                DSA_SKB_CB(skb)->clone = clone;
-               return;
-       }
-
-       kfree_skb(clone);
 }
 
 netdev_tx_t dsa_enqueue_skb(struct sk_buff *skb, struct net_device *dev)
@@ -628,9 +620,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, 
struct net_device *dev)
 
        DSA_SKB_CB(skb)->clone = NULL;
 
-       /* Identify PTP protocol packets, clone them, and pass them to the
-        * switch driver
-        */
+       /* Handle tx timestamp request if has */
        dsa_skb_tx_timestamp(p, skb);
 
        if (dsa_realloc_skb(skb, dev)) {
-- 
2.25.1

Reply via email to