[PATCH net-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support

[PATCH net-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support

[Lorenzo Bianconi]

Rework stmmac_rx() to build the xdp_buff from all the descriptors that make
up a frame, so the XDP program runs on the whole (possibly fragmented)
packet instead of the first buffer only. Fix possible rx_q state leaks
when closing/opening the interface.

---
Changes in v6:
- Drop patch 'net: stmmac: allow non-linear xdp_buff in XDP mode' since
  it requires some XDP fixes.
- Introduce patch 'net: stmmac: drop split frames on AF_XDP zero-copy
  RX' to always drop XDP multi-buff frames for AF_XDP mode since it not
  yet supported there.
- Introduce patch 'net: stmmac: drop multi-buffer frames in XDP mode' to
  fix pending bugs due to missing XDP multi-buffer support before patch
  'net: stmmac: rework stmmac_rx to support XDP rx multi-buff'
- Link to v5: https://lore.kernel.org/r/20261009-stmmac-rx-mb-v5-0-c38fa4eaa138@oss.qualcomm.com

Changes in v5:
- Fix possible read after bound issue in patch 1/3.
- Do not allow parsing non-linear XDP buffers if the program loaded does
  not support frags.
- Add patch 3/3 to remove MTU constraint for XDP prog.
- Link to v4: https://lore.kernel.org/r/20261006-stmmac-rx-mb-v4-0-efa4ca974e3d@oss.qualcomm.com

Changes in v4:
- Reset in_progress flag when the packet is fully consumed.
- Advetise NETDEV_XDP_ACT_RX_SG capability.
- Link to v3: https://lore.kernel.org/r/20261004-stmmac-rx-mb-v3-0-50fa171af9ec@oss.qualcomm.com

Changes in v3:
- Return secondary page in case of error.
- Introduce stmmac_xdp_shrink_tail() to remove the ethernet FCS.
- Add patch 'net: stmmac: take ownership of saved RX state at poll
  entry' from James.
- Link to v2: https://lore.kernel.org/r/20260921-stmmac-rx-mb-v2-1-6e826e1ff306@oss.qualcomm.com

Changes in v2:
- Rely on xdp_buff_add_frag() to create xdp fragments
- Fix bugs in rx_q state recording
- Fix the corner case where we receive more than MAX_SKB_FRAGS fragments
- Cosmetics
- Link to v1: https://lore.kernel.org/r/20260918-stmmac-rx-mb-v1-1-0b4517d404af@oss.qualcomm.com

---
James Hilliard (1):
      net: stmmac: take ownership of saved RX state at poll entry

Lorenzo Bianconi (3):
      net: stmmac: drop split frames on AF_XDP zero-copy RX
      net: stmmac: drop multi-buffer frames in XDP mode
      net: stmmac: rework stmmac_rx to support XDP rx multi-buff

 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 407 ++++++++++++++--------
 2 files changed, 257 insertions(+), 153 deletions(-)
---
base-commit: 29f144016f03608abfddcd6ebaeabafd7aeaadd2
change-id: 20260918-stmmac-rx-mb-16469a2714ea

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

[PATCH net-next v6 1/4] net: stmmac: take ownership of saved RX state at poll entry

[Lorenzo Bianconi]

From: James Hilliard <james.hilliard1@gmail.com>

When a saved partial packet completes with a poll budget of one, the
old loop can leave state_saved and state.skb pointing at an skb that
has already been delivered or freed. The next poll then reuses that
pointer, causing a use-after-free or double free.

Take the saved state at poll entry and clear the stored ownership
immediately. Save it again only if the packet remains incomplete,
including when the next descriptor is still DMA-owned.

Track the frame-in-progress flag independently of the status read from
the next descriptor. The old code keyed the save on the status of the
last descriptor looked at, so a loop stopped at a DMA-owned descriptor
(rx status not yet valid) could drop the saved error/length bookkeeping
in stmmac_rx_zc().

Drop the saved state both when the queue parameters are reset and when
the ring is destroyed. stmmac_reset_rx_queue() is reached from
__stmmac_open(), stmmac_xdp_open(), stmmac_resume() and
stmmac_enable_rx_queue(); without draining it there, a partial frame
saved before suspend (or before a queue is disabled and re-enabled)
survives while cur_rx and dirty_rx are re-armed, and the first poll
resumes that stale skb with descriptors from the freshly initialized
ring. __free_dma_rx_desc_resources() releases it on teardown.

Apply the same state handling to stmmac_rx_zc(), which also leaves
state_saved set when a saved frame finishes at the budget boundary.
That path saves only error and length bookkeeping, not an skb pointer.

Fixes: ec222003bd94 ("net: stmmac: Prepare to add Split Header support")
Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 72 +++++++++++++----------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b8b4de4c5b1a..f66b796f7725 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2180,6 +2180,9 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
 	else
 		dma_free_rx_skbufs(priv, dma_conf, queue);
 
+	dev_kfree_skb_any(rx_q->state.skb);
+	rx_q->state.skb = NULL;
+	rx_q->state_saved = false;
 	rx_q->buf_alloc_num = 0;
 	rx_q->xsk_pool = NULL;
 
@@ -5620,12 +5623,12 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 	unsigned int count = 0, error = 0, len = 0;
 	int dirty = stmmac_rx_dirty(priv, queue);
 	unsigned int next_entry = rx_q->cur_rx;
+	bool in_progress = rx_q->state_saved;
 	u32 rx_errors = 0, rx_dropped = 0;
 	unsigned int desc_size;
 	struct bpf_prog *prog;
 	bool failure = false;
 	int xdp_status = 0;
-	int status = 0;
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head = stmmac_get_rx_desc(priv, rx_q, 0);
@@ -5636,23 +5639,25 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 		stmmac_display_ring(priv, rx_head, priv->dma_conf.dma_rx_size, true,
 				    rx_q->dma_rx_phy, desc_size);
 	}
+
+	if (rx_q->state_saved) {
+		error = rx_q->state.error;
+		len = rx_q->state.len;
+		rx_q->state_saved = false;
+	}
+
 	while (count < limit) {
 		struct stmmac_rx_buffer *buf;
 		struct stmmac_xdp_buff *ctx;
 		unsigned int buf1_len = 0;
 		struct dma_desc *np, *p;
-		int entry;
+		int entry, status;
 		int res;
 
-		if (!count && rx_q->state_saved) {
-			error = rx_q->state.error;
-			len = rx_q->state.len;
-		} else {
-			rx_q->state_saved = false;
+		if (!in_progress) {
 			error = 0;
 			len = 0;
 		}
-
 read_again:
 		if (count >= limit)
 			break;
@@ -5691,6 +5696,8 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 		if (!buf->xdp)
 			break;
 
+		in_progress = status & rx_not_ls;
+
 		if (priv->extend_desc)
 			stmmac_rx_extended_status(priv, &priv->xstats,
 						  rx_q->dma_erx + entry);
@@ -5763,7 +5770,7 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 		count++;
 	}
 
-	if (status & rx_not_ls) {
+	if (in_progress) {
 		rx_q->state_saved = true;
 		rx_q->state.error = error;
 		rx_q->state.len = len;
@@ -5805,9 +5812,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
 	struct stmmac_channel *ch = &priv->channel[queue];
 	unsigned int count = 0, error = 0, len = 0;
-	int status = 0, coe = priv->hw->rx_csum;
 	unsigned int next_entry = rx_q->cur_rx;
+	bool in_progress = rx_q->state_saved;
 	enum dma_data_direction dma_dir;
+	int coe = priv->hw->rx_csum;
 	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
 	struct stmmac_xdp_buff ctx;
@@ -5827,25 +5835,28 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		stmmac_display_ring(priv, rx_head, priv->dma_conf.dma_rx_size, true,
 				    rx_q->dma_rx_phy, desc_size);
 	}
+
+	if (rx_q->state_saved) {
+		skb = rx_q->state.skb;
+		error = rx_q->state.error;
+		len = rx_q->state.len;
+		rx_q->state.skb = NULL;
+		rx_q->state_saved = false;
+	}
+
 	while (count < limit) {
 		unsigned int buf1_len = 0, buf2_len = 0;
 		enum pkt_hash_types hash_type;
 		struct stmmac_rx_buffer *buf;
 		struct dma_desc *np, *p;
-		int entry;
+		int entry, status;
 		u32 hash;
 
-		if (!count && rx_q->state_saved) {
-			skb = rx_q->state.skb;
-			error = rx_q->state.error;
-			len = rx_q->state.len;
-		} else {
-			rx_q->state_saved = false;
+		if (!in_progress) {
 			skb = NULL;
 			error = 0;
 			len = 0;
 		}
-
 read_again:
 		if (count >= limit)
 			break;
@@ -5874,6 +5885,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 		prefetch(np);
 
+		in_progress = status & rx_not_ls;
+
 		if (priv->extend_desc)
 			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry);
 		if (unlikely(status == discard_frame)) {
@@ -5886,11 +5899,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 		if (unlikely(error && (status & rx_not_ls)))
 			goto read_again;
+
 		if (unlikely(error)) {
 			dev_kfree_skb(skb);
-			skb = NULL;
-			count++;
-			continue;
+			goto next;
 		}
 
 		/* Buffer is good. Go on. */
@@ -5964,9 +5976,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 						      STMMAC_XDP_REDIRECT)) {
 					xdp_status |= xdp_res;
 					buf->page = NULL;
-					skb = NULL;
-					count++;
-					continue;
+					goto next;
 				}
 			}
 		}
@@ -6023,10 +6033,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		/* Remove FCS if needed */
 		if (!fcs_stripped && pskb_trim(skb, len)) {
 			dev_kfree_skb_any(skb);
-			skb = NULL;
 			rx_dropped++;
-			count++;
-			continue;
+			goto next;
 		}
 
 		stmmac_get_rx_hwtstamp(priv, p, np, skb);
@@ -6051,14 +6059,15 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 		skb_record_rx_queue(skb, queue);
 		napi_gro_receive(&ch->rx_napi, skb);
-		skb = NULL;
-
 		rx_packets++;
 		rx_bytes += len;
+next:
+		in_progress = false;
+		skb = NULL;
 		count++;
 	}
 
