[PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Jisheng Zhang]

stmmac enables TX checksum insertion for XDP frames whenever the queue
supports it. XDP frames carry no TX checksum offload request, so this
can overwrite a checksum already present in the packet.

Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
so that the packet is sent with its checksum fields unchanged.

Fixes: be8b38a722e6 ("net: stmmac: Add support for XDP_TX action")
Fixes: 8b278a5b69a2 ("net: stmmac: Add support for XDP_REDIRECT action")
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 24656b35350b..8fe75b00974f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5184,7 +5184,6 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 {
 	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
-	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
 	unsigned int entry = tx_q->cur_tx;
 	enum stmmac_txbuf_type buf_type;
 	struct dma_desc *tx_desc;
@@ -5229,7 +5228,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 	stmmac_set_desc_addr(priv, tx_desc, dma_addr);
 
 	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
-			       csum, priv->descriptor_mode, true, true,
+			       false, priv->descriptor_mode, true, true,
 			       xdpf->len);
 
 	tx_q->tx_count_frames++;
-- 
2.53.0

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Andrew Lunn]

On Tue, Sep 29, 2026 at 08:10:25PM +0800, Jisheng Zhang wrote:
> stmmac enables TX checksum insertion for XDP frames whenever the queue
> supports it. XDP frames carry no TX checksum offload request, so this
> can overwrite a checksum already present in the packet.
> 
> Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
> so that the packet is sent with its checksum fields unchanged.

This seems odd to me.

If the frame contains a checksum, it is either correct, and the
hardware calculated one will come out the same, not an issue.  Or the
checksum in the frame is actually wrong, because the frame has got
mangled by eBPF before sending it out, and you want the hardware to
calculate the correct value.

What an i missing?

	Andrew

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Jakub Kicinski]

On Tue, 29 Sep 2026 22:29:14 +0200 Andrew Lunn wrote:
> On Tue, Sep 29, 2026 at 08:10:25PM +0800, Jisheng Zhang wrote:
> > stmmac enables TX checksum insertion for XDP frames whenever the queue
> > supports it. XDP frames carry no TX checksum offload request, so this
> > can overwrite a checksum already present in the packet.
> > 
> > Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
> > so that the packet is sent with its checksum fields unchanged.  
> 
> This seems odd to me.
> 
> If the frame contains a checksum, it is either correct, and the
> hardware calculated one will come out the same, not an issue.  Or the
> checksum in the frame is actually wrong, because the frame has got
> mangled by eBPF before sending it out, and you want the hardware to
> calculate the correct value.
> 
> What an i missing?

Oops, missed this before applying. Fair, but also we shouldn't modify
the frame if user didn't ask for it. Perhaps 0 UDP csum is used, or
user is trying to build a testing tool and intentionally send bad csum.
I applied the patch to net-next. It can hurt as much as help, but the
direction seems right. At the very least any AF_XDP app expecting csum
insertion without asking for it would not be portable to other drivers.

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Jisheng Zhang]

On Tue, Sep 29, 2026 at 10:29:14PM +0200, Andrew Lunn wrote:
> On Tue, Sep 29, 2026 at 08:10:25PM +0800, Jisheng Zhang wrote:
> > stmmac enables TX checksum insertion for XDP frames whenever the queue
> > supports it. XDP frames carry no TX checksum offload request, so this
> > can overwrite a checksum already present in the packet.
> > 
> > Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
> > so that the packet is sent with its checksum fields unchanged.
> 
> This seems odd to me.
> 
> If the frame contains a checksum, it is either correct, and the
> hardware calculated one will come out the same, not an issue.  Or the
> checksum in the frame is actually wrong, because the frame has got
> mangled by eBPF before sending it out, and you want the hardware to
> calculate the correct value.
> 
> What an i missing?
> 
Hi Andrew and all,

some discussions happened in below thread
https://lore.kernel.org/netdev/athAoWfW-xesPhFP@xhacker/

Since it has been merged. Let's move the discussion back to my patch.

AIUI, dwmac can only do HW checksum for normal particular packets
layouts, such as TCP, UDP etc. The stmmac_has_ip_ethertype() has
well demostrated this limitation.
Different with normal tx where the common stack supplies the skb->ip_summed
metadata, XDP path doesn't have the necessary metadata. So to make
the XDP always work properly when transmiting tranmist arbitrary Eth
frames, dwmac HW csum needs to be disabled by default currently.

Thanks

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Andrew Lunn]

