[PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Paolo Valerio]

Tested on Raspberry Pi 5.
All the changes are intended for gem only.

The series consists of two main changes:

- Migration from netdev_alloc_skb() to page pool allocation model,
  enabling skb recycling.
  This also adds support for multi-descriptor frame reception,
  removing the previous single-descriptor approach and avoiding
  potentially large contiguous allocations for e.g. jumbo frames
  with CONFIG_PAGE_SIZE_4KB.

- XDP support: Initial XDP implementation supporting major
  verdicts (XDP_PASS, XDP_DROP, XDP_REDIRECT, XDP_TX) along with
  the ndo_xdp_xmit function for packet redirection.

The driver now advertises NETDEV_XDP_ACT_BASIC, NETDEV_XDP_ACT_REDIRECT,
NETDEV_XDP_ACT_NDO_XMIT capabilities.

Previous versions
=================
  - RFC v1: https://lore.kernel.org/netdev/20251119135330.551835-1-pvalerio@redhat.com/
  - RFC v2: https://lore.kernel.org/netdev/20251220235135.1078587-1-pvalerio@redhat.com/

RFC v2 -> v1
============
  - Removed bp->macbgem_ops.mog_init_rings(bp) call from macb_open()
  - Fixed includes (remove unneeded, moved one from header to macb_main.c)
  - Reverse xmas tree ordering (gem_rx, gem_rx_refill)
  - print_hex_dump_debug() instead of print_hex_dump()
  - Replaced rx frame length check with MACB_BIT(RX_EOF) for data_len
    calculation
  - Removed NET_IP_ALIGN handling in rx buffer size calculation
  - Updated debug format string to include rx_headroom and total size
  - Changed types to unsigned int in helper functions and variable
  - Removed unneeded line break

RFC v1 -> RFC v2
================
  - Squashed 1/6 and 2/6
  - Reworked rx_buffer_size computation. It no longer takes into
    accounts extra room.
  - A bunch of renaming (rx_offset => rx_headroom, removed MACB_MAX_PAD,
    MACB_PP_HEADROOM => XDP_PACKET_HEADROOM, data => ptr, xdp_q => xdp_rxq,
    macb_xdp() => gem_xdp(), macb_xdp_xmit() => gem_xdp_xmit())
  - Deduplicated buffer size computation in gem_xdp_valid_mtu()
    and gem_xdp_setup()
  - gem_xdp_setup() no longer close()/open()
  - Renaming from rx_skbuff to rx_buff is now got split in a separate commit
  - Open-coded gem_page_pool_get_buff()
  - Added missing rx_buff re-initialization in the error path during rx
  - Page pool creation failure now fails the device open
  - Moved xdp buff preparation inside gem_xdp_run()
  - Added missing rcu_access_pointer()
  - Turned return value in -EOPNOTSUPP for macb_xdp() on failure
  - moved tx_skb to tx_buff renaming to a separate commit
  - Removed some unneeded code and set MACB_TYPE_SKB for lp->rm9200_txq[desc].type as well
  - Replaced !!addr with a dedicated bool in macb_xdp_submit_frame()


Paolo Valerio (7):
  net: macb: rename rx_skbuff into rx_buff
  cadence: macb: Add page pool support handle multi-descriptor frame rx
  cadence: macb: use the current queue number for stats
  cadence: macb: add XDP support for gem
  cadence: macb: make macb_tx_skb generic
  cadence: macb: make tx path skb agnostic
  cadence: macb: introduce xmit support

Théo Lebrun (1):
  net: macb: move Rx buffers alloc from link up to open

 drivers/net/ethernet/cadence/Kconfig     |   1 +
 drivers/net/ethernet/cadence/macb.h      |  40 +-
 drivers/net/ethernet/cadence/macb_main.c | 827 +++++++++++++++++------
 3 files changed, 659 insertions(+), 209 deletions(-)

-- 
2.52.0

[PATCH net-next 1/8] net: macb: move Rx buffers alloc from link up to open

[Paolo Valerio]

From: Théo Lebrun <theo.lebrun@bootlin.com>

mog_alloc_rx_buffers(), getting called at open, does not do rx buffer
alloc on GEM. The bulk of the work is done by gem_rx_refill() filling
up all slots with valid buffers.

gem_rx_refill() is called at link up by
gem_init_rings() == bp->macbgem_ops.mog_init_rings().

Move operation to macb_open(), mostly to allow it to fail early and
loudly rather than init the device with Rx mostly broken.

About `bool fail_early`:
 - When called from macb_open(), ring init fails as soon as a queue
   cannot be refilled.
 - When called from macb_hresp_error_task(), we do our best to reinit
   the device: we still iterate over all queues and try refilling all
   even if a previous queue failed.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb.h      |  2 +-
 drivers/net/ethernet/cadence/macb_main.c | 40 +++++++++++++++++-------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 87414a2ddf6e..2cb65ec37d44 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1180,7 +1180,7 @@ struct macb_queue;
 struct macb_or_gem_ops {
 	int	(*mog_alloc_rx_buffers)(struct macb *bp);
 	void	(*mog_free_rx_buffers)(struct macb *bp);
-	void	(*mog_init_rings)(struct macb *bp);
+	int	(*mog_init_rings)(struct macb *bp, bool fail_early);
 	int	(*mog_rx)(struct macb_queue *queue, struct napi_struct *napi,
 			  int budget);
 };
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2d5f3eb09530..5947c2b44bb3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -705,8 +705,8 @@ static void macb_mac_link_up(struct phylink_config *config,
 		if (rx_pause)
 			ctrl |= MACB_BIT(PAE);
 
-		/* Initialize rings & buffers as clearing MACB_BIT(TE) in link down
-		 * cleared the pipeline and control registers.
+		/* Initialize buffer registers as clearing MACB_BIT(TE) in link
+		 * down cleared the pipeline and control registers.
 		 */
 		macb_init_buffers(bp);
 
@@ -1249,13 +1249,14 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	return packets;
 }
 
-static void gem_rx_refill(struct macb_queue *queue)
+static int gem_rx_refill(struct macb_queue *queue)
 {
 	unsigned int		entry;
 	struct sk_buff		*skb;
 	dma_addr_t		paddr;
 	struct macb *bp = queue->bp;
 	struct macb_dma_desc *desc;
+	int err = 0;
 
 	while (CIRC_SPACE(queue->rx_prepared_head, queue->rx_tail,
 			bp->rx_ring_size) > 0) {
@@ -1272,6 +1273,7 @@ static void gem_rx_refill(struct macb_queue *queue)
 			if (unlikely(!skb)) {
 				netdev_err(bp->dev,
 					   "Unable to allocate sk_buff\n");
+				err = -ENOMEM;
 				break;
 			}
 
@@ -1321,6 +1323,7 @@ static void gem_rx_refill(struct macb_queue *queue)
 
 	netdev_vdbg(bp->dev, "rx ring: queue: %p, prepared head %d, tail %d\n",
 			queue, queue->rx_prepared_head, queue->rx_tail);
+	return err;
 }
 
 /* Mark DMA descriptors from begin up to and not including end as unused */
@@ -1773,7 +1776,7 @@ static void macb_hresp_error_task(struct work_struct *work)
 	netif_tx_stop_all_queues(dev);
 	netif_carrier_off(dev);
 
-	bp->macbgem_ops.mog_init_rings(bp);
+	bp->macbgem_ops.mog_init_rings(bp, false);
 
 	/* Initialize TX and RX buffers */
 	macb_init_buffers(bp);
@@ -2546,8 +2549,6 @@ static int macb_alloc_consistent(struct macb *bp)
 		if (!queue->tx_skb)
 			goto out_err;
 	}
-	if (bp->macbgem_ops.mog_alloc_rx_buffers(bp))
-		goto out_err;
 
 	/* Required for tie off descriptor for PM cases */
 	if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE)) {
@@ -2559,6 +2560,11 @@ static int macb_alloc_consistent(struct macb *bp)
 			goto out_err;
 	}
 
+	if (bp->macbgem_ops.mog_alloc_rx_buffers(bp))
+		goto out_err;
+	if (bp->macbgem_ops.mog_init_rings(bp, true))
+		goto out_err;
+
 	return 0;
 
 out_err:
@@ -2579,11 +2585,13 @@ static void macb_init_tieoff(struct macb *bp)
 	desc->ctrl = 0;
 }
 
-static void gem_init_rings(struct macb *bp)
+static int gem_init_rings(struct macb *bp, bool fail_early)
 {
 	struct macb_queue *queue;
 	struct macb_dma_desc *desc = NULL;
+	int last_err = 0;
 	unsigned int q;
+	int err;
 	int i;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
@@ -2599,13 +2607,24 @@ static void gem_init_rings(struct macb *bp)
 		queue->rx_tail = 0;
 		queue->rx_prepared_head = 0;
 
-		gem_rx_refill(queue);
+		/* We get called in two cases:
+		 *  - open: we can propagate alloc errors (so fail early),
+		 *  - HRESP error: cannot propagate, we attempt to reinit
+		 *    all queues in case of failure.
+		 */
+		err = gem_rx_refill(queue);
+		if (err) {
+			last_err = err;
+			if (fail_early)
+				break;
+		}
 	}
 
 	macb_init_tieoff(bp);
+	return last_err;
 }
 
-static void macb_init_rings(struct macb *bp)
+static int macb_init_rings(struct macb *bp, bool fail_early)
 {
 	int i;
 	struct macb_dma_desc *desc = NULL;
@@ -2622,6 +2641,7 @@ static void macb_init_rings(struct macb *bp)
 	desc->ctrl |= MACB_BIT(TX_WRAP);
 
 	macb_init_tieoff(bp);
+	return 0;
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -2953,8 +2973,6 @@ static int macb_open(struct net_device *dev)
 		goto pm_exit;
 	}
 
-	bp->macbgem_ops.mog_init_rings(bp);
-
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		napi_enable(&queue->napi_rx);
 		napi_enable(&queue->napi_tx);
-- 
2.52.0

[PATCH net-next 2/8] net: macb: rename rx_skbuff into rx_buff

[Paolo Valerio]

This is a preparation commit as the field in later patches will no longer
accomomdate skbuffs, but more generic frames.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb.h      |  2 +-
 drivers/net/ethernet/cadence/macb_main.c | 24 ++++++++++++------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 2cb65ec37d44..3b184e9ac771 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1262,7 +1262,7 @@ struct macb_queue {
 	unsigned int		rx_tail;
 	unsigned int		rx_prepared_head;
 	struct macb_dma_desc	*rx_ring;
-	struct sk_buff		**rx_skbuff;
+	void			**rx_buff;
 	void			*rx_buffers;
 	struct napi_struct	napi_rx;
 	struct queue_stats stats;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 5947c2b44bb3..19782f3f46f2 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1267,7 +1267,7 @@ static int gem_rx_refill(struct macb_queue *queue)
 
 		desc = macb_rx_desc(queue, entry);
 
-		if (!queue->rx_skbuff[entry]) {
+		if (!queue->rx_buff[entry]) {
 			/* allocate sk_buff for this free entry in ring */
 			skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size);
 			if (unlikely(!skb)) {
@@ -1286,7 +1286,7 @@ static int gem_rx_refill(struct macb_queue *queue)
 				break;
 			}
 
-			queue->rx_skbuff[entry] = skb;
+			queue->rx_buff[entry] = skb;
 
 			if (entry == bp->rx_ring_size - 1)
 				paddr |= MACB_BIT(RX_WRAP);
@@ -1389,7 +1389,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			queue->stats.rx_dropped++;
 			break;
 		}
-		skb = queue->rx_skbuff[entry];
+		skb = queue->rx_buff[entry];
 		if (unlikely(!skb)) {
 			netdev_err(bp->dev,
 				   "inconsistent Rx descriptor chain\n");
@@ -1398,7 +1398,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			break;
 		}
 		/* now everything is ready for receiving packet */
-		queue->rx_skbuff[entry] = NULL;
+		queue->rx_buff[entry] = NULL;
 		len = ctrl & bp->rx_frm_len_mask;
 
 		netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
@@ -2397,11 +2397,11 @@ static void gem_free_rx_buffers(struct macb *bp)
 	int i;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-		if (!queue->rx_skbuff)
+		if (!queue->rx_buff)
 			continue;
 
 		for (i = 0; i < bp->rx_ring_size; i++) {
-			skb = queue->rx_skbuff[i];
+			skb = queue->rx_buff[i];
 
 			if (!skb)
 				continue;
@@ -2415,8 +2415,8 @@ static void gem_free_rx_buffers(struct macb *bp)
 			skb = NULL;
 		}
 
-		kfree(queue->rx_skbuff);
-		queue->rx_skbuff = NULL;
+		kfree(queue->rx_buff);
+		queue->rx_buff = NULL;
 	}
 }
 
@@ -2479,13 +2479,13 @@ static int gem_alloc_rx_buffers(struct macb *bp)
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		size = bp->rx_ring_size * sizeof(struct sk_buff *);
-		queue->rx_skbuff = kzalloc(size, GFP_KERNEL);
-		if (!queue->rx_skbuff)
+		queue->rx_buff = kzalloc(size, GFP_KERNEL);
+		if (!queue->rx_buff)
 			return -ENOMEM;
 		else
 			netdev_dbg(bp->dev,
-				   "Allocated %d RX struct sk_buff entries at %p\n",
-				   bp->rx_ring_size, queue->rx_skbuff);
+				   "Allocated %d RX buff entries at %p\n",
+				   bp->rx_ring_size, queue->rx_buff);
 	}
 	return 0;
 }
-- 
2.52.0

[PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

Use the page pool allocator for the data buffers and enable skb recycling
support, instead of relying on netdev_alloc_skb allocating the entire skb
during the refill.

The patch also add support for receiving network frames that span multiple
DMA descriptors in the Cadence MACB/GEM Ethernet driver.

The patch removes the requirement that limited frame reception to
a single descriptor (RX_SOF && RX_EOF), also avoiding potential
contiguous multi-page allocation for large frames.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/Kconfig     |   1 +
 drivers/net/ethernet/cadence/macb.h      |   4 +
 drivers/net/ethernet/cadence/macb_main.c | 361 +++++++++++++++--------
 3 files changed, 240 insertions(+), 126 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 5b2a461dfd28..ae500f717433 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -25,6 +25,7 @@ config MACB
 	depends on PTP_1588_CLOCK_OPTIONAL
 	select PHYLINK
 	select CRC32
+	select PAGE_POOL
 	help
 	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
 	  AT91 parts.  This driver also supports the Cadence GEM (Gigabit
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 3b184e9ac771..eb775e576646 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -14,6 +14,7 @@
 #include <linux/interrupt.h>
 #include <linux/phy/phy.h>
 #include <linux/workqueue.h>
+#include <net/page_pool/helpers.h>
 
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
@@ -1266,6 +1267,8 @@ struct macb_queue {
 	void			*rx_buffers;
 	struct napi_struct	napi_rx;
 	struct queue_stats stats;
+	struct page_pool	*page_pool;
+	struct sk_buff		*skb;
 };
 
 struct ethtool_rx_fs_item {
@@ -1289,6 +1292,7 @@ struct macb {
 	struct macb_dma_desc	*rx_ring_tieoff;
 	dma_addr_t		rx_ring_tieoff_dma;
 	size_t			rx_buffer_size;
+	size_t			rx_headroom;
 
 	unsigned int		rx_ring_size;
 	unsigned int		tx_ring_size;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 19782f3f46f2..464bb7b2c04d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1249,14 +1249,22 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	return packets;
 }
 
-static int gem_rx_refill(struct macb_queue *queue)
+static unsigned int gem_total_rx_buffer_size(struct macb *bp)
+{
+	return SKB_HEAD_ALIGN(bp->rx_buffer_size + bp->rx_headroom);
+}
+
+static int gem_rx_refill(struct macb_queue *queue, bool napi)
 {
-	unsigned int		entry;
-	struct sk_buff		*skb;
-	dma_addr_t		paddr;
 	struct macb *bp = queue->bp;
 	struct macb_dma_desc *desc;
+	unsigned int entry;
+	struct page *page;
+	dma_addr_t paddr;
+	gfp_t gfp_alloc;
 	int err = 0;
+	void *data;
+	int offset;
 
 	while (CIRC_SPACE(queue->rx_prepared_head, queue->rx_tail,
 			bp->rx_ring_size) > 0) {
@@ -1268,25 +1276,20 @@ static int gem_rx_refill(struct macb_queue *queue)
 		desc = macb_rx_desc(queue, entry);
 
 		if (!queue->rx_buff[entry]) {
-			/* allocate sk_buff for this free entry in ring */
-			skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size);
-			if (unlikely(!skb)) {
+			gfp_alloc = napi ? GFP_ATOMIC : GFP_KERNEL;
+			page = page_pool_alloc_frag(queue->page_pool, &offset,
+						    gem_total_rx_buffer_size(bp),
+						    gfp_alloc | __GFP_NOWARN);
+			if (!page) {
 				netdev_err(bp->dev,
-					   "Unable to allocate sk_buff\n");
+					   "Unable to allocate page\n");
 				err = -ENOMEM;
 				break;
 			}
 
-			/* now fill corresponding descriptor entry */
-			paddr = dma_map_single(&bp->pdev->dev, skb->data,
-					       bp->rx_buffer_size,
-					       DMA_FROM_DEVICE);
-			if (dma_mapping_error(&bp->pdev->dev, paddr)) {
-				dev_kfree_skb(skb);
-				break;
-			}
-
-			queue->rx_buff[entry] = skb;
+			paddr = page_pool_get_dma_addr(page) + XDP_PACKET_HEADROOM + offset;
+			data = page_address(page) + offset;
+			queue->rx_buff[entry] = data;
 
 			if (entry == bp->rx_ring_size - 1)
 				paddr |= MACB_BIT(RX_WRAP);
@@ -1296,20 +1299,6 @@ static int gem_rx_refill(struct macb_queue *queue)
 			 */
 			dma_wmb();
 			macb_set_addr(bp, desc, paddr);
-
-			/* Properly align Ethernet header.
-			 *
-			 * Hardware can add dummy bytes if asked using the RBOF
-			 * field inside the NCFGR register. That feature isn't
-			 * available if hardware is RSC capable.
-			 *
-			 * We cannot fallback to doing the 2-byte shift before
-			 * DMA mapping because the address field does not allow
-			 * setting the low 2/3 bits.
-			 * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
-			 */
-			if (!(bp->caps & MACB_CAPS_RSC))
-				skb_reserve(skb, NET_IP_ALIGN);
 		} else {
 			desc->ctrl = 0;
 			dma_wmb();
@@ -1350,17 +1339,21 @@ static void discard_partial_frame(struct macb_queue *queue, unsigned int begin,
 static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		  int budget)
 {
+	struct skb_shared_info *shinfo;
 	struct macb *bp = queue->bp;
-	unsigned int		len;
-	unsigned int		entry;
-	struct sk_buff		*skb;
-	struct macb_dma_desc	*desc;
-	int			count = 0;
+	struct macb_dma_desc *desc;
+	unsigned int entry;
+	struct page *page;
+	void *buff_head;
+	int count = 0;
+	int data_len;
+	int nr_frags;
+
 
 	while (count < budget) {
-		u32 ctrl;
+		bool rxused, first_frame, last_frame;
 		dma_addr_t addr;
-		bool rxused;
+		u32 ctrl;
 
 		entry = macb_rx_ring_wrap(bp, queue->rx_tail);
 		desc = macb_rx_desc(queue, entry);
@@ -1374,6 +1367,12 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		if (!rxused)
 			break;
 
+		if (!(bp->caps & MACB_CAPS_RSC))
+			addr += NET_IP_ALIGN;
+
+		dma_sync_single_for_cpu(&bp->pdev->dev,
+					addr, bp->rx_buffer_size,
+					page_pool_get_dma_dir(queue->page_pool));
 		/* Ensure ctrl is at least as up-to-date as rxused */
 		dma_rmb();
 
@@ -1382,58 +1381,117 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		queue->rx_tail++;
 		count++;
 
-		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
-			netdev_err(bp->dev,
-				   "not whole frame pointed by descriptor\n");
-			bp->dev->stats.rx_dropped++;
-			queue->stats.rx_dropped++;
-			break;
-		}
-		skb = queue->rx_buff[entry];
-		if (unlikely(!skb)) {
+		buff_head = queue->rx_buff[entry];
+		if (unlikely(!buff_head)) {
 			netdev_err(bp->dev,
 				   "inconsistent Rx descriptor chain\n");
 			bp->dev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
 		}
-		/* now everything is ready for receiving packet */
-		queue->rx_buff[entry] = NULL;
-		len = ctrl & bp->rx_frm_len_mask;
 
-		netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
+		first_frame = ctrl & MACB_BIT(RX_SOF);
+		last_frame = ctrl & MACB_BIT(RX_EOF);
+
+		if (last_frame) {
+			data_len = ctrl & bp->rx_frm_len_mask;
+			if (!first_frame)
+				data_len -= queue->skb->len;
+		} else {
+			data_len = bp->rx_buffer_size;
+		}
+
+		if (first_frame) {
+			queue->skb = napi_build_skb(buff_head, gem_total_rx_buffer_size(bp));
+			if (unlikely(!queue->skb)) {
+				netdev_err(bp->dev,
+					   "Unable to allocate sk_buff\n");
+				goto free_frags;
+			}
+
+			/* Properly align Ethernet header.
+			 *
+			 * Hardware can add dummy bytes if asked using the RBOF
+			 * field inside the NCFGR register. That feature isn't
+			 * available if hardware is RSC capable.
+			 *
+			 * We cannot fallback to doing the 2-byte shift before
+			 * DMA mapping because the address field does not allow
+			 * setting the low 2/3 bits.
+			 * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
+			 */
+			skb_reserve(queue->skb, bp->rx_headroom);
+			skb_mark_for_recycle(queue->skb);
+			skb_put(queue->skb, data_len);
+			queue->skb->protocol = eth_type_trans(queue->skb, bp->dev);
+
+			skb_checksum_none_assert(queue->skb);
+			if (bp->dev->features & NETIF_F_RXCSUM &&
+			    !(bp->dev->flags & IFF_PROMISC) &&
+			    GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
+				queue->skb->ip_summed = CHECKSUM_UNNECESSARY;
+		} else {
+			if (!queue->skb) {
+				netdev_err(bp->dev,
+					   "Received non-starting frame while expecting it\n");
+				goto free_frags;
+			}
+
+			shinfo = skb_shinfo(queue->skb);
+			page = virt_to_head_page(buff_head);
+			nr_frags = shinfo->nr_frags;
 
-		skb_put(skb, len);
-		dma_unmap_single(&bp->pdev->dev, addr,
-				 bp->rx_buffer_size, DMA_FROM_DEVICE);
+			if (nr_frags >= ARRAY_SIZE(shinfo->frags))
+				goto free_frags;
 
-		skb->protocol = eth_type_trans(skb, bp->dev);
-		skb_checksum_none_assert(skb);
-		if (bp->dev->features & NETIF_F_RXCSUM &&
-		    !(bp->dev->flags & IFF_PROMISC) &&
-		    GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
+			skb_add_rx_frag(queue->skb, nr_frags, page,
+					buff_head - page_address(page) + bp->rx_headroom,
+					data_len, gem_total_rx_buffer_size(bp));
+		}
+
+		/* now everything is ready for receiving packet */
+		queue->rx_buff[entry] = NULL;
 
-		bp->dev->stats.rx_packets++;
-		queue->stats.rx_packets++;
-		bp->dev->stats.rx_bytes += skb->len;
-		queue->stats.rx_bytes += skb->len;
+		netdev_vdbg(bp->dev, "%s %u (len %u)\n", __func__, entry, data_len);
 
-		gem_ptp_do_rxstamp(bp, skb, desc);
+		if (last_frame) {
+			bp->dev->stats.rx_packets++;
+			queue->stats.rx_packets++;
+			bp->dev->stats.rx_bytes += queue->skb->len;
+			queue->stats.rx_bytes += queue->skb->len;
 
+			gem_ptp_do_rxstamp(bp, queue->skb, desc);
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
-		netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
-			    skb->len, skb->csum);
-		print_hex_dump(KERN_DEBUG, " mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
-			       skb_mac_header(skb), 16, true);
-		print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_ADDRESS, 16, 1,
-			       skb->data, 32, true);
+			netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
+				    queue->skb->len, queue->skb->csum);
+			print_hex_dump_debug(" mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
+					     skb_mac_header(queue->skb), 16, true);
+			print_hex_dump_debug("buff_head: ", DUMP_PREFIX_ADDRESS, 16, 1,
+					     buff_head, 32, true);
 #endif
 
-		napi_gro_receive(napi, skb);
+			napi_gro_receive(napi, queue->skb);
+			queue->skb = NULL;
+		}
+
+		continue;
+
+free_frags:
+		if (queue->skb) {
+			dev_kfree_skb(queue->skb);
+			queue->skb = NULL;
+		} else {
+			page_pool_put_full_page(queue->page_pool,
+						virt_to_head_page(buff_head),
+						false);
+		}
+
+		bp->dev->stats.rx_dropped++;
+		queue->stats.rx_dropped++;
+		queue->rx_buff[entry] = NULL;
 	}
 
-	gem_rx_refill(queue);
+	gem_rx_refill(queue, true);
 
 	return count;
 }
@@ -2367,12 +2425,22 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return ret;
 }
 
-static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
+static void macb_init_rx_buffer_size(struct macb *bp, unsigned int mtu)
 {
+	unsigned int overhead;
+	size_t size;
+
 	if (!macb_is_gem(bp)) {
 		bp->rx_buffer_size = MACB_RX_BUFFER_SIZE;
 	} else {
-		bp->rx_buffer_size = size;
+		size = mtu + ETH_HLEN + ETH_FCS_LEN;
+		bp->rx_buffer_size = SKB_DATA_ALIGN(size);
+		if (gem_total_rx_buffer_size(bp) > PAGE_SIZE) {
+			overhead = bp->rx_headroom +
+				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+			bp->rx_buffer_size = rounddown(PAGE_SIZE - overhead,
+						       RX_BUFFER_MULTIPLE);
+		}
 
 		if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
 			netdev_dbg(bp->dev,
@@ -2383,17 +2451,16 @@ static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
 		}
 	}
 
-	netdev_dbg(bp->dev, "mtu [%u] rx_buffer_size [%zu]\n",
-		   bp->dev->mtu, bp->rx_buffer_size);
+	netdev_dbg(bp->dev, "mtu [%u] rx_buffer_size [%zu] rx_headroom [%zu] total [%u]\n",
+		   bp->dev->mtu, bp->rx_buffer_size, bp->rx_headroom,
+		   gem_total_rx_buffer_size(bp));
 }
 
 static void gem_free_rx_buffers(struct macb *bp)
 {
-	struct sk_buff		*skb;
-	struct macb_dma_desc	*desc;
 	struct macb_queue *queue;
-	dma_addr_t		addr;
 	unsigned int q;
+	void *data;
 	int i;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
@@ -2401,22 +2468,20 @@ static void gem_free_rx_buffers(struct macb *bp)
 			continue;
 
 		for (i = 0; i < bp->rx_ring_size; i++) {
-			skb = queue->rx_buff[i];
-
-			if (!skb)
+			data = queue->rx_buff[i];
+			if (!data)
 				continue;
 
-			desc = macb_rx_desc(queue, i);
-			addr = macb_get_addr(bp, desc);
-
-			dma_unmap_single(&bp->pdev->dev, addr, bp->rx_buffer_size,
-					DMA_FROM_DEVICE);
-			dev_kfree_skb_any(skb);
-			skb = NULL;
+			page_pool_put_full_page(queue->page_pool,
+						virt_to_head_page(data),
+						false);
+			queue->rx_buff[i] = NULL;
 		}
 
 		kfree(queue->rx_buff);
 		queue->rx_buff = NULL;
+		page_pool_destroy(queue->page_pool);
+		queue->page_pool = NULL;
 	}
 }
 
@@ -2478,13 +2543,12 @@ static int gem_alloc_rx_buffers(struct macb *bp)
 	int size;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-		size = bp->rx_ring_size * sizeof(struct sk_buff *);
+		size = bp->rx_ring_size * sizeof(*queue->rx_buff);
 		queue->rx_buff = kzalloc(size, GFP_KERNEL);
 		if (!queue->rx_buff)
 			return -ENOMEM;
 		else
-			netdev_dbg(bp->dev,
-				   "Allocated %d RX buff entries at %p\n",
+			netdev_dbg(bp->dev, "Allocated %d RX buff entries at %p\n",
 				   bp->rx_ring_size, queue->rx_buff);
 	}
 	return 0;
@@ -2572,6 +2636,33 @@ static int macb_alloc_consistent(struct macb *bp)
 	return -ENOMEM;
 }
 
+static int gem_create_page_pool(struct macb_queue *queue)
+{
+	struct page_pool_params pp_params = {
+		.order = 0,
+		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+		.pool_size = queue->bp->rx_ring_size,
+		.nid = NUMA_NO_NODE,
+		.dma_dir = DMA_FROM_DEVICE,
+		.dev = &queue->bp->pdev->dev,
+		.napi = &queue->napi_rx,
+		.max_len = PAGE_SIZE,
+	};
+	struct page_pool *pool;
+	int err = 0;
+
+	pool = page_pool_create(&pp_params);
+	if (IS_ERR(pool)) {
+		netdev_err(queue->bp->dev, "cannot create rx page pool\n");
+		err = PTR_ERR(pool);
+		pool = NULL;
+	}
+
+	queue->page_pool = pool;
+
+	return err;
+}
+
 static void macb_init_tieoff(struct macb *bp)
 {
 	struct macb_dma_desc *desc = bp->rx_ring_tieoff;
@@ -2607,12 +2698,24 @@ static int gem_init_rings(struct macb *bp, bool fail_early)
 		queue->rx_tail = 0;
 		queue->rx_prepared_head = 0;
 
+		/* This is a hard failure, so the best we can do is try the
+		 * next queue in case of HRESP error.
+		 */
+		err = gem_create_page_pool(queue);
+		if (err) {
+			last_err = err;
+			if (fail_early)
+				break;
+
+			continue;
+		}
+
 		/* We get called in two cases:
 		 *  - open: we can propagate alloc errors (so fail early),
 		 *  - HRESP error: cannot propagate, we attempt to reinit
 		 *    all queues in case of failure.
 		 */
-		err = gem_rx_refill(queue);
+		err = gem_rx_refill(queue, false);
 		if (err) {
 			last_err = err;
 			if (fail_early)
@@ -2756,39 +2859,40 @@ static void macb_configure_dma(struct macb *bp)
 	unsigned int q;
 	u32 dmacfg;
 
-	buffer_size = bp->rx_buffer_size / RX_BUFFER_MULTIPLE;
-	if (macb_is_gem(bp)) {
-		dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
-		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-			if (q)
-				queue_writel(queue, RBQS, buffer_size);
-			else
-				dmacfg |= GEM_BF(RXBS, buffer_size);
-		}
-		if (bp->dma_burst_length)
-			dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
-		dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
-		dmacfg &= ~GEM_BIT(ENDIA_PKT);
+	if (!macb_is_gem((bp)))
+		return;
 
-		if (bp->native_io)
-			dmacfg &= ~GEM_BIT(ENDIA_DESC);
+	buffer_size = bp->rx_buffer_size / RX_BUFFER_MULTIPLE;
+	dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
+	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+		if (q)
+			queue_writel(queue, RBQS, buffer_size);
 		else
-			dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
+			dmacfg |= GEM_BF(RXBS, buffer_size);
+	}
+	if (bp->dma_burst_length)
+		dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
+	dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
+	dmacfg &= ~GEM_BIT(ENDIA_PKT);
 
-		if (bp->dev->features & NETIF_F_HW_CSUM)
-			dmacfg |= GEM_BIT(TXCOEN);
-		else
-			dmacfg &= ~GEM_BIT(TXCOEN);
+	if (bp->native_io)
+		dmacfg &= ~GEM_BIT(ENDIA_DESC);
+	else
+		dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
 
-		dmacfg &= ~GEM_BIT(ADDR64);
-		if (macb_dma64(bp))
-			dmacfg |= GEM_BIT(ADDR64);
-		if (macb_dma_ptp(bp))
-			dmacfg |= GEM_BIT(RXEXT) | GEM_BIT(TXEXT);
-		netdev_dbg(bp->dev, "Cadence configure DMA with 0x%08x\n",
-			   dmacfg);
-		gem_writel(bp, DMACFG, dmacfg);
-	}
+	if (bp->dev->features & NETIF_F_HW_CSUM)
+		dmacfg |= GEM_BIT(TXCOEN);
+	else
+		dmacfg &= ~GEM_BIT(TXCOEN);
+
+	dmacfg &= ~GEM_BIT(ADDR64);
+	if (macb_dma64(bp))
+		dmacfg |= GEM_BIT(ADDR64);
+	if (macb_dma_ptp(bp))
+		dmacfg |= GEM_BIT(RXEXT) | GEM_BIT(TXEXT);
+	netdev_dbg(bp->dev, "Cadence configure DMA with 0x%08x\n",
+		   dmacfg);
+	gem_writel(bp, DMACFG, dmacfg);
 }
 
 static void macb_init_hw(struct macb *bp)
