Re: [PATCH net-next 2/2] net: stmmac: disable TX checksum offload for jumbo frames on bugged cores

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message's premise doesn't match the tree.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  stmmac_xdp_xmit_xdpf() never looks at priv->plat->tx_coe when deciding…

--

net: stmmac: disable TX checksum offload for jumbo frames on bugged cores

This patch turns off TX checksum insertion in stmmac_xdp_xmit_xdpf() for
frames longer than ETH_FRAME_LEN on bugged_jumbo cores. It is meant to
match the stmmac_fix_features() workaround that the skb path already uses.

> The XDP TX
> path bypasses that workaround, so once XDP is allowed at MTU >
> ETH_DATA_LEN, jumbo frames sent via XDP_TX or redirected into the
> device would leave the interface with a full checksum insertion enabled
> on those cores - exactly what the workaround is meant to avoid.

[Severity: Low]
Can this scenario happen in the current tree? XDP still can't be used with
a jumbo MTU. stmmac_xdp_set_prog() refuses to attach a program when the MTU
is above ETH_DATA_LEN:

drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() {
    ...
	if (prog && dev->mtu > ETH_DATA_LEN) {
		/* For now, the driver doesn't support XDP functionality with
		 * jumbo frames so we return error.
		 */
		NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported");
		return -EOPNOTSUPP;
	}
    ...
}

stmmac_change_mtu() also refuses a jumbo MTU while XDP is enabled:

	if (stmmac_xdp_is_enabled(priv) && new_mtu > ETH_DATA_LEN) {
		netdev_dbg(priv->dev, "Jumbo frames not supported for XDP\n");
		return -EINVAL;
	}

The redirect case looks limited too. Devmap checks every frame with
xdp_ok_fwd_dev(dev, xdp_get_frame_len(xdpf)), which caps the length at
mtu + hard_header_len + VLAN_HLEN. At MTU 1500 that is 1518 bytes.
Redirected frames therefore reach the new ETH_FRAME_LEN check only when
they are 1515 to 1518 bytes long, which is VLAN-tagged normal-size
traffic, not jumbo.

The one path that seems to reach the new check today is XDP_TX after the
program grows the frame. At MTU 1500, stmmac_rx() still sets frame_sz to
at least PAGE_SIZE:

	bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;

A program can therefore use bpf_xdp_adjust_tail() to grow a frame past
1514 bytes and return XDP_TX. stmmac_xdp_xmit_xdpf() never checks that
length against the MTU.

Could the commit message describe the trigger that exists today, and say
whether the bugged_jumbo erratum covers it? If this patch depends on a
separate change that allows XDP at a jumbo MTU, could the message say so?
The Fixes: tag reads as if the problem can be hit now.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ca61ab449e32b..0e29c5d04714c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5276,6 +5276,14 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>  		return STMMAC_XDP_CONSUMED;
>  	}
>  
> +	/* Disable TX checksum offload for jumbo frames on cores where
> +	 * checksum insertion is broken for oversized frames, matching the
> +	 * stmmac_fix_features() workaround used by the skb path.
> +	 */
> +	if (priv->plat->bugged_jumbo &&
> +	    xdp_get_frame_len(xdpf) > ETH_FRAME_LEN)
> +		csum = false;

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. The
comment says this matches stmmac_fix_features(), but that function also
turns off TX checksum offload when tx_coe is unset:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_fix_features() {
    ...
	if (!priv->plat->tx_coe)
		features &= ~NETIF_F_CSUM_MASK;
    ...
}

In stmmac_xdp_xmit_xdpf(), csum comes only from the per-queue flag:

	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;

stmmac_hw_init() clears tx_coe when threshold DMA mode is forced:

		/* TXCOE doesn't work in thresh DMA mode */
		if (priv->plat->force_thresh_dma_mode)
			priv->plat->tx_coe = false;

stmmac_dma_operation_mode() then runs TX in threshold mode
(txmode = tc).

On platforms with snps,force_thresh_dma_mode, do XDP_TX and ndo_xdp_xmit
frames still request full checksum insertion through
stmmac_prepare_tx_desc() in a DMA mode where the driver treats TX COE as
broken? Should the XDP path also check priv->plat->tx_coe here?

>  	while (true) {
>  		skb_frag_t *frag = i ? &sinfo->frags[i - 1] : NULL;
>  		int len = frag ? skb_frag_size(frag) : xdpf->len;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-xdp-tx-pre-fixes-v1-0-05fd0c90f7e1%40oss.qualcomm.com