-	if (status & rx_not_ls || skb) {
+	if (in_progress || skb) {
 		rx_q->state_saved = true;
 		rx_q->state.skb = skb;
 		rx_q->state.error = error;
@@ -8378,6 +8387,9 @@ static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue)
 {
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
 
+	dev_kfree_skb_any(rx_q->state.skb);
+	rx_q->state.skb = NULL;
+	rx_q->state_saved = false;
 	rx_q->cur_rx = 0;
 	rx_q->dirty_rx = 0;
 }

-- 
2.56.0

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

[Lorenzo Bianconi]

stmmac_rx_zc() assumes a 1:1 mapping between a received frame and an
XSK pool buffer and drops any frame that spans more than one RX
descriptor:

        /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
        if (likely(status & rx_not_ls)) {
                xsk_buff_free(buf->xdp);
                buf->xdp = NULL;
                rx_dropped++;
                error = 1;
                dirty++;
                goto read_again;
        }

The non-last descriptors are freed and dropped, but the frame is not
marked as errored. When the poll reaches the last descriptor of the
same frame, error is still 0, so the fragment is handled as a complete
frame: it is passed to the XDP program, copied into an skb on XDP_PASS,
or converted for XDP_TX. The tail starts in the middle of the original
payload at a predictable offset and, parsed as a fresh frame (len == 0),
stmmac_rx_buf1_len() reports the full dma_buf_sz length for it even
though the DMA engine only wrote the tail remainder into that buffer, so
the reads extend past the received bytes and can reach beyond the end of
the XSK pool buffer. GMAC4 always enables jumbo frames (GMAC_CONFIG_JE
in GMAC_CORE_INIT), so an on-link peer can trigger such a split even
when the interface MTU is left at 1500.

Set error when the 1:1 mapping check drops a descriptor, so the
remaining descriptors of the frame - including the last one - are
drained and released with xsk_buff_free() instead of being processed as
a new frame. Account every freed buffer with dirty and count the
dropped frame once in the poll budget / rx_pkt_n, and once in
rx_dropped.

Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f66b796f7725..426bb68c9bbc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5701,18 +5701,21 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 		if (priv->extend_desc)
 			stmmac_rx_extended_status(priv, &priv->xstats,
 						  rx_q->dma_erx + entry);
-		if (unlikely(status == discard_frame)) {
-			xsk_buff_free(buf->xdp);
-			buf->xdp = NULL;
-			dirty++;
+
+		if (unlikely(status == discard_frame) && !error) {
 			error = 1;
 			if (!priv->hwts_rx_en)
 				rx_errors++;
 		}
 
-		if (unlikely(error && (status & rx_not_ls)))
-			goto read_again;
 		if (unlikely(error)) {
+			xsk_buff_free(buf->xdp);
+			buf->xdp = NULL;
+			dirty++;
+
+			if (status & rx_not_ls)
+				goto read_again;
+
 			count++;
 			continue;
 		}
@@ -5721,8 +5724,9 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 		if (likely(status & rx_not_ls)) {
 			xsk_buff_free(buf->xdp);
 			buf->xdp = NULL;
+			rx_dropped++;
+			error = 1;
 			dirty++;
-			count++;
 			goto read_again;
 		}
 

-- 
2.56.0

[PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[Lorenzo Bianconi]

stmmac_rx() runs the XDP program once per descriptor buffer and uses
skb == NULL as the only marker for the start of a frame. A frame larger
than one buffer is split by the hardware across several descriptors;
GMAC4 enables jumbo frames unconditionally (GMAC_CONFIG_JE in
GMAC_CORE_INIT), so an on-link peer can deliver such a frame even when
the interface MTU is left at 1500.

When an XDP verdict ends a non-last descriptor, skb is cleared and the
next descriptor of the same hardware frame is parsed as a new frame.
For XDP_CONSUMED this happens via read_again with len preserved, so
stmmac_rx_buf1_len() then returns min(dma_buf_sz, plen - len) for the
last descriptor, which can be as small as 0-3 bytes. buf1_len -=
ETH_FCS_LEN underflows and xdp_prepare_buff() puts data_end far past
data; on XDP_PASS this produces an skb whose length is close to 4G that
reaches the network stack (skb->tail and skb->len are 32-bit and wrap
below skb->end, so skb_over_panic() does not fire). A 4+ byte tail is
instead offered to the XDP program and delivered as a standalone
mid-payload Ethernet frame on XDP_PASS. XDP_TX/XDP_REDIRECT instead
falls through to the next iteration, resetting the running length, so
the tail is re-parsed as a full-size new frame.

XDP does not support scatter-gather frames until the multi-buffer
rework, so drop the whole frame instead: mark it as errored as soon as
a non-last descriptor is seen while an XDP program is attached, drain
and release every descriptor - including the last - and account the
frame once in rx_dropped. Removing the re-parse lets both XDP verdict
branches simply complete the packet via next.

Note: the subsequent "net: stmmac: rework stmmac_rx to support XDP rx
multi-buff" accumulates the whole frame into a single xdp_buff and
supersedes this patch; the fix is kept so the net portion of the series
stands on its own.

Fixes: 5fabb01207a2 ("net: stmmac: Add initial XDP support")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 28 ++++++++++-------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 426bb68c9bbc..8c08d44fdbd3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 		if (priv->extend_desc)
 			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry);
-		if (unlikely(status == discard_frame)) {
-			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
-			buf->page = NULL;
+
+		if (unlikely(!error && status == discard_frame)) {
 			error = 1;
 			if (!priv->hwts_rx_en)
 				rx_errors++;
 		}
 
-		if (unlikely(error && (status & rx_not_ls)))
-			goto read_again;
+		/* We do not support SG in XDP mode */
+		if (unlikely(!error && (status & rx_not_ls) &&
+			     stmmac_xdp_is_enabled(priv))) {
+			rx_dropped++;
+			error = 1;
+		}
 
 		if (unlikely(error)) {
+			if (status & rx_not_ls)
+				goto read_again;
+
 			dev_kfree_skb(skb);
 			goto next;
 		}
@@ -5965,17 +5971,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 							   sync_len, true);
 					buf->page = NULL;
 					rx_dropped++;
-
-					/* Clear skb as it was set as
-					 * status by XDP program.
-					 */
-					skb = NULL;
-
-					if (unlikely((status & rx_not_ls)))
-						goto read_again;
-
-					count++;
-					continue;
+					goto next;
 				} else if (xdp_res & (STMMAC_XDP_TX |
 						      STMMAC_XDP_REDIRECT)) {
 					xdp_status |= xdp_res;

-- 
2.56.0

[PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[Lorenzo Bianconi]

Build the xdp_buff by accumulating all the descriptors that make up a
frame, so the XDP program runs on the full (possibly fragmented) packet
instead of just the first buffer. When the frame is not consumed by the
program, assemble the skb from the head buffer and the collected
fragments via napi_build_skb()/xdp_update_skb_frags_info().

Note that XDP still rejects an MTU larger than ETH_DATA_LEN
(stmmac_xdp_set_prog(), stmmac_change_mtu()) and disables SPH, so an
ordinary frame fits in a single buffer. This path is therefore exercised
by frames that do span several descriptors (for example jumbo frames the
MAC admits through its jumbo-enable bit) and is groundwork for extending
XDP to jumbo/SPH; it does not enable XDP jumbo support by itself.

For those multi-descriptor frames the old code was buggy as well: it ran
the XDP program once per descriptor buffer, so for a multi-descriptor
frame the program was also invoked on continuation buffers holding
mid-frame payload. It also subtracted ETH_FCS_LEN from the first buffer
even when it was not the last descriptor (its second buffer empty),
dropping four bytes from the middle of the frame while marking the FCS as
already stripped and skipping the final pskb_trim(). Accumulating the
whole frame before running XDP fixes both.

To do so, store the in-progress xdp_buff in rx_q->state instead of the
partially built skb, so the accumulated head and fragments survive a
NAPI poll boundary (mid-frame dma_own or dirty_rx break). The state is
saved only while a frame is in progress and cleared once it completes,
leaving it untouched when the poll does not process anything (e.g.
netpoll invoked with a zero budget).

In addition:
- advertise NETDEV_XDP_ACT_RX_SG, since the RX path can now deliver
  non-linear buffers to XDP;
- drop multi-buffer frames when the attached program was not loaded with
  BPF_F_XDP_HAS_FRAGS, since a single-buffer program must not be handed a
  non-linear xdp_buff;
- build the skb head with napi_build_skb() passing xdp->frame_sz, so
  skb_shinfo() lands on the same shared_info the fragments were
  accumulated into;
- carry the XDP metadata written with bpf_xdp_adjust_meta() into the skb
  via skb_metadata_set(), so it is no longer dropped and TC/BPF programs
  can read it through data_meta;
- attach fragments with xdp_buff_add_frag(), which initializes all the
  shared_info fields and takes care of the pfmemalloc bit;
- release the buffers collected in the xdp_buff when a frame is dropped
  on RX errors; the previous code relied on dev_kfree_skb() recycling the
  skb frags, which no longer applies now that the head and fragments live
  in an xdp_buff rather than a partially built skb;
- drop the whole frame when it would exceed MAX_SKB_FRAGS, where the
  previous skb_add_rx_frag() calls were unbounded;
- strip the Ethernet FCS from the accumulated xdp_buff with a
  driver-local tail shrink (stmmac_xdp_shrink_tail()) for both linear
  and fragmented frames, instead of trimming the skb or the first
  descriptor length.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 327 ++++++++++++++--------
 2 files changed, 211 insertions(+), 119 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 9278378407e8..63020e8edd8a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -132,7 +132,8 @@ struct stmmac_rx_queue {
 	dma_addr_t dma_rx_phy;
 	unsigned int state_saved;
 	struct {
-		struct sk_buff *skb;
+		struct xdp_buff xdp;
+		unsigned int frames;
 		unsigned int len;
 		unsigned int error;
 	} state;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8c08d44fdbd3..fd04368543c2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -158,6 +158,9 @@ static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
 static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
 					  u32 rxmode, u32 chan);
 static void stmmac_vlan_restore(struct stmmac_priv *priv);
+static void stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q,
+				struct xdp_buff *xdp, int sync_len,
+				bool allow_direct);
 
 #ifdef CONFIG_DEBUG_FS
 static const struct net_device_ops stmmac_netdev_ops;
@@ -2180,8 +2183,10 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
 	else
 		dma_free_rx_skbufs(priv, dma_conf, queue);
 
-	dev_kfree_skb_any(rx_q->state.skb);
-	rx_q->state.skb = NULL;
+	if (rx_q->state.frames && !rx_q->xsk_pool) {
+		stmmac_xdp_put_buff(rx_q, &rx_q->state.xdp, -1, false);
+		rx_q->state.frames = 0;
+	}
 	rx_q->state_saved = false;
 	rx_q->buf_alloc_num = 0;
 	rx_q->xsk_pool = NULL;
@@ -5460,8 +5465,8 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv,
 static struct sk_buff *stmmac_xdp_run_prog(struct stmmac_priv *priv,
 					   struct xdp_buff *xdp)
 {
+	int res = STMMAC_XDP_CONSUMED;
 	struct bpf_prog *prog;
-	int res;
 
 	prog = READ_ONCE(priv->xdp_prog);
 	if (!prog) {
@@ -5469,7 +5474,8 @@ static struct sk_buff *stmmac_xdp_run_prog(struct stmmac_priv *priv,
 		goto out;
 	}
 
-	res = __stmmac_xdp_run_prog(priv, prog, xdp);
+	if (likely(!xdp_buff_has_frags(xdp) || prog->aux->xdp_has_frags))
+		res = __stmmac_xdp_run_prog(priv, prog, xdp);
 out:
 	return ERR_PTR(-res);
 }
@@ -5801,6 +5807,118 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 	return failure ? limit : (int)count;
 }
 
+static void
+stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q, struct xdp_buff *xdp,
+		    int sync_len, bool allow_direct)
+{
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
+	int i;
+
+	if (likely(!xdp_buff_has_frags(xdp)))
+		goto out;
+
+	for (i = 0; i < sinfo->nr_frags; i++)
+		page_pool_put_full_page(rx_q->page_pool,
+					skb_frag_page(&sinfo->frags[i]),
+					allow_direct);
+out:
+	page_pool_put_page(rx_q->page_pool, virt_to_head_page(xdp->data),
+			   sync_len, allow_direct);
+}
+
+static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
+{
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
+	u32 metasize = xdp->data - xdp->data_meta;
+	struct sk_buff *skb;
+	u8 num_frags = 0;
+
+	if (unlikely(xdp_buff_has_frags(xdp)))
+		num_frags = sinfo->nr_frags;
+
+	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
+	if (!skb)
+		return NULL;
+
+	skb_mark_for_recycle(skb);
+	skb_reserve(skb, xdp->data - xdp->data_hard_start);
+	skb_put(skb, xdp->data_end - xdp->data);
+	if (metasize)
+		skb_metadata_set(skb, metasize);
+
+	if (unlikely(xdp_buff_has_frags(xdp)))
+		xdp_update_skb_frags_info(skb, num_frags, sinfo->xdp_frags_size,
+					  num_frags * xdp->frame_sz,
+					  xdp_buff_get_skb_flags(xdp));
+	return skb;
+}
+
+static bool stmmac_build_xdp_frags(struct stmmac_priv *priv,
+				   struct stmmac_rx_queue *rx_q,
+				   unsigned int len, struct page *page,
+				   unsigned int offset,
+				   enum dma_data_direction dma_dir,
+				   struct xdp_buff *xdp)
+{
+	dma_addr_t dma_addr = page_pool_get_dma_addr(page) + offset;
+
+	dma_sync_single_for_cpu(priv->device, dma_addr, len, dma_dir);
+	if (!xdp_buff_add_frag(xdp, page_to_netmem(page), offset, len,
+			       xdp->frame_sz)) {
+		page_pool_put_full_page(rx_q->page_pool, page, true);
+		return false;
+	}
+
+	return true;
+}
+
+static int stmmac_xdp_shrink_tail(struct stmmac_rx_queue *rx_q,
+				  struct xdp_buff *xdp, int offset)
+{
+	struct skb_shared_info *sinfo;
+	int i;
+
+	if (unlikely(offset < 0 ||
+		     offset > (int)xdp_get_buff_len(xdp) - ETH_HLEN))
+		return -EINVAL;
+
+	if (likely(!xdp_buff_has_frags(xdp))) {
+		xdp->data_end -= offset;
+		return 0;
+	}
+
+	sinfo = xdp_get_shared_info_from_buff(xdp);
+	for (i = sinfo->nr_frags - 1; i >= 0 && offset > 0; i--) {
+		skb_frag_t *frag = &sinfo->frags[i];
+		int delta = min_t(int, offset, skb_frag_size(frag));
+
+		if (delta == skb_frag_size(frag)) {
+			/* The whole frag is consumed by the strip: return it
+			 * to the page pool right away. Its ring slot still
+			 * carries the page DMA address until stmmac_rx_refill()
+			 * re-arms it at the end of the NAPI poll. This is
+			 * safe because HW cannot touch the slot until then.
+			 */
+			page_pool_put_full_page(rx_q->page_pool,
+						skb_frag_page(frag), true);
+			sinfo->nr_frags--;
+		} else {
+			skb_frag_size_sub(frag, delta);
+		}
+
+		sinfo->xdp_frags_size -= delta;
+		offset -= delta;
+	}
+
+	if (unlikely(!sinfo->nr_frags)) {
+		xdp_buff_clear_frags_flag(xdp);
+		xdp_buff_clear_frag_pfmemalloc(xdp);
+		xdp->data_end -= offset;
+	}
+
+	return 0;
+}
+
 /**
  * stmmac_rx - manage the receive process
  * @priv: driver private structure
@@ -5814,21 +5932,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	u32 rx_errors = 0, rx_dropped = 0, rx_bytes = 0, rx_packets = 0;
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
+	unsigned int frames = 0, next_entry = rx_q->cur_rx;
 	struct stmmac_channel *ch = &priv->channel[queue];
 	unsigned int count = 0, error = 0, len = 0;
-	unsigned int next_entry = rx_q->cur_rx;
 	bool in_progress = rx_q->state_saved;
 	enum dma_data_direction dma_dir;
 	int coe = priv->hw->rx_csum;
-	unsigned int desc_size;
-	struct sk_buff *skb = NULL;
 	struct stmmac_xdp_buff ctx;
-	bool fcs_stripped = false;
+	unsigned int desc_size;
 	int xdp_status = 0;
 	int bufsz;
 
 	dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
-	bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
+	bufsz = rx_q->napi_skb_frag_size;
+	ctx.priv = priv;
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head = stmmac_get_rx_desc(priv, rx_q, 0);
@@ -5841,23 +5958,25 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	}
 
 	if (rx_q->state_saved) {
-		skb = rx_q->state.skb;
+		ctx.xdp = rx_q->state.xdp;
 		error = rx_q->state.error;
+		frames = rx_q->state.frames;
 		len = rx_q->state.len;
-		rx_q->state.skb = NULL;
 		rx_q->state_saved = false;
+		rx_q->state.frames = 0;
 	}
 
 	while (count < limit) {
 		unsigned int buf1_len = 0, buf2_len = 0;
+		unsigned int pre_len, sync_len;
 		enum pkt_hash_types hash_type;
 		struct stmmac_rx_buffer *buf;
 		struct dma_desc *np, *p;
+		struct sk_buff *skb;
 		int entry, status;
 		u32 hash;
 
 		if (!in_progress) {
-			skb = NULL;
 			error = 0;
 			len = 0;
 		}
@@ -5900,19 +6019,11 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 				rx_errors++;
 		}
 
-		/* We do not support SG in XDP mode */
-		if (unlikely(!error && (status & rx_not_ls) &&
-			     stmmac_xdp_is_enabled(priv))) {
-			rx_dropped++;
-			error = 1;
-		}
-
 		if (unlikely(error)) {
 			if (status & rx_not_ls)
 				goto read_again;
 
-			dev_kfree_skb(skb);
-			goto next;
+			goto error_free_frag;
 		}
 
 		/* Buffer is good. Go on. */
@@ -5922,121 +6033,89 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
 		len += buf2_len;
 
-		/* ACS is disabled; strip manually. */
-		if (likely(!(status & rx_not_ls)))
-			len -= ETH_FCS_LEN;
-
-		if (!skb) {
-			unsigned int pre_len, sync_len;
-
-			/* Each frame starts here: reset the FCS handling */
-			fcs_stripped = false;
-
+		if (!frames) {
 			dma_sync_single_for_cpu(priv->device, buf->addr,
 						buf1_len, dma_dir);
 			net_prefetch(page_address(buf->page) +
 				     buf->page_offset);
 
-			if (stmmac_xdp_is_enabled(priv) && !buf2_len) {
-				buf1_len -= ETH_FCS_LEN;
-				fcs_stripped = true;
-			}
-
 			xdp_init_buff(&ctx.xdp, bufsz, &rx_q->xdp_rxq);
 			xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
 					 buf->page_offset, buf1_len, true);
-
-			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
-				  buf->page_offset;
-
-			ctx.priv = priv;
-			ctx.desc = p;
-			ctx.ndesc = np;
-
-			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
-			/* Due xdp_adjust_tail: DMA sync for_device
-			 * cover max len CPU touch
-			 */
-			sync_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
-				   buf->page_offset;
-			sync_len = max(sync_len, pre_len);
-
-			/* For Not XDP_PASS verdict */
-			if (IS_ERR(skb)) {
-				unsigned int xdp_res = -PTR_ERR(skb);
-
-				if (xdp_res & STMMAC_XDP_CONSUMED) {
-					page_pool_put_page(rx_q->page_pool,
-							   virt_to_head_page(ctx.xdp.data),
-							   sync_len, true);
-					buf->page = NULL;
-					rx_dropped++;
-					goto next;
-				} else if (xdp_res & (STMMAC_XDP_TX |
-						      STMMAC_XDP_REDIRECT)) {
-					xdp_status |= xdp_res;
-					buf->page = NULL;
-					goto next;
-				}
-			}
-		}
-
-		if (!skb) {
-			unsigned int head_pad_len;
-
-			/* XDP program may expand or reduce tail */
-			buf1_len = ctx.xdp.data_end - ctx.xdp.data;
-
-			skb = napi_build_skb(page_address(buf->page),
-					     rx_q->napi_skb_frag_size);
-			if (!skb) {
-				page_pool_recycle_direct(rx_q->page_pool,
-							 buf->page);
-				buf->page = NULL;
-				rx_dropped++;
-				count++;
-				goto drain_data;
-			}
-
-			/* XDP program may adjust header */
-			head_pad_len = ctx.xdp.data - ctx.xdp.data_hard_start;
-			skb_reserve(skb, head_pad_len);
-			skb_put(skb, buf1_len);
-			skb_mark_for_recycle(skb);
 			buf->page = NULL;
 		} else if (buf1_len) {
-			dma_sync_single_for_cpu(priv->device, buf->addr,
-						buf1_len, dma_dir);
-			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-					buf->page, buf->page_offset, buf1_len,
-					priv->dma_conf.dma_buf_sz);
+			if (!stmmac_build_xdp_frags(priv, rx_q, buf1_len,
+						    buf->page,
+						    buf->page_offset,
+						    dma_dir, &ctx.xdp)) {
+				if (!error)
+					rx_dropped++;
+				error = 1;
+			}
 			buf->page = NULL;
 		}
 
 		if (buf2_len) {
-			dma_sync_single_for_cpu(priv->device, buf->sec_addr,
-						buf2_len, dma_dir);
-			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-					buf->sec_page, 0, buf2_len,
-					priv->dma_conf.dma_buf_sz);
+			if (!stmmac_build_xdp_frags(priv, rx_q, buf2_len,
+						    buf->sec_page, 0,
+						    dma_dir, &ctx.xdp)) {
+				if (!error)
+					rx_dropped++;
+				error = 1;
+			}
 			buf->sec_page = NULL;
 		}
