From: Vladimir Oltean <vladimir.olt...@nxp.com>

The ocelot_port->ts_id is used to:
- populate skb->cb[0] for matching the TX timestamp in the PTP IRQ with
  an skb.
- populate the REW_OP from the injection header of the ongoing skb.
Only then is ocelot_port->ts_id incremented.

This is a problem because, at least theoretically, another timestampable
skb might use the same ocelot_port->ts_id before that is incremented. So
the logic of using and incrementing the timestamp id should be atomic
per port.

The solution is to use the global ocelot_port->ts_id only while
protected by the associated ocelot_port->ts_id_lock. That's where we
populate skb->cb[0]. Note that for ocelot,
ocelot_port_add_txtstamp_skb() is called for the actual skb, but for
felix, it is called for the skb's clone. That is something which will
also be changed.

Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c     | 13 ++++++++++++-
 drivers/net/ethernet/mscc/ocelot_net.c |  6 ++----
 include/soc/mscc/ocelot.h              |  1 +
 net/dsa/tag_ocelot.c                   | 11 +++++++----
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c 
b/drivers/net/ethernet/mscc/ocelot.c
index 5abb7d2b0a9e..8caf3afd464d 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -421,10 +421,15 @@ int ocelot_port_add_txtstamp_skb(struct ocelot_port 
*ocelot_port,
 
        if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP &&
            ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
+               spin_lock(&ocelot_port->ts_id_lock);
+
                shinfo->tx_flags |= SKBTX_IN_PROGRESS;
                /* Store timestamp ID in cb[0] of sk_buff */
-               skb->cb[0] = ocelot_port->ts_id % 4;
+               skb->cb[0] = ocelot_port->ts_id;
+               ocelot_port->ts_id = (ocelot_port->ts_id + 1) % 4;
                skb_queue_tail(&ocelot_port->tx_skbs, skb);
+
+               spin_unlock(&ocelot_port->ts_id_lock);
                return 0;
        }
        return -ENODATA;
@@ -1443,6 +1448,12 @@ int ocelot_init(struct ocelot *ocelot)
        if (!ocelot->stats_queue)
                return -ENOMEM;
 
+       for (port = 0; port < ocelot->num_phys_ports; port++) {
+               struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+               spin_lock_init(&ocelot_port->ts_id_lock);
+       }
+
        INIT_LIST_HEAD(&ocelot->multicast);
        ocelot_mact_init(ocelot);
        ocelot_vlan_init(ocelot);
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c 
b/drivers/net/ethernet/mscc/ocelot_net.c
index cacabc23215a..8490e42e9e2d 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -349,10 +349,8 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct 
net_device *dev)
 
        if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP) {
                info.rew_op = ocelot_port->ptp_cmd;
-               if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
-                       info.rew_op |= (ocelot_port->ts_id  % 4) << 3;
-                       ocelot_port->ts_id++;
-               }
+               if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+                       info.rew_op |= skb->cb[0] << 3;
        }
 
        ocelot_gen_ifh(ifh, &info);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index da369b12005f..4521dd602ddc 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -566,6 +566,7 @@ struct ocelot_port {
        u8                              ptp_cmd;
        struct sk_buff_head             tx_skbs;
        u8                              ts_id;
+       spinlock_t                      ts_id_lock;
 
        phy_interface_t                 phy_mode;
 
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 42f327c06dca..b4fc05cafaa6 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -160,11 +160,14 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
        packing(injection, &qos_class, 19,  17, OCELOT_TAG_LEN, PACK, 0);
 
        if (ocelot->ptp && (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
+               struct sk_buff *clone = DSA_SKB_CB(skb)->clone;
+
                rew_op = ocelot_port->ptp_cmd;
-               if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
-                       rew_op |= (ocelot_port->ts_id  % 4) << 3;
-                       ocelot_port->ts_id++;
-               }
+               /* Retrieve timestamp ID populated inside skb->cb[0] of the
+                * clone by ocelot_port_add_txtstamp_skb
+                */
+               if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+                       rew_op |= clone->cb[0] << 3;
 
                packing(injection, &rew_op, 125, 117, OCELOT_TAG_LEN, PACK, 0);
        }
-- 
2.25.1

Reply via email to