From: Vladimir Oltean <olteanv@gmail.com>
To: davem@davemloft.net, netdev@vger.kernel.org
Cc: yangbo.lu@nxp.com, xiaoliang.yang_1@nxp.com,
UNGLinuxDriver@microchip.com, claudiu.manoil@nxp.com,
alexandre.belloni@bootlin.com, andrew@lunn.ch,
vivien.didelot@gmail.com, f.fainelli@gmail.com, kuba@kernel.org
Subject: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
Date: Tue, 15 Sep 2020 21:22:24 +0300 [thread overview]
Message-ID: <20200915182229.69529-3-olteanv@gmail.com> (raw)
In-Reply-To: <20200915182229.69529-1-olteanv@gmail.com>
From: Vladimir Oltean <vladimir.oltean@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.oltean@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
next prev parent reply other threads:[~2020-09-15 18:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
2020-09-15 18:22 ` Vladimir Oltean [this message]
2020-09-16 11:12 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Alexandre Belloni
2020-09-16 12:25 ` Vladimir Oltean
2020-09-17 0:19 ` David Miller
2020-09-17 23:43 ` Vladimir Oltean
2020-09-17 23:57 ` David Miller
2020-09-15 18:22 ` [PATCH net 3/7] net: dsa: seville: fix buffer size of the queue system Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
2020-09-17 0:21 ` David Miller
2020-09-15 18:22 ` [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
2020-09-17 0:24 ` David Miller
2020-09-15 18:22 ` [PATCH net 6/7] net: mscc: ocelot: refactor ports parsing code into a dedicated function Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 7/7] net: mscc: ocelot: unregister net devices on unbind Vladimir Oltean
2020-09-15 21:19 ` [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Horatiu Vultur
2020-09-16 9:56 ` Alexandre Belloni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200915182229.69529-3-olteanv@gmail.com \
--to=olteanv@gmail.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@gmail.com \
--cc=xiaoliang.yang_1@nxp.com \
--cc=yangbo.lu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).