Re: [PATCH net-next v3 5/8] net: bcmgenet: derive the receive buffer length from the MTU

[Nicolai Buchwitz]

On 8.10.2026 11:40, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let 
> patchwork
> know by including "pw-bot: cr" as a separate line at the end of your 
> reply
> (one such reply per series is enough).
>
> - [Medium] In bcmgenet_set_mtu_regs()
>   (drivers/net/ethernet/broadcom/genet/bcmgenet.c), UMAC_MAX_FRAME_LEN…
>
> --
>
> net: bcmgenet: derive the receive buffer length from the MTU
>
> This commit computes the RBUF packet ready threshold from the MTU and
> derives the receive buffer length from it, replacing the fixed 2048 
> byte
> RX_BUF_LENGTH. UMAC_MAX_FRAME_LEN and the TDMA flow period are now also
> derived from the MTU, and the TBUF threshold is programmed at its 
> maximum.
>
> > diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> > b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > index 83ffc4a2a2a72..4a4b0c46f8986 100644
> > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>
> [ ... ]
>
> > @@ -2630,6 +2648,44 @@ static void bcmgenet_link_intr_enable(struct 
> > bcmgenet_priv *priv)
>
> [ ... ]
>
> > +/* Program the MTU dependent registers. Call with the MAC disabled. 
> > */
> > +static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, 
> > unsigned int mtu)
> > +{
> > +	u32 thld = bcmgenet_pkt_rdy_thld(mtu);
> > +
> > +	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), 
> > UMAC_MAX_FRAME_LEN);
>
> [Severity: Medium]
>
> Will this drop standard size frames once the MTU is lowered?
>
> Before this patch, UMAC_MAX_FRAME_LEN was always 1536. Now it is
> ENET_MAX_FRAME_LEN(mtu), which is mtu + 36, and nothing keeps it from
> going lower.
>
> The commit message says:
>
>   The MTU is still fixed at ETH_DATA_LEN
>
> Only the upper bound is fixed, though. bcmgenet does not override
> min_mtu, so ether_setup() still allows any MTU from 68 up to 1500:
>
> net/ethernet/eth.c:ether_setup() {
>     ...
> 	dev->min_mtu		= ETH_MIN_MTU;
> 	dev->max_mtu		= ETH_DATA_LEN;
>     ...
> }
>
> This commit has no ndo_change_mtu, so __netif_set_mtu() only updates
> dev->mtu. The next open or full resume then writes the smaller value:
>
> init_umac()
>   bcmgenet_set_mtu_regs(priv, priv->dev->mtu)
>     bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), 
> UMAC_MAX_FRAME_LEN)
>
> Take an MTU below about 1482, for example 1400 for a tunnel overlay, or
> 1280. Full size 1514/1518 byte frames from LAN peers would then be over
> the MAC limit. They would get DMA_RX_LG and be dropped in
> bcmgenet_desc_rx():
>
> 	if (dma_flag & DMA_RX_LG)
> 		u64_stats_inc(&stats->length_errors);
> 	...
> 	page_pool_put_full_page(ring->page_pool, rx_page,
> 				true);
>
> Before this patch, the MAC accepted these frames whatever the MTU was.
> Full size UDP, broadcast, multicast or forwarded traffic that used to 
> be
> received would now be lost.
>
> This still happens at the end of the series. The later commit "net:
> bcmgenet: allow the MTU to be changed" adds bcmgenet_change_mtu(), 
> which
> applies the smaller limit straight away on a running interface.
>
> That later commit does fix two related cases here. One is the stale,
> too-small limit left after raising the MTU on a running interface. The
> other is the WoL fast resume mismatch between TDMA_FLOW_PERIOD and
> UMAC_MAX_FRAME_LEN.
>
> Would it work to program max(mtu, ETH_DATA_LEN) into 
> UMAC_MAX_FRAME_LEN,
> and to update the commit message to match?

Sure. Will do in v4

---
pw-bot: cr