On Thu, Oct 22, 2026 at 07:33:44PM +0800, Jisheng Zhang wrote:
> On Tue, Sep 29, 2026 at 10:29:14PM +0200, Andrew Lunn wrote:
> > On Tue, Sep 29, 2026 at 08:10:25PM +0800, Jisheng Zhang wrote:
> > > stmmac enables TX checksum insertion for XDP frames whenever the queue
> > > supports it. XDP frames carry no TX checksum offload request, so this
> > > can overwrite a checksum already present in the packet.
> > > 
> > > Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
> > > so that the packet is sent with its checksum fields unchanged.
> > 
> > This seems odd to me.
> > 
> > If the frame contains a checksum, it is either correct, and the
> > hardware calculated one will come out the same, not an issue.  Or the
> > checksum in the frame is actually wrong, because the frame has got
> > mangled by eBPF before sending it out, and you want the hardware to
> > calculate the correct value.
> > 
> > What an i missing?
> > 
> Hi Andrew and all,
> 
> some discussions happened in below thread
> https://lore.kernel.org/netdev/athAoWfW-xesPhFP@xhacker/
> 
> Since it has been merged. Let's move the discussion back to my patch.
> 
> AIUI, dwmac can only do HW checksum for normal particular packets
> layouts, such as TCP, UDP etc. The stmmac_has_ip_ethertype() has
> well demostrated this limitation.
> Different with normal tx where the common stack supplies the skb->ip_summed
> metadata, XDP path doesn't have the necessary metadata. So to make
> the XDP always work properly when transmiting tranmist arbitrary Eth
> frames, dwmac HW csum needs to be disabled by default currently.

I don't know the XDP code. But...

Zoom out, look at the big picture:

What do other MAC drivers does? Do they also disable hardware checksum
when XDP is enabled on a queue?

Should XDP actually have the needed meta data? Can it be added in a
low overhead way? Does the overhead of having it cost less than
forcing software check sums?

	Andrew

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[Jisheng Zhang]

On Thu, Oct 22, 2026 at 03:54:54PM +0200, Andrew Lunn wrote:
> On Thu, Oct 22, 2026 at 07:33:44PM +0800, Jisheng Zhang wrote:
> > On Tue, Sep 29, 2026 at 10:29:14PM +0200, Andrew Lunn wrote:
> > > On Tue, Sep 29, 2026 at 08:10:25PM +0800, Jisheng Zhang wrote:
> > > > stmmac enables TX checksum insertion for XDP frames whenever the queue
> > > > supports it. XDP frames carry no TX checksum offload request, so this
> > > > can overwrite a checksum already present in the packet.
> > > > 
> > > > Pass false to stmmac_prepare_tx_desc() when transmitting an XDP frame
> > > > so that the packet is sent with its checksum fields unchanged.
> > > 
> > > This seems odd to me.
> > > 
> > > If the frame contains a checksum, it is either correct, and the
> > > hardware calculated one will come out the same, not an issue.  Or the
> > > checksum in the frame is actually wrong, because the frame has got
> > > mangled by eBPF before sending it out, and you want the hardware to
> > > calculate the correct value.
> > > 
> > > What an i missing?
> > > 
> > Hi Andrew and all,
> > 
> > some discussions happened in below thread
> > https://lore.kernel.org/netdev/athAoWfW-xesPhFP@xhacker/
> > 
> > Since it has been merged. Let's move the discussion back to my patch.
> > 
> > AIUI, dwmac can only do HW checksum for normal particular packets
> > layouts, such as TCP, UDP etc. The stmmac_has_ip_ethertype() has
> > well demostrated this limitation.
> > Different with normal tx where the common stack supplies the skb->ip_summed
> > metadata, XDP path doesn't have the necessary metadata. So to make
> > the XDP always work properly when transmiting tranmist arbitrary Eth
> > frames, dwmac HW csum needs to be disabled by default currently.
> 
> I don't know the XDP code. But...
> 
> Zoom out, look at the big picture:
> 
> What do other MAC drivers does? Do they also disable hardware checksum
> when XDP is enabled on a queue?

I checked ixgbe and i40e. AIUI, their ordinary XDP tx paths don't
request checksum insertion: ixgbe_xmit_xdp_ring() leaves the checksum
offload bits clear, and i40e_xmit_xdp_ring() supplies no checksum
command or header offsets.

So this patch likewise changes the descriptor for each XDP frame. NOTE,
it doesn't disable checksum for normal skb transmissions.

Also disabling insertion doesn't introduce an automatic software
checksum calculation. An unchanged forwarded pkt already has its
checksum. If an XDP prog modifies checksum-covered fields, it's
responsible for updating the checksum.

> 
> Should XDP actually have the needed meta data? Can it be added in a
> low overhead way? Does the overhead of having it cost less than
> forcing software check sums?

This needs XDP common code path changes, I think, it needs a separate
discussion.

Re: [PATCH] net: stmmac: Disable checksum insertion for XDP frame TX

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch fixes only one of the two stmmac XDP-family TX
  paths.
