[PATCH net-next 0/8] net: macb: add XSK support

[PATCH net-next 0/8] net: macb: add XSK support

[Théo Lebrun]

Add XSK support to the MACB/GEM driver.
Tested on Mobileye EyeQ5 (MIPS) evaluation board.
Applies on top of net-next (4ad96a7c9e2c) and Paolo's XDP work [0].

I don't have good Rx benchmark numbers yet, sorry, mostly because of
userspace tooling issues around eBPF/XDP and MIPS. In copy mode it only
means slowdowns, but in zero-copy, as we work with a fixed amount of
buffers, it causes allocation errors.

--

The bulk of the work is dealing with a second allocator. Throughout, we
now use queue->page_pool or queue->xsk_pool. The former gives us raw
buffers which we need to wrap inside xdp_buff and the latter allocates
xdp_buff, meaning less work.

To simplify the implementation, attaching an XSK pool implies closing
and reopening the interface. It could be improved over time as
currently attaching AF_XDP socket in zero-copy mode means we
close/reopen twice: once for the XDP program, once for the XSK pool.

First three patches are cleanup.

   [PATCH net-next 1/8] net: macb: make rx error messages rate-limited
   [PATCH net-next 2/8] net: macb: account for stats in Rx XDP codepaths
   [PATCH net-next 3/8] net: macb: account for stats in Tx XDP codepaths

Then comes preparation work.

   [PATCH net-next 4/8] net: macb: drop handling of recycled buffers in gem_rx_refill()
   [PATCH net-next 5/8] net: macb: move macb_xdp_submit_frame() body to helper function

And finally the XSK codepaths.

   [PATCH net-next 6/8] net: macb: add infrastructure for XSK buffer pool
   [PATCH net-next 7/8] net: macb: add Rx zero-copy AF_XDP support
   [PATCH net-next 8/8] net: macb: add Tx zero-copy AF_XDP support

Thanks,
Have a nice day,
Théo

[0]: https://lore.kernel.org/netdev/20260302115232.1430640-1-pvalerio@redhat.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Théo Lebrun (8):
      net: macb: make rx error messages rate-limited
      net: macb: account for stats in Rx XDP codepaths
      net: macb: account for stats in Tx XDP codepaths
      net: macb: drop handling of recycled buffers in gem_rx_refill()
      net: macb: move macb_xdp_submit_frame() body to helper function
      net: macb: add infrastructure for XSK buffer pool
      net: macb: add Rx zero-copy AF_XDP support
      net: macb: add Tx zero-copy AF_XDP support

 drivers/net/ethernet/cadence/macb.h      |   2 +
 drivers/net/ethernet/cadence/macb_main.c | 668 +++++++++++++++++++++----------
 2 files changed, 468 insertions(+), 202 deletions(-)
---
base-commit: 06d25a140f34f5879d0731117d4d62a7dd3824a9
change-id: 20260225-macb-xsk-452c0c802436

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH net-next 1/8] net: macb: make rx error messages rate-limited

[Théo Lebrun]

If Rx codepath error messages trigger, they do not interrupt reception.
Kernel log gets spammed, we lose useful history and everything crawls
to a halt. Instead, make them rate-limited to keep old useful
information in the log and keep the system responsive.

