Hi David, On Tue, 28 Jul 2020 17:52:02 -0700 (PDT) David Miller wrote:
> > > > @@ -3651,7 +3651,8 @@ static void mvneta_stop_dev(struct mvneta_port *pp) > > > > set_bit(__MVNETA_DOWN, &pp->state); > > > > - if (device_may_wakeup(&pp->dev->dev)) > > + if (device_may_wakeup(&pp->dev->dev) && > > + pp->pkt_size == MVNETA_RX_PKT_SIZE(pp->dev->mtu)) > > phylink_speed_down(pp->phylink, false); > > > > This is too much for me. > > You shouldn't have to shut down the entire device and take it back up > again just to change the MTU. > > Unfortunately, this is a common pattern in many drivers and it is very > dangerous to take this lazy path of just doing "stop/start" around > the MTU change. > > It means you can't recover from partial failures properly, > f.e. recovering from an inability to allocate queue resources for the > new MTU. > > To solve this properly, you must restructure the MTU change such that > is specifically stops the necessary and only the units of the chip > necessary to change the MTU. > > It should next try to allocate the necessary resources to satisfy the > MTU change, keeping the existing resources allocated in case of > failure. > > Then, only is all resources are successfully allocated, it should > commit the MTU change fully and without errors. > > Then none of these link flapping issues are even possible. Thanks a lot for pointing out the correct direction. Refactoring change mtu method needs more time(maybe for linux-5.10 is reasonable), so I just drop patch2 in v2.