+		frames++;
 
-drain_data:
 		if (likely(status & rx_not_ls))
 			goto read_again;
-		if (!skb)
-			continue;
 
-		/* Got entire packet into SKB. Finish it. */
+		/* ACS is disabled; strip manually. */
+		len -= ETH_FCS_LEN;
+		if (stmmac_xdp_shrink_tail(rx_q, &ctx.xdp, ETH_FCS_LEN)) {
+			if (!error)
+				rx_dropped++;
+			error = 1;
+		}
+
+		if (unlikely(error))
+			goto error_free_frag;
+
+		pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start;
+
+		ctx.desc = p;
+		ctx.ndesc = np;
+
+		skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
+		/* Due xdp_adjust_tail: DMA sync for_device
+		 * cover max len CPU touch
+		 */
+		sync_len = ctx.xdp.data_end - ctx.xdp.data_hard_start;
+		sync_len = max(sync_len, pre_len);
+
+		/* 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;
+			}
 
-		/* Remove FCS if needed */
-		if (!fcs_stripped && pskb_trim(skb, len)) {
-			dev_kfree_skb_any(skb);
-			rx_dropped++;
 			goto next;
 		}
 
+		skb = stmmac_build_skb(&ctx.xdp);
+		if (!skb) {
+			rx_dropped++;
+			goto error_free_frag;
+		}
+
+		/* Got entire packet into SKB. Finish it. */
 		stmmac_get_rx_hwtstamp(priv, p, np, skb);
 
 		if (priv->hw->hw_vlan_en)
@@ -6063,15 +6142,23 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		rx_bytes += len;
 next:
 		in_progress = false;
-		skb = NULL;
+		frames = 0;
+		count++;
+		continue;
+error_free_frag:
+		if (frames)
+			stmmac_xdp_put_buff(rx_q, &ctx.xdp, -1, true);
+		in_progress = false;
+		frames = 0;
 		count++;
 	}
 
