summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKoichiro Den <den@valinux.co.jp>2026-08-31 00:16:17 +0900
committerJakub Kicinski <kuba@kernel.org>2026-08-31 20:09:35 -0700
commit545b63503c696c4ce0663b3fcd37f41169aec1eb (patch)
treeff8a94cba5563bd55941754592c706af50b7a1d5
parent975b5b067f525a1b1338c4a3bee1c46545801518 (diff)
net: ntb_netdev: Fix statistics races
ntb_netdev updates shared net_device stats from per-QP RX and TX callbacks. Once multiple queues are enabled, concurrent updates can be lost. Use per-CPU tstats for packet and byte counters and DEV_STATS_INC() for less frequent drop and error counters. Callbacks can run synchronously in the xmit path or asynchronously from a tasklet or the memcpy kthread. Pin TX updates against migration in the kthread path. Use the IRQ-safe u64_stats helpers because netpoll can invoke the synchronous path with IRQs disabled. Let the core manage tstats while keeping transport teardown after unregister_netdev(), outside RTNL. RCU lets unregister wait for TX completions already updating stats, while later completions only consume the skb and skip accounting and queue wake. Fixes: 24d9e73c7e00 ("net: ntb_netdev: Support ethtool channels for multi-queue") Cc: stable@vger.kernel.org Suggested-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Koichiro Den <den@valinux.co.jp> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20260830151617.3546585-1-den@valinux.co.jp Signed-off-by: Jakub Kicinski <kuba@kernel.org>
-rw-r--r--drivers/net/ntb_netdev.c47
1 files changed, 34 insertions, 13 deletions
diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 9c171697e762..2c04be6d61a8 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -127,8 +127,10 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
{
struct ntb_netdev_queue *q = qp_data;
struct ntb_netdev *dev = q->ntdev;
+ struct pcpu_sw_netstats *tstats;
struct sk_buff *skb, *new_skb;
struct net_device *ndev;
+ unsigned long flags;
int rc;
ndev = dev->ndev;
@@ -139,17 +141,20 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
netdev_dbg(ndev, "%s: %d byte payload received\n", __func__, len);
if (len < 0) {
- ndev->stats.rx_errors++;
- ndev->stats.rx_length_errors++;
+ DEV_STATS_INC(ndev, rx_errors);
+ DEV_STATS_INC(ndev, rx_length_errors);
goto enqueue_again;
}
- ndev->stats.rx_packets++;
- ndev->stats.rx_bytes += len;
+ tstats = this_cpu_ptr(ndev->tstats);
+ flags = u64_stats_update_begin_irqsave(&tstats->syncp);
+ u64_stats_inc(&tstats->rx_packets);
+ u64_stats_add(&tstats->rx_bytes, len);
+ u64_stats_update_end_irqrestore(&tstats->syncp, flags);
new_skb = netdev_alloc_skb(ndev, ndev->mtu + ETH_HLEN);
if (!new_skb) {
- ndev->stats.rx_dropped++;
+ DEV_STATS_INC(ndev, rx_dropped);
goto enqueue_again;
}
@@ -166,8 +171,8 @@ enqueue_again:
rc = ntb_transport_rx_enqueue(qp, skb, skb->data, ndev->mtu + ETH_HLEN);
if (rc) {
dev_kfree_skb_any(skb);
- ndev->stats.rx_errors++;
- ndev->stats.rx_fifo_errors++;
+ DEV_STATS_INC(ndev, rx_errors);
+ DEV_STATS_INC(ndev, rx_fifo_errors);
}
}
@@ -210,25 +215,39 @@ static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp, void *qp_data,
{
struct ntb_netdev_queue *q = qp_data;
struct ntb_netdev *dev = q->ntdev;
+ struct pcpu_sw_netstats *tstats;
struct net_device *ndev;
struct sk_buff *skb;
+ unsigned long flags;
+ bool registered;
ndev = dev->ndev;
skb = data;
if (!skb || !ndev)
return;
+ rcu_read_lock();
+ registered = READ_ONCE(ndev->reg_state) == NETREG_REGISTERED;
+ if (!registered)
+ goto free_skb;
+
if (len > 0) {
- ndev->stats.tx_packets++;
- ndev->stats.tx_bytes += skb->len;
+ /* The memcpy kthread can migrate, so pin the per-CPU update. */
+ tstats = get_cpu_ptr(ndev->tstats);
+ flags = u64_stats_update_begin_irqsave(&tstats->syncp);
+ u64_stats_inc(&tstats->tx_packets);
+ u64_stats_add(&tstats->tx_bytes, skb->len);
+ u64_stats_update_end_irqrestore(&tstats->syncp, flags);
+ put_cpu_ptr(ndev->tstats);
} else {
- ndev->stats.tx_errors++;
- ndev->stats.tx_aborted_errors++;
+ DEV_STATS_INC(ndev, tx_errors);
+ DEV_STATS_INC(ndev, tx_aborted_errors);
}
+free_skb:
dev_kfree_skb_any(skb);
- if (ntb_transport_tx_free_entry(qp) >= tx_start) {
+ if (registered && ntb_transport_tx_free_entry(qp) >= tx_start) {
/* Make sure anybody stopping the queue after this sees the new
* value of ntb_transport_tx_free_entry()
*/
@@ -237,6 +256,7 @@ static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp, void *qp_data,
ntb_transport_link_query(q->qp))
netif_wake_subqueue(ndev, q->qid);
}
+ rcu_read_unlock();
}
static const struct ntb_queue_handlers ntb_netdev_handlers = {
@@ -277,7 +297,7 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
drop:
dev_kfree_skb_any(skb);
- ndev->stats.tx_dropped++;
+ DEV_STATS_INC(ndev, tx_dropped);
return NETDEV_TX_OK;
}
@@ -647,6 +667,7 @@ static int ntb_netdev_probe(struct device *client_dev)
}
ndev->features = NETIF_F_HIGHDMA;
+ ndev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;