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

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

[Maxime Coquelin]

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().

---

 drivers/net/mlx5/mlx5_ethdev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 7708a0b808..f2ae75a8e1 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -647,6 +647,14 @@ mlx5_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 	ret = mlx5_get_mtu(dev, &kern_mtu);
 	if (ret)
 		return ret;
+
+	if (kern_mtu == mtu) {
+		priv->mtu = mtu;
+		DRV_LOG(DEBUG, "port %u adapter MTU was already set to %u",
+			dev->data->port_id, mtu);
+		return 0;
+	}
+
 	/* Set kernel interface MTU first. */
 	ret = mlx5_set_mtu(dev, mtu);
 	if (ret)
--
2.49.0

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

[David Marchand]

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().

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.


-- 
David Marchand

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

[Dariusz Sosnowski]

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.

> 
> 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.

I'll prepare a patch later which does that.
Thanks for bringing that up.

For posperity - this cached value is used only during
rte_eth_rx_queue_setup() while validating shared RX queue
configuration.

Best regards,
Dariusz Sosnowski

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

[Dariusz Sosnowski]

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.
> 
> > 
> > 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".

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

[Dariusz Sosnowski]

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

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

[David Marchand]

On Tue, Jun 3, 2025 at 7:54 PM Dariusz Sosnowski <dsosnowski@nvidia.com> wrote:
> > 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.

Good point, which makes this priv->mtu even less useful.

Thanks Dariusz.

-- 
David Marchand