Re: [PATCH net-next v8 7/7] net: bcmgenet: reject MTU changes incompatible with XDP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The XDP MTU validation logic contains a mathematical error (ignoring L2 headers) and is structurally unreachable (dead code) because the device's maximum MTU is fixed at 1500.
--

commit b8018c0a123b4927db5ca8c39c8395c751532069
Author: Nicolai Buchwitz <nb@tipi-net.de>

net: bcmgenet: reject MTU changes incompatible with XDP

This commit introduces a minimal ndo_change_mtu function to the bcmgenet
driver. It checks if an XDP program is attached and rejects MTU values that
would exceed the capacity of a single-page XDP buffer.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 02ad2f410d6c..4d1ec68ec0c5 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -4085,6 +4085,20 @@ static int bcmgenet_xdp_xmit(struct net_device *dev, int num_frames,
[ ... ]
> +static int bcmgenet_change_mtu(struct net_device *dev, int new_mtu)
> +{
> +	struct bcmgenet_priv *priv = netdev_priv(dev);
> +
> +	if (priv->xdp_prog && new_mtu > PAGE_SIZE - GENET_RX_HEADROOM -
> +	    SKB_DATA_ALIGN(sizeof(struct skb_shared_info))) {

Is this condition actually reachable, and does it correctly account for L2
headers?

The bcmgenet driver does not appear to override dev->max_mtu, which
defaults to ETH_DATA_LEN (1500). Because dev_validate_mtu() in the core
networking stack checks that new_mtu <= dev->max_mtu before calling
ndo_change_mtu, new_mtu will never exceed 1500.

Given that PAGE_SIZE (typically 4096) minus GENET_RX_HEADROOM and the
skb_shared_info alignment is roughly 3454 bytes, the check new_mtu > 3454
will always evaluate to false.

Also, new_mtu represents the L3 payload size. Shouldn't the check add the
L2 header overhead when comparing against the L2 buffer capacity?

Furthermore, is it possible for an MTU change to corrupt the XDP buffer
layout as the commit message claims?

The hardware DMA appears to be statically configured to write a maximum of
RX_BUF_LENGTH (2048) bytes per buffer, and bcmgenet_desc_rx() explicitly
drops any packet larger than 2048 bytes. It seems structurally impossible
to overflow the 4096-byte page and corrupt skb_shared_info.

> +		netdev_warn(dev, "MTU too large for single-page XDP buffer\n");
> +		return -EINVAL;
> +	}
> +
> +	WRITE_ONCE(dev->mtu, new_mtu);
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428205846.2625550-1-nb@tipi-net.de?part=7