@@ -2951,7 +3055,6 @@ static void macb_set_rx_mode(struct net_device *dev)
 
 static int macb_open(struct net_device *dev)
 {
-	size_t bufsz = dev->mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN;
 	struct macb *bp = netdev_priv(dev);
 	struct macb_queue *queue;
 	unsigned int q;
@@ -2964,7 +3067,7 @@ static int macb_open(struct net_device *dev)
 		return err;
 
 	/* RX buffers initialization */
-	macb_init_rx_buffer_size(bp, bufsz);
+	macb_init_rx_buffer_size(bp, dev->mtu);
 
 	err = macb_alloc_consistent(bp);
 	if (err) {
@@ -5625,6 +5728,12 @@ static int macb_probe(struct platform_device *pdev)
 	if (err)
 		goto err_out_phy_exit;
 
+	if (macb_is_gem(bp)) {
+		bp->rx_headroom = XDP_PACKET_HEADROOM;
+		if (!(bp->caps & MACB_CAPS_RSC))
+			bp->rx_headroom += NET_IP_ALIGN;
+	}
+
 	netif_carrier_off(dev);
 
 	err = register_netdev(dev);
-- 
2.52.0

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Andrew Lunn]

On Thu, Jan 15, 2026 at 11:25:26PM +0100, Paolo Valerio wrote:
> Use the page pool allocator for the data buffers and enable skb recycling
> support, instead of relying on netdev_alloc_skb allocating the entire skb
> during the refill.

Do you have any benchmark numbers for this change? Often swapping to
page pool improves the performance of the driver, and i use it as a
selling point for doing the conversion, independent of XDP.

Thanks
	Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

On 16 Jan 2026 at 06:16:16 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Jan 15, 2026 at 11:25:26PM +0100, Paolo Valerio wrote:
>> Use the page pool allocator for the data buffers and enable skb recycling
>> support, instead of relying on netdev_alloc_skb allocating the entire skb
>> during the refill.
>
> Do you have any benchmark numbers for this change? Often swapping to
> page pool improves the performance of the driver, and i use it as a
> selling point for doing the conversion, independent of XDP.
>

Good point. I'll collect some numbers and add them here.

> Thanks
> 	Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

On 16 Jan 2026 at 06:16:16 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Jan 15, 2026 at 11:25:26PM +0100, Paolo Valerio wrote:
>> Use the page pool allocator for the data buffers and enable skb recycling
>> support, instead of relying on netdev_alloc_skb allocating the entire skb
>> during the refill.
>
> Do you have any benchmark numbers for this change? Often swapping to
> page pool improves the performance of the driver, and i use it as a
> selling point for doing the conversion, independent of XDP.
>

I finally got the chance to get my hands on the board.

On the rpi5 I simply run xdp-bench in skb-mode to drop and collect the
stats.

Page size is 4k and stats include the driver consuming a full page
setting mtu such that rx_buffer_size + overhead exceed half page and the
other way around for 2 fragments.

             |    64     |    128    |
  baseline   |  533,158  |  531,618  |
  pp page    |  530,929  |  529,682  |
  pp 2 frags |  530,781  |  529,116  |

> Thanks
> 	Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Andrew Lunn]

On Thu, Jan 22, 2026 at 11:24:05PM +0100, Paolo Valerio wrote:
> On 16 Jan 2026 at 06:16:16 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > On Thu, Jan 15, 2026 at 11:25:26PM +0100, Paolo Valerio wrote:
> >> Use the page pool allocator for the data buffers and enable skb recycling
> >> support, instead of relying on netdev_alloc_skb allocating the entire skb
> >> during the refill.
> >
> > Do you have any benchmark numbers for this change? Often swapping to
> > page pool improves the performance of the driver, and i use it as a
> > selling point for doing the conversion, independent of XDP.
> >
> 
> I finally got the chance to get my hands on the board.
> 
> On the rpi5 I simply run xdp-bench in skb-mode to drop and collect the
> stats.
> 
> Page size is 4k and stats include the driver consuming a full page
> setting mtu such that rx_buffer_size + overhead exceed half page and the
> other way around for 2 fragments.
> 
>              |    64     |    128    |
>   baseline   |  533,158  |  531,618  |
>   pp page    |  530,929  |  529,682  |
>   pp 2 frags |  530,781  |  529,116  |

I was more interested in plain networking, not XDP. Does it perform
better with page pool? You at least need to show it is not worse, you
need to avoid performance regressions.

     Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

On 23 Jan 2026 at 12:04:55 AM, Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Jan 22, 2026 at 11:24:05PM +0100, Paolo Valerio wrote:
>> On 16 Jan 2026 at 06:16:16 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> 
>> > On Thu, Jan 15, 2026 at 11:25:26PM +0100, Paolo Valerio wrote:
>> >> Use the page pool allocator for the data buffers and enable skb recycling
>> >> support, instead of relying on netdev_alloc_skb allocating the entire skb
>> >> during the refill.
>> >
>> > Do you have any benchmark numbers for this change? Often swapping to
>> > page pool improves the performance of the driver, and i use it as a
>> > selling point for doing the conversion, independent of XDP.
>> >
>> 
>> I finally got the chance to get my hands on the board.
>> 
>> On the rpi5 I simply run xdp-bench in skb-mode to drop and collect the
>> stats.
>> 
>> Page size is 4k and stats include the driver consuming a full page
>> setting mtu such that rx_buffer_size + overhead exceed half page and the
>> other way around for 2 fragments.
>> 
>>              |    64     |    128    |
>>   baseline   |  533,158  |  531,618  |
>>   pp page    |  530,929  |  529,682  |
>>   pp 2 frags |  530,781  |  529,116  |
>
> I was more interested in plain networking, not XDP. Does it perform
> better with page pool? You at least need to show it is not worse, you
> need to avoid performance regressions.
>

I retested with iperf3. The target has a single rx queue with iperf3
running with no cpu affinity set.

|              |  64 | 128 |
| baseline     | 273 | 545 |
| pp (page)    | 273 | 544 |
| pp (2 frags) | 272 | 544 |



>      Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Andrew Lunn]

> > I was more interested in plain networking, not XDP. Does it perform
> > better with page pool? You at least need to show it is not worse, you
> > need to avoid performance regressions.
> >
> 
> I retested with iperf3. The target has a single rx queue with iperf3
> running with no cpu affinity set.
> 
> |              |  64 | 128 |
> | baseline     | 273 | 545 |
> | pp (page)    | 273 | 544 |
> | pp (2 frags) | 272 | 544 |

So no real difference. That is unusual, it is typically faster, or if
it is always doing line rate, it uses less CPU time. That might
suggest the page pool integration is not optimal?

	Andrew

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Théo Lebrun]

On Mon Jan 26, 2026 at 3:29 PM CET, Andrew Lunn wrote:
>> > I was more interested in plain networking, not XDP. Does it perform
>> > better with page pool? You at least need to show it is not worse, you
>> > need to avoid performance regressions.
>> 
>> I retested with iperf3. The target has a single rx queue with iperf3
>> running with no cpu affinity set.
>> 
>> |              |  64 | 128 |
>> | baseline     | 273 | 545 |
>> | pp (page)    | 273 | 544 |
>> | pp (2 frags) | 272 | 544 |
>
> So no real difference. That is unusual, it is typically faster, or if
> it is always doing line rate, it uses less CPU time. That might
> suggest the page pool integration is not optimal?

One more data point. I get line rate with & without page_pool so below
are CPU times from /proc/stat:

         upstream    pp
user            1     1
system        179    91 (!!!)
idle         7874  7303
softirq        35    37

16K pages on Mobileye EyeQ5 (MIPS), 7 fragments per page.

Paolo shared 64 versus 128 measurements but I am unsure what those stand
for; I doubt it can be packet size as xdp-bench does not have it as a
parameter. https://man.archlinux.org/man/extra/xdp-tools/xdp-bench.8.en

Measurement incantation:

   cat /proc/stat > /tmp/a && \
   iperf3 -c $IP && \
   cat /proc/stat > /tmp/b && \
   awk 'NR==FNR && $1=="cpu" {user=$2;sys=$4;idle=$5;softirq=$8;next}
        $1=="cpu" {printf "user\t%5d\n", $2-user}
        $1=="cpu" {printf "system\t%5d\n", $4-sys}
        $1=="cpu" {printf "idle\t%5d\n", $5-idle}
        $1=="cpu" {printf "softirq\t%5d\n", $8-softirq}
   ' /tmp/a /tmp/b

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

On 26 Jan 2026 at 07:45:29 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> On Mon Jan 26, 2026 at 3:29 PM CET, Andrew Lunn wrote:
>>> > I was more interested in plain networking, not XDP. Does it perform
>>> > better with page pool? You at least need to show it is not worse, you
>>> > need to avoid performance regressions.
>>> 
>>> I retested with iperf3. The target has a single rx queue with iperf3
>>> running with no cpu affinity set.
>>> 
>>> |              |  64 | 128 |
>>> | baseline     | 273 | 545 |
>>> | pp (page)    | 273 | 544 |
>>> | pp (2 frags) | 272 | 544 |
>>
>> So no real difference. That is unusual, it is typically faster, or if
>> it is always doing line rate, it uses less CPU time. That might
>> suggest the page pool integration is not optimal?
>
> One more data point. I get line rate with & without page_pool so below
> are CPU times from /proc/stat:
>
>          upstream    pp
> user            1     1
> system        179    91 (!!!)
> idle         7874  7303
> softirq        35    37
>
> 16K pages on Mobileye EyeQ5 (MIPS), 7 fragments per page.
>
> Paolo shared 64 versus 128 measurements but I am unsure what those stand
> for; I doubt it can be packet size as xdp-bench does not have it as a
> parameter. https://man.archlinux.org/man/extra/xdp-tools/xdp-bench.8.en
>

64 and 128 are packet size in bytes.
For the first test I used xdp-trafficgen on the sender side and
xdp-bench (skb-mode) to count the drops in pps on my board.

For the stack test I used iperf3 (UDP) similarly with 64 and 128 for the
length option.

> Measurement incantation:
>
>    cat /proc/stat > /tmp/a && \
>    iperf3 -c $IP && \
>    cat /proc/stat > /tmp/b && \
>    awk 'NR==FNR && $1=="cpu" {user=$2;sys=$4;idle=$5;softirq=$8;next}
>         $1=="cpu" {printf "user\t%5d\n", $2-user}
>         $1=="cpu" {printf "system\t%5d\n", $4-sys}
>         $1=="cpu" {printf "idle\t%5d\n", $5-idle}
>         $1=="cpu" {printf "softirq\t%5d\n", $8-softirq}
>    ' /tmp/a /tmp/b
>
> Thanks,
>
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Théo Lebrun]

On Tue Jan 27, 2026 at 12:51 AM CET, Paolo Valerio wrote:
> On 26 Jan 2026 at 07:45:29 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>> On Mon Jan 26, 2026 at 3:29 PM CET, Andrew Lunn wrote:
>>>> > I was more interested in plain networking, not XDP. Does it perform
>>>> > better with page pool? You at least need to show it is not worse, you
>>>> > need to avoid performance regressions.
>>>> 
>>>> I retested with iperf3. The target has a single rx queue with iperf3
>>>> running with no cpu affinity set.
>>>> 
>>>> |              |  64 | 128 |
>>>> | baseline     | 273 | 545 |
>>>> | pp (page)    | 273 | 544 |
>>>> | pp (2 frags) | 272 | 544 |
>>>
>>> So no real difference. That is unusual, it is typically faster, or if
>>> it is always doing line rate, it uses less CPU time. That might
>>> suggest the page pool integration is not optimal?
>>
>> One more data point. I get line rate with & without page_pool so below
>> are CPU times from /proc/stat:
>>
>>          upstream    pp
>> user            1     1
>> system        179    91 (!!!)
>> idle         7874  7303
>> softirq        35    37
>>
>> 16K pages on Mobileye EyeQ5 (MIPS), 7 fragments per page.
>>
>> Paolo shared 64 versus 128 measurements but I am unsure what those stand
>> for; I doubt it can be packet size as xdp-bench does not have it as a
>> parameter. https://man.archlinux.org/man/extra/xdp-tools/xdp-bench.8.en
>
> 64 and 128 are packet size in bytes.
> For the first test I used xdp-trafficgen on the sender side and
> xdp-bench (skb-mode) to count the drops in pps on my board.
>
> For the stack test I used iperf3 (UDP) similarly with 64 and 128 for the
> length option.

|           |    64 | 128 |
| baseline  |  75.3 | 145 |
| page_pool |  79.3 | 154 |

Call is: iperf3 -c $IP -u -A2 -b1G -l64   # or -l128

CPU affinity is important. I get approx -30% on CPU0 (where IRQs land)
and -12% on CPU1 (2nd thread of CPU0).

I looked at ftrace function_graph and it looks sensible. Baseline spends
16% of macb_rx_poll() in gem_rx_refill(). This goes down to 10% with
page_pool.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Valerio]

On 26 Jan 2026 at 03:29:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:

>> > I was more interested in plain networking, not XDP. Does it perform
>> > better with page pool? You at least need to show it is not worse, you
>> > need to avoid performance regressions.
>> >
>> 
>> I retested with iperf3. The target has a single rx queue with iperf3
>> running with no cpu affinity set.
>> 
>> |              |  64 | 128 |
>> | baseline     | 273 | 545 |
>> | pp (page)    | 273 | 544 |
>> | pp (2 frags) | 272 | 544 |
>
> So no real difference. That is unusual, it is typically faster, or if
> it is always doing line rate, it uses less CPU time. That might
> suggest the page pool integration is not optimal?
>

Thanks Andrew,

I can't tell, but the same numbers were a little surprising to me as well.
I'll spend some more time on this on my hw.

