Re: [PATCH net-next v3 2/2] 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:
- [High] net: stmmac: XDP_TX and XDP_REDIRECT silently truncate fragmented multi-buffer frames
--

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

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

This patch accumulates descriptors that make up a frame to build an xdp_buff
so the XDP program runs on the full fragmented packet. If not consumed by the
program, it constructs the skb from the head buffer and collected fragments.

> 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

[ ... ]

> @@ -5874,135 +5996,94 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)

[ ... ]

> +		/* For Not XDP_PASS verdict */
> +		if (IS_ERR(skb)) {
> +			unsigned int xdp_res = -PTR_ERR(skb);
> +
> +			if (xdp_res & STMMAC_XDP_CONSUMED) {
> +				stmmac_xdp_put_buff(rx_q, &ctx.xdp, sync_len,
> +						    true);
> +				rx_dropped++;
> +			} else if (xdp_res & (STMMAC_XDP_TX |
> +					      STMMAC_XDP_REDIRECT)) {
> +				xdp_status |= xdp_res;
> +			}

[Severity: High]
Does this silently truncate fragmented multi-buffer frames when the XDP program 
returns XDP_TX or XDP_REDIRECT?

Looking at the transmit path in stmmac_xdp_xmit_xdpf(), it maps and prepares
only the linear data of the xdp_frame:

	if (dma_map) {
		dma_addr = dma_map_single(priv->device, xdpf->data,
					  xdpf->len, DMA_TO_DEVICE);

It does not appear to check xdp_frame_has_frags() or map the fragments stored
in skb_shared_info.

Since this patch allows stmmac_rx() to build and process fragmented xdp_buffs, 
if the program forwards a fragmented packet, won't the hardware transmit only
the linear portion and drop the rest?

> +
> +			/* Clear skb as it was set as status by
> +			 * the XDP program
> +			 */
> +			frames = 0;
>  			count++;
>  			continue;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-stmmac-rx-mb-v3-0-50fa171af9ec@oss.qualcomm.com?part=2