Subject: Re: [PATCH net] bgmac: reduce max frame size to support just MTU 1500

From: Simon Horman <[email-masked]>
Date: Mon, 27 Jan 2025 17:22:05 +0000

On Fri, Jan 24, 2025 at 11:14:04AM -0800, Florian Fainelli wrote:
> From: Rafał Miłecki <[email-masked]>
> 
> bgmac allocates new replacement buffer before handling each received
> frame. Allocating & DMA-preparing 9724 B each time consumes a lot of CPU
> time. Ideally bgmac should just respect currently set MTU but it isn't
> the case right now. For now just revert back to the old limited frame
> size.
> 
> This change bumps NAT masquerade speed by ~95%.
> 
> Since commit 8218f62c9c9b ("mm: page_frag: use initial zero offset for
> page_frag_alloc_align()"), the bgmac driver fails to open its network
> interface successfully and runs out of memory in the following call
> stack:
> 
> bgmac_open
>   -> bgmac_dma_init
>     -> bgmac_dma_rx_skb_for_slot
>       -> netdev_alloc_frag
> 
> BGMAC_RX_ALLOC_SIZE = 10048 and PAGE_FRAG_CACHE_MAX_SIZE = 32768.
> 
> Eventually we land into __page_frag_alloc_align() with the following
> parameters across multiple successive calls:
> 
> __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=0
> __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=10048
> __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=20096
> __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=30144
> 
> So in that case we do indeed have offset + fragsz (40192) > size (32768)
> and so we would eventually return NULL. Reverting to the older 1500
> bytes MTU allows the network driver to be usable again.
> 
> Fixes: 8c7da63978f1 ("bgmac: configure MTU and add support for frames beyond 8192 byte size")
> Signed-off-by: Rafał Miłecki <[email-masked]>
> [florian: expand commit message about recent commits]
> Signed-off-by: Florian Fainelli <[email-masked]>
> Change-Id: Ie70d714cb4f00e45a34e9a015d0eb4bff60fac6e

Hi Florian,

I think the Change-Id line needs to be dropped,
but otherwise this looks good to me.

Reviewed-by: Simon Horman <[email-masked]>