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

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

On 8.10.2026 11:40, netdev-bot+sashiko@kernel.org 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