On 09/07/2026 12:52, Jiri Pirko wrote:

Thu, Jul 09, 2026 at 08:00:19AM +0200, mbloch@nvidia.com wrote:

quoted

On 08/07/2026 11:34, Jiri Pirko wrote:

quoted

Tue, Jul 07, 2026 at 07:45:27PM +0200, mbloch@nvidia.com wrote:

quoted

Apply devlink_eswitch_mode= boot defaults for mlx5 after the initial
probe finishes device initialization while holding the devlink instance
lock.

At this point the devlink instance is registered and mlx5 can perform an
eswitch mode change. Calling devl_apply_default_esw_mode() also clears
any pending default apply work queued by devl_register(), so the queued
work will not apply the same default again.

Keep this call in mlx5_init_one() rather than the lower-level
devl-locked init helper. That helper is also used by devlink reload, and
devlink core already applies the boot default after a successful
DRIVER_REINIT reload.

Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 643b4aac2033..0712efea74cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c

@@ -1392,6 +1392,17 @@ static void mlx5_unload(struct mlx5_core_dev *dev)

	mlx5_free_bfreg(dev, &dev->priv.bfreg);
}

+static void mlx5_devl_apply_default_esw_mode(struct mlx5_core_dev *dev)
+{
+	struct devlink *devlink = priv_to_devlink(dev);
+
+	if (!MLX5_ESWITCH_MANAGER(dev))
+		return;
+
+	devl_assert_locked(devlink);
+	devl_apply_default_esw_mode(devlink);
+}
+
int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
{
	bool light_probe = mlx5_dev_is_lightweight(dev);

@@ -1471,6 +1482,8 @@ int mlx5_init_one(struct mlx5_core_dev *dev)

	err = mlx5_init_one_devl_locked(dev);
	if (err)
		devl_unregister(devlink);
+	else
+		mlx5_devl_apply_default_esw_mode(dev);

I don't understand why this patch is needed at all. Just leave the job
to the devlink core, no? That was the point to not pollute drivers with
code like this. Is it some kind of leftover?

It was discussed with Jakub here:
https://lore.kernel.org/all/20260611085440.4fe36bf2@kernel.org/ (local)

The main reason is timing. If the default is applied only by devlink
core, it has to wait until the driver drops the devlink lock.

I don't follow.

<quote>
	devl_lock(devlink);
	if (dev->shd) {
		err = devl_nested_devlink_set(dev->shd, devlink);
		if (err)
			goto unlock;
	}
	devl_register(devlink);
	err = mlx5_init_one_devl_locked(dev);
	if (err)
		devl_unregister(devlink);
unlock:
	devl_unlock(devlink);
</quote>

devlink lock is droped right after.

Earlier versions had the fw reset / recovery flows covered as well.
The regular probe/init in mlx5 need some changes in mlx5 so I can
move it earlier, left the API just to make sure we are aligned and
it could be used be a later patches.

Anyway, I guess I can drop this as well and continue with followup
patches just to keep forward progress.

quoted

For mlx5, that usually happens very late in the init sequence. I
wanted drivers to be able to apply the default as soon as the driver
is ready for it, because on NICs with a DPU the host PF can remain
stuck until the ECPF moves to switchdev.

This API is also useful beyond the initial devlink registration path.
Follow-up patches will use it for driver controlled paths that are
not covered by the devlink core, such as recovery and FW reset.

There is also a race window where userspace may take the devlink lock
before the core gets a chance to apply the default. Letting the driver
explicitly apply the default at the right point avoids that scenario.

Thinking about this again, maybe the simpler approach is to apply the
default from devl_unlock(). That would avoid the whole workqueue
infra.

I avoided doing that earlier because applying a default mode as a
side effect of devl_unlock() feels a bit odd. But compared to adding
dedicated workqueue handling maybe it is the lesser evil here.

What do you think?

I was under impression that you need the work to resolve nested locking.
If not, drop it, sure.

I think that should be fine. Once the instance is registered and the
devlink lock is dropped, the driver should be ready for normal devlink
operations anyway, since a user could trigger the same path at that
point, so we just don't drop the lock and call the driver directly,
should work.

The only caveat I can think of is a driver holding some other lock
while dropping the devlink lock, and then trying to take that same
lock again from the mode-setting callback. But that sounds like a
driver bug rather than something devlink should work around.

I’ll drop the workqueue approach and rework this for v6 :)

Mark

quoted

About the extra API, I still think it's useful and would like to keep
it if possible.

Mark

quoted

quoted

unlock:
	devl_unlock(devlink);
	return err;
-- 
2.43.0