diff options
| author | Koichiro Den <den@valinux.co.jp> | 2026-08-31 00:16:17 +0900 |
|---|---|---|
| committer | Jakub Kicinski <kuba@kernel.org> | 2026-08-31 20:09:35 -0700 |
| commit | 545b63503c696c4ce0663b3fcd37f41169aec1eb (patch) | |
| tree | ff8a94cba5563bd55941754592c706af50b7a1d5 | |
| parent | 975b5b067f525a1b1338c4a3bee1c46545801518 (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.c | 47 |
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; |
