Date:   Mon, 20 Mar 2023 12:33:17 +0100
From:   Francois Romieu <romieu@fr.zoreil.com>
To:     Haiyang Zhang <haiyangz@microsoft.com>
Cc:     "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Dexuan Cui <decui@microsoft.com>,
        KY Srinivasan <kys@microsoft.com>,
        Paul Rosswurm <paulros@microsoft.com>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "leon@kernel.org" <leon@kernel.org>,
        Long Li <longli@microsoft.com>,
        "ssengar@linux.microsoft.com" <ssengar@linux.microsoft.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next] net: mana: Add support for jumbo frame
Message-ID: <20230320113317.GA290683@electric-eye.fr.zoreil.com>
References: <1679261264-26375-1-git-send-email-haiyangz@microsoft.com>
 <20230319224642.GA239003@electric-eye.fr.zoreil.com>
 <PH7PR21MB31162F5F9E5C8C146760AF10CA809@PH7PR21MB3116.namprd21.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PH7PR21MB31162F5F9E5C8C146760AF10CA809@PH7PR21MB3116.namprd21.prod.outlook.com>
Precedence: bulk

Haiyang Zhang <haiyangz@microsoft.com> :
> > From: Francois Romieu <romieu@fr.zoreil.com>
[...]
> > 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.

In a ideal world, I would expect change_mtu() to allocate the new resources,
bail out if some allocation fails, stop the traffic, swap the old and new
resources, then restart the traffic and release the old resources.
This way the device is never left in a failed state.

-- 
Ueimor