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

[Nicolai Buchwitz <nb@tipi-net.de>]

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