RE: [PATCH net-next] net: mana: Add support for jumbo frame

[Haiyang Zhang]

> -----Original Message-----
> From: Francois Romieu <romieu@xxxxxxxxxxxxx>
> Sent: Sunday, March 19, 2023 6:47 PM
> To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: linux-hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Dexuan Cui
> <decui@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>; Paul Rosswurm
> <paulros@xxxxxxxxxxxxx>; olaf@xxxxxxxxx; vkuznets@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx; wei.liu@xxxxxxxxxx; edumazet@xxxxxxxxxx;
> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; leon@xxxxxxxxxx; Long Li
> <longli@xxxxxxxxxxxxx>; ssengar@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next] net: mana: Add support for jumbo frame
> 
> [Some people who received this message don't often get email from
> romieu@xxxxxxxxxxxxx. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> :
> [...]
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index 492474b4d8aa..07738b7e85f2 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -427,6 +427,34 @@ static u16 mana_select_queue(struct net_device
> *ndev, struct sk_buff *skb,
> >       return txq;
> >  }
> >
> > +static int mana_change_mtu(struct net_device *ndev, int new_mtu)
> > +{
> > +     unsigned int old_mtu = ndev->mtu;
> > +     int err, err2;
> > +
> > +     err = mana_detach(ndev, false);
> > +     if (err) {
> > +             netdev_err(ndev, "mana_detach failed: %d\n", err);
> > +             return err;
> > +     }
> > +
> > +     ndev->mtu = new_mtu;
> > +
> > +     err = mana_attach(ndev);
> > +     if (!err)
> > +             return 0;
> > +
> > +     netdev_err(ndev, "mana_attach failed: %d\n", err);
> > +
> > +     /* Try to roll it back to the old configuration. */
> > +     ndev->mtu = old_mtu;
> > +     err2 = mana_attach(ndev);
> > +     if (err2)
> > +             netdev_err(ndev, "mana re-attach failed: %d\n", err2);
> > +
> > +     return err;
> > +}
> 
> I do not see where the driver could depend on the MTU. Even if it fails,
> a single call to mana_change_mtu should thus never wreck the old working
> state/configuration.
> 
> Stated differently, the detach/attach implementation is simple but
> it makes the driver less reliable than it could be.
> 
> No ?

No, it doesn't make the driver less reliable. To safely remove and reallocate 
DMA buffers with different size, we have to stop the traffic. So, mana_detach() 
is called. We also call mana_detach() in mana_close(). So the process in 
mana_change_mtu() is no more risky than ifdown/ifup of the NIC. 

In some rare cases, if the system memory is running really low, the bigger 
buffer allocation may fail, so we re-try with the previous MTU. I don't expect 
it to fail again. But we still check & log the error code for completeness and 
debugging.

Thanks,
- Haiyang