Re: [net-next,3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
cadence: macb: Add page pool support handle multi-descriptor frame rx

This patch converts the MACB/GEM driver from netdev_alloc_skb() to
page pool allocation and adds support for receiving frames that span
multiple DMA descriptors.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 19782f3f46f2..464bb7b2c04d 100644

[ ... ]

> @@ -1350,17 +1339,21 @@ static void discard_partial_frame(struct macb_queue *queue, unsigned int begin,
>  static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
>  		  int budget)
>  {
> +	struct skb_shared_info *shinfo;
>  	struct macb *bp = queue->bp;
> -	unsigned int		len;
> -	unsigned int		entry;
> -	struct sk_buff		*skb;
> -	struct macb_dma_desc	*desc;
> -	int			count = 0;
> +	struct macb_dma_desc *desc;
> +	unsigned int entry;
> +	struct page *page;
> +	void *buff_head;
> +	int count = 0;
> +	int data_len;
> +	int nr_frags;
> +
>
>  	while (count < budget) {
> -		u32 ctrl;
> +		bool rxused, first_frame, last_frame;
>  		dma_addr_t addr;
> -		bool rxused;
> +		u32 ctrl;

[ ... ]

> -		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
> -			netdev_err(bp->dev,
> -				   "not whole frame pointed by descriptor\n");
> -			bp->dev->stats.rx_dropped++;
> -			queue->stats.rx_dropped++;
> -			break;
> -		}
> -		skb = queue->rx_buff[entry];
> -		if (unlikely(!skb)) {
> +		buff_head = queue->rx_buff[entry];
> +		if (unlikely(!buff_head)) {
>  			netdev_err(bp->dev,
>  				   "inconsistent Rx descriptor chain\n");
>  			bp->dev->stats.rx_dropped++;
>  			queue->stats.rx_dropped++;
>  			break;
>  		}
> -		/* now everything is ready for receiving packet */
> -		queue->rx_buff[entry] = NULL;
> -		len = ctrl & bp->rx_frm_len_mask;
>
> -		netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
> +		first_frame = ctrl & MACB_BIT(RX_SOF);
> +		last_frame = ctrl & MACB_BIT(RX_EOF);
> +
> +		if (last_frame) {
> +			data_len = ctrl & bp->rx_frm_len_mask;
> +			if (!first_frame)
> +				data_len -= queue->skb->len;
                                     ^^^^^^^^^^^^^^

If a descriptor arrives with RX_EOF set but RX_SOF clear (e.g., hardware
error or link reset mid-frame), and queue->skb is NULL from a previously
completed frame, this will dereference NULL. The check for queue->skb
being NULL at the "else" branch below happens after this calculation.
Should the data_len calculation be moved after the first_frame handling,
or should there be a NULL check on queue->skb before accessing its len?

> +		} else {
> +			data_len = bp->rx_buffer_size;
> +		}
> +
> +		if (first_frame) {

[ ... ]

> +		} else {
> +			if (!queue->skb) {
> +				netdev_err(bp->dev,
> +					   "Received non-starting frame while expecting it\n");
> +				goto free_frags;
> +			}
> +
> +			shinfo = skb_shinfo(queue->skb);
> +			page = virt_to_head_page(buff_head);
> +			nr_frags = shinfo->nr_frags;
> +
> +			if (nr_frags >= ARRAY_SIZE(shinfo->frags))
> +				goto free_frags;
> +
> +			skb_add_rx_frag(queue->skb, nr_frags, page,
> +					buff_head - page_address(page) + bp->rx_headroom,
> +					data_len, gem_total_rx_buffer_size(bp));
> +		}

[ ... ]

> +	free_frags:
> +		if (queue->skb) {
> +			dev_kfree_skb(queue->skb);
> +			queue->skb = NULL;
> +		} else {
> +			page_pool_put_full_page(queue->page_pool,
> +						virt_to_head_page(buff_head),
> +						false);
> +		}

When the fragment overflow check (nr_frags >= ARRAY_SIZE) triggers,
queue->skb is non-NULL so dev_kfree_skb() runs. However, the current
buff_head (the fragment that could not be added) is not returned to
the page pool since the else branch is not taken. Does this leak the
page for that buffer?

> +
> +		bp->dev->stats.rx_dropped++;
> +		queue->stats.rx_dropped++;
> +		queue->rx_buff[entry] = NULL;
>  	}

[ ... ]

> +static int gem_create_page_pool(struct macb_queue *queue)
> +{
> +	struct page_pool_params pp_params = {
> +		.order = 0,
> +		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
> +		.pool_size = queue->bp->rx_ring_size,
> +		.nid = NUMA_NO_NODE,
> +		.dma_dir = DMA_FROM_DEVICE,
> +		.dev = &queue->bp->pdev->dev,
> +		.napi = &queue->napi_rx,
> +		.max_len = PAGE_SIZE,
> +	};
> +	struct page_pool *pool;
> +	int err = 0;
> +
> +	pool = page_pool_create(&pp_params);
> +	if (IS_ERR(pool)) {
> +		netdev_err(queue->bp->dev, "cannot create rx page pool\n");
> +		err = PTR_ERR(pool);
> +		pool = NULL;
> +	}
> +
> +	queue->page_pool = pool;
> +
> +	return err;
> +}

[ ... ]

> @@ -2607,12 +2698,24 @@ static int gem_init_rings(struct macb *bp, bool fail_early)
>  		queue->rx_tail = 0;
>  		queue->rx_prepared_head = 0;
>
> +		/* This is a hard failure, so the best we can do is try the
> +		 * next queue in case of HRESP error.
> +		 */
> +		err = gem_create_page_pool(queue);
                      ^^^^^^^^^^^^^^^^^^^^^^^^

When macb_hresp_error_task() calls gem_init_rings(bp, false), this
creates a new page pool without destroying the existing one. The old
queue->page_pool pointer is overwritten, leaking the previous page pool.
Additionally, the rx_buff[] entries still reference pages from the old
pool. When those buffers are later processed in gem_rx() and returned
via page_pool_put_full_page(), they would be returned to the new pool
instead of their original pool. Should the old page pool be destroyed
and rx buffers cleaned up before creating the new pool in the HRESP
recovery path?

> +		if (err) {
> +			last_err = err;
> +			if (fail_early)
> +				break;
> +
> +			continue;
> +		}

[ ... ]

Re: [net-next,3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Théo Lebrun]

On Mon Jan 19, 2026 at 8:36 PM CET, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html

This page indicates the prompt is at `review-prompt/prompt.md` but the
repo never has had any `prompt.md` file. Guessing from
`kernel/skills/kernel.md` it seems to be `kernel/review-core.md`
instead.

Agreed with the LLM's comments.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [net-next,3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Jakub Kicinski]

On Thu, 22 Jan 2026 15:39:51 +0100 Théo Lebrun wrote:
> On Mon Jan 19, 2026 at 8:36 PM CET, Jakub Kicinski wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> >
> > For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html  
> 
> This page indicates the prompt is at `review-prompt/prompt.md` but the
> repo never has had any `prompt.md` file. Guessing from
> `kernel/skills/kernel.md` it seems to be `kernel/review-core.md`
> instead.

Thanks for pointing out, it got renamed yesterday evening.
Let me update the help page.

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Théo Lebrun]

Hi Paolo,

On Thu Jan 15, 2026 at 11:25 PM CET, Paolo Valerio wrote:
> Use the page pool allocator for the data buffers and enable skb recycling
> support, instead of relying on netdev_alloc_skb allocating the entire skb
> during the refill.
>
> The patch also add support for receiving network frames that span multiple
> DMA descriptors in the Cadence MACB/GEM Ethernet driver.
>
> The patch removes the requirement that limited frame reception to
> a single descriptor (RX_SOF && RX_EOF), also avoiding potential
> contiguous multi-page allocation for large frames.

[...]

> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -14,6 +14,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/phy/phy.h>
>  #include <linux/workqueue.h>
> +#include <net/page_pool/helpers.h>
>  
>  #define MACB_GREGS_NBR 16
>  #define MACB_GREGS_VERSION 2
> @@ -1266,6 +1267,8 @@ struct macb_queue {
>  	void			*rx_buffers;
>  	struct napi_struct	napi_rx;
>  	struct queue_stats stats;
> +	struct page_pool	*page_pool;
> +	struct sk_buff		*skb;

There might be a cleanup issue with the GRO queue->skb field.
It gets cleaned up by gem_rx() in
 - various error codepaths with the free_frags label, or,
 - the happy path where we hand off the skb with napi_gro_receive().

If we get closed in the middle of a transfer we have queue->skb that is
valid. It sounds like an issue if we shutdown (and leak the SKB) or if
we reopen (and have the old SKB laying around). I have not encountered
this on HW, but I haven't tried.

Do you agree? A job for gem_free_rx_buffers()?

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

[Théo Lebrun]

On Thu Jan 15, 2026 at 11:25 PM CET, Paolo Valerio wrote:
> @@ -1382,58 +1381,117 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
>  #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> -		netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> -			    skb->len, skb->csum);
> -		print_hex_dump(KERN_DEBUG, " mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
> -			       skb_mac_header(skb), 16, true);
> -		print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_ADDRESS, 16, 1,
> -			       skb->data, 32, true);
> +			netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> +				    queue->skb->len, queue->skb->csum);
> +			print_hex_dump_debug(" mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
> +					     skb_mac_header(queue->skb), 16, true);
> +			print_hex_dump_debug("buff_head: ", DUMP_PREFIX_ADDRESS, 16, 1,
> +					     buff_head, 32, true);
>  #endif

I think the print_hex_*() calls need some updating. The first call
prints the MAC header (which is sensible) but the second one prints the
first 32 bytes of buff_head, which points to the headroom.

I have been using those calls today to debug something on a new
platform, and I replaced it by:

#if defined(DEBUG) && defined(VERBOSE_DEBUG)
         BUG_ON(!first_frame);
         netdev_vdbg(bp->dev, "received skb of length %u (len %d), csum: %08x\n",
                queue->skb->len, data_len, queue->skb->csum);
         print_hex_dump_debug("buffer: ", DUMP_PREFIX_ADDRESS, 16, 1,
                    buff_head + bp->rx_headroom, data_len, true);
#endif

So I replaced it by a single call that prints the full buffer.
 - It prints packets in their entirety, in a single call. Being a single
   call is important because then with scripting it can be imported
   into wireshark. Yes this is ugly but tcpdump is too late for my
   debugging needs.
 - This does not work if an SKB spans >1 frame, which is why I put the
   BUG_ON() call. It is fine for my usecase but upstream might need
   something more resistant.

Here is one proposal for upstream:
 - We move the printk from last_frame to first_frame.
   That way the debugging code avoids dealing with fragmentation.
 - We print `buff_head + bp->rx_headroom` with a size of
   `min(±64, data_len)`, to avoid printing full packets.

I can deal with it if you want. If I do, you should probably drop the
print_hex_dump_debug() calls in your series because they are broken
as-is. Or you do it, as you want!

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH net-next 4/8] cadence: macb: use the current queue number for stats

[Paolo Valerio]

gem_get_ethtool_stats calculates the size of the statistics
data to copy always considering maximum number of queues.

The patch makes sure the statistics are copied only for the
active queues as returned in the string set count op.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 464bb7b2c04d..89f0c4dc3884 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3249,7 +3249,7 @@ static void gem_get_ethtool_stats(struct net_device *dev,
 	spin_lock_irq(&bp->stats_lock);
 	gem_update_stats(bp);
 	memcpy(data, &bp->ethtool_stats, sizeof(u64)
-			* (GEM_STATS_LEN + QUEUE_STATS_LEN * MACB_MAX_QUEUES));
+			* (GEM_STATS_LEN + QUEUE_STATS_LEN * bp->num_queues));
 	spin_unlock_irq(&bp->stats_lock);
 }
 
-- 
2.52.0

[PATCH net-next 5/8] cadence: macb: add XDP support for gem

[Paolo Valerio]

Introduce basic XDP support for macb/gem with the XDP_PASS,
XDP_DROP, XDP_REDIRECT verdict support.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb.h      |   3 +
 drivers/net/ethernet/cadence/macb_main.c | 178 +++++++++++++++++++++--
 2 files changed, 167 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index eb775e576646..d46d8523198d 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -15,6 +15,7 @@
 #include <linux/phy/phy.h>
 #include <linux/workqueue.h>
 #include <net/page_pool/helpers.h>
