Re: [PATCH net-next v6 2/4] net: stmmac: drop split frames on AF_XDP zero-copy RX

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] net: stmmac: inverted likely() branch prediction hint penalizes fast path
--

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

net: stmmac: drop split frames on AF_XDP zero-copy RX

This commit fixes the AF_XDP zero-copy receive path to properly drop and
drain frames that span multiple RX descriptors. It ensures the fragmented
descriptors are released without being incorrectly processed as a new frame.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -5721,8 +5724,9 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
>  		if (likely(status & rx_not_ls)) {

[Severity: Low]
Does this use of likely() invert the branch prediction hint?

Since XDP limits MTU to 1500, valid frames should fit completely into a 
single buffer, making rx_not_ls 0 in the normal fast path. By using likely() 
here, the compiler is instructed to optimize for split or dropped frames.

Would it be better to use unlikely() so the fast path for valid packets isn't
penalized? A few lines further down, stmmac_rx_zc() correctly uses 
likely(!(status & rx_not_ls)) for the valid packet case.

>  			xsk_buff_free(buf->xdp);
>  			buf->xdp = NULL;
> +			rx_dropped++;
> +			error = 1;
>  			dirty++;
> -			count++;
>  			goto read_again;
>  		}

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