Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: lost increments of global rx_dropped/rx_errors in concurrent NAPI polls
--

commit 45e89e4ad29cf5d670b87e898ffe94eedf0e0127
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

net: stmmac: drop multi-buffer frames in XDP mode

stmmac_rx() runs the XDP program once per descriptor buffer and uses skb ==
NULL as the only marker for the start of a frame. A frame larger than one
buffer is split by the hardware across several descriptors. XDP does not
support scatter-gather frames until the multi-buffer rework, so this commit
drops the whole frame instead.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 426bb68c9bbce..8c08d44fdbd39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  
>  		if (priv->extend_desc)
>  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry);
> -		if (unlikely(status == discard_frame)) {
> -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> -			buf->page = NULL;
> +
> +		if (unlikely(!error && status == discard_frame)) {
>  			error = 1;
>  			if (!priv->hwts_rx_en)
>  				rx_errors++;
>  		}
>  
> -		if (unlikely(error && (status & rx_not_ls)))
> -			goto read_again;
> +		/* We do not support SG in XDP mode */
> +		if (unlikely(!error && (status & rx_not_ls) &&
> +			     stmmac_xdp_is_enabled(priv))) {
> +			rx_dropped++;

[Severity: Medium]
Will this lead to lost increments in the global rx_dropped and rx_errors
statistics?

The local rx_dropped and rx_errors counters are updated here, and at the
end of stmmac_rx() they are added directly to the global statistics:

	priv->xstats.rx_dropped += rx_dropped;
	priv->xstats.rx_errors += rx_errors;

When multiple NAPI handlers for different RX queues run concurrently on
different CPUs, could they race on updating these global counters without
synchronization?

> +			error = 1;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-stmmac-rx-mb-v6-0-2225050915c1@oss.qualcomm.com?part=3