No netdev_*_ratelimited() variants exist so we switch to dev_*().

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a79daad275ba..ab73d1a522c2 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1303,11 +1303,12 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 static inline int gem_rx_data_len(struct macb *bp, struct macb_queue *queue,
 				  u32 desc_ctrl, bool rx_sof, bool rx_eof)
 {
+	struct device *dev = &bp->pdev->dev;
 	int len;
 
 	if (unlikely(!rx_sof && !queue->skb)) {
-		netdev_err(bp->dev,
-			   "Received non-starting frame while expecting a starting one\n");
+		dev_err_ratelimited(dev,
+				    "Received non-starting frame while expecting a starting one\n");
 		return -1;
 	}
 
@@ -1322,7 +1323,7 @@ static inline int gem_rx_data_len(struct macb *bp, struct macb_queue *queue,
 
 	if (rx_eof && !rx_sof) {
 		if (unlikely(queue->skb->len > len)) {
-			netdev_err(bp->dev, "Unexpected frame len: %d\n", len);
+			dev_err_ratelimited(dev, "Unexpected frame len: %d\n", len);
 			return -1;
 		}
 
@@ -1382,8 +1383,8 @@ static int gem_rx_refill(struct macb_queue *queue, bool napi)
 						    gem_total_rx_buffer_size(bp),
 						    gfp_alloc | __GFP_NOWARN);
 			if (!page) {
-				netdev_err(bp->dev,
-					   "Unable to allocate rx buffer\n");
+				dev_err_ratelimited(&bp->pdev->dev,
+						    "Unable to allocate rx buffer\n");
 				err = -ENOMEM;
 				break;
 			}
@@ -1666,8 +1667,8 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 
 		buff_head = queue->rx_buff[entry];
 		if (unlikely(!buff_head)) {
-			netdev_err(bp->dev,
-				   "inconsistent Rx descriptor chain\n");
+			dev_err_ratelimited(&bp->pdev->dev,
+					    "inconsistent Rx descriptor chain\n");
 			bp->dev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;

-- 
2.53.0

[PATCH net-next 2/8] 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 | 47 +++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ab73d1a522c2..1aa90499343a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1627,6 +1627,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;
@@ -1669,8 +1670,7 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		if (unlikely(!buff_head)) {
 			dev_err_ratelimited(&bp->pdev->dev,
 					    "inconsistent Rx descriptor chain\n");
-			bp->dev->stats.rx_dropped++;
-			queue->stats.rx_dropped++;
+			dropped++;
 			break;
 		}
 
@@ -1700,11 +1700,29 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			if (last_frame) {
 				ret = gem_xdp_run(queue, buff_head, &data_len,
 						  &headroom, addr - gem_rx_pad(bp));
-				if (ret == XDP_REDIRECT)
-					xdp_flush = true;
 
-				if (ret != XDP_PASS)
-					goto next_frame;
+				switch (ret) {
+				/* continue to SKB handling codepath */
+				case XDP_PASS:
+					break;
+
+				/* dropped packet cases */
+				case XDP_ABORTED:
+				case XDP_DROP:
+					dropped++;
+					queue->rx_buff[entry] = NULL;
+					continue;
+
+				/* redirect/tx cases */
+				case XDP_REDIRECT:
+					xdp_flush = true;
+					fallthrough;
+				case XDP_TX:
+					packets++;
+					bytes += data_len;
+					queue->rx_buff[entry] = NULL;
+					continue;
+				}
 			}
 
 			queue->skb = napi_build_skb(buff_head, gem_total_rx_buffer_size(bp));
@@ -1743,10 +1761,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;
 
 			queue->skb->protocol = eth_type_trans(queue->skb, bp->dev);
 			skb_checksum_none_assert(queue->skb);
@@ -1769,7 +1785,6 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 			queue->skb = NULL;
 		}
 
-next_frame:
 		queue->rx_buff[entry] = NULL;
 		continue;
 
@@ -1784,11 +1799,17 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 						virt_to_head_page(buff_head),
 						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 net-next 3/8] net: macb: account for stats in Tx XDP codepaths

[Théo Lebrun]

macb_tx_complete() processing loop assumes a packet is composed of
multiple frames and composes around this idea. However, this is only
true in the SKB case ie `tx_buff->type == MACB_TYPE_SKB`.

Rework macb_tx_complete() to bring the tx_buff->type switch statement
outside and the frame iteration loop now lives only inside the SKB
case.

Fix Tx XDP stats that were not accounted for, in the XDP_TX|NDO cases.
Only increment statistics once per macb_tx_complete() call rather than
once per frame.

The `bytes` and `packets` stack variables now gets incremented for
completed XDP XMIT/TX packets. This implies the DQL subsystem through
netdev_tx_completed_queue() now gets notified of those packets
completing. We must therefore also report those bytes as sent, using
netdev_tx_sent_queue(), in macb_xdp_submit_frame() called by:
 - Rx XDP programs returning action XDP_TX and,
 - the .ndo_xdp_xmit() callback.

Incrementing `packets` also implies XDP packets are accounted for in our
NAPI budget calculation.

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 1aa90499343a..c1677f1d8f23 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1212,7 +1212,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 {
 	struct macb *bp = queue->bp;
 	unsigned long flags;
-	int skb_packets = 0;
+	int xsk_frames = 0;
 	unsigned int tail;
 	unsigned int head;
 	u16 queue_index;
@@ -1227,7 +1227,6 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		struct macb_tx_buff *tx_buff;
 		struct macb_dma_desc *desc;
 		struct sk_buff *skb;
-		void *data = NULL;
 		u32 ctrl;
 
 		desc = macb_tx_desc(queue, tail);
@@ -1243,52 +1242,46 @@ 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;
-				packets++;
-				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->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;
-				skb_packets++;
-				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);
-
-			/* data is set only for the last buffer of the frame.
-			 * WARNING: at this point the buffer 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),
-				  skb_packets, bytes);
+				  packets, bytes);
 
 	queue->tx_tail = tail;
 	if (__netif_subqueue_stopped(bp->dev, queue_index) &&
@@ -1529,6 +1522,8 @@ static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
 	spin_unlock(&bp->lock);
 
+	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index), xdpf->len);
+
 	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1)
 		netif_stop_subqueue(dev, queue_index);
 

-- 
2.53.0

[PATCH net-next 4/8] net: macb: drop handling of recycled buffers in gem_rx_refill()

[Théo Lebrun]

The refill operation supports detecting if a buffer is present in a
slot; if it is, then it updates its DMA descriptor reusing the same
buffer.

This behavior can be dropped; all codepaths of gem_rx() letting a buffer
lay around to be reused by refill have disappeared. Said another way:
every time queue->tx_tail is incremented, queue->rx_buff[entry] is set
to NULL.

On the same occasion, move `gfp_alloc` assignment out of the loop and
into variable declarations. Its value is constant across the function's
lifetime. Also fix tiny alignment issue with the while statement.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c1677f1d8f23..ed94f9f0894b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1351,18 +1351,18 @@ static unsigned int gem_total_rx_buffer_size(struct macb *bp)
 
 static int gem_rx_refill(struct macb_queue *queue, bool napi)
 {
+	gfp_t gfp_alloc = napi ? GFP_ATOMIC : GFP_KERNEL;
 	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) {
+			  bp->rx_ring_size) > 0) {
 		entry = macb_rx_ring_wrap(bp, queue->rx_prepared_head);
 
 		/* Make hw descriptor updates visible to CPU */
@@ -1370,41 +1370,33 @@ static int gem_rx_refill(struct macb_queue *queue, bool napi)
 
 		desc = macb_rx_desc(queue, entry);
 
-		if (!queue->rx_buff[entry]) {
-			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) {
-				dev_err_ratelimited(&bp->pdev->dev,
-						    "Unable to allocate rx buffer\n");
-				err = -ENOMEM;
-				break;
-			}
-
-			paddr = page_pool_get_dma_addr(page) +
-				gem_rx_pad(bp) + offset;
-
-			dma_sync_single_for_device(&bp->pdev->dev,
-						   paddr, bp->rx_buffer_size,
-						   page_pool_get_dma_dir(queue->page_pool));
-
-			data = page_address(page) + offset;
-			queue->rx_buff[entry] = data;
-
-			if (entry == bp->rx_ring_size - 1)
-				paddr |= MACB_BIT(RX_WRAP);
-			desc->ctrl = 0;
-			/* Setting addr clears RX_USED and allows reception,
-			 * make sure ctrl is cleared first to avoid a race.
-			 */
-			dma_wmb();
-			macb_set_addr(bp, desc, paddr);
-		} else {
-			desc->ctrl = 0;
-			dma_wmb();
-			desc->addr &= ~MACB_BIT(RX_USED);
+		page = page_pool_alloc_frag(queue->page_pool, &offset,
+					    gem_total_rx_buffer_size(bp),
+					    gfp_alloc | __GFP_NOWARN);
+		if (!page) {
+			dev_err_ratelimited(&bp->pdev->dev,
+					    "Unable to allocate rx buffer\n");
+			err = -ENOMEM;
+			break;
 		}
+
+		paddr = page_pool_get_dma_addr(page) + gem_rx_pad(bp) + offset;
+
+		dma_sync_single_for_device(&bp->pdev->dev,
+					   paddr, bp->rx_buffer_size,
+					   page_pool_get_dma_dir(queue->page_pool));
+
+		data = page_address(page) + offset;
+		queue->rx_buff[entry] = data;
+
+		if (entry == bp->rx_ring_size - 1)
+			paddr |= MACB_BIT(RX_WRAP);
+		desc->ctrl = 0;
+		/* Setting addr clears RX_USED and allows reception,
+		 * make sure ctrl is cleared first to avoid a race.
+		 */
+		dma_wmb();
+		macb_set_addr(bp, desc, paddr);
 		queue->rx_prepared_head++;
 	}
 

