Re: [PATCH net-next v5 09/13] net: mana: keep per-queue statistics in the port context
From: sashiko-bot@kernel.org
Date: 2026-09-10 22:25:27
Also in:
linux-hyperv
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] u64_stats_update_begin called with preemption enabled causes potential deadlock. Pre-existing issues: - [High] Missing Mutual Exclusion on tx_stats updates between mana_xdp_xmit and mana_start_xmit. -- commit 45f5d2a315921074d282869dd20e8233e5bb463b Author: Long Li [off-list ref] Subject: net: mana: keep per-queue statistics in the port context Move RX/TX counters into port-lifetime arrays so queue rebuilds do not reset them. Sum all max_queues slots for ndo_get_stats64(), keep the current queue count for ethtool, and fold retiring RX queues count into private drain_stats upon destruction.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c index debf0e7765862..2060e08d82277 100644 --- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c +++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -73,7 +73,7 @@ int mana_xdp_xmit(struct net_device *ndev, int n, struct xdp_frame **frames, count++; } - tx_stats = &apc->tx_qp[q_idx]->txq.stats; + tx_stats = apc->tx_qp[q_idx]->txq.stats; u64_stats_update_begin(&tx_stats->syncp); tx_stats->xdp_xmit += count; u64_stats_update_end(&tx_stats->syncp);
[Severity: High] This is a pre-existing issue, but is there a missing mutual exclusion between mana_xdp_xmit() and mana_start_xmit() when updating tx_stats->syncp? mana_xdp_xmit() bypasses the qdisc HARD_TX_LOCK and maps q_idx using smp_processor_id(). As a result, it can run concurrently with mana_start_xmit() which uses the skb's queue mapping. Because the skb queue mapping could map to the same queue from another CPU, both functions might run simultaneously on different CPUs and call u64_stats_update_begin() on the same syncp. This would corrupt the sequence counter.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c index 5752e3745ccc6..98007bb3ef7ca 100644 --- a/drivers/net/ethernet/microsoft/mana/mana_en.c +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -575,7 +575,7 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev) /* Populated the packet and bytes counters based on post GSO packet * calculations */ - tx_stats = &txq->stats; + tx_stats = txq->stats; u64_stats_update_begin(&tx_stats->syncp); tx_stats->packets += num_gso_seg; tx_stats->bytes += len + ((num_gso_seg - 1) * gso_hs);
[Severity: High] This is a pre-existing issue, but this is the primary packet transmission path that can run concurrently with the XDP transmission path in mana_xdp_xmit() mentioned above, contributing to the lost updates on the shared tx_stats struct. [ ... ]
+static void mana_fold_rxq_stats(struct mana_port_context *apc,
+ struct mana_rxq *rxq)
+{
+ struct mana_stats_rx *src = &rxq->drain_stats;
+ struct mana_stats_rx *dst;
+ unsigned int i;
+
+ ASSERT_RTNL();
+
+ if (!apc->rxq_stats_ret || rxq->rxq_idx >= apc->max_queues)
+ return;
+
+ dst = &apc->rxq_stats_ret[rxq->rxq_idx];
+
+ u64_stats_update_begin(&dst->syncp);
+ dst->packets += src->packets;
+ dst->bytes += src->bytes;[Severity: High] Does mana_fold_rxq_stats() need to disable preemption or bottom halves before acquiring the sequence lock here? This function executes in process context holding the RTNL lock, but it calls u64_stats_update_begin() with preemption enabled. On 32-bit systems, if the thread is preempted or interrupted by a softirq on the same CPU that subsequently reads the stats via mana_get_stats64(), the reader could spin infinitely waiting for the sequence lock to become even. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909222416.884246-1-longli@microsoft.com?part=9