Re: [PATCH] net/mlx5: avoid setting kernel MTU if not needed

[Dariusz Sosnowski <dsosnowski@nvidia.com>]

Hi Maxime,

On Tue, Jun 03, 2025 at 07:56:11PM +0200, Dariusz Sosnowski wrote:
> On Tue, Jun 03, 2025 at 07:53:45PM +0200, Dariusz Sosnowski wrote:
> > Hi,
> > 
> > On Wed, May 28, 2025 at 02:12:37PM +0200, David Marchand wrote:
> > > Hello,
> > > 
> > > On Wed, May 28, 2025 at 11:36 AM Maxime Coquelin
> > > <maxime.coquelin@redhat.com> wrote:
> > > >
> > > > This patch checks whether the Kernel MTU has the same value
> > > > as the requested one at port configuration time, and skip
> > > > setting it if it is the same.
> > > >
> > > > Doing this, we can avoid the application to require
> > > > NET_ADMIN capability, as in v23.11.
> > > >
> > > > Fixes: 10859ecf09c4 ("net/mlx5: fix MTU configuration")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > > ---
> > > >
> > > > Hi Dariuz,
> > > >
> > > > I set priv->mtu as it is done after the mlx5_set_mtu() call,
> > > > but I'm not sure it is necessary, as is the existing call to
> > > > mlx5_get_mtu() because it seems done in mlx5_dev_spawn().
> > 
> > Correct, this additional update of priv->mtu is not needed here.
> > It can be removed.

As a matter of fact, the setting of priv->mtu can stay as is in the
patch and the current version of the patch can be applied in my opinion.
The "future" patch which removes mtu field from mlx5_priv
would take care of that.

Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

> > 
> > > 
> > > It seems there were some back and forth on this priv->mtu topic
> > > between Nelio and other devs in the past.
> > > 
> > > Atm, I don't see the need for keeping such a cached mtu value in priv.
> > > There is only one user of the value, and it is for configuration
> > > operation that can do a query to the kernel.
> > 
> > I agree. It's not really needed, especially since the same value is also
> > stored in dev->data->mtu, so the kernel query can easily be replaced
> > with reading dev->data->mtu.
> 
> Correction - "so the use of cached value can easily be replaced with
> reading dev->data->mtu".

Best regards,
Dariusz Sosnowski