-	if (in_progress || skb) {
-		rx_q->state_saved = true;
-		rx_q->state.skb = skb;
+	if (in_progress || frames) {
+		rx_q->state.xdp = ctx.xdp;
+		rx_q->state.frames = frames;
 		rx_q->state.error = error;
 		rx_q->state.len = len;
+		rx_q->state_saved = true;
 	}
 
 	stmmac_finalize_xdp_rx(priv, xdp_status);
@@ -8067,6 +8154,7 @@ static int __stmmac_dvr_probe(struct device *device,
 	ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 			    NETIF_F_RXCSUM;
 	ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
+			     NETDEV_XDP_ACT_RX_SG |
 			     NETDEV_XDP_ACT_XSK_ZEROCOPY;
 
 	ret = stmmac_tc_init(priv, priv);
@@ -8387,9 +8475,12 @@ static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue)
 {
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
 
-	dev_kfree_skb_any(rx_q->state.skb);
-	rx_q->state.skb = NULL;
+	if (rx_q->state.frames && !rx_q->xsk_pool) {
+		stmmac_xdp_put_buff(rx_q, &rx_q->state.xdp, -1, false);
+		rx_q->state.frames = 0;
+	}
 	rx_q->state_saved = false;
+
 	rx_q->cur_rx = 0;
 	rx_q->dirty_rx = 0;
 }

-- 
2.56.0

Re: [PATCH net-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support

[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-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

> 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.

The issues were spotted by sashiko.

> 
>  - 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.

These are real issues spotted by sashiko.

> 
>  - 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.

I tested them on a Qualcomm Rb3-gen2 board.

Regards,
Lorenzo

> 
> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v6 1/4] net: stmmac: take ownership of saved RX state at poll entry

[Nicolai Buchwitz]

On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> From: James Hilliard <james.hilliard1@gmail.com>
> 
> When a saved partial packet completes with a poll budget of one, the
> old loop can leave state_saved and state.skb pointing at an skb that
> has already been delivered or freed. The next poll then reuses that
> pointer, causing a use-after-free or double free.
> 
> Take the saved state at poll entry and clear the stored ownership
> immediately. Save it again only if the packet remains incomplete,
> including when the next descriptor is still DMA-owned.
> 
> Track the frame-in-progress flag independently of the status read from
> the next descriptor. The old code keyed the save on the status of the
> last descriptor looked at, so a loop stopped at a DMA-owned descriptor
> (rx status not yet valid) could drop the saved error/length bookkeeping
> in stmmac_rx_zc().
> 
> Drop the saved state both when the queue parameters are reset and when
> the ring is destroyed. stmmac_reset_rx_queue() is reached from
> __stmmac_open(), stmmac_xdp_open(), stmmac_resume() and
> stmmac_enable_rx_queue(); without draining it there, a partial frame
> saved before suspend (or before a queue is disabled and re-enabled)
> survives while cur_rx and dirty_rx are re-armed, and the first poll
> resumes that stale skb with descriptors from the freshly initialized
> ring. __free_dma_rx_desc_resources() releases it on teardown.
> 
> Apply the same state handling to stmmac_rx_zc(), which also leaves
> state_saved set when a saved frame finishes at the budget boundary.
> That path saves only error and length bookkeeping, not an skb pointer.
> 
> Fixes: ec222003bd94 ("net: stmmac: Prepare to add Split Header 
> support")
> Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---

> [...]

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

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

[Nicolai Buchwitz]

On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> stmmac_rx_zc() assumes a 1:1 mapping between a received frame and an
> XSK pool buffer and drops any frame that spans more than one RX
> descriptor:
> 
>         /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
>         if (likely(status & rx_not_ls)) {
>                 xsk_buff_free(buf->xdp);
>                 buf->xdp = NULL;
>                 rx_dropped++;
>                 error = 1;
>                 dirty++;
>                 goto read_again;
>         }
> 
> The non-last descriptors are freed and dropped, but the frame is not
> marked as errored. When the poll reaches the last descriptor of the
> same frame, error is still 0, so the fragment is handled as a complete
> frame: it is passed to the XDP program, copied into an skb on XDP_PASS,
> or converted for XDP_TX. The tail starts in the middle of the original
> payload at a predictable offset and, parsed as a fresh frame (len == 
> 0),
> stmmac_rx_buf1_len() reports the full dma_buf_sz length for it even
> though the DMA engine only wrote the tail remainder into that buffer, 
> so
> the reads extend past the received bytes and can reach beyond the end 
> of
> the XSK pool buffer. GMAC4 always enables jumbo frames (GMAC_CONFIG_JE
> in GMAC_CORE_INIT), so an on-link peer can trigger such a split even
> when the interface MTU is left at 1500.
> 
> Set error when the 1:1 mapping check drops a descriptor, so the
> remaining descriptors of the frame - including the last one - are
> drained and released with xsk_buff_free() instead of being processed as
> a new frame. Account every freed buffer with dirty and count the
> dropped frame once in the poll budget / rx_pkt_n, and once in
> rx_dropped.
> 
> Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---

> [...]

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[Nicolai Buchwitz]

Hi Lorenzo

On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> stmmac_rx() runs the XDP program once per descriptor buffer and uses
> skb == NULL as the only marker for the start of a frame. A frame larger
> than one buffer is split by the hardware across several descriptors;
> GMAC4 enables jumbo frames unconditionally (GMAC_CONFIG_JE in
> GMAC_CORE_INIT), so an on-link peer can deliver such a frame even when
> the interface MTU is left at 1500.
> 
> When an XDP verdict ends a non-last descriptor, skb is cleared and the
> next descriptor of the same hardware frame is parsed as a new frame.
> For XDP_CONSUMED this happens via read_again with len preserved, so
> stmmac_rx_buf1_len() then returns min(dma_buf_sz, plen - len) for the
> last descriptor, which can be as small as 0-3 bytes. buf1_len -=
> ETH_FCS_LEN underflows and xdp_prepare_buff() puts data_end far past
> data; on XDP_PASS this produces an skb whose length is close to 4G that
> reaches the network stack (skb->tail and skb->len are 32-bit and wrap
> below skb->end, so skb_over_panic() does not fire). A 4+ byte tail is
> instead offered to the XDP program and delivered as a standalone
> mid-payload Ethernet frame on XDP_PASS. XDP_TX/XDP_REDIRECT instead
> falls through to the next iteration, resetting the running length, so
> the tail is re-parsed as a full-size new frame.
> 
> XDP does not support scatter-gather frames until the multi-buffer
> rework, so drop the whole frame instead: mark it as errored as soon as
> a non-last descriptor is seen while an XDP program is attached, drain
> and release every descriptor - including the last - and account the
> frame once in rx_dropped. Removing the re-parse lets both XDP verdict
> branches simply complete the packet via next.
> 
> Note: the subsequent "net: stmmac: rework stmmac_rx to support XDP rx
> multi-buff" accumulates the whole frame into a single xdp_buff and
> supersedes this patch; the fix is kept so the net portion of the series
> stands on its own.
> 
> Fixes: 5fabb01207a2 ("net: stmmac: Add initial XDP support")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 28 
> ++++++++++-------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 426bb68c9bbc..8c08d44fdbd3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv, 
> int limit, u32 queue)
> 
>  		if (priv->extend_desc)
>  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + 
> entry);
> -		if (unlikely(status == discard_frame)) {
> -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> -			buf->page = NULL;
> +
> +		if (unlikely(!error && status == discard_frame)) {
>  			error = 1;
>  			if (!priv->hwts_rx_en)
>  				rx_errors++;
>  		}
> 
> -		if (unlikely(error && (status & rx_not_ls)))
> -			goto read_again;
> +		/* We do not support SG in XDP mode */
> +		if (unlikely(!error && (status & rx_not_ls) &&
> +			     stmmac_xdp_is_enabled(priv))) {
> +			rx_dropped++;
> +			error = 1;
> +		}

AFAIU GMAC4 and XGMAC always use the second buffer of a descriptor. If
so, at MTU 1500 a frame of up to 3072 bytes can fit in one descriptor,
and this check would not catch it?

The XDP program would then only see the first buffer. With XDP_TX the
frame is cut and sent too short. With XDP_PASS the second buffer is 
added
after the program already decided.

Patch 4 fixes this. But if this patch should work on its own, should it
also drop the frame when buf2_len is not zero? Or am I missing
something?

> [...]

Thanks,
Nicolai

Re: [PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[Nicolai Buchwitz]

Hi Lorenzo

On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> Build the xdp_buff by accumulating all the descriptors that make up a
> frame, so the XDP program runs on the full (possibly fragmented) packet
> instead of just the first buffer. When the frame is not consumed by the
> program, assemble the skb from the head buffer and the collected
> fragments via napi_build_skb()/xdp_update_skb_frags_info().
> 
> Note that XDP still rejects an MTU larger than ETH_DATA_LEN
> (stmmac_xdp_set_prog(), stmmac_change_mtu()) and disables SPH, so an
> ordinary frame fits in a single buffer. This path is therefore 
> exercised
> by frames that do span several descriptors (for example jumbo frames 
> the
> MAC admits through its jumbo-enable bit) and is groundwork for 
> extending
> XDP to jumbo/SPH; it does not enable XDP jumbo support by itself.
> 
> For those multi-descriptor frames the old code was buggy as well: it 
> ran
> the XDP program once per descriptor buffer, so for a multi-descriptor
> frame the program was also invoked on continuation buffers holding
> mid-frame payload. It also subtracted ETH_FCS_LEN from the first buffer
> even when it was not the last descriptor (its second buffer empty),
> dropping four bytes from the middle of the frame while marking the FCS 
> as
> already stripped and skipping the final pskb_trim(). Accumulating the
> whole frame before running XDP fixes both.

With patch 3/4 applied, these frames are already dropped in XDP mode,
so this paragraph no longer matches the code this patch changes?

> 
> To do so, store the in-progress xdp_buff in rx_q->state instead of the
> partially built skb, so the accumulated head and fragments survive a
> NAPI poll boundary (mid-frame dma_own or dirty_rx break). The state is
> saved only while a frame is in progress and cleared once it completes,
> leaving it untouched when the poll does not process anything (e.g.
> netpoll invoked with a zero budget).
> 
> In addition:
> - advertise NETDEV_XDP_ACT_RX_SG, since the RX path can now deliver
>   non-linear buffers to XDP;
> - drop multi-buffer frames when the attached program was not loaded 
> with
>   BPF_F_XDP_HAS_FRAGS, since a single-buffer program must not be handed 
> a
>   non-linear xdp_buff;
> - build the skb head with napi_build_skb() passing xdp->frame_sz, so
>   skb_shinfo() lands on the same shared_info the fragments were
>   accumulated into;
> - carry the XDP metadata written with bpf_xdp_adjust_meta() into the 
> skb
>   via skb_metadata_set(), so it is no longer dropped and TC/BPF 
> programs
>   can read it through data_meta;
> - attach fragments with xdp_buff_add_frag(), which initializes all the
>   shared_info fields and takes care of the pfmemalloc bit;
> - release the buffers collected in the xdp_buff when a frame is dropped
>   on RX errors; the previous code relied on dev_kfree_skb() recycling 
> the
>   skb frags, which no longer applies now that the head and fragments 
> live
>   in an xdp_buff rather than a partially built skb;
> - drop the whole frame when it would exceed MAX_SKB_FRAGS, where the
>   previous skb_add_rx_frag() calls were unbounded;
> - strip the Ethernet FCS from the accumulated xdp_buff with a
>   driver-local tail shrink (stmmac_xdp_shrink_tail()) for both linear
>   and fragmented frames, instead of trimming the skb or the first
>   descriptor length.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---

> [...]

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

> [...]

> +static void
> +stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q, struct xdp_buff 
> *xdp,
> +		    int sync_len, bool allow_direct)

This looks like xdp_return_buff(), apart from the sync length of the
first buffer. Could the NAPI paths (XDP_CONSUMED and error_free_frag)
use xdp_return_buff() instead? The teardown paths need allow_direct =
false, so they would still need something local.

> +{
> +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> +	int i;
> +
> +	if (likely(!xdp_buff_has_frags(xdp)))
> +		goto out;
> +
> +	for (i = 0; i < sinfo->nr_frags; i++)
> +		page_pool_put_full_page(rx_q->page_pool,
> +					skb_frag_page(&sinfo->frags[i]),
> +					allow_direct);
> +out:
> +	page_pool_put_page(rx_q->page_pool, virt_to_head_page(xdp->data),
> +			   sync_len, allow_direct);
> +}
> +
> +static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
> +{

This is very close to xdp_build_skb_from_buff(). Have you checked if 
this might
be a fit?

> +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> +	u32 metasize = xdp->data - xdp->data_meta;
> +	struct sk_buff *skb;
> +	u8 num_frags = 0;
> +
> +	if (unlikely(xdp_buff_has_frags(xdp)))
> +		num_frags = sinfo->nr_frags;
> +
> +	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
> +	if (!skb)
> +		return NULL;
> +
> +	skb_mark_for_recycle(skb);
> +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> +	skb_put(skb, xdp->data_end - xdp->data);
> +	if (metasize)
> +		skb_metadata_set(skb, metasize);
> +
> +	if (unlikely(xdp_buff_has_frags(xdp)))
> +		xdp_update_skb_frags_info(skb, num_frags, sinfo->xdp_frags_size,
> +					  num_frags * xdp->frame_sz,
> +					  xdp_buff_get_skb_flags(xdp));
> +	return skb;
> +}

> [...]

> +static int stmmac_xdp_shrink_tail(struct stmmac_rx_queue *rx_q,
> +				  struct xdp_buff *xdp, int offset)

This is only used to drop the FCS. On the last descriptor we already
know the frame length, so could the 4 bytes be taken off buf1_len /
buf2_len before the buffers are added? Then only the rare case where
the last buffer has fewer than 4 bytes would need to trim the previous
frag, and this helper could go away.

> [...]

Thanks,
Nicolai

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4582 bytes --]

On Oct 12, Nicolai Buchwitz wrote:
> Hi Lorenzo
> 
> On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> > stmmac_rx() runs the XDP program once per descriptor buffer and uses
> > skb == NULL as the only marker for the start of a frame. A frame larger
> > than one buffer is split by the hardware across several descriptors;
> > GMAC4 enables jumbo frames unconditionally (GMAC_CONFIG_JE in
> > GMAC_CORE_INIT), so an on-link peer can deliver such a frame even when
> > the interface MTU is left at 1500.
> > 
> > When an XDP verdict ends a non-last descriptor, skb is cleared and the
> > next descriptor of the same hardware frame is parsed as a new frame.
> > For XDP_CONSUMED this happens via read_again with len preserved, so
> > stmmac_rx_buf1_len() then returns min(dma_buf_sz, plen - len) for the
> > last descriptor, which can be as small as 0-3 bytes. buf1_len -=
> > ETH_FCS_LEN underflows and xdp_prepare_buff() puts data_end far past
> > data; on XDP_PASS this produces an skb whose length is close to 4G that
> > reaches the network stack (skb->tail and skb->len are 32-bit and wrap
> > below skb->end, so skb_over_panic() does not fire). A 4+ byte tail is
> > instead offered to the XDP program and delivered as a standalone
> > mid-payload Ethernet frame on XDP_PASS. XDP_TX/XDP_REDIRECT instead
> > falls through to the next iteration, resetting the running length, so
> > the tail is re-parsed as a full-size new frame.
> > 
> > XDP does not support scatter-gather frames until the multi-buffer
> > rework, so drop the whole frame instead: mark it as errored as soon as
> > a non-last descriptor is seen while an XDP program is attached, drain
> > and release every descriptor - including the last - and account the
> > frame once in rx_dropped. Removing the re-parse lets both XDP verdict
> > branches simply complete the packet via next.
> > 
> > Note: the subsequent "net: stmmac: rework stmmac_rx to support XDP rx
> > multi-buff" accumulates the whole frame into a single xdp_buff and
> > supersedes this patch; the fix is kept so the net portion of the series
> > stands on its own.
> > 
> > Fixes: 5fabb01207a2 ("net: stmmac: Add initial XDP support")
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 28
> > ++++++++++-------------
> >  1 file changed, 12 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 426bb68c9bbc..8c08d44fdbd3 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv,
> > int limit, u32 queue)
> > 
> >  		if (priv->extend_desc)
> >  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx +
> > entry);
> > -		if (unlikely(status == discard_frame)) {
> > -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> > -			buf->page = NULL;
> > +
> > +		if (unlikely(!error && status == discard_frame)) {
> >  			error = 1;
> >  			if (!priv->hwts_rx_en)
> >  				rx_errors++;
> >  		}
> > 
> > -		if (unlikely(error && (status & rx_not_ls)))
> > -			goto read_again;
> > +		/* We do not support SG in XDP mode */
> > +		if (unlikely(!error && (status & rx_not_ls) &&
> > +			     stmmac_xdp_is_enabled(priv))) {
> > +			rx_dropped++;
> > +			error = 1;
> > +		}
> 
> AFAIU GMAC4 and XGMAC always use the second buffer of a descriptor. If
> so, at MTU 1500 a frame of up to 3072 bytes can fit in one descriptor,
> and this check would not catch it?
> 
> The XDP program would then only see the first buffer. With XDP_TX the
> frame is cut and sent too short. With XDP_PASS the second buffer is added
> after the program already decided.
> 
> Patch 4 fixes this. But if this patch should work on its own, should it
> also drop the frame when buf2_len is not zero? Or am I missing
> something?

Hi Nicolai,

IIUC your comment we are missing the case where we have a single descriptor
(rx_not_ls is not set in status) but the packet is splitted over buf1 and buf2,
right? Something like:

	...
	buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
	len += buf2_len;

	if (unlikely(stmmac_xdp_is_enabled(priv) && buf2_len)) {
		rx_dropped++;
		dev_kfree_skb(skb);
		goto next;
	}
	...

Is my understanding correct?

Regards,
Lorenzo

> 
> > [...]
> 
> Thanks,
> Nicolai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[Nicolai Buchwitz]

Hi Lorenzo

On 13.10.2026 10:17, Lorenzo Bianconi wrote:
> On Oct 12, Nicolai Buchwitz wrote:

> [...]

>> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > index 426bb68c9bbc..8c08d44fdbd3 100644
>> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv,
>> > int limit, u32 queue)
>> >
>> >  		if (priv->extend_desc)
>> >  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx +
>> > entry);
>> > -		if (unlikely(status == discard_frame)) {
>> > -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
>> > -			buf->page = NULL;
>> > +
>> > +		if (unlikely(!error && status == discard_frame)) {
>> >  			error = 1;
>> >  			if (!priv->hwts_rx_en)
>> >  				rx_errors++;
>> >  		}
>> >
>> > -		if (unlikely(error && (status & rx_not_ls)))
>> > -			goto read_again;
>> > +		/* We do not support SG in XDP mode */
>> > +		if (unlikely(!error && (status & rx_not_ls) &&
>> > +			     stmmac_xdp_is_enabled(priv))) {
>> > +			rx_dropped++;
>> > +			error = 1;
>> > +		}
>> 
>> AFAIU GMAC4 and XGMAC always use the second buffer of a descriptor. If
>> so, at MTU 1500 a frame of up to 3072 bytes can fit in one descriptor,
>> and this check would not catch it?
>> 
>> The XDP program would then only see the first buffer. With XDP_TX the
>> frame is cut and sent too short. With XDP_PASS the second buffer is 
>> added
>> after the program already decided.
>> 
>> Patch 4 fixes this. But if this patch should work on its own, should 
>> it
>> also drop the frame when buf2_len is not zero? Or am I missing
>> something?
> 
> Hi Nicolai,
> 
> IIUC your comment we are missing the case where we have a single 
> descriptor
> (rx_not_ls is not set in status) but the packet is splitted over buf1 
> and buf2,
> right? Something like:
> 
> 	...
> 	buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
> 	len += buf2_len;
> 
> 	if (unlikely(stmmac_xdp_is_enabled(priv) && buf2_len)) {
> 		rx_dropped++;
> 		dev_kfree_skb(skb);
> 		goto next;
> 	}
> 	...
> 
> Is my understanding correct?

Yes, exactly.

I think skb is always NULL there in XDP mode, so the dev_kfree_skb()
can go. Both pages stay in the slot and stmmac_rx_refill() reuses them,
so nothing leaks.

Maybe extend the "We do not support SG in XDP mode" comment to cover
this case too.

> [...]

Thanks,
Nicolai

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]

