Re: [PATCH net-next v7 07/13] net: mana: swap queue sets in mana_change_mtu

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mana: data race on TX stats syncp in mana_xdp_xmit
--

commit 03c88ebc19bffbae5acfce1a1948eb4cb597ee75
Author: Long Li <longli@microsoft.com>

net: mana: swap queue sets in mana_change_mtu

Carry the MTU in the queue set and size replacement RX buffers from it.
Publish ndev->mtu after RSS configuration succeeds; allocation failure
leaves the live queues and advertised MTU unchanged.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> index 484f061bc75e4..687b9ce7d9a3b 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> @@ -68,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;

[Severity: Medium]
Does this stats update in mana_xdp_xmit() lack mutual exclusion when racing
with other CPUs?

When multiple CPUs redirect traffic via ndo_xdp_xmit, they can map to the
same q_idx (since q_idx = smp_processor_id() % real_num_tx_queues) and
execute u64_stats_update_begin() concurrently.

This could also race with mana_start_xmit(), which updates the same
per-queue syncp:

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 489558176a5de..c1ff4d9b8d45f 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -551,7 +579,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;

While mana_start_xmit() holds __netif_tx_lock, that lock does not protect
against lockless mana_xdp_xmit() calls.

On architectures where u64_stats_update_begin() uses seqcounts, could this
concurrent modification corrupt the seqcount and cause readers (like
mana_get_stats64()) to infinite loop or read torn values?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791710735.git.weh@linux.microsoft.com?part=7