[PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[Nicolai Buchwitz]

Since the page_pool conversion every RX buffer is a whole page and the
skb truesize is the page, although the hardware writes at most 2 KiB of
it. On 64 KiB pages a packet therefore counts 65792 bytes against the
socket buffer where it used to count 2752. As a result a UDP socket
with the default buffer starts to drop after three packets, and the RX
rings pin 16 MiB for 512 KiB of buffers.

Fix this and allocate the buffers as page fragments, so the truesize is
what a packet occupies. 4 KiB pages stay one buffer per page.

Sync each buffer in the refill path, as page_pool can only sync a whole
page on recycle. On 4 KiB pages that is twice what the hardware wrote.

Fixes: 7bc054c2d4ed ("net: bcmgenet: convert RX path to page_pool")
Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
Closes: https://lore.kernel.org/all/20260924065839.56793-1-kmehltretter@gmail.com/
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 .../net/ethernet/broadcom/genet/bcmgenet.c    | 29 ++++++++++++++-----
 .../net/ethernet/broadcom/genet/bcmgenet.h    |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 21668e41b696..f725d26e6020 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -57,6 +57,9 @@
  */
 #define GENET_RSB_PAD		(sizeof(struct status_64) + 2)
 
+/* RX buffer plus the skb_shared_info napi_build_skb() places behind it */
+#define GENET_RX_BUF_SIZE	SKB_HEAD_ALIGN(RX_BUF_LENGTH)
+
 /* Tx/Rx DMA register offset, skip 256 descriptors */
 #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
 #define DMA_DESC_SIZE		(WORDS_PER_BD(priv) * sizeof(u32))
@@ -2254,11 +2257,12 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
 			      struct enet_cb *cb)
 {
 	struct bcmgenet_priv *priv = ring->priv;
+	unsigned int size = GENET_RX_BUF_SIZE;
+	unsigned int offset;
 	dma_addr_t mapping;
 	struct page *page;
 
-	page = page_pool_alloc_pages(ring->page_pool,
-				     GFP_ATOMIC);
+	page = page_pool_dev_alloc(ring->page_pool, &offset, &size);
 	if (!page) {
 		priv->mib.alloc_rx_buff_failed++;
 		netif_err(priv, rx_err, priv->dev,
@@ -2267,9 +2271,13 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
 	}
 
 	/* page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
-	mapping = page_pool_get_dma_addr(page);
+	mapping = page_pool_get_dma_addr(page) + offset;
+	dma_sync_single_for_device(&priv->pdev->dev, mapping, RX_BUF_LENGTH,
+				   DMA_FROM_DEVICE);
 
 	cb->rx_page = page;
+	cb->rx_offset = offset;
+	cb->rx_size = size;
 	dmadesc_set_addr(priv, cb->bd_addr, mapping);
 
 	return 0;
@@ -2323,6 +2331,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 
 	while ((rxpktprocessed < rxpkttoprocess) &&
 	       (rxpktprocessed < budget)) {
+		unsigned int rx_offset, rx_size;
 		struct status_64 *status;
 		struct page *rx_page;
 		void *hard_start;
@@ -2332,6 +2341,8 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 
 		/* Save the received page before refilling */
 		rx_page = cb->rx_page;
+		rx_offset = cb->rx_offset;
+		rx_size = cb->rx_size;
 
 		if (bcmgenet_rx_refill(ring, cb)) {
 			BCMGENET_STATS64_INC(stats, dropped);
@@ -2341,10 +2352,10 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 		/* Sync the full buffer; the HW may have written anywhere
 		 * up to RX_BUF_LENGTH.
 		 */
-		page_pool_dma_sync_for_cpu(ring->page_pool, rx_page, 0,
+		page_pool_dma_sync_for_cpu(ring->page_pool, rx_page, rx_offset,
 					   RX_BUF_LENGTH);
 
-		hard_start = page_address(rx_page);
+		hard_start = page_address(rx_page) + rx_offset;
 		status = (struct status_64 *)hard_start;
 		dma_length_status = status->length_status;
 
@@ -2410,7 +2421,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 		/* Build SKB from the page - data starts at hard_start,
 		 * frame begins after RSB(64) + pad(2) = 66 bytes.
 		 */
-		skb = napi_build_skb(hard_start, PAGE_SIZE);
+		skb = napi_build_skb(hard_start, rx_size);
 		if (unlikely(!skb)) {
 			BCMGENET_STATS64_INC(stats, dropped);
 			page_pool_put_full_page(ring->page_pool, rx_page,
@@ -2762,14 +2773,16 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 static int bcmgenet_rx_ring_create_pool(struct bcmgenet_priv *priv,
 					struct bcmgenet_rx_ring *ring)
 {
+	/* Buffers share a page. bcmgenet_rx_refill() syncs each one for the
+	 * device, PP_FLAG_DMA_SYNC_DEV would sync the whole page.
+	 */
 	struct page_pool_params pp_params = {
 		.order = 0,
-		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+		.flags = PP_FLAG_DMA_MAP,
 		.pool_size = ring->size,
 		.nid = NUMA_NO_NODE,
 		.dev = &priv->pdev->dev,
 		.dma_dir = DMA_FROM_DEVICE,
-		.max_len = RX_BUF_LENGTH,
 	};
 	int err;
 
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 22a958ba9902..86f2aed20dbe 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -470,6 +470,8 @@ struct bcmgenet_rx_stats64 {
 struct enet_cb {
 	struct sk_buff      *skb;
 	struct page         *rx_page;
+	unsigned int        rx_offset;
+	unsigned int        rx_size;
 	void __iomem *bd_addr;
 	DEFINE_DMA_UNMAP_ADDR(dma_addr);
 	DEFINE_DMA_UNMAP_LEN(dma_len);
-- 
2.53.0

Re: [PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[Simon Horman]

On Thu, Sep 24, 2026 at 12:19:21PM +0200, Nicolai Buchwitz wrote:
> Since the page_pool conversion every RX buffer is a whole page and the
> skb truesize is the page, although the hardware writes at most 2 KiB of
> it. On 64 KiB pages a packet therefore counts 65792 bytes against the
> socket buffer where it used to count 2752. As a result a UDP socket
> with the default buffer starts to drop after three packets, and the RX
> rings pin 16 MiB for 512 KiB of buffers.
> 
> Fix this and allocate the buffers as page fragments, so the truesize is
> what a packet occupies. 4 KiB pages stay one buffer per page.
> 
> Sync each buffer in the refill path, as page_pool can only sync a whole
> page on recycle. On 4 KiB pages that is twice what the hardware wrote.
> 
> Fixes: 7bc054c2d4ed ("net: bcmgenet: convert RX path to page_pool")
> Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
> Closes: https://lore.kernel.org/all/20260924065839.56793-1-kmehltretter@gmail.com/
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

...

> @@ -2254,11 +2257,12 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
>  			      struct enet_cb *cb)
>  {
>  	struct bcmgenet_priv *priv = ring->priv;
> +	unsigned int size = GENET_RX_BUF_SIZE;
> +	unsigned int offset;
>  	dma_addr_t mapping;
>  	struct page *page;
>  
> -	page = page_pool_alloc_pages(ring->page_pool,
> -				     GFP_ATOMIC);
> +	page = page_pool_dev_alloc(ring->page_pool, &offset, &size);

I think it is not so important either way, but I'm wondering
if you considered using page_pool_alloc_frag() here.

>  	if (!page) {
>  		priv->mib.alloc_rx_buff_failed++;
>  		netif_err(priv, rx_err, priv->dev,

...

Re: [PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[Nicolai Buchwitz]

Hi Simon

On 25.9.2026 15:47, Simon Horman wrote:
> On Thu, Sep 24, 2026 at 12:19:21PM +0200, Nicolai Buchwitz wrote:
>> Since the page_pool conversion every RX buffer is a whole page and the
>> skb truesize is the page, although the hardware writes at most 2 KiB 
>> of
>> it. On 64 KiB pages a packet therefore counts 65792 bytes against the
>> socket buffer where it used to count 2752. As a result a UDP socket
>> with the default buffer starts to drop after three packets, and the RX
>> rings pin 16 MiB for 512 KiB of buffers.
>> 
>> Fix this and allocate the buffers as page fragments, so the truesize 
>> is
>> what a packet occupies. 4 KiB pages stay one buffer per page.
>> 
>> Sync each buffer in the refill path, as page_pool can only sync a 
>> whole
>> page on recycle. On 4 KiB pages that is twice what the hardware wrote.
>> 
>> Fixes: 7bc054c2d4ed ("net: bcmgenet: convert RX path to page_pool")
>> Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
>> Closes: 
>> https://lore.kernel.org/all/20260924065839.56793-1-kmehltretter@gmail.com/
>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> 
> ...
> 
>> @@ -2254,11 +2257,12 @@ static int bcmgenet_rx_refill(struct 
>> bcmgenet_rx_ring *ring,
>>  			      struct enet_cb *cb)
>>  {
>>  	struct bcmgenet_priv *priv = ring->priv;
>> +	unsigned int size = GENET_RX_BUF_SIZE;
>> +	unsigned int offset;
>>  	dma_addr_t mapping;
>>  	struct page *page;
>> 
>> -	page = page_pool_alloc_pages(ring->page_pool,
>> -				     GFP_ATOMIC);
>> +	page = page_pool_dev_alloc(ring->page_pool, &offset, &size);
> 
> I think it is not so important either way, but I'm wondering
> if you considered using page_pool_alloc_frag() here.

Yes, I did, but page_pool_alloc() seemed a better fit as it hands back 
the usable
size in *size. Also it falls back to a whole page above half a page and 
avoids an
underestimate at the end of a page.

> 
>>  	if (!page) {
>>  		priv->mib.alloc_rx_buff_failed++;
>>  		netif_err(priv, rx_err, priv->dev,
> 
> ...

Thanks,
Nicolai

Re: [PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[Simon Horman]

On Fri, Sep 25, 2026 at 03:59:57PM +0200, Nicolai Buchwitz wrote:
> Hi Simon
> 
> On 25.9.2026 15:47, Simon Horman wrote:
> > On Thu, Sep 24, 2026 at 12:19:21PM +0200, Nicolai Buchwitz wrote:
> > > Since the page_pool conversion every RX buffer is a whole page and the
> > > skb truesize is the page, although the hardware writes at most 2 KiB
> > > of
> > > it. On 64 KiB pages a packet therefore counts 65792 bytes against the
> > > socket buffer where it used to count 2752. As a result a UDP socket
> > > with the default buffer starts to drop after three packets, and the RX
> > > rings pin 16 MiB for 512 KiB of buffers.
> > > 
> > > Fix this and allocate the buffers as page fragments, so the truesize
> > > is
> > > what a packet occupies. 4 KiB pages stay one buffer per page.
> > > 
> > > Sync each buffer in the refill path, as page_pool can only sync a
> > > whole
> > > page on recycle. On 4 KiB pages that is twice what the hardware wrote.
> > > 
> > > Fixes: 7bc054c2d4ed ("net: bcmgenet: convert RX path to page_pool")
> > > Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
> > > Closes: https://lore.kernel.org/all/20260924065839.56793-1-kmehltretter@gmail.com/
> > > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> > 
> > ...
> > 
> > > @@ -2254,11 +2257,12 @@ static int bcmgenet_rx_refill(struct
> > > bcmgenet_rx_ring *ring,
> > >  			      struct enet_cb *cb)
> > >  {
> > >  	struct bcmgenet_priv *priv = ring->priv;
> > > +	unsigned int size = GENET_RX_BUF_SIZE;
> > > +	unsigned int offset;
> > >  	dma_addr_t mapping;
> > >  	struct page *page;
> > > 
> > > -	page = page_pool_alloc_pages(ring->page_pool,
> > > -				     GFP_ATOMIC);
> > > +	page = page_pool_dev_alloc(ring->page_pool, &offset, &size);
> > 
> > I think it is not so important either way, but I'm wondering
> > if you considered using page_pool_alloc_frag() here.
> 
> Yes, I did, but page_pool_alloc() seemed a better fit as it hands back
> the usable size in *size. Also it falls back to a whole page above half a
> page and avoids an underestimate at the end of a page.

Thanks for the clarification.
All looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net] net: bcmgenet: allocate RX buffers as page fragments

[Karl Mehltretter]

On Thu, Sep 24, 2026 at 12:19:21PM +0100, Nicolai Buchwitz wrote:
> Fix this and allocate the buffers as page fragments, so the truesize is
> what a packet occupies. 4 KiB pages stay one buffer per page.
> 

Tested-by: Karl Mehltretter <kmehltretter@gmail.com>

I applied this unchanged to current net at 11536ee3d3e0, current net-next at
42a9fb3382fc, and after patches 129 and 130 from 6.18.54-rc1. I tested each
with a 64 KiB arm64 kernel in a custom QEMU Pi 400 model with GENET v5 DMA.

My LLM agent helped me running these tests.

Results with a 524,288-byte receive buffer (repeated over five rounds):

                             Unpatched net   Patched net/net-next
Charge per 512-byte UDP      65,792 bytes    2,624 bytes
Datagrams queued (of 1,000)  7               195
Socket drops                 993             805

Thanks,
Karl