Re: [PATCH net-next v2] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: stmmac: missing drop accounting when XDP fragment addition fails
--

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

net: stmmac: rework stmmac_rx to support XDP rx multi-buff

This commit builds the xdp_buff by accumulating descriptors so the XDP
program runs on the full packet. It saves the state in rx_q->state across
NAPI poll boundaries and builds the skb from the head buffer and fragments.

Link: https://lore.kernel.org/r/20260918-stmmac-rx-mb-v1-1-0b4517d404af@oss.qualcomm.com

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index bf9e7e4cb1c31..7d1149511c55d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -5866,6 +5932,33 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  			xdp_init_buff(&ctx.xdp, bufsz, &rx_q->xdp_rxq);
>  			xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
>  					 buf->page_offset, buf1_len, true);
> +			first_desc = false;
> +			buf->page = NULL;
> +		} else if (buf1_len) {
> +			error |= !stmmac_build_xdp_frags(priv, rx_q, buf1_len,
> +							 buf->page,
> +							 buf->page_offset,
> +							 dma_dir, &ctx.xdp);

[Severity: Medium]
When stmmac_build_xdp_frags() returns false, the error flag is set.

This could happen when receiving large frames (e.g. Jumbo frames) on hardware
configured with small DMA buffers, leading to many descriptors per frame.

Will this cause the packet to be silently dropped without updating
rx_dropped or rx_errors?

> +			buf->page = NULL;
> +		}
> +
> +		if (buf2_len) {
> +			error |= !stmmac_build_xdp_frags(priv, rx_q, buf2_len,
> +							 buf->sec_page,
> +							 buf->page_offset,
> +							 dma_dir, &ctx.xdp);
> +			buf->sec_page = NULL;
> +		}
> +
> +		if (likely(status & rx_not_ls))
> +			goto read_again;
> +
> +		if (unlikely(error))
> +			goto error_free_frag;

[ ... ]

> @@ -5989,13 +6036,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  		rx_packets++;
>  		rx_bytes += len;
>  		count++;
> +		continue;
> +error_free_frag:
> +		if (!first_desc) {
> +			stmmac_xdp_put_buff(rx_q, &ctx.xdp, -1);
> +			first_desc = true;
> +		}
> +		dev_kfree_skb(skb);
> +		skb = NULL;
> +		count++;

[Severity: Medium]
If we reach this teardown path due to an error, the frame is consumed and
the polling count is incremented, but no interface drop statistics appear to
be updated.

Could this make debugging MTU or fragmentation issues difficult, since drops
(such as exceeding MAX_SKB_FRAGS) would be invisible to monitoring? Should
rx_dropped or rx_errors be incremented here?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-stmmac-rx-mb-v2-1-6e826e1ff306@oss.qualcomm.com?part=1