Re: [PATCH v2] ravb: implement MTU change while device is up

[Ulrich Hecht <uli@fpond.eu>]

> On June 6, 2019 at 4:08 AM David Miller <davem@davemloft.net> wrote:
> 
> 
> From: Ulrich Hecht <uli+renesas@fpond.eu>
> Date: Wed,  5 Jun 2019 17:14:20 +0200
> 
> > @@ -1811,11 +1811,14 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd)
> >  static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
> >  {
> >  	if (netif_running(ndev))
> > -		return -EBUSY;
> > +		ravb_close(ndev);
> >  
> >  	ndev->mtu = new_mtu;
> >  	netdev_update_features(ndev);
> >  
> > +	if (netif_running(ndev))
> > +		return ravb_open(ndev);
> > +
> 
> And if ravb_open() fails?  The user sees a failure, but to the user the failure
> means the MTU change can't be done, yet the device has the new MTU set still.
> 
> This really is terrible behavior.
> 
> If you must do a prepare/commit kind of sequence for this to work properly if
> you are going to go down the road of taking the device down to change the MTU
> when the device is UP.

So would rolling back the MTU change in case of a re-open failure be acceptable?

If so, is there additional action that needs to be taken if a device unexpectedly goes down as a side-effect of an MTU change?

CU
Uli