Re: [PATCH net-next v2 08/13] net: mana: keep per-queue statistics in the port context

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrent writers to the same `u64_stats_sync` structure during RX queue swap.
--

--- Patch [8]: [PATCH net-next v2 08/13] net: mana: keep per-queue statistics in the port context ---
commit 48e144b542daf3b7f2ff07b74db60098e784159d
Author: Long Li <longli@microsoft.com>

net: mana: keep per-queue statistics in the port context

This commit moves per-queue statistics out of the queue structures and into a
port-context array to prevent statistics from resetting when the queue is
reconfigured. The stats are indexed by the queue index, and the arrays outlive
the queues so totals stay monotonic.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -1059,6 +1065,48 @@ static void mana_cleanup_port_context(struct mana_port_context *apc)
>  	apc->rxqs = NULL;
>  }
>  
> +/* Counters belong to the port, not the queues, so a queue-set replacement
> + * does not reset them. Sized to max_queues, allocated once.
> + *
> + * A swap adds no writer to a TX slot. RX slots do overlap briefly, since a
> + * retiring rxq keeps its NAPI until mana_free_qset() destroys it. MANA is
> + * 64-bit only, so u64_stats_sync has no seqcount and at worst a few
> + * increments are lost; the alternatives are a lock in the receive path or
> + * per-set slots that make ndo_get_stats64() dip during a swap.
> + */

[Severity: Medium]
Is it safe to allow concurrent writers to the same u64_stats_sync structure?

The u64_stats_sync API requires that writers must be mutually exclusive for a
given syncp. While MANA is 64-bit only and lacks a seqcount in the syncp,
allowing both the retiring RX queue's NAPI and the new RX queue's NAPI to
concurrently update the same apc->rxq_stats[rxq_idx] introduces a data race.

For example, both could be running mana_rx_skb() at the same time:

	u64_stats_update_begin(&rx_stats->syncp);
	rx_stats->packets++;
	rx_stats->bytes += pkt_len;

Could this lead to undefined behavior or KCSAN warnings when the queues
overlap during a swap?

> +static int mana_alloc_queue_stats(struct mana_port_context *apc)
> +{
> +	unsigned int i;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813050418.2906468-1-longli@microsoft.com?part=8