On Oct 13, Nicolai Buchwitz wrote:
> Hi Lorenzo
> 
> On 13.10.2026 10:17, Lorenzo Bianconi wrote:
> > On Oct 12, Nicolai Buchwitz wrote:
> 
> > [...]
> 
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > index 426bb68c9bbc..8c08d44fdbd3 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv,
> > > > int limit, u32 queue)
> > > >
> > > >  		if (priv->extend_desc)
> > > >  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx +
> > > > entry);
> > > > -		if (unlikely(status == discard_frame)) {
> > > > -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> > > > -			buf->page = NULL;
> > > > +
> > > > +		if (unlikely(!error && status == discard_frame)) {
> > > >  			error = 1;
> > > >  			if (!priv->hwts_rx_en)
> > > >  				rx_errors++;
> > > >  		}
> > > >
> > > > -		if (unlikely(error && (status & rx_not_ls)))
> > > > -			goto read_again;
> > > > +		/* We do not support SG in XDP mode */
> > > > +		if (unlikely(!error && (status & rx_not_ls) &&
> > > > +			     stmmac_xdp_is_enabled(priv))) {
> > > > +			rx_dropped++;
> > > > +			error = 1;
> > > > +		}
> > > 
> > > AFAIU GMAC4 and XGMAC always use the second buffer of a descriptor. If
> > > so, at MTU 1500 a frame of up to 3072 bytes can fit in one descriptor,
> > > and this check would not catch it?
> > > 
> > > The XDP program would then only see the first buffer. With XDP_TX the
> > > frame is cut and sent too short. With XDP_PASS the second buffer is
> > > added
> > > after the program already decided.
> > > 
> > > Patch 4 fixes this. But if this patch should work on its own, should
> > > it
> > > also drop the frame when buf2_len is not zero? Or am I missing
> > > something?
> > 
> > Hi Nicolai,
> > 
> > IIUC your comment we are missing the case where we have a single
> > descriptor
> > (rx_not_ls is not set in status) but the packet is splitted over buf1
> > and buf2,
> > right? Something like:
> > 
> > 	...
> > 	buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
> > 	len += buf2_len;
> > 
> > 	if (unlikely(stmmac_xdp_is_enabled(priv) && buf2_len)) {
> > 		rx_dropped++;
> > 		dev_kfree_skb(skb);
> > 		goto next;
> > 	}
> > 	...
> > 
> > Is my understanding correct?
> 
> Yes, exactly.
> 
> I think skb is always NULL there in XDP mode, so the dev_kfree_skb()
> can go. Both pages stay in the slot and stmmac_rx_refill() reuses them,
> so nothing leaks.
> 
> Maybe extend the "We do not support SG in XDP mode" comment to cover
> this case too.

ack, thx for the review. I will fix it in v7.

Regards,
Lorenzo

> 
> > [...]
> 
> Thanks,
> Nicolai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 7938 bytes --]

> Hi Lorenzo
> 
> On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> > Build the xdp_buff by accumulating all the descriptors that make up a
> > frame, so the XDP program runs on the full (possibly fragmented) packet
> > instead of just the first buffer. When the frame is not consumed by the
> > program, assemble the skb from the head buffer and the collected
> > fragments via napi_build_skb()/xdp_update_skb_frags_info().
> > 
> > Note that XDP still rejects an MTU larger than ETH_DATA_LEN
> > (stmmac_xdp_set_prog(), stmmac_change_mtu()) and disables SPH, so an
> > ordinary frame fits in a single buffer. This path is therefore exercised
> > by frames that do span several descriptors (for example jumbo frames the
> > MAC admits through its jumbo-enable bit) and is groundwork for extending
> > XDP to jumbo/SPH; it does not enable XDP jumbo support by itself.
> > 
> > For those multi-descriptor frames the old code was buggy as well: it ran
> > the XDP program once per descriptor buffer, so for a multi-descriptor
> > frame the program was also invoked on continuation buffers holding
> > mid-frame payload. It also subtracted ETH_FCS_LEN from the first buffer
> > even when it was not the last descriptor (its second buffer empty),
> > dropping four bytes from the middle of the frame while marking the FCS
> > as
> > already stripped and skipping the final pskb_trim(). Accumulating the
> > whole frame before running XDP fixes both.
> 
> With patch 3/4 applied, these frames are already dropped in XDP mode,
> so this paragraph no longer matches the code this patch changes?

ack, I will remove it in v7.

> 
> > 
> > To do so, store the in-progress xdp_buff in rx_q->state instead of the
> > partially built skb, so the accumulated head and fragments survive a
> > NAPI poll boundary (mid-frame dma_own or dirty_rx break). The state is
> > saved only while a frame is in progress and cleared once it completes,
> > leaving it untouched when the poll does not process anything (e.g.
> > netpoll invoked with a zero budget).
> > 
> > In addition:
> > - advertise NETDEV_XDP_ACT_RX_SG, since the RX path can now deliver
> >   non-linear buffers to XDP;
> > - drop multi-buffer frames when the attached program was not loaded with
> >   BPF_F_XDP_HAS_FRAGS, since a single-buffer program must not be handed
> > a
> >   non-linear xdp_buff;
> > - build the skb head with napi_build_skb() passing xdp->frame_sz, so
> >   skb_shinfo() lands on the same shared_info the fragments were
> >   accumulated into;
> > - carry the XDP metadata written with bpf_xdp_adjust_meta() into the skb
> >   via skb_metadata_set(), so it is no longer dropped and TC/BPF programs
> >   can read it through data_meta;
> > - attach fragments with xdp_buff_add_frag(), which initializes all the
> >   shared_info fields and takes care of the pfmemalloc bit;
> > - release the buffers collected in the xdp_buff when a frame is dropped
> >   on RX errors; the previous code relied on dev_kfree_skb() recycling
> > the
> >   skb frags, which no longer applies now that the head and fragments
> > live
> >   in an xdp_buff rather than a partially built skb;
> > - drop the whole frame when it would exceed MAX_SKB_FRAGS, where the
> >   previous skb_add_rx_frag() calls were unbounded;
> > - strip the Ethernet FCS from the accumulated xdp_buff with a
> >   driver-local tail shrink (stmmac_xdp_shrink_tail()) for both linear
> >   and fragmented frames, instead of trimming the skb or the first
> >   descriptor length.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > ---
> 
> > [...]
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> 
> > [...]
> 
> > +static void
> > +stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q, struct xdp_buff *xdp,
> > +		    int sync_len, bool allow_direct)
> 
> This looks like xdp_return_buff(), apart from the sync length of the
> first buffer. Could the NAPI paths (XDP_CONSUMED and error_free_frag)
> use xdp_return_buff() instead? The teardown paths need allow_direct =
> false, so they would still need something local.

