Thread (20 messages) 20 messages, 5 authors, 2026-06-17

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

From: Simon Horman <horms@kernel.org>
Date: 2026-03-25 17:45:32

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
---
net: macb: Add page pool support handle multi-descriptor frame rx

This commit adds page pool support for the Cadence MACB/GEM Ethernet
driver and enables multi-descriptor frame reception. It replaces
netdev_alloc_skb() with page pool allocation and removes the limitation
that frames must fit in a single descriptor.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index fca7b9c6b670..84989ff0c3a9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2600,12 +2700,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 = MIN(size, RX_BUFFER_MAX);
+		size = mtu + ETH_HLEN + ETH_FCS_LEN;
+		bp->rx_buffer_size = SKB_DATA_ALIGN(size + bp->rx_ip_align);
+		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,
Can this overflow the hardware register field? The old code clamped size
against RX_BUFFER_MAX (16320), but the new code only guards against
PAGE_SIZE. Looking at the RXBS field in the hardware (GEM_RXBS_SIZE=8 bits
at macb.h:356), it can only represent values 0-255, encoding buffer sizes
0-16320 (value * RX_BUFFER_MULTIPLE where RX_BUFFER_MULTIPLE=64).

On systems with large page sizes (e.g., ARM64 with CONFIG_ARM64_64K_PAGES),
the PAGE_SIZE guard doesn't trigger for jumbo MTU values. If rx_buffer_size
reaches 16384 (SKB_DATA_ALIGN(16320+2)), then buffer_size = 16384/64 = 256,
which overflows the 8-bit field to 0 when written via GEM_BF(RXBS,
buffer_size) in macb_configure_dma():

drivers/net/ethernet/cadence/macb_main.c:macb_configure_dma() {
	buffer_size = bp->rx_buffer_size / RX_BUFFER_MULTIPLE;
	...
	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);
	}
}

This would cause the hardware to use an incorrect (zero) buffer size.

Should the code clamp rx_buffer_size to ensure buffer_size stays within
0-255 after the division?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help