Re: [PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

From: Andrew Lunn

Date: Sun Oct 18 2026 - 23:12:58 EST


On Sun, Oct 18, 2026 at 06:44:38PM -0700, Rosen Penev wrote:
> The rx buffers are sized from the MTU in ag71xx_open(), but
> ag71xx_change_mtu() writes the new maximum frame length straight to
> the MAC while the interface is running. The rx descriptors carry no
> buffer length, so after raising the MTU the MAC accepts and DMAs
> frames larger than the mapped buffer, overwriting the skb_shared_info
> at the end of the fragment and whatever follows it.
>
> This is reachable without user action: DSA raises the MTU of a
> running conduit to make room for the switch tag.
>
> Restart the interface when the MTU changes while it is up, so the
> rings are refilled with buffers sized for the new MTU before the MAC
> is programmed with the new frame length.
>
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
> drivers/net/ethernet/atheros/ag71xx.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index 8a3c689f6825..3c9de8c2ba64 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -1761,11 +1761,19 @@ static irqreturn_t ag71xx_interrupt(int irq, void *dev_id)
>
> static int ag71xx_change_mtu(struct net_device *ndev, int new_mtu)
> {
> - struct ag71xx *ag = netdev_priv(ndev);
> + bool running = netif_running(ndev);
> +
> + /* The rx buffers are sized for the MTU at open time and the rx
> + * descriptors carry no buffer length, so a larger MTU must not
> + * reach the MAC before the rings have been refilled.
> + */
> + if (running)
> + ag71xx_stop(ndev);

This appears to free the rings.

>
> WRITE_ONCE(ndev->mtu, new_mtu);
> - ag71xx_wr(ag, AG71XX_REG_MAC_MFL,
> - ag71xx_max_frame_len(ndev->mtu));
> +
> + if (running)
> + return ag71xx_open(ndev);

And this tries to allocate them again. If this fails, there is no way
back, the interface is dead.

This is not how MTU change should work. If it fails, it should leave
the old MTU in place.

Andrew

---
pw-bot: cr