Theoretically I agree, but, since we still need stmmac_xdp_put_buff()
for the teardown path, I guess we can keep it since it is even an
optimized version with respect to xdp_return_buff(). What do yo think?

> 
> > +{
> > +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > +	int i;
> > +
> > +	if (likely(!xdp_buff_has_frags(xdp)))
> > +		goto out;
> > +
> > +	for (i = 0; i < sinfo->nr_frags; i++)
> > +		page_pool_put_full_page(rx_q->page_pool,
> > +					skb_frag_page(&sinfo->frags[i]),
> > +					allow_direct);
> > +out:
> > +	page_pool_put_page(rx_q->page_pool, virt_to_head_page(xdp->data),
> > +			   sync_len, allow_direct);
> > +}
> > +
> > +static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
> > +{
> 
> This is very close to xdp_build_skb_from_buff(). Have you checked if this
> might
> be a fit?

yes, but we can't use xdp_build_skb_from_buff() since it runs eth_type_trans()
modifying the skb->data pointer (in this way we will corrupt stmmac_rx_vlan()).
Agree?

> 
> > +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > +	u32 metasize = xdp->data - xdp->data_meta;
> > +	struct sk_buff *skb;
> > +	u8 num_frags = 0;
> > +
> > +	if (unlikely(xdp_buff_has_frags(xdp)))
> > +		num_frags = sinfo->nr_frags;
> > +
> > +	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
> > +	if (!skb)
> > +		return NULL;
> > +
> > +	skb_mark_for_recycle(skb);
> > +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> > +	skb_put(skb, xdp->data_end - xdp->data);
> > +	if (metasize)
> > +		skb_metadata_set(skb, metasize);
> > +
> > +	if (unlikely(xdp_buff_has_frags(xdp)))
> > +		xdp_update_skb_frags_info(skb, num_frags, sinfo->xdp_frags_size,
> > +					  num_frags * xdp->frame_sz,
> > +					  xdp_buff_get_skb_flags(xdp));
> > +	return skb;
> > +}
> 
> > [...]
> 
> > +static int stmmac_xdp_shrink_tail(struct stmmac_rx_queue *rx_q,
> > +				  struct xdp_buff *xdp, int offset)
> 
> This is only used to drop the FCS. On the last descriptor we already
> know the frame length, so could the 4 bytes be taken off buf1_len /
> buf2_len before the buffers are added? Then only the rare case where
> the last buffer has fewer than 4 bytes would need to trim the previous
> frag, and this helper could go away.

uhm, right. I guess we could do something like (and drop
stmmac_xdp_shrink_tail()):

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e80793bd6553..5cb8a6883f44 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6027,6 +6027,19 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
 		len += buf2_len;
 
