Re: Minimum MTU Mess

[Jarod Wilson <jarod@redhat.com>]

On Wed, Sep 07, 2016 at 10:31:12PM +0200, Andrew Lunn wrote:
> Hi Jarod
> 
> > -	/*	MTU must be positive.	 */
> > -	if (new_mtu < 0)
> > +	if (new_mtu < dev->min_mtu) {
> > +		netdev_err(dev, "Invalid MTU %d requested, hw min %d\n",
> > +			   new_mtu, dev->min_mtu);
> >  		return -EINVAL;
> > +	}
> > +
> > +	if (new_mtu > dev->max_mtu) {
> > +		netdev_err(dev, "Invalid MTU %d requested, hw max %d\n",
> > +			   new_mtu, dev->min_mtu);
> > +		return -EINVAL;
> > +	}
> 
> I doubt you can make such a big change like this in one go. Can you
> really guarantee all interfaces, of what ever type, will have some
> value for dev->min_mtu and dev->max_mtu? What may fly is something
> more like:
> 
> > +	if (dev->max_mtu && new_mtu > dev->max_mtu) {
> > +		netdev_err(dev, "Invalid MTU %d requested, hw max %d\n",
> > +			   new_mtu, dev->min_mtu);
> > +		return -EINVAL;
> > +	}
> 
> Maybe in a few cycles you can add a WARN_ON(!dev->max_mtu), and a few
> cycles after that go with (new_mtu > dev->max_mtu).

My local tree actually has if (dev->max_mtu > 0 && new_mtu > dev->max_mtu)
since just after I'd sent my mail for exactly that reason, though looking
at alloc_netdev_mqs(), it does seem we're at least guaranteed the value
will be 0 if not otherwise initialized, so your version looks perfectly
fine, and in the min_mtu case, without any additional handling, things
behave exactly as they did before. This is definitely going to require a
few passes... (Working my way through every driver with an ndo_change_mtu
wired up right now to see just how crazy this might get).

-- 
Jarod Wilson
jarod@redhat.com