[PATCH net 1/3] net: stmmac: honour Rx checksum errors in the AF_XDP zero-copy path

[PATCH net 1/3] net: stmmac: honour Rx checksum errors in the AF_XDP zero-copy path

[Grische]

Commit ee0aace5f844 ("net: stmmac: Correctly handle Rx checksum offload
errors") taught stmmac_rx() to honour the csum_none status, but
stmmac_dispatch_skb_zc() still ignores it and sets CHECKSUM_UNNECESSARY
for every IP frame whenever the checksum offload engine (CoE) is
enabled.

Since commit fe4042797651 ("net: stmmac: dwmac4: stop hardware from
dropping checksum-error packets") dwmac4 delivers such frames instead of
dropping them, so this path receives them.

Mark the skb CHECKSUM_NONE when csum_none is set, as stmmac_rx() does.

Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Cc: <stable@vger.kernel.org>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Grische <opensource@grische.xyz>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9741f97fa37a..32810f949245 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5463,7 +5463,7 @@ static struct sk_buff *stmmac_construct_skb_zc(struct stmmac_channel *ch,
 
 static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue,
 				   struct dma_desc *p, struct dma_desc *np,
-				   struct xdp_buff *xdp)
+				   struct xdp_buff *xdp, int status)
 {
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_channel *ch = &priv->channel[queue];
@@ -5488,7 +5488,8 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue,
 		stmmac_rx_vlan(priv->dev, skb);
 	skb->protocol = eth_type_trans(skb, priv->dev);
 
-	if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
+	if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) ||
+	    (status & csum_none))
 		skb_checksum_none_assert(skb);
 	else
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -5698,7 +5699,8 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 
 		switch (res) {
 		case STMMAC_XDP_PASS:
-			stmmac_dispatch_skb_zc(priv, queue, p, np, buf->xdp);
+			stmmac_dispatch_skb_zc(priv, queue, p, np, buf->xdp,
+					       status);
 			xsk_buff_free(buf->xdp);
 			break;
 		case STMMAC_XDP_CONSUMED:
-- 
2.54.0

[PATCH net 2/3] net: stmmac: dwmac1000: don't discard frames with only a checksum error

[Grische]

On dwmac1000 with enhanced descriptors, the hardware sets the Error
Summary (ES) bit of the Rx descriptor not only for L2 errors but also
for an L3/L4 checksum error. enh_desc_get_rx_status() returns
discard_frame whenever ES is set, and since commit 1b746ce8b397 ("net:
stmmac: don't overwrite discard_frame status") the checksum status is
only evaluated for good frames.

Return discard_frame only for L2 errors. A frame with only a checksum
error is then reported as csum_none and reaches the stack as
CHECKSUM_NONE, so the stack verifies the checksum itself. This matches
dwmac4, whose Error Summary check does not include the checksum bits.

Also drop the per-frame pr_err in enh_desc_get_rx_status(). Once the MAC
delivers checksum-error frames, it can be triggered from the wire.

This prepares for setting DMA_CONTROL_DT, which makes the MAC deliver
checksum-error frames instead of dropping them in its Rx FIFO.

Fixes: 1b746ce8b397 ("net: stmmac: don't overwrite discard_frame status")
Cc: <stable@vger.kernel.org> # 6.6.x: ee0aace5f844: net: stmmac: Correctly handle Rx checksum offload errors
Cc: <stable@vger.kernel.org> # 6.6.x
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Grische <opensource@grische.xyz>
---
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 051253601225..5c43d32a2f22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -195,9 +195,6 @@ static int enh_desc_get_rx_status(struct stmmac_extra_stats *x,
 		if (unlikely(rdes0 & RDES0_OVERFLOW_ERROR))
 			x->rx_gmac_overflow++;
 
-		if (unlikely(rdes0 & RDES0_IPC_CSUM_ERROR))
-			pr_err("\tIPC Csum Error/Giant frame\n");
-
 		if (unlikely(rdes0 & RDES0_COLLISION))
 			x->rx_collision++;
 		if (unlikely(rdes0 & RDES0_RECEIVE_WATCHDOG))
@@ -209,7 +206,11 @@ static int enh_desc_get_rx_status(struct stmmac_extra_stats *x,
 		if (unlikely(rdes0 & RDES0_CRC_ERROR)) {
 			x->rx_crc_errors++;
 		}
-		ret = discard_frame;
+		if (rdes0 & (RDES0_DESCRIPTOR_ERROR | RDES0_OVERFLOW_ERROR |
+			     RDES0_COLLISION | RDES0_RECEIVE_WATCHDOG |
+			     RDES0_MII_ERROR | RDES0_CRC_ERROR |
+			     RDES0_LENGTH_ERROR))
+			ret = discard_frame;
 	}
 
 	/* After a payload csum error, the ES bit is set.
-- 
2.54.0

Re: [PATCH net 2/3] net: stmmac: dwmac1000: don't discard frames with only a checksum error

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] enh_desc_get_rx_status()
  (drivers/net/ethernet/stmicro/stmmac/enh_desc.c:209-213) now discards…

--

net: stmmac: dwmac1000: don't discard frames with only a checksum error

This patch changes enh_desc_get_rx_status() so that a descriptor with the
Error Summary bit set is discarded only when one of the L2 error bits is
also set. Frames with only an L3/L4 checksum error then reach the stack as
CHECKSUM_NONE. The patch also removes the per-frame pr_err for
RDES0_IPC_CSUM_ERROR.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> index 0512536012251..5c43d32a2f22b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> @@ -195,9 +195,6 @@ static int enh_desc_get_rx_status(struct stmmac_extra_stats *x,
>  		if (unlikely(rdes0 & RDES0_OVERFLOW_ERROR))
>  			x->rx_gmac_overflow++;
>  
> -		if (unlikely(rdes0 & RDES0_IPC_CSUM_ERROR))
> -			pr_err("\tIPC Csum Error/Giant frame\n");
> -

[ ... ]

> @@ -209,7 +206,11 @@ static int enh_desc_get_rx_status(struct stmmac_extra_stats *x,
>  		if (unlikely(rdes0 & RDES0_CRC_ERROR)) {
>  			x->rx_crc_errors++;
>  		}
> -		ret = discard_frame;
> +		if (rdes0 & (RDES0_DESCRIPTOR_ERROR | RDES0_OVERFLOW_ERROR |
> +			     RDES0_COLLISION | RDES0_RECEIVE_WATCHDOG |
> +			     RDES0_MII_ERROR | RDES0_CRC_ERROR |
> +			     RDES0_LENGTH_ERROR))
> +			ret = discard_frame;

[Severity: High]
Can this mask let giant frames through when Rx checksum offload is
disabled?

RDES0[7] (RDES0_IPC_CSUM_ERROR) means an IPC checksum error only while
GMAC_CONTROL_IPC is set. With IPC clear, the same bit reports a Giant
Frame. That is an L2 error, and it also sets ES. The pr_err string removed
above ("IPC Csum Error/Giant frame") names both meanings.

dwmac1000_rx_ipc_enable() clears IPC whenever hw->rx_csum is 0:

dwmac1000_rx_ipc_enable() {
    ...
	if (hw->rx_csum)
		value |= GMAC_CONTROL_IPC;
	else
		value &= ~GMAC_CONTROL_IPC;
    ...
}

This happens with "ethtool -K <if> rx off" through stmmac_set_features(),
when the platform has no RX COE, or when the IPC read-back in
stmmac_hw_setup() fails.

In that state, a giant frame where bit 7 is the only ES contributor stays
good_frame here. It is then passed to enh_desc_coe_rdes0() with
ipc_err = 1. That function returns csum_none for status 0x6/0x7 and
good_frame for the "Reserved" 0x2 case. It never returns discard_frame,
so stmmac_rx() passes the oversized frame to XDP and the stack. Before
this patch the frame was dropped.

What happens when the giant frame is larger than dma_buf_sz and spans two
descriptors? One example is threshold DMA mode with
snps,force_thresh_dma_mode, where the FIFO can no longer drop the frame.

The non-LS descriptor returns discard_frame without rx_not_ls. So
stmmac_rx() frees it and resets error and len. The LS descriptor is then
handled as the start of a new frame:

stmmac_rx_buf1_len() {
    ...
	plen = stmmac_get_rx_frame_len(priv, p, coe);
    ...
	return min_t(unsigned int, priv->dma_conf.dma_buf_sz, plen - len);
}

With len == 0 this reports dma_buf_sz bytes, but DMA only wrote
plen - dma_buf_sz bytes to that buffer. Would the stack then see a frame
whose Ethernet header comes from the middle of the giant frame's payload,
followed by stale page_pool contents?

This also seems to disagree with the commit message:

  "Return discard_frame only for L2 errors."

  "This matches dwmac4, whose Error Summary check does not include the
  checksum bits."

dwmac4_wrback_get_rx_status() still discards RDES3_GIANT_PACKET inside its
ES block, and ndesc_get_rx_status() discards every ES frame.

The later patch in this series that sets DMA_CONTROL_DT ("net: stmmac:
dwmac1000: stop hardware from dropping checksum-error packets") only
affects COE-detected errors, so it doesn't appear to cover this case.

Should bit 7 also count as a discard reason unless IPC (hw->rx_csum) is
enabled?

>  	}
>  
>  	/* After a payload csum error, the ES bit is set.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020190319.884375-1-opensource%40grische.xyz

[PATCH net 3/3] net: stmmac: dwmac1000: stop hardware from dropping checksum-error packets

[Grische]

When the checksum offload engine (CoE) is enabled, the dwmac1000 MAC
silently drops any frame for which the engine detects a checksum error,
before the driver sees it.

This breaks VXLAN over IPv6 with zero UDP checksums, which RFC 6935 and
RFC 6936 allow for tunnels.

Set DMA_CONTROL_DT so that such frames are delivered instead of dropped,
as commit fe4042797651 ("net: stmmac: dwmac4: stop hardware from
dropping checksum-error packets") did for dwmac4. CoE remains enabled,
so the driver still sees the error status and marks the skb
CHECKSUM_NONE for the stack to verify.

Tested on an Aruba AP-325 (ipq806x-gmac-dwmac) with Rx checksum offload
on.

Link: https://lore.kernel.org/all/20250625132117.1b3264e8@kernel.org/
Link: https://github.com/freifunk-gluon/gluon/issues/3783
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Cc: <stable@vger.kernel.org> # 6.6.x: ee0aace5f844: net: stmmac: Correctly handle Rx checksum offload errors
Cc: <stable@vger.kernel.org> # 6.6.x
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Grische <opensource@grische.xyz>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h     | 1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
index 1de1f929d61c..004e6e30a68b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
@@ -163,6 +163,7 @@ enum rx_tx_priority_ratio {
 
 /* DMA operation mode defines (start/stop tx/rx are placed in common header)*/
 /* Disable Drop TCP/IP csum error */
+#define DMA_CONTROL_DT		0x04000000
 #define DMA_CONTROL_RSF		0x02000000	/* Receive Store and Forward */
 #define DMA_CONTROL_DFF		0x01000000	/* Disaable flushing */
 /* Threshold for Activating the FC */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index 3ac7a7949529..00c8d4dfb553 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -129,6 +129,8 @@ static void dwmac1000_dma_operation_mode_rx(struct stmmac_priv *priv,
 {
 	u32 csr6 = readl(ioaddr + DMA_CHAN_CONTROL(channel));
 
+	csr6 |= DMA_CONTROL_DT;
+
 	if (mode == SF_DMA_MODE) {
 		pr_debug("GMAC: enable RX store and forward mode\n");
 		csr6 |= DMA_CONTROL_RSF | DMA_CONTROL_DFF;
-- 
2.54.0

Re: [PATCH net 3/3] net: stmmac: dwmac1000: stop hardware from dropping checksum-error packets

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] dwmac1000_dma_operation_mode_rx() now sets CSR6.DT
  (DMA_CONTROL_DT) on every dwmac1000 core, whichever descriptor format…

--

net: stmmac: dwmac1000: stop hardware from dropping checksum-error packets

This patch sets DMA_CONTROL_DT in dwmac1000_dma_operation_mode_rx(). With
that bit set, the dwmac1000 MAC hands up frames that fail the Rx checksum
offload check instead of dropping them. The aim is to make VXLAN over IPv6
with zero UDP checksums work, with the driver marking those frames
CHECKSUM_NONE.

Link: https://lore.kernel.org/all/20250625132117.1b3264e8@kernel.org/
Link: https://github.com/freifunk-gluon/gluon/issues/3783

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> index 3ac7a7949529c..00c8d4dfb553b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> @@ -129,6 +129,8 @@ static void dwmac1000_dma_operation_mode_rx(struct stmmac_priv *priv,
>  {
>  	u32 csr6 = readl(ioaddr + DMA_CHAN_CONTROL(channel));
>  
> +	csr6 |= DMA_CONTROL_DT;

[Severity: High]
Does this work on dwmac1000 cores that use normal descriptors?

DT is now set no matter which descriptor format is in use. Only the
enhanced descriptor parser turns checksum-only errors into csum_none, via
enh_desc_get_rx_status()->enh_desc_coe_rdes0().

stmmac_hw_init() reads enh_desc and rx_coe from separate HW capability
bits:

    priv->plat->enh_desc = priv->dma_cap.enh_desc;
    ...
    if (priv->dma_cap.rx_coe_type2)
        priv->plat->rx_coe = STMMAC_RX_COE_TYPE2;

stmmac_dwmac1_quirks() then picks ndesc_ops whenever enh_desc is 0. So the
driver allows a core with normal descriptors and Type-2 Rx COE.

ndesc_get_rx_status() can only return good_frame or discard_frame. It
never sets csum_none, and nothing in the driver checks
RDES0_PAYLOAD_CSUM_ERR from descs.h:

    if (unlikely(rdes0 & RDES0_ERROR_SUMMARY)) {
        ...
        if (unlikely(rdes0 & RDES0_IPC_CSUM_ERROR))
            x->ipc_csum_error++;
        ...
        ret = discard_frame;
    }

If ES is set for the checksum error, stmmac_rx() now drops the frame in
software and counts it as an rx error. Wouldn't VXLAN over IPv6 with zero
UDP checksums still be broken on these cores?

The comment in enh_desc_get_rx_status() says the databook does not
require ES for payload-only checksum errors. If ES is clear in that case,
ndesc_get_rx_status() returns good_frame, and stmmac_rx() does:

    if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) ||
        (status & csum_none))
        skb_checksum_none_assert(skb);
    else
        skb->ip_summed = CHECKSUM_UNNECESSARY;

Could a frame with a bad L4 checksum, which the hardware used to drop, now
reach the stack marked CHECKSUM_UNNECESSARY?

Either way, this part of the commit message would not hold for normal
descriptors:

  "CoE remains enabled, so the driver still sees the error status and
   marks the skb CHECKSUM_NONE for the stack to verify."

Would it make sense to set DT only when enhanced descriptors are in use,
or to make ndesc_get_rx_status() report csum_none for checksum errors?

> +
>  	if (mode == SF_DMA_MODE) {
>  		pr_debug("GMAC: enable RX store and forward mode\n");
>  		csr6 |= DMA_CONTROL_RSF | DMA_CONTROL_DFF;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020190319.884375-1-opensource%40grische.xyz