- [Medium] The patch changes user-visible behavior in a way the commit
  message does not mention.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  stmmac_xdp_xmit_xdpf() in…

--

net: stmmac: Disable checksum insertion for XDP frame TX

This commit stops stmmac_xdp_xmit_xdpf() from requesting hardware
checksum insertion for XDP_TX and ndo_xdp_xmit frames. It now passes
false as the csum argument to stmmac_prepare_tx_desc(), so these frames
go out with their checksum fields unchanged.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ec62fa7418f41..81c1a93c9d90d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -5254,7 +5253,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>  	stmmac_set_desc_addr(priv, tx_desc, dma_addr);
>  
>  	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
> -			       csum, priv->descriptor_mode, true, true,
> +			       false, priv->descriptor_mode, true, true,
>  			       xdpf->len);

[Severity: Medium]
Does the AF_XDP zero-copy TX path need the same change? It shares this
TX ring, and stmmac_xdp_xmit_zc() still sets csum based only on what the
queue supports:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_xdp_xmit_zc() {
    ...
	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
    ...
		stmmac_prepare_tx_desc(priv, tx_desc, 1, xdp_desc.len,
				       csum, priv->descriptor_mode, true, true,
				       xdp_desc.len);
    ...
}

When csum_flag is true, every descriptor backend turns on full checksum
insertion. That is TX_CIC_FULL for the dwmac4, enhanced and normal
descriptors, and XGMAC_TDES3_CIC = 0x3 for xgmac.

An XSK frame cannot ask for checksum offload, because
stmmac_xsk_tx_metadata_ops has no .tmo_request_checksum:

static const struct xsk_tx_metadata_ops stmmac_xsk_tx_metadata_ops = {
	.tmo_request_timestamp		= stmmac_xsk_request_timestamp,
	.tmo_fill_timestamp		= stmmac_xsk_fill_timestamp,
	.tmo_request_launch_time	= stmmac_xsk_request_launch_time,
};

For the same reason, netdev_nl_dev_fill() never reports
NETDEV_XSK_FLAGS_TX_CHECKSUM for stmmac. Yet on a queue that supports
checksum offload, the MAC still rewrites the IP and L4 checksums of every
AF_XDP zero-copy frame.

The commit message describes exactly this case: a frame with no TX
checksum offload request whose existing checksum gets overwritten.

Should stmmac_xdp_xmit_zc() also pass false? Another option is to
implement tmo_request_checksum, so insertion is enabled only for frames
that ask for it. Either change would probably need its own Fixes: tag
pointing at the XSK zero-copy TX commit.

[Severity: Medium]
Could frames that relied on the old behavior now go out with bad L4
checksums? With CIC=FULL the hardware computes the pseudo-header itself
and ignores the existing checksum field. That used to fix up redirected
frames whose L4 checksum held only the pseudo-header sum.

veth is one source of such frames. VETH_FEATURES includes
NETIF_F_HW_CSUM, so TCP/UDP traffic from the peer is CHECKSUM_PARTIAL.
veth_xdp_rcv_skb() then converts the skb to an xdp_buff without
completing the checksum:

veth_xdp_rcv_skb()
  XDP_REDIRECT -> xdp_do_redirect()
    stmmac_xdp_xmit()
      stmmac_xdp_xmit_xdpf()   /* csum is now always false */

After this patch those frames leave with only the partial checksum, and
receivers drop them.

The new behavior matches other XDP drivers, and the root cause is
arguably on the sending side. However, the commit message only says:

    so that the packet is sent with its checksum fields unchanged.

Could the message mention this trade-off? A concrete case where the old
behavior caused harm would also help maintainers and stable backporters
weigh the change.

>  
>  	tx_q->tx_count_frames++;

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. Can the
modulo just after this line divide by zero?

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_xdp_xmit_xdpf() {
    ...
	tx_q->tx_count_frames++;

	if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)
		set_ic = true;
    ...
}

__stmmac_set_coalesce() rejects the settings only when both values are
zero:

	if ((ec->tx_coalesce_usecs == 0) &&
	    (ec->tx_max_coalesced_frames == 0))
		return -EINVAL;

So a command like "ethtool -C ethX tx-usecs 1000 tx-frames 0" stores 0
in priv->tx_coal_frames[queue]. After that, every XDP_TX frame from
stmmac_xdp_xmit_back() and every frame from stmmac_xdp_xmit() hits this
modulo. On x86 that causes a divide error oops in softirq context.

stmmac_xdp_xmit_zc() and the skb xmit paths already check for this:

		if (!priv->tx_coal_frames[queue])
			set_ic = false;
		else if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)

Should stmmac_xdp_xmit_xdpf() have the same check?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929121025.20821-1-jszhang%40kernel.org