-- 
2.53.0

[PATCH net-next 5/8] net: macb: move macb_xdp_submit_frame() body to helper function

[Théo Lebrun]

Part of macb_xdp_submit_frame() is specific to the handling of an XDP
buffer (pick a queue for emission, DMA map or sync, report emitted
bytes), part is chitchat with hardware to update DMA descriptor and
start transmit.

Move the hardware specific code out of macb_xdp_submit_frame() into a
macb_xdp_submit_buff() helper function. The goal is to make code
reusable to support XSK buffers.

The macb_xdp_submit_frame() body is modified slightly: we bring the
dma_map_single() call outside of the queue->tx_ptr_lock critical
section, to minimise its span.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ed94f9f0894b..65c2ec2a843c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1208,6 +1208,52 @@ static bool ptp_one_step_sync(struct sk_buff *skb)
 	return false;
 }
 
+static void macb_xdp_submit_buff(struct macb *bp, unsigned int queue_index,
+				 struct macb_tx_buff buff)
+{
+	struct macb_queue *queue = &bp->queues[queue_index];
+	struct net_device *netdev = bp->dev;
+	struct macb_tx_buff *tx_buff;
+	struct macb_dma_desc *desc;
+	unsigned int next_head;
+	u32 ctrl;
+
+	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 = buff;
+
+	ctrl = (u32)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, 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(netdev, queue_index);
+}
+
 static int macb_tx_complete(struct macb_queue *queue, int budget)
 {
 	struct macb *bp = queue->bp;
@@ -1430,44 +1476,25 @@ 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,
+				 struct net_device *netdev, bool dma_map,
 				 dma_addr_t addr)
 {
+	struct device *dev = &bp->pdev->dev;
 	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;
-		}
+		mapping = dma_map_single(dev, xdpf->data, xdpf->len, DMA_TO_DEVICE);
+		err = dma_mapping_error(&bp->pdev->dev, mapping);
+		if (unlikely(err))
+			return err;
+
+		buff_type = MACB_TYPE_XDP_NDO;
 	} else {
 		/* progs can adjust the head. Sync and set the adjusted one.
 		 * This also implicitly takes into account ip alignment,
@@ -1476,52 +1503,38 @@ static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
 		mapping = addr + xdpf->headroom + sizeof(*xdpf);
 		dma_sync_single_for_device(&bp->pdev->dev, mapping,
 					   xdpf->len, DMA_BIDIRECTIONAL);
+
+		buff_type = MACB_TYPE_XDP_TX;
 	}
 
-	next_head = queue->tx_head + 1;
+	queue_index = cpu % bp->num_queues;
+	queue = &bp->queues[queue_index];
 
-	ctrl = MACB_BIT(TX_USED);
-	desc = macb_tx_desc(queue, next_head);
-	desc->ctrl = ctrl;
+	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
 
-	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 = dma_map ? mapping : 0;
-	tx_buff->size = xdpf->len;
-	tx_buff->mapped_as_page = false;
+	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1) {
+		/* This is a hard error, log it. */
+		netif_stop_subqueue(netdev, queue_index);
+		netdev_dbg(netdev, "tx_head = %u, tx_tail = %u\n",
+			   queue->tx_head, queue->tx_tail);
+		err = -ENOMEM;
+	} else {
+		macb_xdp_submit_buff(bp, queue_index, (struct macb_tx_buff){
+			.ptr = xdpf,
+			.mapping = dma_map ? mapping : 0,
+			.size = xdpf->len,
+			.mapped_as_page = false,
+			.type = buff_type,
+		});
 
-	ctrl = (u32)tx_buff->size;
-	ctrl |= MACB_BIT(TX_LAST);
+		netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index), xdpf->len);
+	}
 