+		if (!(status & rx_not_ls)) {
+			/* ACS is disabled; strip manually. */
+			if (buf2_len > ETH_FCS_LEN) {
+				buf2_len -= ETH_FCS_LEN;
+			} else if (!buf2_len) {
+				buf1_len -= ETH_FCS_LEN;
+			} else {
+				buf1_len -= (ETH_FCS_LEN - buf2_len);
+				buf1_len = 0;
+			}
+			len -= ETH_FCS_LEN;
+		}
+
 		if (!frames) {
 			dma_sync_single_for_cpu(priv->device, buf->addr,
 						buf1_len, dma_dir);
@@ -6064,8 +6077,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		if (likely(status & rx_not_ls))
 			goto read_again;
 
-		/* ACS is disabled; strip manually. */
-		len -= ETH_FCS_LEN;
 		if (stmmac_xdp_shrink_tail(rx_q, &ctx.xdp, ETH_FCS_LEN)) {
 			if (!error)
 				rx_dropped++;

What do you think?

Regards,
Lorenzo

> 
> > [...]
> 
> Thanks,
> Nicolai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 8801 bytes --]

On Oct 13, Lorenzo Bianconi wrote:
> > Hi Lorenzo
> > 
> > On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> > > Build the xdp_buff by accumulating all the descriptors that make up a
> > > frame, so the XDP program runs on the full (possibly fragmented) packet
> > > instead of just the first buffer. When the frame is not consumed by the
> > > program, assemble the skb from the head buffer and the collected
> > > fragments via napi_build_skb()/xdp_update_skb_frags_info().
> > > 
> > > Note that XDP still rejects an MTU larger than ETH_DATA_LEN
> > > (stmmac_xdp_set_prog(), stmmac_change_mtu()) and disables SPH, so an
> > > ordinary frame fits in a single buffer. This path is therefore exercised
> > > by frames that do span several descriptors (for example jumbo frames the
> > > MAC admits through its jumbo-enable bit) and is groundwork for extending
> > > XDP to jumbo/SPH; it does not enable XDP jumbo support by itself.
> > > 
> > > For those multi-descriptor frames the old code was buggy as well: it ran
> > > the XDP program once per descriptor buffer, so for a multi-descriptor
> > > frame the program was also invoked on continuation buffers holding
> > > mid-frame payload. It also subtracted ETH_FCS_LEN from the first buffer
> > > even when it was not the last descriptor (its second buffer empty),
> > > dropping four bytes from the middle of the frame while marking the FCS
> > > as
> > > already stripped and skipping the final pskb_trim(). Accumulating the
> > > whole frame before running XDP fixes both.
> > 
> > With patch 3/4 applied, these frames are already dropped in XDP mode,
> > so this paragraph no longer matches the code this patch changes?
> 
> ack, I will remove it in v7.
> 
> > 
> > > 
> > > To do so, store the in-progress xdp_buff in rx_q->state instead of the
> > > partially built skb, so the accumulated head and fragments survive a
> > > NAPI poll boundary (mid-frame dma_own or dirty_rx break). The state is
> > > saved only while a frame is in progress and cleared once it completes,
> > > leaving it untouched when the poll does not process anything (e.g.
> > > netpoll invoked with a zero budget).
> > > 
> > > In addition:
> > > - advertise NETDEV_XDP_ACT_RX_SG, since the RX path can now deliver
> > >   non-linear buffers to XDP;
> > > - drop multi-buffer frames when the attached program was not loaded with
> > >   BPF_F_XDP_HAS_FRAGS, since a single-buffer program must not be handed
> > > a
> > >   non-linear xdp_buff;
> > > - build the skb head with napi_build_skb() passing xdp->frame_sz, so
> > >   skb_shinfo() lands on the same shared_info the fragments were
> > >   accumulated into;
> > > - carry the XDP metadata written with bpf_xdp_adjust_meta() into the skb
> > >   via skb_metadata_set(), so it is no longer dropped and TC/BPF programs
> > >   can read it through data_meta;
> > > - attach fragments with xdp_buff_add_frag(), which initializes all the
> > >   shared_info fields and takes care of the pfmemalloc bit;
> > > - release the buffers collected in the xdp_buff when a frame is dropped
> > >   on RX errors; the previous code relied on dev_kfree_skb() recycling
> > > the
> > >   skb frags, which no longer applies now that the head and fragments
> > > live
> > >   in an xdp_buff rather than a partially built skb;
> > > - drop the whole frame when it would exceed MAX_SKB_FRAGS, where the
> > >   previous skb_add_rx_frag() calls were unbounded;
> > > - strip the Ethernet FCS from the accumulated xdp_buff with a
> > >   driver-local tail shrink (stmmac_xdp_shrink_tail()) for both linear
> > >   and fragmented frames, instead of trimming the skb or the first
> > >   descriptor length.
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > ---
> > 
> > > [...]
> > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > 
> > > [...]
> > 
> > > +static void
> > > +stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q, struct xdp_buff *xdp,
> > > +		    int sync_len, bool allow_direct)
> > 
> > This looks like xdp_return_buff(), apart from the sync length of the
> > first buffer. Could the NAPI paths (XDP_CONSUMED and error_free_frag)
> > use xdp_return_buff() instead? The teardown paths need allow_direct =
> > false, so they would still need something local.
> 
> Theoretically I agree, but, since we still need stmmac_xdp_put_buff()
> for the teardown path, I guess we can keep it since it is even an
> optimized version with respect to xdp_return_buff(). What do yo think?
> 
> > 
> > > +{
> > > +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > > +	int i;
> > > +
> > > +	if (likely(!xdp_buff_has_frags(xdp)))
> > > +		goto out;
> > > +
> > > +	for (i = 0; i < sinfo->nr_frags; i++)
> > > +		page_pool_put_full_page(rx_q->page_pool,
> > > +					skb_frag_page(&sinfo->frags[i]),
> > > +					allow_direct);
> > > +out:
> > > +	page_pool_put_page(rx_q->page_pool, virt_to_head_page(xdp->data),
> > > +			   sync_len, allow_direct);
> > > +}
> > > +
> > > +static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
> > > +{
> > 
> > This is very close to xdp_build_skb_from_buff(). Have you checked if this
> > might
> > be a fit?
> 
> yes, but we can't use xdp_build_skb_from_buff() since it runs eth_type_trans()
> modifying the skb->data pointer (in this way we will corrupt stmmac_rx_vlan()).
> Agree?
> 
> > 
> > > +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > > +	u32 metasize = xdp->data - xdp->data_meta;
> > > +	struct sk_buff *skb;
> > > +	u8 num_frags = 0;
> > > +
> > > +	if (unlikely(xdp_buff_has_frags(xdp)))
> > > +		num_frags = sinfo->nr_frags;
> > > +
> > > +	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
> > > +	if (!skb)
> > > +		return NULL;
> > > +
> > > +	skb_mark_for_recycle(skb);
> > > +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> > > +	skb_put(skb, xdp->data_end - xdp->data);
> > > +	if (metasize)
> > > +		skb_metadata_set(skb, metasize);
> > > +
> > > +	if (unlikely(xdp_buff_has_frags(xdp)))
> > > +		xdp_update_skb_frags_info(skb, num_frags, sinfo->xdp_frags_size,
> > > +					  num_frags * xdp->frame_sz,
> > > +					  xdp_buff_get_skb_flags(xdp));
> > > +	return skb;
> > > +}
> > 
> > > [...]
> > 
> > > +static int stmmac_xdp_shrink_tail(struct stmmac_rx_queue *rx_q,
> > > +				  struct xdp_buff *xdp, int offset)
> > 
> > This is only used to drop the FCS. On the last descriptor we already
> > know the frame length, so could the 4 bytes be taken off buf1_len /
> > buf2_len before the buffers are added? Then only the rare case where
> > the last buffer has fewer than 4 bytes would need to trim the previous
> > frag, and this helper could go away.
> 
> uhm, right. I guess we could do something like (and drop
> stmmac_xdp_shrink_tail()):
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e80793bd6553..5cb8a6883f44 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6027,6 +6027,19 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  		buf2_len = stmmac_rx_buf2_len(priv, p, status, len);
>  		len += buf2_len;
>  
> +		if (!(status & rx_not_ls)) {
> +			/* ACS is disabled; strip manually. */
> +			if (buf2_len > ETH_FCS_LEN) {
> +				buf2_len -= ETH_FCS_LEN;
> +			} else if (!buf2_len) {
> +				buf1_len -= ETH_FCS_LEN;
> +			} else {
> +				buf1_len -= (ETH_FCS_LEN - buf2_len);
> +				buf1_len = 0;

Here there is an obvious typo (buf2_len and not buf1_len) but thinking again
at the suggested approach I think there is corner case we can't really cover,
the FCS can be theoretically split over different DMA descriptor, so we need
to reconstruct the full xdp buffer first and then remove it. In other words,
I do not think we can get rid of stmmac_xdp_shrink_tail(). What do you think?

Regards,
Lorenzo

> +			}
> +			len -= ETH_FCS_LEN;
> +		}
> +
>  		if (!frames) {
>  			dma_sync_single_for_cpu(priv->device, buf->addr,
>  						buf1_len, dma_dir);
> @@ -6064,8 +6077,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  		if (likely(status & rx_not_ls))
>  			goto read_again;
>  
> -		/* ACS is disabled; strip manually. */
> -		len -= ETH_FCS_LEN;
>  		if (stmmac_xdp_shrink_tail(rx_q, &ctx.xdp, ETH_FCS_LEN)) {
>  			if (!error)
>  				rx_dropped++;
> 
> What do you think?
> 
> Regards,
> Lorenzo
> 
> > 
> > > [...]
> > 
> > Thanks,
> > Nicolai



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[sashiko-bot]

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

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: lost increments of global rx_dropped/rx_errors in concurrent NAPI polls
--

commit 45e89e4ad29cf5d670b87e898ffe94eedf0e0127
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

net: stmmac: drop multi-buffer frames in XDP mode

stmmac_rx() runs the XDP program once per descriptor buffer and uses skb ==
NULL as the only marker for the start of a frame. A frame larger than one
buffer is split by the hardware across several descriptors. XDP does not
support scatter-gather frames until the multi-buffer rework, so this commit
drops the whole frame instead.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 426bb68c9bbce..8c08d44fdbd39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  
>  		if (priv->extend_desc)
>  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry);
> -		if (unlikely(status == discard_frame)) {
> -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> -			buf->page = NULL;
> +
> +		if (unlikely(!error && status == discard_frame)) {
>  			error = 1;
>  			if (!priv->hwts_rx_en)
>  				rx_errors++;
>  		}
>  
> -		if (unlikely(error && (status & rx_not_ls)))
> -			goto read_again;
> +		/* We do not support SG in XDP mode */
> +		if (unlikely(!error && (status & rx_not_ls) &&
> +			     stmmac_xdp_is_enabled(priv))) {
> +			rx_dropped++;

[Severity: Medium]
Will this lead to lost increments in the global rx_dropped and rx_errors
statistics?

The local rx_dropped and rx_errors counters are updated here, and at the
end of stmmac_rx() they are added directly to the global statistics:

	priv->xstats.rx_dropped += rx_dropped;
	priv->xstats.rx_errors += rx_errors;

When multiple NAPI handlers for different RX queues run concurrently on
different CPUs, could they race on updating these global counters without
synchronization?

> +			error = 1;
> +		}

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

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

[netdev-bot+sashiko]

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

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

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] The fix is incomplete on cores that use normal or enhanced RX
  descriptors (dwmac100 and dwmac1000/GMAC 3.x).
- [Low] On GMAC4/XGMAC, stmmac_rx_zc() now counts a multi-descriptor
  frame with a hardware error in rx_dropped instead of rx_errors.
- [Low] The commit message presents the patched code as the existing
  buggy code.

Pre-existing issues:
- [Low] Each RX NAPI poll, in stmmac_rx_zc() (stmmac_main.c:5789-5790)
  and stmmac_rx() (6091-6092), merges its local counts into the device-…

--

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

This patch changes stmmac_rx_zc() so that, in AF_XDP zero-copy mode, a
frame spanning more than one RX descriptor sets error on its first
descriptor. The remaining descriptors, including the last one, are then
freed instead of being processed as a new frame. It also moves the buffer
free into the common error block and adjusts the rx_dropped and count
accounting.

> stmmac_rx_zc() assumes a 1:1 mapping between a received frame and an
> XSK pool buffer and drops any frame that spans more than one RX
> descriptor:
>
>         /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
>         if (likely(status & rx_not_ls)) {
>                 xsk_buff_free(buf->xdp);
>                 buf->xdp = NULL;
>                 rx_dropped++;
>                 error = 1;
>                 dirty++;
>                 goto read_again;
>         }
>
> The non-last descriptors are freed and dropped, but the frame is not
> marked as errored.

[Severity: Low]
Is this snippet the post-patch version rather than the existing code? It
contains rx_dropped++ and error = 1 and has no count++. Those are the
exact lines this patch adds and removes. The pre-patch branch was:

        if (likely(status & rx_not_ls)) {
                xsk_buff_free(buf->xdp);
                buf->xdp = NULL;
                dirty++;
                count++;
                goto read_again;
        }

The next sentence says the frame is not marked as errored. That
contradicts the error = 1 in the quoted snippet.

Could the snippet be replaced with the original code?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f66b796f7725b..426bb68c9bbce 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5701,18 +5701,21 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
>  		if (priv->extend_desc)
>  			stmmac_rx_extended_status(priv, &priv->xstats,
>  						  rx_q->dma_erx + entry);
> -		if (unlikely(status == discard_frame)) {
> -			xsk_buff_free(buf->xdp);
> -			buf->xdp = NULL;
> -			dirty++;
> +
> +		if (unlikely(status == discard_frame) && !error) {
>  			error = 1;
>  			if (!priv->hwts_rx_en)
>  				rx_errors++;
>  		}

[Severity: Low]
On GMAC4/XGMAC, does this change how split frames with a hardware error
are counted?

The first descriptor of a multi-descriptor frame returns rx_not_ls from
dwmac4_wrback_get_rx_status(). It takes the 1:1 path further down, which
now does rx_dropped++ and error = 1.

The last descriptor may then return discard_frame because ES is set (CRC,
overflow, watchdog, giant). In that case the && !error guard here is
false, so rx_errors++ is skipped.

Before the patch, the 1:1 path did not set error. The last descriptor's
discard_frame incremented rx_errors, and rx_dropped was not touched.

The per-cause xstats counters are still incremented. However, ip -s link
now reports the frame as dropped rather than errored. Is that intended?
The commit message only says the frame is counted once in rx_dropped.

>  
> -		if (unlikely(error && (status & rx_not_ls)))
> -			goto read_again;
>  		if (unlikely(error)) {
> +			xsk_buff_free(buf->xdp);
> +			buf->xdp = NULL;
> +			dirty++;
> +
> +			if (status & rx_not_ls)
> +				goto read_again;
> +
>  			count++;
>  			continue;
>  		}

[Severity: High]
Does this fix also cover cores that use normal or enhanced RX descriptors
(dwmac100 and dwmac1000)? The continuation logic here relies on non-last
descriptors reporting rx_not_ls. Only dwmac4_wrback_get_rx_status() and
the dwxgmac2 callback do that.

enh_desc_get_rx_status() and ndesc_get_rx_status() report a non-last
descriptor like this:

drivers/net/ethernet/stmicro/stmmac/enh_desc.c:enh_desc_get_rx_status() {
    ...
	if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
		x->rx_length++;
		return discard_frame;
	}
    ...
}

With that encoding, a frame split across two descriptors goes through
stmmac_rx_zc() like this:

first descriptor:
    in_progress = status & rx_not_ls;          /* 0 */
    status == discard_frame && !error  ->  error = 1
    error block frees buf->xdp, dirty++
    (status & rx_not_ls) is 0  ->  count++; continue;

next loop iteration:
    if (!in_progress) { error = 0; len = 0; }

last descriptor:
    good status, so it skips the error block and the 1:1 rx_not_ls check

The tail is then processed as a complete new frame. With len == 0,
stmmac_rx_buf1_len() returns:

	return min_t(unsigned int, priv->dma_conf.dma_buf_sz, plen - len);

That value is dma_buf_sz, but the DMA only wrote the tail remainder into
that XSK buffer. The tail starts mid-payload and goes to the XDP program,
to XDP_TX, or to stmmac_dispatch_skb_zc() on XDP_PASS.

Isn't that the same symptom the commit message says this patch removes?

This path looks reachable:

- __stmmac_dvr_probe() advertises NETDEV_XDP_ACT_XSK_ZEROCOPY for every
  core.
- stmmac_xdp_enable_pool() only checks the queue indices and the frame
  size. It does not check the core or the descriptor format.
- dwmac1000 and dwmac100 have no .set_bfsize op. The buffer size
  programmed by stmmac_clear_rx_descriptors() is therefore dma_buf_sz,
  which is 1536 at MTU 1500.

dwmac1000_core_init() only enables the larger frame modes above
MTU 1500:

	if (mtu > 1500)
		value |= GMAC_CONTROL_2K;
	if (mtu > 2000)
		value |= GMAC_CONTROL_JE;

XDP limits the MTU to 1500, so neither bit is set. It appears an on-link
peer could still send a frame between 1536 bytes and the 2048-byte
watchdog limit, and the hardware would split it across two descriptors.

Would it work better to track continuation from the hardware LD/FD bits?
Other options are to have the enh/norm callbacks report rx_not_ls for
non-last descriptors, or to refuse XSK zero-copy on these descriptor
formats.

> @@ -5721,8 +5724,9 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
>  		if (likely(status & rx_not_ls)) {
>  			xsk_buff_free(buf->xdp);
>  			buf->xdp = NULL;
> +			rx_dropped++;
> +			error = 1;
>  			dirty++;
> -			count++;
>  			goto read_again;
>  		}

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. The
new rx_dropped++ feeds into this accumulator at the end of
stmmac_rx_zc():

	priv->xstats.rx_dropped += rx_dropped;
	priv->xstats.rx_errors += rx_errors;

stmmac_rx() does the same thing. Each channel has its own NAPI instance.
RX queues polling on different CPUs can therefore do these non-atomic
read-modify-writes on the device-wide unsigned long fields at the same
time. stmmac_get_stats64() also reads them without any synchronization.

Can increments to rx_dropped and rx_errors be lost when two queues finish
their polls at the same time?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-rx-mb-v6-0-2225050915c1%40oss.qualcomm.com

Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 (1) · 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] On GMAC4/XGMAC, a frame that fills buffer 1 and buffer 2 of a
  single descriptor gets past the new 'We do not support SG in XDP mode'…
