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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230320074202.GH36557@unreal>
Date:   Mon, 20 Mar 2023 09:42:02 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Haiyang Zhang <haiyangz@...rosoft.com>
Cc:     linux-hyperv@...r.kernel.org, netdev@...r.kernel.org,
        decui@...rosoft.com, kys@...rosoft.com, paulros@...rosoft.com,
        olaf@...fle.de, vkuznets@...hat.com, davem@...emloft.net,
        wei.liu@...nel.org, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, longli@...rosoft.com,
        ssengar@...ux.microsoft.com, linux-kernel@...r.kernel.org
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@...rosoft.com>
> ---
>  .../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.

Thanks

> +	if (err2)
> +		netdev_err(ndev, "mana re-attach failed: %d\n", err2);
> +
> +	return err;