netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] bgmac: reduce max frame size to support just MTU 1500
@ 2025-01-24 19:14 Florian Fainelli
  2025-01-27 17:22 ` Simon Horman
  0 siblings, 1 reply; 2+ messages in thread
From: Florian Fainelli @ 2025-01-24 19:14 UTC (permalink / raw)
  To: netdev
  Cc: Rafał Miłecki, Florian Fainelli,
	Broadcom internal kernel review list, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Murali Krishna Policharla, Ray Jui, Vladimir Oltean,
	Arun Parameswaran, open list

From: Rafał Miłecki <rafal@milecki.pl>

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 <rafal@milecki.pl>
[florian: expand commit message about recent commits]
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Change-Id: Ie70d714cb4f00e45a34e9a015d0eb4bff60fac6e
---
 drivers/net/ethernet/broadcom/bgmac.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index d73ef262991d..6fee9a41839c 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -328,8 +328,7 @@
 #define BGMAC_RX_FRAME_OFFSET			30		/* There are 2 unused bytes between header and real data */
 #define BGMAC_RX_BUF_OFFSET			(NET_SKB_PAD + NET_IP_ALIGN - \
 						 BGMAC_RX_FRAME_OFFSET)
-/* Jumbo frame size with FCS */
-#define BGMAC_RX_MAX_FRAME_SIZE			9724
+#define BGMAC_RX_MAX_FRAME_SIZE			1536
 #define BGMAC_RX_BUF_SIZE			(BGMAC_RX_FRAME_OFFSET + BGMAC_RX_MAX_FRAME_SIZE)
 #define BGMAC_RX_ALLOC_SIZE			(SKB_DATA_ALIGN(BGMAC_RX_BUF_SIZE + BGMAC_RX_BUF_OFFSET) + \
 						 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] bgmac: reduce max frame size to support just MTU 1500
  2025-01-24 19:14 [PATCH net] bgmac: reduce max frame size to support just MTU 1500 Florian Fainelli
@ 2025-01-27 17:22 ` Simon Horman
  0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2025-01-27 17:22 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Rafał Miłecki,
	Broadcom internal kernel review list, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Murali Krishna Policharla, Ray Jui, Vladimir Oltean,
	Arun Parameswaran, open list

On Fri, Jan 24, 2025 at 11:14:04AM -0800, Florian Fainelli wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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 <rafal@milecki.pl>
> [florian: expand commit message about recent commits]
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> 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 <horms@kernel.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-01-27 17:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-24 19:14 [PATCH net] bgmac: reduce max frame size to support just MTU 1500 Florian Fainelli
2025-01-27 17:22 ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).