-	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, 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);
-
-	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index), xdpf->len);
-
-	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 && dma_map)
+		dma_unmap_single(dev, mapping, xdpf->len, DMA_TO_DEVICE);
+
 	return err;
 }
 

-- 
2.53.0

[PATCH net-next 5/8] net: macb: move macb_xdp_submit_frame() body to helper function

[Théo Lebrun]

Part of macb_xdp_submit_frame() is specific to the handling of an XDP
buffer (pick a queue for emission, DMA map or sync, report emitted
bytes), part is chitchat with hardware to update DMA descriptor and
start transmit.

Move the hardware specific code out of macb_xdp_submit_frame() into a
macb_xdp_submit_buff() helper function. The goal is to make code
reusable to support XSK buffers.

The macb_xdp_submit_frame() body is modified slightly: we bring the
dma_map_single() call outside of the queue->tx_ptr_lock critical
section, to minimise its span.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ed94f9f0894b..65c2ec2a843c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1208,6 +1208,52 @@ static bool ptp_one_step_sync(struct sk_buff *skb)
 	return false;
 }
 
+static void macb_xdp_submit_buff(struct macb *bp, unsigned int queue_index,
+				 struct macb_tx_buff buff)
+{
+	struct macb_queue *queue = &bp->queues[queue_index];
+	struct net_device *netdev = bp->dev;
+	struct macb_tx_buff *tx_buff;
+	struct macb_dma_desc *desc;
+	unsigned int next_head;
+	u32 ctrl;
+
+	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 = buff;
+
+	ctrl = (u32)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, 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(netdev, queue_index);
+}
+
 static int macb_tx_complete(struct macb_queue *queue, int budget)
 {
 	struct macb *bp = queue->bp;
@@ -1430,44 +1476,25 @@ 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,
+				 struct net_device *netdev, bool dma_map,
 				 dma_addr_t addr)
 {
+	struct device *dev = &bp->pdev->dev;
 	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;
-		}
+		mapping = dma_map_single(dev, xdpf->data, xdpf->len, DMA_TO_DEVICE);
+		err = dma_mapping_error(&bp->pdev->dev, mapping);
+		if (unlikely(err))
+			return err;
+
+		buff_type = MACB_TYPE_XDP_NDO;
 	} else {
 		/* progs can adjust the head. Sync and set the adjusted one.
 		 * This also implicitly takes into account ip alignment,
@@ -1476,52 +1503,38 @@ static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
 		mapping = addr + xdpf->headroom + sizeof(*xdpf);
 		dma_sync_single_for_device(&bp->pdev->dev, mapping,
 					   xdpf->len, DMA_BIDIRECTIONAL);
+
+		buff_type = MACB_TYPE_XDP_TX;
 	}
 
-	next_head = queue->tx_head + 1;
+	queue_index = cpu % bp->num_queues;
+	queue = &bp->queues[queue_index];
 
-	ctrl = MACB_BIT(TX_USED);
-	desc = macb_tx_desc(queue, next_head);
-	desc->ctrl = ctrl;
+	spin_lock_irqsave(&queue->tx_ptr_lock, flags);
 
-	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 = dma_map ? mapping : 0;
-	tx_buff->size = xdpf->len;
-	tx_buff->mapped_as_page = false;
+	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1) {
+		/* This is a hard error, log it. */
+		netif_stop_subqueue(netdev, queue_index);
+		netdev_dbg(netdev, "tx_head = %u, tx_tail = %u\n",
+			   queue->tx_head, queue->tx_tail);
+		err = -ENOMEM;
+	} else {
+		macb_xdp_submit_buff(bp, queue_index, (struct macb_tx_buff){
+			.ptr = xdpf,
+			.mapping = dma_map ? mapping : 0,
+			.size = xdpf->len,
+			.mapped_as_page = false,
+			.type = buff_type,
+		});
 
-	ctrl = (u32)tx_buff->size;
-	ctrl |= MACB_BIT(TX_LAST);
+		netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index), xdpf->len);
+	}
 
-	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, 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);
-
-	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index), xdpf->len);
-
-	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 && dma_map)
+		dma_unmap_single(dev, mapping, xdpf->len, DMA_TO_DEVICE);
+
 	return err;
 }
 

-- 
2.53.0