- [Medium] The new XDP guard in stmmac_rx() and the drain logic (`if
  (status & rx_not_ls) goto read_again;`, `in_progress = status &…

Pre-existing issues:
- [Medium] dwmac4_wrback_get_rx_status() sets ret = discard_frame when
  RDES3_ERROR_SUMMARY is set (CRC, receive error, overflow, watchdog,…

--

net: stmmac: drop multi-buffer frames in XDP mode

With an XDP program attached, this commit makes stmmac_rx() mark a frame
as errored when it sees a non-last descriptor. It then drains every
descriptor of that frame and counts it once in rx_dropped. The
XDP_CONSUMED re-parse path is removed, so both XDP verdict branches now
finish the packet through next.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 426bb68c9bbce..8c08d44fdbd39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  
>  		if (priv->extend_desc)
>  			stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry);
> -		if (unlikely(status == discard_frame)) {
> -			page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> -			buf->page = NULL;
> +
> +		if (unlikely(!error && status == discard_frame)) {
>  			error = 1;
>  			if (!priv->hwts_rx_en)
>  				rx_errors++;
>  		}

[Severity: Medium]
This is a pre-existing issue and wasn't introduced by this patch. Should
this check test the bit instead of comparing for equality? The
rx_frame_status values are bit flags, and dwmac4_wrback_get_rx_status()
can return discard_frame | csum_none:

dwmac4_wrback_get_rx_status() {
	...
	if (unlikely(rdes3 & RDES3_ERROR_SUMMARY)) {
		...
		ret = discard_frame;
	}
	...
	if (rdes1 & RDES1_IP_HDR_ERROR) {
		x->ip_hdr_err++;
		ret |= csum_none;
	}
	...
	if (rdes1 & RDES1_IP_PAYLOAD_ERROR) {
		x->ip_payload_err++;
		ret |= csum_none;
	}
	...
}

Take a frame that has an error summary bit (CRC, overflow, giant, ...)
and also an IP header or payload error. The status is 0x3, so it fails
status == discard_frame and goes to XDP or the stack as a good frame.

The stack path gets CHECKSUM_NONE and checks the checksum in software,
so the XDP path is the one mainly affected. This patch only adds
!error && to the same comparison, and the comparison is unchanged at
the end of the series.

>  
> -		if (unlikely(error && (status & rx_not_ls)))
> -			goto read_again;
> +		/* We do not support SG in XDP mode */
> +		if (unlikely(!error && (status & rx_not_ls) &&
> +			     stmmac_xdp_is_enabled(priv))) {
> +			rx_dropped++;
> +			error = 1;
> +		}

[Severity: Medium]
Does this guard catch a frame that fits in one descriptor but spills
into buffer 2? It only tests rx_not_ls, so a last descriptor with data
in both buf1 and buf2 gets past it.

On GMAC4/XGMAC, stmmac_rx_check_buf2_cap() returns true whenever
set_sec_addr exists. It ignores SPH and XDP:

stmmac_rx_check_buf2_cap() {
	return priv->hw->desc && priv->hw->desc->set_sec_addr;
}

So stmmac_rx_refill() arms sec_page, and dwmac4_set_sec_addr() sets
RDES3_BUFFER2_VALID_ADDR.

With MTU 1500 (dma_buf_sz 1536) and GMAC_CONFIG_JE always set, a peer
can send a 1537..3072 byte frame. It lands in one descriptor with LS
set, so buf1_len is 1536 and buf2_len is non-zero.

Further down in stmmac_rx(), the xdp_buff is built from buffer 1 only,
and the FCS isn't stripped:

	if (stmmac_xdp_is_enabled(priv) && !buf2_len) {
		buf1_len -= ETH_FCS_LEN;
		fcs_stripped = true;
	}

	xdp_init_buff(&ctx.xdp, bufsz, &rx_q->xdp_rxq);
	xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
			 buf->page_offset, buf1_len, true);

Wouldn't the XDP program see a truncated frame here?

On XDP_TX and XDP_REDIRECT, the truncated buffer 1 is forwarded.
buf->sec_page stays in the slot, so the buffer 2 data is dropped. On
XDP_PASS, a buffer 1 that the program may have modified is followed by
an unmodified buffer 2 frag.

So for this case the subject line ("drop multi-buffer frames in XDP
mode") doesn't hold. Commit 88095728511e ("net: stmmac: fix rx
Scatter-Gather support") already lists this as a known limitation.

The later patch in this series, "net: stmmac: rework stmmac_rx to
support XDP rx multi-buff", fixes it by adding buf2 as an xdp frag. The
standalone net/stable portion would still have the gap. Should frames
with buf2_len != 0 also be dropped here while XDP is enabled?

>  
>  		if (unlikely(error)) {
> +			if (status & rx_not_ls)
> +				goto read_again;
> +

[Severity: Medium]
Does this drain work on enh_desc and norm_desc cores? The new XDP guard
and this goto read_again both expect stmmac_rx_status() to return
rx_not_ls for a non-last descriptor.

Only dwmac4_wrback_get_rx_status() and the dwxgmac2 rx_status do that.
The legacy parsers return plain discard_frame:

enh_desc_get_rx_status() / ndesc_get_rx_status() {
	...
	if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
		x->rx_length++;
		return discard_frame;
	}
	...
}

ndev->xdp_features is set unconditionally, so XDP is advertised on
these cores too.

On dwmac1000/dwmac100, the first descriptor of a split frame sets
error = 1 and counts rx_errors, not rx_dropped. rx_not_ls is clear, so
the code takes goto next and doesn't drain. The next iteration then
resets error, len and skb.

The last descriptor is then parsed as a new frame with len == 0. In
that case stmmac_rx_buf1_len() returns min(dma_buf_sz, plen), and plen
is the full frame length.

Could this give XDP (and the stack on XDP_PASS) a mid-payload frame?
Its length would run up to dma_buf_sz and cover stale page_pool bytes
that this packet didn't write.

The commit message says this outcome is removed ("drain and release
every descriptor - including the last"), but it is still there at the
end of the series.

Whether it can happen depends on the legacy MAC delivering a frame
larger than dma_buf_sz at MTU 1500. dwmac1000_core_init() leaves JE/2K
off but doesn't set WD, so frames up to 2048 bytes can reach the FIFO.
Whether they reach memory without ES set depends on how the hardware is
configured, for example store-and-forward with FEF=0 versus threshold
mode.

>  			dev_kfree_skb(skb);
>  			goto next;
>  		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-rx-mb-v6-0-2225050915c1%40oss.qualcomm.com

Re: [PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[netdev-bot+sashiko]

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

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

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] stmmac_build_skb() now calls skb_metadata_set(skb, metasize)
  (stmmac_main.c:5847), so the XDP metadata sits right before skb->data.
- [Low] The commit message says the old stmmac_rx() had two bugs on
  multi-descriptor frames, and that accumulating the frame before…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but this patch
  fixes it without saying so, and the net portion of the series leaves…
- [Medium] This problem wasn't introduced by this patch, but
  stmmac_construct_skb_zc() (AF_XDP zero-copy XDP_PASS path) allocates a…
- [Medium] This problem wasn't introduced by this patch, but
  dwmac4_wrback_get_rx_status() loads RDES1 and RDES2 before it loads…

--

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

This commit reworks stmmac_rx() so that all descriptors of a frame are
collected into an xdp_buff before the XDP program runs. On XDP_PASS the
skb is built from that xdp_buff with napi_build_skb() and
xdp_update_skb_frags_info(). The in-progress xdp_buff is kept in
rx_q->state across NAPI polls, XDP metadata is passed to the skb, the
FCS is stripped with a driver-local tail shrink, and
NETDEV_XDP_ACT_RX_SG is advertised.

> For those multi-descriptor frames the old code was buggy as well: it ran
> the XDP program once per descriptor buffer, so for a multi-descriptor
> frame the program was also invoked on continuation buffers holding
> mid-frame payload. It also subtracted ETH_FCS_LEN from the first buffer
> even when it was not the last descriptor (its second buffer empty),
> dropping four bytes from the middle of the frame while marking the FCS as
> already stripped and skipping the final pskb_trim(). Accumulating the
> whole frame before running XDP fixes both.

[Severity: Low]
Does this describe the code being replaced? The parent commit "net:
stmmac: drop multi-buffer frames in XDP mode" added this check, and this
patch removes it:

    /* We do not support SG in XDP mode */
    if (unlikely(!error && (status & rx_not_ls) &&
                 stmmac_xdp_is_enabled(priv))) {
        rx_dropped++;
        error = 1;
    }

With a program attached, every non-last descriptor is marked as an error
before it reaches the block that runs the program and does:

    if (stmmac_xdp_is_enabled(priv) && !buf2_len) {
        buf1_len -= ETH_FCS_LEN;
        fcs_stripped = true;
    }

In XDP mode only single-descriptor frames get there. Neither problem
seems reachable in the parent.

The message also does not say what actually changes relative to the
parent. Multi-descriptor frames that used to be dropped in XDP mode now
go to programs loaded with BPF_F_XDP_HAS_FRAGS.

Could the commit message describe that change instead?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 8c08d44fdbd39..fd04368543c23 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -5801,6 +5807,118 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)

[ ... ]

> +static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
> +{
> +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> +	u32 metasize = xdp->data - xdp->data_meta;
> +	struct sk_buff *skb;
> +	u8 num_frags = 0;
> +
> +	if (unlikely(xdp_buff_has_frags(xdp)))
> +		num_frags = sinfo->nr_frags;
> +
> +	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
> +	if (!skb)
> +		return NULL;
> +
> +	skb_mark_for_recycle(skb);
> +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> +	skb_put(skb, xdp->data_end - xdp->data);
> +	if (metasize)
> +		skb_metadata_set(skb, metasize);

[Severity: Medium]
Now that the regular RX path sets metadata, does the software VLAN
stripping later in stmmac_rx() corrupt it?

When priv->hw->hw_vlan_en is false (GMAC 3.x and dwmac100 cores),
stmmac_rx() calls stmmac_rx_vlan(), which does this for a tagged frame:

    memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
    skb_pull(skb, VLAN_HLEN);

The metadata is not moved. eth_type_trans() then sets mac_header at the
pulled data, and skb_metadata_end() returns skb_mac_header(skb).

So data_meta as seen by TC/BPF would cover
[orig_data + 4 - metasize, orig_data + 4). The first 4 metadata bytes
are lost, and the last 4 bytes are the old start of the destination MAC.

skb_reorder_vlan_header() and the skb_postpull_data_move()/skb_data_move()
helpers move the metadata together with the data. Should
stmmac_rx_vlan() do the same?

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. The
AF_XDP zero-copy counterpart, stmmac_construct_skb_zc(), still sets the
metadata length without copying the metadata:

    skb = napi_alloc_skb(&ch->rxtx_napi,
                         xdp->data_end - xdp->data_hard_start);
    ...
    skb_reserve(skb, xdp->data - xdp->data_hard_start);
    memcpy(__skb_put(skb, datasize), xdp->data, datasize);
    if (metasize)
        skb_metadata_set(skb, metasize);

The bytes in front of skb->data in the new skb are uninitialized
headroom, not what the program wrote with bpf_xdp_adjust_meta().
xdp_build_skb_from_zc() in net/core/xdp.c copies from xdp->data_meta
before it sets the metadata length.

Should the ZC XDP_PASS path copy the metadata too?

[ ... ]

> @@ -5900,19 +6019,11 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>  				rx_errors++;
>  		}
>  

[Severity: Medium]
This isn't a bug introduced by this patch, but the status used here
comes from dwmac4_wrback_get_rx_status(). That function loads the other
descriptor words before the ownership word and has no barrier after the
OWN check:

    u32 rdes1 = le32_to_cpu(p->des1);
    u32 rdes2 = le32_to_cpu(p->des2);
    u32 rdes3 = le32_to_cpu(p->des3);
    ...
    if (unlikely(rdes3 & RDES3_OWN))
        return dma_own;

If the DMA write-back lands between these loads, can a descriptor with
OWN already clear be paired with stale read-format RDES1/RDES2 contents?

That could give wrong checksum-error bits (and CHECKSUM_UNNECESSARY), a
spurious discard_frame from the filter-fail bits, or wrong PTP stats.
The XGMAC descriptor ops use dma_rmb() for this.

Would reading RDES3 first, followed by dma_rmb(), be needed here?

[ ... ]

> @@ -5922,121 +6033,89 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)

[ ... ]

> -		if (!skb) {
> -			unsigned int head_pad_len;
> -
> -			/* XDP program may expand or reduce tail */
> -			buf1_len = ctx.xdp.data_end - ctx.xdp.data;
> -
> -			skb = napi_build_skb(page_address(buf->page),
> -					     rx_q->napi_skb_frag_size);
> -			if (!skb) {
> -				page_pool_recycle_direct(rx_q->page_pool,
> -							 buf->page);
> -				buf->page = NULL;
> -				rx_dropped++;
> -				count++;
> -				goto drain_data;
> -			}

[Severity: High]
This is a pre-existing issue, not one introduced by this patch, but this
hunk also removes an existing crash path that the commit message does
not mention.

Without XDP, the old code uses skb == NULL as the only start-of-frame
marker. If napi_build_skb() fails on the first descriptor of a
multi-descriptor frame, the code reaches drain_data with skb still NULL
and error still 0:

    drain_data:
        if (likely(status & rx_not_ls))
            goto read_again;

The next continuation descriptor then goes through the if (!skb)
head-building branch. With no program attached, stmmac_xdp_run_prog()
returns ERR_PTR(0), which is NULL. A new skb is then built from
mid-frame payload.

With SPH active, stmmac_rx_buf1_len() returns 0 for non-first
descriptors:

    if (priv->sph_active && len)
        return 0;

So the rebuilt skb has an empty linear area and all of its data in
frags. pskb_trim(skb, len) does nothing because len > skb->len. Can
that then reach:

stmmac_rx()
  eth_type_trans()
    eth_skb_pull_mac()
      __skb_pull()
        BUG()    /* skb->len < skb->data_len */

Without SPH, a piece of mid-frame payload is passed up as a standalone
Ethernet frame.

The new code builds the skb only at the last descriptor and sends a
stmmac_build_skb() failure to error_free_frag, so the problem is gone
after this patch. "net: stmmac: drop multi-buffer frames in XDP mode"
only covers XDP mode, and this rework has no Fixes tag. Backports that
take only the Fixes-tagged patches would keep the crash.

Should the napi_build_skb() failure path be fixed separately for net,
for example by setting error so the rest of the frame is drained? Should
this commit message also mention it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-rx-mb-v6-0-2225050915c1%40oss.qualcomm.com