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

[Haiyang Zhang]

> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Monday, March 20, 2023 3:42 AM
> 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; Long Li <longli@xxxxxxxxxxxxx>;
> ssengar@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next] net: mana: Add support for jumbo frame
> 
> On Sun, Mar 19, 2023 at 02:27:44PM -0700, Haiyang Zhang wrote:
> > During probe, get the hardware allowed max MTU by querying the device
> > configuration. Users can select MTU up to the device limit. Also,
> > when XDP is in use, we currently limit the buffer size to one page.
> >
> > Updated RX data path to allocate and use RX queue DMA buffers with
> > proper size based on the MTU setting.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > ---
> >  .../net/ethernet/microsoft/mana/mana_bpf.c    |  22 +-
> >  drivers/net/ethernet/microsoft/mana/mana_en.c | 229 ++++++++++++----
> --
> >  include/net/mana/gdma.h                       |   4 +
> >  include/net/mana/mana.h                       |  18 +-
> >  4 files changed, 183 insertions(+), 90 deletions(-)
> 
> <...>
> 
> > +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);
> 
> I second to Francois and agree with him that it is very questionable.
> If mana_attach() failed for first try, you should bail out and not
> retry with some hope that it will pass.
> 

Will consider. Thanks.