+#include <net/xdp.h>
 
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
@@ -1269,6 +1270,7 @@ struct macb_queue {
 	struct queue_stats stats;
 	struct page_pool	*page_pool;
 	struct sk_buff		*skb;
+	struct xdp_rxq_info	xdp_rxq;
 };
 
 struct ethtool_rx_fs_item {
@@ -1368,6 +1370,7 @@ struct macb {
 
 	struct macb_pm_data pm_data;
 	const struct macb_usrio_config *usrio;
+	struct bpf_prog	__rcu *prog;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 89f0c4dc3884..1f62100a4c4d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -6,6 +6,7 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/bpf_trace.h>
 #include <linux/circ_buf.h>
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
@@ -1249,9 +1250,19 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	return packets;
 }
 
+static unsigned int gem_max_rx_data_size(int base_sz)
+{
+	return SKB_DATA_ALIGN(base_sz + ETH_HLEN + ETH_FCS_LEN);
+}
+
+static unsigned int gem_max_rx_buffer_size(int data_sz, struct macb *bp)
+{
+	return SKB_HEAD_ALIGN(data_sz + bp->rx_headroom);
+}
+
 static unsigned int gem_total_rx_buffer_size(struct macb *bp)
 {
-	return SKB_HEAD_ALIGN(bp->rx_buffer_size + bp->rx_headroom);
+	return gem_max_rx_buffer_size(bp->rx_buffer_size, bp);
 }
 
 static int gem_rx_refill(struct macb_queue *queue, bool napi)
@@ -1336,12 +1347,61 @@ static void discard_partial_frame(struct macb_queue *queue, unsigned int begin,
 	 */
 }
 
+static u32 gem_xdp_run(struct macb_queue *queue, void *buff_head,
+		       unsigned int len)
+{
+	struct net_device *dev;
+	struct bpf_prog *prog;
+	struct xdp_buff xdp;
+
+	u32 act = XDP_PASS;
+
+	rcu_read_lock();
+
+	prog = rcu_dereference(queue->bp->prog);
+	if (!prog)
+		goto out;
+
+	xdp_init_buff(&xdp, gem_total_rx_buffer_size(queue->bp), &queue->xdp_rxq);
+	xdp_prepare_buff(&xdp, buff_head, queue->bp->rx_headroom, len, false);
+	xdp_buff_clear_frags_flag(&xdp);
+	dev = queue->bp->dev;
+
+	act = bpf_prog_run_xdp(prog, &xdp);
+	switch (act) {
+	case XDP_PASS:
+		goto out;
+	case XDP_REDIRECT:
+		if (unlikely(xdp_do_redirect(dev, &xdp, prog))) {
+			act = XDP_DROP;
+			break;
+		}
+		goto out;
+	default:
+		bpf_warn_invalid_xdp_action(dev, prog, act);
+		fallthrough;
+	case XDP_ABORTED:
+		trace_xdp_exception(dev, prog, act);
+		fallthrough;
+	case XDP_DROP:
+		break;
+	}
+
+	page_pool_put_full_page(queue->page_pool,
+				virt_to_head_page(xdp.data), true);
+out:
+	rcu_read_unlock();
+
+	return act;
+}
+
 static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		  int budget)
 {
 	struct skb_shared_info *shinfo;
 	struct macb *bp = queue->bp;
 	struct macb_dma_desc *desc;
+	bool xdp_flush = false;
 	unsigned int entry;
 	struct page *page;
 	void *buff_head;
@@ -1349,11 +1409,11 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 	int data_len;
 	int nr_frags;
 
-
 	while (count < budget) {
 		bool rxused, first_frame, last_frame;
 		dma_addr_t addr;
 		u32 ctrl;
+		u32 ret;
 
 		entry = macb_rx_ring_wrap(bp, queue->rx_tail);
 		desc = macb_rx_desc(queue, entry);
@@ -1401,6 +1461,17 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			data_len = bp->rx_buffer_size;
 		}
 
+		if (!(first_frame && last_frame))
+			goto skip_xdp;
+
+		ret = gem_xdp_run(queue, buff_head, data_len);
+		if (ret == XDP_REDIRECT)
+			xdp_flush = true;
+
+		if (ret != XDP_PASS)
+			goto next_frame;
+
+skip_xdp:
 		if (first_frame) {
 			queue->skb = napi_build_skb(buff_head, gem_total_rx_buffer_size(bp));
 			if (unlikely(!queue->skb)) {
@@ -1450,10 +1521,6 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		}
 
 		/* now everything is ready for receiving packet */
-		queue->rx_buff[entry] = NULL;
-
-		netdev_vdbg(bp->dev, "%s %u (len %u)\n", __func__, entry, data_len);
-
 		if (last_frame) {
 			bp->dev->stats.rx_packets++;
 			queue->stats.rx_packets++;
@@ -1474,6 +1541,8 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			queue->skb = NULL;
 		}
 
+next_frame:
+		queue->rx_buff[entry] = NULL;
 		continue;
 
 free_frags:
@@ -1491,6 +1560,9 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		queue->rx_buff[entry] = NULL;
 	}
 
+	if (xdp_flush)
+		xdp_do_flush();
+
 	gem_rx_refill(queue, true);
 
 	return count;
@@ -2428,13 +2500,11 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 static void macb_init_rx_buffer_size(struct macb *bp, unsigned int mtu)
 {
 	unsigned int overhead;
-	size_t size;
 
 	if (!macb_is_gem(bp)) {
 		bp->rx_buffer_size = MACB_RX_BUFFER_SIZE;
 	} else {
-		size = mtu + ETH_HLEN + ETH_FCS_LEN;
-		bp->rx_buffer_size = SKB_DATA_ALIGN(size);
+		bp->rx_buffer_size = gem_max_rx_data_size(mtu);
 		if (gem_total_rx_buffer_size(bp) > PAGE_SIZE) {
 			overhead = bp->rx_headroom +
 				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
@@ -2480,6 +2550,8 @@ static void gem_free_rx_buffers(struct macb *bp)
 
 		kfree(queue->rx_buff);
 		queue->rx_buff = NULL;
+		if (xdp_rxq_info_is_reg(&queue->xdp_rxq))
+			xdp_rxq_info_unreg(&queue->xdp_rxq);
 		page_pool_destroy(queue->page_pool);
 		queue->page_pool = NULL;
 	}
@@ -2636,30 +2708,55 @@ static int macb_alloc_consistent(struct macb *bp)
 	return -ENOMEM;
 }
 
-static int gem_create_page_pool(struct macb_queue *queue)
+static int gem_create_page_pool(struct macb_queue *queue, int qid)
 {
 	struct page_pool_params pp_params = {
 		.order = 0,
 		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
 		.pool_size = queue->bp->rx_ring_size,
 		.nid = NUMA_NO_NODE,
-		.dma_dir = DMA_FROM_DEVICE,
+		.dma_dir = rcu_access_pointer(queue->bp->prog)
+				? DMA_BIDIRECTIONAL
+				: DMA_FROM_DEVICE,
 		.dev = &queue->bp->pdev->dev,
 		.napi = &queue->napi_rx,
 		.max_len = PAGE_SIZE,
 	};
 	struct page_pool *pool;
-	int err = 0;
+	int err;
 
 	pool = page_pool_create(&pp_params);
 	if (IS_ERR(pool)) {
 		netdev_err(queue->bp->dev, "cannot create rx page pool\n");
 		err = PTR_ERR(pool);
-		pool = NULL;
+		goto clear_pool;
 	}
 
 	queue->page_pool = pool;
 
+	err = xdp_rxq_info_reg(&queue->xdp_rxq, queue->bp->dev, qid,
+			       queue->napi_rx.napi_id);
+	if (err < 0) {
+		netdev_err(queue->bp->dev, "xdp: failed to register rxq info\n");
+		goto destroy_pool;
+	}
+
+	err = xdp_rxq_info_reg_mem_model(&queue->xdp_rxq, MEM_TYPE_PAGE_POOL,
+					 queue->page_pool);
+	if (err) {
+		netdev_err(queue->bp->dev, "xdp: failed to register rxq memory model\n");
+		goto unreg_info;
+	}
+
+	return 0;
+
+unreg_info:
+	xdp_rxq_info_unreg(&queue->xdp_rxq);
+destroy_pool:
+	page_pool_destroy(pool);
+clear_pool:
+	queue->page_pool = NULL;
+
 	return err;
 }
 
@@ -2701,7 +2798,7 @@ static int gem_init_rings(struct macb *bp, bool fail_early)
 		/* This is a hard failure, so the best we can do is try the
 		 * next queue in case of HRESP error.
 		 */
-		err = gem_create_page_pool(queue);
+		err = gem_create_page_pool(queue, q);
 		if (err) {
 			last_err = err;
 			if (fail_early)
@@ -3152,11 +3249,27 @@ static int macb_close(struct net_device *dev)
 	return 0;
 }
 
+static bool gem_xdp_valid_mtu(struct macb *bp, int mtu)
+{
+	int max_frame_size;
+
+	max_frame_size = gem_max_rx_buffer_size(gem_max_rx_data_size(mtu), bp);
+
+	return max_frame_size <= PAGE_SIZE;
+}
+
 static int macb_change_mtu(struct net_device *dev, int new_mtu)
 {
+	struct macb *bp = netdev_priv(dev);
+
 	if (netif_running(dev))
 		return -EBUSY;
 
+	if (rcu_access_pointer(bp->prog) && !gem_xdp_valid_mtu(bp, new_mtu)) {
+		netdev_err(dev, "MTU %d too large for XDP", new_mtu);
+		return -EINVAL;
+	}
+
 	WRITE_ONCE(dev->mtu, new_mtu);
 
 	return 0;
@@ -3174,6 +3287,39 @@ static int macb_set_mac_addr(struct net_device *dev, void *addr)
 	return 0;
 }
 
+static int gem_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
+			 struct netlink_ext_ack *extack)
+{
+	struct macb *bp = netdev_priv(dev);
+	struct bpf_prog *old_prog;
+
+	if (prog && !gem_xdp_valid_mtu(bp, dev->mtu)) {
+		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
+		return -EOPNOTSUPP;
+	}
+
+	old_prog = rcu_replace_pointer(bp->prog, prog, lockdep_rtnl_is_held());
+	if (old_prog)
+		bpf_prog_put(old_prog);
+
+	return 0;
+}
+
+static int gem_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+	struct macb *bp = netdev_priv(dev);
+
+	if (!macb_is_gem(bp))
+		return -EOPNOTSUPP;
+
+	switch (xdp->command) {
+	case XDP_SETUP_PROG:
+		return gem_xdp_setup(dev, xdp->prog, xdp->extack);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static void gem_update_stats(struct macb *bp)
 {
 	struct macb_queue *queue;
@@ -4427,6 +4573,7 @@ static const struct net_device_ops macb_netdev_ops = {
 	.ndo_hwtstamp_set	= macb_hwtstamp_set,
 	.ndo_hwtstamp_get	= macb_hwtstamp_get,
 	.ndo_setup_tc		= macb_setup_tc,
+	.ndo_bpf		= gem_xdp,
 };
 
 /* Configure peripheral capabilities according to device tree
@@ -5732,6 +5879,9 @@ static int macb_probe(struct platform_device *pdev)
 		bp->rx_headroom = XDP_PACKET_HEADROOM;
 		if (!(bp->caps & MACB_CAPS_RSC))
 			bp->rx_headroom += NET_IP_ALIGN;
+
+		dev->xdp_features = NETDEV_XDP_ACT_BASIC |
+				    NETDEV_XDP_ACT_REDIRECT;
 	}
 
 	netif_carrier_off(dev);
-- 
2.52.0

Re: [net-next,5/8] cadence: macb: add XDP support for gem

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
cadence: macb: add XDP support for gem

This commit introduces XDP support for the macb/gem driver with XDP_PASS,
XDP_DROP, and XDP_REDIRECT verdicts.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 89f0c4dc3884..1f62100a4c4d 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

[ ... ]

> -static int gem_create_page_pool(struct macb_queue *queue)
> +static int gem_create_page_pool(struct macb_queue *queue, int qid)
>  {
>  	struct page_pool_params pp_params = {
>  		.order = 0,
>  		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
>  		.pool_size = queue->bp->rx_ring_size,
>  		.nid = NUMA_NO_NODE,
> -		.dma_dir = DMA_FROM_DEVICE,
> +		.dma_dir = rcu_access_pointer(queue->bp->prog)
> +				? DMA_BIDIRECTIONAL
> +				: DMA_FROM_DEVICE,

The page pool is created with DMA direction based on whether an XDP program
is attached at creation time. However, gem_xdp_setup() below allows attaching
XDP programs to a running interface without recreating the page pool.

If the interface is opened without XDP (page pool uses DMA_FROM_DEVICE), and
then an XDP program is attached via ndo_bpf, the pool will still use
DMA_FROM_DEVICE. For XDP redirect, DMA_BIDIRECTIONAL is needed for proper
cache coherency on non-cache-coherent architectures.

The stmmac driver handles this by calling stmmac_xdp_release() and
stmmac_xdp_open() when XDP program state changes on a running interface,
which recreates the page pool with the correct DMA direction.

Should gem_xdp_setup() reset the interface (or at least the page pools) when
attaching or detaching XDP programs on a running interface to ensure the DMA
direction matches?

[ ... ]

> +static int gem_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
> +			 struct netlink_ext_ack *extack)
> +{
> +	struct macb *bp = netdev_priv(dev);
> +	struct bpf_prog *old_prog;
> +
> +	if (prog && !gem_xdp_valid_mtu(bp, dev->mtu)) {
> +		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	old_prog = rcu_replace_pointer(bp->prog, prog, lockdep_rtnl_is_held());
> +	if (old_prog)
> +		bpf_prog_put(old_prog);
> +
> +	return 0;
> +}

This function swaps the XDP program via RCU but does not reset the interface
or recreate the page pools. This relates to the DMA direction concern above.

[PATCH net-next 6/8] cadence: macb: make macb_tx_skb generic

[Paolo Valerio]

The macb_tx_skb structure is renamed to macb_tx_buff with
no functional changes.

This is a preparatory step for adding xdp xmit support.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb.h      | 10 +--
 drivers/net/ethernet/cadence/macb_main.c | 96 ++++++++++++------------
 2 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d46d8523198d..970ad3f945fb 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -959,7 +959,7 @@ struct macb_dma_desc_ptp {
 /* Scaled PPM fraction */
 #define PPM_FRACTION	16
 
-/* struct macb_tx_skb - data about an skb which is being transmitted
+/* struct macb_tx_buff - data about an skb which is being transmitted
  * @skb: skb currently being transmitted, only set for the last buffer
  *       of the frame
  * @mapping: DMA address of the skb's fragment buffer
@@ -967,8 +967,8 @@ struct macb_dma_desc_ptp {
  * @mapped_as_page: true when buffer was mapped with skb_frag_dma_map(),
  *                  false when buffer was mapped with dma_map_single()
  */
-struct macb_tx_skb {
-	struct sk_buff		*skb;
+struct macb_tx_buff {
+	void			*skb;
 	dma_addr_t		mapping;
 	size_t			size;
 	bool			mapped_as_page;
@@ -1253,7 +1253,7 @@ struct macb_queue {
 	spinlock_t		tx_ptr_lock;
 	unsigned int		tx_head, tx_tail;
 	struct macb_dma_desc	*tx_ring;
-	struct macb_tx_skb	*tx_skb;
+	struct macb_tx_buff	*tx_buff;
 	dma_addr_t		tx_ring_dma;
 	struct work_struct	tx_error_task;
 	bool			txubr_pending;
@@ -1331,7 +1331,7 @@ struct macb {
 	phy_interface_t		phy_interface;
 
 	/* AT91RM9200 transmit queue (1 on wire + 1 queued) */
-	struct macb_tx_skb	rm9200_txq[2];
+	struct macb_tx_buff	rm9200_txq[2];
 	unsigned int		max_tx_length;
 
 	u64			ethtool_stats[GEM_STATS_LEN + QUEUE_STATS_LEN * MACB_MAX_QUEUES];
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 1f62100a4c4d..26af371b3b1e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -157,10 +157,10 @@ static struct macb_dma_desc *macb_tx_desc(struct macb_queue *queue,
 	return &queue->tx_ring[index];
 }
 
-static struct macb_tx_skb *macb_tx_skb(struct macb_queue *queue,
-				       unsigned int index)
+static struct macb_tx_buff *macb_tx_buff(struct macb_queue *queue,
+					 unsigned int index)
 {
-	return &queue->tx_skb[macb_tx_ring_wrap(queue->bp, index)];
+	return &queue->tx_buff[macb_tx_ring_wrap(queue->bp, index)];
 }
 
 static dma_addr_t macb_tx_dma(struct macb_queue *queue, unsigned int index)
@@ -969,21 +969,21 @@ static int macb_halt_tx(struct macb *bp)
 					bp, TSR);
 }
 
-static void macb_tx_unmap(struct macb *bp, struct macb_tx_skb *tx_skb, int budget)
+static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff, int budget)
 {
-	if (tx_skb->mapping) {
-		if (tx_skb->mapped_as_page)
-			dma_unmap_page(&bp->pdev->dev, tx_skb->mapping,
-				       tx_skb->size, DMA_TO_DEVICE);
+	if (tx_buff->mapping) {
+		if (tx_buff->mapped_as_page)
+			dma_unmap_page(&bp->pdev->dev, tx_buff->mapping,
+				       tx_buff->size, DMA_TO_DEVICE);
 		else
-			dma_unmap_single(&bp->pdev->dev, tx_skb->mapping,
-					 tx_skb->size, DMA_TO_DEVICE);
-		tx_skb->mapping = 0;
+			dma_unmap_single(&bp->pdev->dev, tx_buff->mapping,
+					 tx_buff->size, DMA_TO_DEVICE);
+		tx_buff->mapping = 0;
 	}
 
-	if (tx_skb->skb) {
-		napi_consume_skb(tx_skb->skb, budget);
-		tx_skb->skb = NULL;
+	if (tx_buff->skb) {
+		napi_consume_skb(tx_buff->skb, budget);
+		tx_buff->skb = NULL;
 	}
 }
 
@@ -1029,7 +1029,7 @@ static void macb_tx_error_task(struct work_struct *work)
 	u32			queue_index;
 	u32			packets = 0;
 	u32			bytes = 0;
-	struct macb_tx_skb	*tx_skb;
+	struct macb_tx_buff	*tx_buff;
 	struct macb_dma_desc	*desc;
 	struct sk_buff		*skb;
 	unsigned int		tail;
@@ -1069,16 +1069,16 @@ static void macb_tx_error_task(struct work_struct *work)
 
 		desc = macb_tx_desc(queue, tail);
 		ctrl = desc->ctrl;
-		tx_skb = macb_tx_skb(queue, tail);
-		skb = tx_skb->skb;
+		tx_buff = macb_tx_buff(queue, tail);
+		skb = tx_buff->skb;
 
 		if (ctrl & MACB_BIT(TX_USED)) {
 			/* skb is set for the last buffer of the frame */
 			while (!skb) {
-				macb_tx_unmap(bp, tx_skb, 0);
+				macb_tx_unmap(bp, tx_buff, 0);
 				tail++;
-				tx_skb = macb_tx_skb(queue, tail);
-				skb = tx_skb->skb;
+				tx_buff = macb_tx_buff(queue, tail);
+				skb = tx_buff->skb;
 			}
 
 			/* ctrl still refers to the first buffer descriptor
@@ -1107,7 +1107,7 @@ static void macb_tx_error_task(struct work_struct *work)
 			desc->ctrl = ctrl | MACB_BIT(TX_USED);
 		}
 
-		macb_tx_unmap(bp, tx_skb, 0);
+		macb_tx_unmap(bp, tx_buff, 0);
 	}
 
 	netdev_tx_completed_queue(netdev_get_tx_queue(bp->dev, queue_index),
@@ -1185,7 +1185,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
 	head = queue->tx_head;
 	for (tail = queue->tx_tail; tail != head && packets < budget; tail++) {
-		struct macb_tx_skb	*tx_skb;
+		struct macb_tx_buff	*tx_buff;
 		struct sk_buff		*skb;
 		struct macb_dma_desc	*desc;
 		u32			ctrl;
@@ -1205,8 +1205,8 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 
 		/* Process all buffers of the current transmitted frame */
 		for (;; tail++) {
-			tx_skb = macb_tx_skb(queue, tail);
-			skb = tx_skb->skb;
+			tx_buff = macb_tx_buff(queue, tail);
+			skb = tx_buff->skb;
 
 			/* First, update TX stats if needed */
 			if (skb) {
@@ -1226,7 +1226,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 			}
 
 			/* Now we can safely release resources */
-			macb_tx_unmap(bp, tx_skb, budget);
+			macb_tx_unmap(bp, tx_buff, budget);
 
 			/* skb is set only for the last buffer of the frame.
 			 * WARNING: at this point skb has been freed by
@@ -2130,8 +2130,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int len, i, tx_head = queue->tx_head;
 	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
+	struct macb_tx_buff *tx_buff = NULL;
 	unsigned int eof = 1, mss_mfs = 0;
-	struct macb_tx_skb *tx_skb = NULL;
 	struct macb_dma_desc *desc;
 	unsigned int offset, size;
 	dma_addr_t mapping;
@@ -2154,7 +2154,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 	offset = 0;
 	while (len) {
-		tx_skb = macb_tx_skb(queue, tx_head);
+		tx_buff = macb_tx_buff(queue, tx_head);
 
 		mapping = dma_map_single(&bp->pdev->dev,
 					 skb->data + offset,
@@ -2163,10 +2163,10 @@ static unsigned int macb_tx_map(struct macb *bp,
 			goto dma_error;
 
 		/* Save info to properly release resources */
-		tx_skb->skb = NULL;
-		tx_skb->mapping = mapping;
-		tx_skb->size = size;
-		tx_skb->mapped_as_page = false;
+		tx_buff->skb = NULL;
+		tx_buff->mapping = mapping;
+		tx_buff->size = size;
+		tx_buff->mapped_as_page = false;
 
 		len -= size;
 		offset += size;
@@ -2183,7 +2183,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		offset = 0;
 		while (len) {
 			size = umin(len, bp->max_tx_length);
-			tx_skb = macb_tx_skb(queue, tx_head);
+			tx_buff = macb_tx_buff(queue, tx_head);
 
 			mapping = skb_frag_dma_map(&bp->pdev->dev, frag,
 						   offset, size, DMA_TO_DEVICE);
@@ -2191,10 +2191,10 @@ static unsigned int macb_tx_map(struct macb *bp,
 				goto dma_error;
 
 			/* Save info to properly release resources */
-			tx_skb->skb = NULL;
-			tx_skb->mapping = mapping;
-			tx_skb->size = size;
-			tx_skb->mapped_as_page = true;
+			tx_buff->skb = NULL;
+			tx_buff->mapping = mapping;
+			tx_buff->size = size;
+			tx_buff->mapped_as_page = true;
 
 			len -= size;
 			offset += size;
@@ -2203,13 +2203,13 @@ static unsigned int macb_tx_map(struct macb *bp,
 	}
 
 	/* Should never happen */
-	if (unlikely(!tx_skb)) {
+	if (unlikely(!tx_buff)) {
 		netdev_err(bp->dev, "BUG! empty skb!\n");
 		return 0;
 	}
 
 	/* This is the last buffer of the frame: save socket buffer */
-	tx_skb->skb = skb;
+	tx_buff->skb = skb;
 
 	/* Update TX ring: update buffer descriptors in reverse order
 	 * to avoid race condition
@@ -2240,10 +2240,10 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 	do {
 		i--;
-		tx_skb = macb_tx_skb(queue, i);
+		tx_buff = macb_tx_buff(queue, i);
 		desc = macb_tx_desc(queue, i);
 
-		ctrl = (u32)tx_skb->size;
+		ctrl = (u32)tx_buff->size;
 		if (eof) {
 			ctrl |= MACB_BIT(TX_LAST);
 			eof = 0;
@@ -2266,7 +2266,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 			ctrl |= MACB_BF(MSS_MFS, mss_mfs);
 
 		/* Set TX buffer descriptor */
-		macb_set_addr(bp, desc, tx_skb->mapping);
+		macb_set_addr(bp, desc, tx_buff->mapping);
 		/* desc->addr must be visible to hardware before clearing
 		 * 'TX_USED' bit in desc->ctrl.
 		 */
@@ -2282,9 +2282,9 @@ static unsigned int macb_tx_map(struct macb *bp,
 	netdev_err(bp->dev, "TX DMA map failed\n");
 
 	for (i = queue->tx_head; i != tx_head; i++) {
-		tx_skb = macb_tx_skb(queue, i);
+		tx_buff = macb_tx_buff(queue, i);
 
-		macb_tx_unmap(bp, tx_skb, 0);
+		macb_tx_unmap(bp, tx_buff, 0);
 	}
 
 	return -ENOMEM;
@@ -2601,8 +2601,8 @@ static void macb_free_consistent(struct macb *bp)
 	dma_free_coherent(dev, size, bp->queues[0].rx_ring, bp->queues[0].rx_ring_dma);
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-		kfree(queue->tx_skb);
-		queue->tx_skb = NULL;
+		kfree(queue->tx_buff);
+		queue->tx_buff = NULL;
 		queue->tx_ring = NULL;
 		queue->rx_ring = NULL;
 	}
@@ -2680,9 +2680,9 @@ static int macb_alloc_consistent(struct macb *bp)
 		queue->rx_ring = rx + macb_rx_ring_size_per_queue(bp) * q;
 		queue->rx_ring_dma = rx_dma + macb_rx_ring_size_per_queue(bp) * q;
 
-		size = bp->tx_ring_size * sizeof(struct macb_tx_skb);
-		queue->tx_skb = kmalloc(size, GFP_KERNEL);
-		if (!queue->tx_skb)
+		size = bp->tx_ring_size * sizeof(struct macb_tx_buff);
+		queue->tx_buff = kmalloc(size, GFP_KERNEL);
+		if (!queue->tx_buff)
 			goto out_err;
 	}
 
-- 
2.52.0

[PATCH net-next 7/8] cadence: macb: make tx path skb agnostic

[Paolo Valerio]

Rename macb_tx_buff member skb to ptr and introduce macb_tx_buff_type
to identify the buffer type macb_tx_buff represents.

This is the last preparatory step for xdp xmit support.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb.h      | 23 +++++++++++-----
 drivers/net/ethernet/cadence/macb_main.c | 35 ++++++++++++++----------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 970ad3f945fb..c17e894dfcdb 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -959,19 +959,28 @@ struct macb_dma_desc_ptp {
 /* Scaled PPM fraction */
 #define PPM_FRACTION	16
 
-/* struct macb_tx_buff - data about an skb which is being transmitted
- * @skb: skb currently being transmitted, only set for the last buffer
- *       of the frame
+enum macb_tx_buff_type {
+	MACB_TYPE_SKB,
+	MACB_TYPE_XDP_TX,
+	MACB_TYPE_XDP_NDO,
+};
+
+/* struct macb_tx_buff - data about an skb or xdp frame which is being
+ * transmitted.
+ * @ptr: pointer to skb or xdp frame being transmitted, only set
+ *        for the last buffer for sk_buff
  * @mapping: DMA address of the skb's fragment buffer
  * @size: size of the DMA mapped buffer
  * @mapped_as_page: true when buffer was mapped with skb_frag_dma_map(),
  *                  false when buffer was mapped with dma_map_single()
+ * @type: type of buffer (MACB_TYPE_SKB, MACB_TYPE_XDP_TX, MACB_TYPE_XDP_NDO)
  */
 struct macb_tx_buff {
-	void			*skb;
-	dma_addr_t		mapping;
-	size_t			size;
-	bool			mapped_as_page;
+	void				*ptr;
+	dma_addr_t			mapping;
+	size_t				size;
+	bool				mapped_as_page;
+	enum macb_tx_buff_type		type;
 };
 
 /* Hardware-collected statistics. Used when updating the network
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 26af371b3b1e..afd8c0f2d895 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -969,7 +969,8 @@ static int macb_halt_tx(struct macb *bp)
 					bp, TSR);
 }
 
-static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff, int budget)
+static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff,
+			  int budget)
 {
 	if (tx_buff->mapping) {
 		if (tx_buff->mapped_as_page)
@@ -981,9 +982,9 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff, int bud
 		tx_buff->mapping = 0;
 	}
 
-	if (tx_buff->skb) {
-		napi_consume_skb(tx_buff->skb, budget);
-		tx_buff->skb = NULL;
+	if (tx_buff->ptr) {
+		napi_consume_skb(tx_buff->ptr, budget);
+		tx_buff->ptr = NULL;
 	}
 }
 
@@ -1070,7 +1071,7 @@ static void macb_tx_error_task(struct work_struct *work)
 		desc = macb_tx_desc(queue, tail);
 		ctrl = desc->ctrl;
 		tx_buff = macb_tx_buff(queue, tail);
-		skb = tx_buff->skb;
+		skb = tx_buff->ptr;
 
 		if (ctrl & MACB_BIT(TX_USED)) {
 			/* skb is set for the last buffer of the frame */
@@ -1078,7 +1079,7 @@ static void macb_tx_error_task(struct work_struct *work)
 				macb_tx_unmap(bp, tx_buff, 0);
 				tail++;
 				tx_buff = macb_tx_buff(queue, tail);
-				skb = tx_buff->skb;
+				skb = tx_buff->ptr;
 			}
 
 			/* ctrl still refers to the first buffer descriptor
@@ -1206,7 +1207,9 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		/* Process all buffers of the current transmitted frame */
 		for (;; tail++) {
 			tx_buff = macb_tx_buff(queue, tail);
-			skb = tx_buff->skb;
+
+			if (tx_buff->type == MACB_TYPE_SKB)
+				skb = tx_buff->ptr;
 
 			/* First, update TX stats if needed */
 			if (skb) {
@@ -2163,7 +2166,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 			goto dma_error;
 
 		/* Save info to properly release resources */
-		tx_buff->skb = NULL;
+		tx_buff->ptr = NULL;
+		tx_buff->type = MACB_TYPE_SKB;
 		tx_buff->mapping = mapping;
 		tx_buff->size = size;
 		tx_buff->mapped_as_page = false;
@@ -2191,7 +2195,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 				goto dma_error;
 
 			/* Save info to properly release resources */
-			tx_buff->skb = NULL;
+			tx_buff->ptr = NULL;
+			tx_buff->type = MACB_TYPE_SKB;
 			tx_buff->mapping = mapping;
 			tx_buff->size = size;
 			tx_buff->mapped_as_page = true;
@@ -2209,7 +2214,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 	}
 
 	/* This is the last buffer of the frame: save socket buffer */
-	tx_buff->skb = skb;
+	tx_buff->ptr = skb;
+	tx_buff->type = MACB_TYPE_SKB;
 
 	/* Update TX ring: update buffer descriptors in reverse order
 	 * to avoid race condition
@@ -5096,8 +5102,9 @@ static netdev_tx_t at91ether_start_xmit(struct sk_buff *skb,
 		netif_stop_queue(dev);
 
 		/* Store packet information (to free when Tx completed) */
-		lp->rm9200_txq[desc].skb = skb;
+		lp->rm9200_txq[desc].ptr = skb;
 		lp->rm9200_txq[desc].size = skb->len;
+		lp->rm9200_txq[desc].type = MACB_TYPE_SKB;
 		lp->rm9200_txq[desc].mapping = dma_map_single(&lp->pdev->dev, skb->data,
 							      skb->len, DMA_TO_DEVICE);
 		if (dma_mapping_error(&lp->pdev->dev, lp->rm9200_txq[desc].mapping)) {
@@ -5189,9 +5196,9 @@ static irqreturn_t at91ether_interrupt(int irq, void *dev_id)
 			dev->stats.tx_errors++;
 
 		desc = 0;
-		if (lp->rm9200_txq[desc].skb) {
-			dev_consume_skb_irq(lp->rm9200_txq[desc].skb);
-			lp->rm9200_txq[desc].skb = NULL;
+		if (lp->rm9200_txq[desc].ptr) {
+			dev_consume_skb_irq(lp->rm9200_txq[desc].ptr);
+			lp->rm9200_txq[desc].ptr = NULL;
 			dma_unmap_single(&lp->pdev->dev, lp->rm9200_txq[desc].mapping,
 					 lp->rm9200_txq[desc].size, DMA_TO_DEVICE);
 			dev->stats.tx_packets++;
-- 
2.52.0

[PATCH net-next 8/8] cadence: macb: introduce xmit support

[Paolo Valerio]

Add XDP_TX verdict support, also introduce ndo_xdp_xmit function for
redirection, and update macb_tx_unmap() to handle both skbs and xdp
frames advertising NETDEV_XDP_ACT_NDO_XMIT capability and the ability
to process XDP_TX verdicts.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 165 +++++++++++++++++++++--
 1 file changed, 157 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index afd8c0f2d895..32f8629bcb25 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -969,6 +969,17 @@ static int macb_halt_tx(struct macb *bp)
 					bp, TSR);
 }
 
+static void release_buff(void *buff, enum macb_tx_buff_type type, int budget)
+{
+	if (type == MACB_TYPE_SKB) {
+		napi_consume_skb(buff, budget);
+	} else if (type == MACB_TYPE_XDP_TX) {
+		xdp_return_frame_rx_napi(buff);
+	} else {
+		xdp_return_frame(buff);
+	}
+}
+
 static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff,
 			  int budget)
 {
@@ -983,7 +994,7 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff,
 	}
 
 	if (tx_buff->ptr) {
-		napi_consume_skb(tx_buff->ptr, budget);
+		release_buff(tx_buff->ptr, tx_buff->type, budget);
 		tx_buff->ptr = NULL;
 	}
 }
@@ -1071,6 +1082,10 @@ static void macb_tx_error_task(struct work_struct *work)
 		desc = macb_tx_desc(queue, tail);
 		ctrl = desc->ctrl;
 		tx_buff = macb_tx_buff(queue, tail);
+
+		if (tx_buff->type != MACB_TYPE_SKB)
+			goto unmap;
+
 		skb = tx_buff->ptr;
 
 		if (ctrl & MACB_BIT(TX_USED)) {
@@ -1108,6 +1123,7 @@ static void macb_tx_error_task(struct work_struct *work)
 			desc->ctrl = ctrl | MACB_BIT(TX_USED);
 		}
 
+unmap:
 		macb_tx_unmap(bp, tx_buff, 0);
 	}
 
@@ -1186,6 +1202,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
 	head = queue->tx_head;
 	for (tail = queue->tx_tail; tail != head && packets < budget; tail++) {
+		void			*data = NULL;
 		struct macb_tx_buff	*tx_buff;
 		struct sk_buff		*skb;
 		struct macb_dma_desc	*desc;
@@ -1208,11 +1225,16 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		for (;; tail++) {
 			tx_buff = macb_tx_buff(queue, tail);
 
-			if (tx_buff->type == MACB_TYPE_SKB)
-				skb = tx_buff->ptr;
+			if (tx_buff->type != MACB_TYPE_SKB) {
+				data = tx_buff->ptr;
+				goto unmap;
+			}
 
 			/* First, update TX stats if needed */
-			if (skb) {
+			if (tx_buff->type == MACB_TYPE_SKB && tx_buff->ptr) {
+				data = tx_buff->ptr;
+				skb = tx_buff->ptr;
+
 				if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
 				    !ptp_one_step_sync(skb))
 					gem_ptp_do_txstamp(bp, skb, desc);
@@ -1228,6 +1250,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 				bytes += skb->len;
 			}
 
+unmap:
 			/* Now we can safely release resources */
 			macb_tx_unmap(bp, tx_buff, budget);
 
@@ -1235,7 +1258,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 			 * WARNING: at this point skb has been freed by
 			 * macb_tx_unmap().
 			 */
-			if (skb)
+			if (data)
 				break;
 		}
 	}
@@ -1350,10 +1373,127 @@ static void discard_partial_frame(struct macb_queue *queue, unsigned int begin,
 	 */
 }
 
+static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
+				 struct net_device *dev, bool dma_map,
+				 dma_addr_t addr)
+{
+	enum macb_tx_buff_type buff_type;
+	struct macb_tx_buff *tx_buff;
+	int cpu = smp_processor_id();
+	struct macb_dma_desc *desc;
+	struct macb_queue *queue;
+	unsigned int next_head;
+	unsigned long flags;
+	dma_addr_t mapping;
+	u16 queue_index;
+	int err = 0;
+	u32 ctrl;
+
+	queue_index = cpu % bp->num_queues;
+	queue = &bp->queues[queue_index];
+	buff_type = dma_map ? MACB_TYPE_XDP_NDO : MACB_TYPE_XDP_TX;
+
+	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
+
+	/* This is a hard error, log it. */
+	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1) {
+		netif_stop_subqueue(dev, queue_index);
+		netdev_dbg(bp->dev, "tx_head = %u, tx_tail = %u\n",
+			   queue->tx_head, queue->tx_tail);
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	if (dma_map) {
+		mapping = dma_map_single(&bp->pdev->dev,
+					 xdpf->data,
+					 xdpf->len, DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(&bp->pdev->dev, mapping))) {
+			err = -ENOMEM;
+			goto unlock;
+		}
+	} else {
+		mapping = addr;
+		dma_sync_single_for_device(&bp->pdev->dev, mapping,
+					   xdpf->len, DMA_BIDIRECTIONAL);
+	}
+
+	next_head = queue->tx_head + 1;
+
+	ctrl = MACB_BIT(TX_USED);
+	desc = macb_tx_desc(queue, next_head);
+	desc->ctrl = ctrl;
+
+	desc = macb_tx_desc(queue, queue->tx_head);
+	tx_buff = macb_tx_buff(queue, queue->tx_head);
+	tx_buff->ptr = xdpf;
+	tx_buff->type = buff_type;
+	tx_buff->mapping = mapping;
+	tx_buff->size = xdpf->len;
+	tx_buff->mapped_as_page = false;
+
+	ctrl = (u32)tx_buff->size;
+	ctrl |= MACB_BIT(TX_LAST);
+
+	if (unlikely(macb_tx_ring_wrap(bp, queue->tx_head) == (bp->tx_ring_size - 1)))
+		ctrl |= MACB_BIT(TX_WRAP);
+
+	/* Set TX buffer descriptor */
+	macb_set_addr(bp, desc, tx_buff->mapping);
+	/* desc->addr must be visible to hardware before clearing
+	 * 'TX_USED' bit in desc->ctrl.
+	 */
+	wmb();
+	desc->ctrl = ctrl;
+	queue->tx_head = next_head;
+
+	/* Make newly initialized descriptor visible to hardware */
+	wmb();
+
+	spin_lock(&bp->lock);
+	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
+	spin_unlock(&bp->lock);
+
+	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1)
+		netif_stop_subqueue(dev, queue_index);
+
+unlock:
+	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
+
+	if (err)
+		release_buff(xdpf, buff_type, 0);
+
+	return err;
+}
+
+static int gem_xdp_xmit(struct net_device *dev, int num_frame,
+			struct xdp_frame **frames, u32 flags)
+{
+	struct macb *bp = netdev_priv(dev);
+	u32 xmitted = 0;
+	int i;
+
+	if (!macb_is_gem(bp))
+		return -EOPNOTSUPP;
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+		return -EINVAL;
+
+	for (i = 0; i < num_frame; i++) {
+		if (macb_xdp_submit_frame(bp, frames[i], dev, true, 0))
+			break;
+
+		xmitted++;
+	}
+
+	return xmitted;
+}
+
 static u32 gem_xdp_run(struct macb_queue *queue, void *buff_head,
-		       unsigned int len)
+		       unsigned int len, dma_addr_t addr)
 {
 	struct net_device *dev;
+	struct xdp_frame *xdpf;
 	struct bpf_prog *prog;
 	struct xdp_buff xdp;
 
@@ -1380,6 +1520,13 @@ static u32 gem_xdp_run(struct macb_queue *queue, void *buff_head,
 			break;
 		}
 		goto out;
+	case XDP_TX:
+		xdpf = xdp_convert_buff_to_frame(&xdp);
+
+		if (!xdpf || macb_xdp_submit_frame(queue->bp, xdpf, dev, false,
+						   addr))
+			act = XDP_DROP;
+		goto out;
 	default:
 		bpf_warn_invalid_xdp_action(dev, prog, act);
 		fallthrough;
@@ -1467,7 +1614,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		if (!(first_frame && last_frame))
 			goto skip_xdp;
 
-		ret = gem_xdp_run(queue, buff_head, data_len);
+		ret = gem_xdp_run(queue, buff_head, data_len, addr);
 		if (ret == XDP_REDIRECT)
 			xdp_flush = true;
 
@@ -4580,6 +4727,7 @@ static const struct net_device_ops macb_netdev_ops = {
 	.ndo_hwtstamp_get	= macb_hwtstamp_get,
 	.ndo_setup_tc		= macb_setup_tc,
 	.ndo_bpf		= gem_xdp,
+	.ndo_xdp_xmit		= gem_xdp_xmit,
 };
 
 /* Configure peripheral capabilities according to device tree
@@ -5888,7 +6036,8 @@ static int macb_probe(struct platform_device *pdev)
 			bp->rx_headroom += NET_IP_ALIGN;
 
 		dev->xdp_features = NETDEV_XDP_ACT_BASIC |
-				    NETDEV_XDP_ACT_REDIRECT;
+				    NETDEV_XDP_ACT_REDIRECT |
+				    NETDEV_XDP_ACT_NDO_XMIT;
 	}
 
 	netif_carrier_off(dev);
-- 
2.52.0

Re: [net-next,8/8] cadence: macb: introduce xmit support

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
cadence: macb: introduce xmit support

This patch adds XDP_TX verdict support and introduces the ndo_xdp_xmit
function for XDP redirection.  It updates macb_tx_unmap() to handle both
skbs and xdp frames.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index afd8c0f2d895..32f8629bcb25 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

[ ... ]

> @@ -983,7 +994,7 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff,
>  	}
>
>  	if (tx_buff->ptr) {
> -		napi_consume_skb(tx_buff->ptr, budget);
> +		release_buff(tx_buff->ptr, tx_buff->type, budget);
>  		tx_buff->ptr = NULL;
>  	}
>  }

[ ... ]

> +static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
> +				 struct net_device *dev, bool dma_map,
> +				 dma_addr_t addr)
> +{

[ ... ]

> +	if (dma_map) {
> +		mapping = dma_map_single(&bp->pdev->dev,
> +					 xdpf->data,
> +					 xdpf->len, DMA_TO_DEVICE);
> +		if (unlikely(dma_mapping_error(&bp->pdev->dev, mapping))) {
> +			err = -ENOMEM;
> +			goto unlock;
> +		}
> +	} else {
> +		mapping = addr;
> +		dma_sync_single_for_device(&bp->pdev->dev, mapping,
> +					   xdpf->len, DMA_BIDIRECTIONAL);
> +	}

[ ... ]

> +	tx_buff->ptr = xdpf;
> +	tx_buff->type = buff_type;
> +	tx_buff->mapping = mapping;
                          ^^^^^^^

For XDP_TX (dma_map=false), the mapping here is the page pool's DMA address
passed via the addr parameter.  Later, macb_tx_unmap() unconditionally calls
dma_unmap_single() when tx_buff->mapping is non-zero.

Should macb_tx_unmap() skip the DMA unmap for MACB_TYPE_XDP_TX buffers?
Page pool manages its own DMA mappings, and calling dma_unmap_single() on
them appears incorrect.  For comparison, stmmac_tx_clean() explicitly
checks "buf_type != STMMAC_TXBUF_T_XDP_TX" before unmapping.

[ ... ]

> +	case XDP_TX:
> +		xdpf = xdp_convert_buff_to_frame(&xdp);
> +
> +		if (!xdpf || macb_xdp_submit_frame(queue->bp, xdpf, dev, false,
> +						   addr))
> +			act = XDP_DROP;
> +		goto out;

When xdp_convert_buff_to_frame() returns NULL, this sets act = XDP_DROP but
then unconditionally executes "goto out", which skips the page_pool_put_full_page()
call that handles XDP_DROP.  Could this leak the page when
xdp_convert_buff_to_frame() fails due to insufficient headroom?

When macb_xdp_submit_frame() fails, it internally calls release_buff() which
returns the page, so that path seems fine.  But the xdpf == NULL case appears
to need a "break" instead of falling through to "goto out".

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Théo Lebrun]

On Thu Jan 15, 2026 at 11:25 PM CET, Paolo Valerio wrote:
> Paolo Valerio (7):
>   net: macb: rename rx_skbuff into rx_buff
>   cadence: macb: Add page pool support handle multi-descriptor frame rx
>   cadence: macb: use the current queue number for stats
>   cadence: macb: add XDP support for gem
>   cadence: macb: make macb_tx_skb generic
>   cadence: macb: make tx path skb agnostic
>   cadence: macb: introduce xmit support
>
> Théo Lebrun (1):
>   net: macb: move Rx buffers alloc from link up to open

`net: macb:` is the right prefix for MACB. Two commits use it but not
the other ones. Sending this one-off comment as I just noticed it
(while generating the XSK series).

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 1/6] net: macb: rename release_buff() -> macb_tx_release_buff()

[Théo Lebrun]

release_buff() as a function name is ambiguous:
 - it misses the driver prefix and,
 - it does not mention whether it is a Tx or Rx buffer.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d155f4571c74..70f5dc9c6529 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -969,7 +969,7 @@ static int macb_halt_tx(struct macb *bp)
 					bp, TSR);
 }
 
-static void release_buff(void *buff, enum macb_tx_buff_type type, int budget)
+static void macb_tx_release_buff(void *buff, enum macb_tx_buff_type type, int budget)
 {
 	if (type == MACB_TYPE_SKB) {
 		napi_consume_skb(buff, budget);
@@ -994,7 +994,7 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff,
 	}
 
 	if (tx_buff->ptr) {
-		release_buff(tx_buff->ptr, tx_buff->type, budget);
+		macb_tx_release_buff(tx_buff->ptr, tx_buff->type, budget);
 		tx_buff->ptr = NULL;
 	}
 }
@@ -1461,7 +1461,7 @@ static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
 	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
 
 	if (err)
-		release_buff(xdpf, buff_type, 0);
+		macb_tx_release_buff(xdpf, buff_type, 0);
 
 	return err;
 }
-- 
2.53.0

[PATCH 2/6] net: macb: drop two labels in gem_rx()

[Théo Lebrun]

Avoid skip_xdp goto and label that skip over five lines of code. If
received buffer is fragmented, we know for sure it cannot be XDP as we
limit the MTU to ensure packets fit one buffer.

Second label removed is next_frame which jumps to one assignement
statement and a continue. Duplicate to remove label and goto.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 70f5dc9c6529..918eee5c8a5b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1611,17 +1611,17 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			data_len = bp->rx_buffer_size;
 		}
 
-		if (!(first_frame && last_frame))
-			goto skip_xdp;
+		if (first_frame && last_frame) {
+			ret = gem_xdp_run(queue, buff_head, data_len, addr);
+			if (ret == XDP_REDIRECT)
+				xdp_flush = true;
 
-		ret = gem_xdp_run(queue, buff_head, data_len, addr);
-		if (ret == XDP_REDIRECT)
-			xdp_flush = true;
+			if (ret != XDP_PASS) {
+				queue->rx_buff[entry] = NULL;
+				continue;
+			}
+		}
 
-		if (ret != XDP_PASS)
-			goto next_frame;
-
-skip_xdp:
 		if (first_frame) {
 			queue->skb = napi_build_skb(buff_head, gem_total_rx_buffer_size(bp));
 			if (unlikely(!queue->skb)) {
@@ -1691,7 +1691,6 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			queue->skb = NULL;
 		}
 
-next_frame:
 		queue->rx_buff[entry] = NULL;
 		continue;
 
-- 
2.53.0

[PATCH 3/6] net: macb: always use DMA_BIDIRECTIONAL on page pool buffers

[Théo Lebrun]

Creating a page pool on open requires knowing the DMA direction. We want
BIDI if an XDP program is loaded (because it might modify and XDP_TX the
buffer) and only need FROM_DEVICE otherwise.

However, we do not reopen on .ndo_bpf() to load an XDP program. This
means the DMA direction might be wrong. Options:
 - Reopen on XDP program load if no program was loaded before.
 - Always use BIDI.

Option 2 shows no reproducible performance costs on my target. It
implies an additional dma_sync_single_for_device() when page pool
buffers are recycled; see __page_pool_put_page().

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 918eee5c8a5b..05bbbaf260ce 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1582,7 +1582,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 
 		dma_sync_single_for_cpu(&bp->pdev->dev,
 					addr, bp->rx_buffer_size,
-					page_pool_get_dma_dir(queue->page_pool));
+					DMA_BIDIRECTIONAL);
 		/* Ensure ctrl is at least as up-to-date as rxused */
 		dma_rmb();
 
@@ -2867,9 +2867,7 @@ static int gem_create_page_pool(struct macb_queue *queue, int qid)
 		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
 		.pool_size = queue->bp->rx_ring_size,
 		.nid = NUMA_NO_NODE,
-		.dma_dir = rcu_access_pointer(queue->bp->prog)
-				? DMA_BIDIRECTIONAL
-				: DMA_FROM_DEVICE,
+		.dma_dir = DMA_BIDIRECTIONAL,
 		.dev = &queue->bp->pdev->dev,
 		.napi = &queue->napi_rx,
 		.max_len = PAGE_SIZE,
-- 
2.53.0

[PATCH 4/6] net: macb: account for stats in Rx XDP codepaths

[Théo Lebrun]

gem_xdp_run() returns an action. Wrt stats, we land in three different
cases:
 - Packet is handed to the stack (XDP_PASS), turns into an SKB and gets
   accounted for below in gem_rx(). No fix here.
 - Packet is dropped (XDP_DROP|ABORTED), we must increment the dropped
   counter. Missing; add it.
 - Packet is passed along (XDP_TX|REDIRECT), we must increment bytes &
   packets counters. Missing; add it.

Along the way, use local variables to store rx_bytes, rx_packets and
rx_dropped. Then increase stats only once at the end of gem_rx(). This
is simpler because all three stats must modified on a per interface and
per queue basis.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 37 +++++++++++++++++-------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 05bbbaf260ce..01b4299dc34b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1548,6 +1548,7 @@ static u32 gem_xdp_run(struct macb_queue *queue, void *buff_head,
 static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		  int budget)
 {
+	unsigned int packets = 0, dropped = 0, bytes = 0;
 	struct skb_shared_info *shinfo;
 	struct macb *bp = queue->bp;
 	struct macb_dma_desc *desc;
@@ -1595,8 +1596,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		if (unlikely(!buff_head)) {
 			netdev_err(bp->dev,
 				   "inconsistent Rx descriptor chain\n");
-			bp->dev->stats.rx_dropped++;
-			queue->stats.rx_dropped++;
+			dropped++;
 			break;
 		}
 
@@ -1613,10 +1613,21 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 
 		if (first_frame && last_frame) {
 			ret = gem_xdp_run(queue, buff_head, data_len, addr);
-			if (ret == XDP_REDIRECT)
-				xdp_flush = true;
 
-			if (ret != XDP_PASS) {
+			switch (ret) {
+			case XDP_PASS:
+				break; /* fallthrough to SKB handling */
+			case XDP_ABORTED:
+			case XDP_DROP:
+				dropped++;
+				queue->rx_buff[entry] = NULL;
+				continue;
+			case XDP_REDIRECT:
+				xdp_flush = true;
+				fallthrough;
+			case XDP_TX:
+				packets++;
+				bytes += data_len;
 				queue->rx_buff[entry] = NULL;
 				continue;
 			}
@@ -1672,10 +1683,8 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 
 		/* now everything is ready for receiving packet */
 		if (last_frame) {
-			bp->dev->stats.rx_packets++;
-			queue->stats.rx_packets++;
-			bp->dev->stats.rx_bytes += queue->skb->len;
-			queue->stats.rx_bytes += queue->skb->len;
+			packets++;
+			bytes += queue->skb->len;
 
 			gem_ptp_do_rxstamp(bp, queue->skb, desc);
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
@@ -1704,11 +1713,17 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 						false);
 		}
 
-		bp->dev->stats.rx_dropped++;
-		queue->stats.rx_dropped++;
+		dropped++;
 		queue->rx_buff[entry] = NULL;
 	}
 
+	bp->dev->stats.rx_packets += packets;
+	queue->stats.rx_packets += packets;
+	bp->dev->stats.rx_dropped += dropped;
+	queue->stats.rx_dropped += dropped;
+	bp->dev->stats.rx_bytes += bytes;
+	queue->stats.rx_bytes += bytes;
+
 	if (xdp_flush)
 		xdp_do_flush();
 
-- 
2.53.0

[PATCH 5/6] net: macb: improve Rx refill error message

[Théo Lebrun]

Message is "Unable to allocate page" in case of alloc failure.
 - It does not mention the buffer direction (Rx/Tx).
 - We allocate page fragments, not pages.

Fix by replacing with "Unable to allocate rx buffer".

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 01b4299dc34b..2c7644f2215a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1319,7 +1319,7 @@ static int gem_rx_refill(struct macb_queue *queue, bool napi)
 						    gfp_alloc | __GFP_NOWARN);
 			if (!page) {
 				netdev_err(bp->dev,
-					   "Unable to allocate page\n");
+					   "Unable to allocate rx buffer\n");
 				err = -ENOMEM;
 				break;
 			}
-- 
2.53.0

[PATCH 6/6] net: macb: rework macb_tx_complete() processing loop

[Théo Lebrun]

The SKB case of macb_tx_complete() must iterate over buffers taking into
account frames might be multi-buffer. Following that, the code for XDP
got integrated into the same loop. But in XDP cases, frames must be
single buffer.

Knowing that, we reverse operations: first we check the buffer type,
then we iterate to consume all frame buffers if it is an SKB buffer.

Side-effects:

 - Statistics tx_packets and tx_bytes are now incremented for
   XDP_TX and XDP_NDO frames as well.

 - Only increment statistics once per macb_tx_complete() call rather
   than once per frame.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 71 +++++++++++-------------
 1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c7644f2215a..6c33af667405 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1202,11 +1202,10 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
 	head = queue->tx_head;
 	for (tail = queue->tx_tail; tail != head && packets < budget; tail++) {
-		void			*data = NULL;
-		struct macb_tx_buff	*tx_buff;
-		struct sk_buff		*skb;
-		struct macb_dma_desc	*desc;
-		u32			ctrl;
+		struct macb_tx_buff *tx_buff;
+		struct macb_dma_desc *desc;
+		struct sk_buff *skb;
+		u32 ctrl;
 
 		desc = macb_tx_desc(queue, tail);
 
@@ -1221,48 +1220,44 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		if (!(ctrl & MACB_BIT(TX_USED)))
 			break;
 
-		/* Process all buffers of the current transmitted frame */
-		for (;; tail++) {
-			tx_buff = macb_tx_buff(queue, tail);
+		tx_buff = macb_tx_buff(queue, tail);
 
-			if (tx_buff->type != MACB_TYPE_SKB) {
-				data = tx_buff->ptr;
-				goto unmap;
+		switch (tx_buff->type) {
+		case MACB_TYPE_SKB:
+			/* Process all buffers of the current transmitted frame */
+			while (!tx_buff->ptr) {
+				macb_tx_unmap(bp, tx_buff, budget);
+				tail++;
+				tx_buff = macb_tx_buff(queue, tail);
 			}
 
-			/* First, update TX stats if needed */
-			if (tx_buff->type == MACB_TYPE_SKB && tx_buff->ptr) {
-				data = tx_buff->ptr;
-				skb = tx_buff->ptr;
+			skb = tx_buff->ptr;
 
-				if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
-				    !ptp_one_step_sync(skb))
-					gem_ptp_do_txstamp(bp, skb, desc);
+			if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+			    !ptp_one_step_sync(skb))
+				gem_ptp_do_txstamp(bp, skb, desc);
 
-				netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
-					    macb_tx_ring_wrap(bp, tail),
-					    skb->data);
-				bp->dev->stats.tx_packets++;
-				queue->stats.tx_packets++;
-				bp->dev->stats.tx_bytes += skb->len;
-				queue->stats.tx_bytes += skb->len;
-				packets++;
-				bytes += skb->len;
-			}
+			netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
+				    macb_tx_ring_wrap(bp, tail),
+				    skb->data);
+			bytes += skb->len;
+			break;
 
-unmap:
-			/* Now we can safely release resources */
-			macb_tx_unmap(bp, tx_buff, budget);
-
-			/* skb is set only for the last buffer of the frame.
-			 * WARNING: at this point skb has been freed by
-			 * macb_tx_unmap().
-			 */
-			if (data)
-				break;
+		case MACB_TYPE_XDP_TX:
+		case MACB_TYPE_XDP_NDO:
+			bytes += tx_buff->size;
+			break;
 		}
+
+		packets++;
+		macb_tx_unmap(bp, tx_buff, budget);
 	}
 
+	bp->dev->stats.tx_packets += packets;
+	queue->stats.tx_packets += packets;
+	bp->dev->stats.tx_bytes += bytes;
+	queue->stats.tx_bytes += bytes;
+
 	netdev_tx_completed_queue(netdev_get_tx_queue(bp->dev, queue_index),
 				  packets, bytes);
 
-- 
2.53.0

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Théo Lebrun]

Hello Paolo,

My XSK series (based on top of yours) is ready.
I won't be sending it while net-next is closed, of course.

It starts with a few cleanup/rework patches that have no direct link to
XSK and touch code you add in this series. They could be appended to
your next revision or even squashed (with the right SoB &
Co-developed-by trailers) for most.

Some contain bug fixes, mostly related to stats accounting. I expect
some amount of discussion as well, mostly regarding the page pool DMA
direction.

Patches don't have the proper Fixes tags, sorry about that.
I'll send them as reply to this email.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Théo Lebrun]

On Fri Feb 13, 2026 at 5:57 PM CET, Théo Lebrun wrote:
> I'll send them as reply to this email.

Not using b4, I had to make a mistake.
Patches have been sent as a reply to the wrong email in this thread.

https://lore.kernel.org/netdev/20260213165759.225449-1-theo.lebrun@bootlin.com/

Have a nice week-end,
Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Paolo Valerio]

On 13 Feb 2026 at 05:57:17 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Hello Paolo,
>
> My XSK series (based on top of yours) is ready.
> I won't be sending it while net-next is closed, of course.
>

Nice!
Same for this series, will respin when net-next will reopen.

> It starts with a few cleanup/rework patches that have no direct link to
> XSK and touch code you add in this series. They could be appended to
> your next revision or even squashed (with the right SoB &
> Co-developed-by trailers) for most.
>
> Some contain bug fixes, mostly related to stats accounting. I expect
> some amount of discussion as well, mostly regarding the page pool DMA
> direction.
>

I quickly looked at the patches.

As we briefly discussed off-list, in my plans stats were considered as a
follow up. I intentionally kept them out of the series and to keep it
smaller.
Given you added them, I guess it's perfectly fine if you include
the patch directly in your series (unless maintainers prefer/suggest
otherwise). The same should apply for macb_tx_complete() rework.
That being said, IMO, we should account xdp stats separately instead of
including them in the generic ones.

The one about DMA_BIDIRECTIONAL is already part of this review cycle
(see bot's reply to 5/8) and already incorporated. I'm also considering
the possibility a change that make this no longer relevant anyways, but
I'm not sure as it was planned as a follow up.

Regarding the patches containing simple renames, I don't think they
require a Co-developed-by. The reasoning follows the same logic I
applied to the patch you authored in my series; while I addressed a NULL
dereference, it didn't seem significant enough to mark the entire patch
as co-developed.
The same goes for the style change patch, it doesn't change any logic,
it's just a cosmetic change that normally go through the regular review
process, but if you think it requires a separate patch, feel free to
follow up on this in your series.

Paolo

> Patches don't have the proper Fixes tags, sorry about that.
> I'll send them as reply to this email.
>
> Thanks,
>
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Théo Lebrun]

On Sat Feb 14, 2026 at 4:37 PM CET, Paolo Valerio wrote:
> On 13 Feb 2026 at 05:57:17 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>> My XSK series (based on top of yours) is ready.
>> I won't be sending it while net-next is closed, of course.
>
> Nice!
> Same for this series, will respin when net-next will reopen.
>
>> It starts with a few cleanup/rework patches that have no direct link to
>> XSK and touch code you add in this series. They could be appended to
>> your next revision or even squashed (with the right SoB &
>> Co-developed-by trailers) for most.
>>
>> Some contain bug fixes, mostly related to stats accounting. I expect
>> some amount of discussion as well, mostly regarding the page pool DMA
>> direction.
>
> I quickly looked at the patches.
>
> As we briefly discussed off-list, in my plans stats were considered as a
> follow up. I intentionally kept them out of the series and to keep it
> smaller.
> Given you added them, I guess it's perfectly fine if you include
> the patch directly in your series (unless maintainers prefer/suggest
> otherwise). The same should apply for macb_tx_complete() rework.
> That being said, IMO, we should account xdp stats separately instead of
> including them in the generic ones.

ACK so I keep those patches in my series.

  [PATCH 4/_] net: macb: account for stats in Rx XDP codepaths
  [PATCH 6/_] net: macb: rework macb_tx_complete() processing loop

If my diff related to statistics grows bigger, it'll deserve its own
series. Until then I'll keep it part of XSK.

> The one about DMA_BIDIRECTIONAL is already part of this review cycle
> (see bot's reply to 5/8) and already incorporated. I'm also considering
> the possibility a change that make this no longer relevant anyways, but
> I'm not sure as it was planned as a follow up.

Yes indeed, Jakub's LLM pointed it out. I looked into this for a bit and
couldn't find any good solution. In the end I couldn't find any
measurable performance improvement so no need to worry about it (on my
platform). I guess the only valid option is to reopen
if `running && (!!old_prog != !!new_prog)`?

  [PATCH 3/_] net: macb: always use DMA_BIDIRECTIONAL on page pool buffers

> Regarding the patches containing simple renames, I don't think they
> require a Co-developed-by. The reasoning follows the same logic I
> applied to the patch you authored in my series; while I addressed a NULL
> dereference, it didn't seem significant enough to mark the entire patch
> as co-developed.
> The same goes for the style change patch, it doesn't change any logic,
> it's just a cosmetic change that normally go through the regular review
> process, but if you think it requires a separate patch, feel free to
> follow up on this in your series.

Yes for sure! Squash the remaining patches as if they were ordinary
review messages. It was simpler for me to send those as patches along
the other three.

  [PATCH 1/_] net: macb: rename release_buff() -> macb_tx_release_buff()
  [PATCH 2/_] net: macb: drop two labels in gem_rx()
  [PATCH 5/_] net: macb: improve Rx refill error message

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Paolo Valerio]

On 16 Feb 2026 at 10:17:39 AM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> On Sat Feb 14, 2026 at 4:37 PM CET, Paolo Valerio wrote:
>> On 13 Feb 2026 at 05:57:17 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>>> My XSK series (based on top of yours) is ready.
>>> I won't be sending it while net-next is closed, of course.
>>
>> Nice!
>> Same for this series, will respin when net-next will reopen.
>>
>>> It starts with a few cleanup/rework patches that have no direct link to
>>> XSK and touch code you add in this series. They could be appended to
>>> your next revision or even squashed (with the right SoB &
>>> Co-developed-by trailers) for most.
>>>
>>> Some contain bug fixes, mostly related to stats accounting. I expect
>>> some amount of discussion as well, mostly regarding the page pool DMA
>>> direction.
>>
>> I quickly looked at the patches.
>>
>> As we briefly discussed off-list, in my plans stats were considered as a
>> follow up. I intentionally kept them out of the series and to keep it
>> smaller.
>> Given you added them, I guess it's perfectly fine if you include
>> the patch directly in your series (unless maintainers prefer/suggest
>> otherwise). The same should apply for macb_tx_complete() rework.
>> That being said, IMO, we should account xdp stats separately instead of
>> including them in the generic ones.
>
> ACK so I keep those patches in my series.
>
>   [PATCH 4/_] net: macb: account for stats in Rx XDP codepaths
>   [PATCH 6/_] net: macb: rework macb_tx_complete() processing loop
>
> If my diff related to statistics grows bigger, it'll deserve its own
> series. Until then I'll keep it part of XSK.
>
>> The one about DMA_BIDIRECTIONAL is already part of this review cycle
>> (see bot's reply to 5/8) and already incorporated. I'm also considering
>> the possibility a change that make this no longer relevant anyways, but
>> I'm not sure as it was planned as a follow up.
>
> Yes indeed, Jakub's LLM pointed it out. I looked into this for a bit and
> couldn't find any good solution. In the end I couldn't find any
> measurable performance improvement so no need to worry about it (on my
> platform). I guess the only valid option is to reopen
> if `running && (!!old_prog != !!new_prog)`?
>

yeah, which I guess is fine, after all.
Also, this way at some point we may even consider to remove the xdp
headroom for the skb case and reserve less like NET_SKB_PAD. This would
have the extra bonus to not require a full page for 1500 mtu with 4k
pages.

>   [PATCH 3/_] net: macb: always use DMA_BIDIRECTIONAL on page pool buffers
>
>> Regarding the patches containing simple renames, I don't think they
>> require a Co-developed-by. The reasoning follows the same logic I
>> applied to the patch you authored in my series; while I addressed a NULL
>> dereference, it didn't seem significant enough to mark the entire patch
>> as co-developed.
>> The same goes for the style change patch, it doesn't change any logic,
>> it's just a cosmetic change that normally go through the regular review
>> process, but if you think it requires a separate patch, feel free to
>> follow up on this in your series.
>
> Yes for sure! Squash the remaining patches as if they were ordinary
> review messages. It was simpler for me to send those as patches along
> the other three.
>
>   [PATCH 1/_] net: macb: rename release_buff() -> macb_tx_release_buff()
>   [PATCH 2/_] net: macb: drop two labels in gem_rx()
>   [PATCH 5/_] net: macb: improve Rx refill error message
>
> Thanks,
>
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH net-next 0/8] net: macb: Add XDP support and page pool integration

[Théo Lebrun]

On Thu Feb 19, 2026 at 7:05 PM CET, Paolo Valerio wrote:
> On 16 Feb 2026 at 10:17:39 AM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>> On Sat Feb 14, 2026 at 4:37 PM CET, Paolo Valerio wrote:
>>> On 13 Feb 2026 at 05:57:17 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>>> The one about DMA_BIDIRECTIONAL is already part of this review cycle
>>> (see bot's reply to 5/8) and already incorporated. I'm also considering
>>> the possibility a change that make this no longer relevant anyways, but
>>> I'm not sure as it was planned as a follow up.
>>
>> Yes indeed, Jakub's LLM pointed it out. I looked into this for a bit and
>> couldn't find any good solution. In the end I couldn't find any
>> measurable performance improvement so no need to worry about it (on my
>> platform). I guess the only valid option is to reopen
>> if `running && (!!old_prog != !!new_prog)`?
>
> yeah, which I guess is fine, after all.
> Also, this way at some point we may even consider to remove the xdp
> headroom for the skb case and reserve less like NET_SKB_PAD. This would
> have the extra bonus to not require a full page for 1500 mtu with 4k
> pages.

I will be adding a close/open cycle with the XSK pool introduction.
That is why in my branch I preferred to lean in the direction of not
closing/reopening in the XDP program load (to avoid 2x reopens on AF_XDP
zero-copy). I get your point though.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com