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

Thu, Jul 09, 2026 at 07:45:20AM +0200, mbloch@nvidia.com wrote:

quoted

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

quoted

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

quoted

Apply parsed devlink_eswitch_mode= defaults after devlink registration
and after successful reload.

devl_register() may still be called before the device is ready for an
eswitch mode change. Keep the registration path passive and let the
regular devl_unlock() path queue the async apply work once the instance
is registered and the default is still pending.

The queueing path runs while the devlink instance lock is held, so the
queued work gets its devlink reference before the caller drops the lock.
The worker then takes the devlink instance lock normally and applies the
default only if the instance is still registered and the default is still
pending.

This is very code-descriptive. What's the benefit of that?

The point is that there is still a window before the queued work
runs where the user can explicitly set the eswitch mode. If they 
do, the default will no longer be pending, so the worker will skip
applying it.

I'll reword.

quoted

quoted

For successful reloads that performed DRIVER_REINIT, devlink_reload()
already holds the devlink instance lock and the driver has completed
reload_up(). Clear pending work and apply the default directly from the
reload path instead of queueing work.

Preserve the user configured mode when it is set before devlink applies
the default.

[..]

quoted

+void devlink_default_esw_mode_apply_locked(struct devlink *devlink)
+{
+	const struct devlink_ops *ops = devlink->ops;
+	int err;
+
+	devl_assert_locked(devlink);
+
+	if (!devlink_default_esw_mode_match(devlink))
+		return;
+
+	if (!ops->eswitch_mode_set) {
+		if (!devlink_default_esw_mode_match_all)
+			devl_warn(devlink,
+				  "devlink_eswitch_mode= selected this device but eswitch mode setting is not supported\n");
+		return;
+	}
+
+	err = devlink_eswitch_mode_set(devlink, devlink_default_esw_mode, NULL);
+	if (err)
+		devl_warn(devlink,
+			  "Couldn't apply default eswitch mode, err %d\n",
+			  err);
+}
+
+void devlink_default_esw_mode_queue_apply_work(struct devlink *devlink)

eswitch/esw - we call it "eswitch" consistently everywhere. Why "esw"
here?

Ack

quoted

quoted

+{
+	devl_assert_locked(devlink);
+
+	if (!devlink_default_esw_mode_enabled || !devlink_default_esw_mode_wq)
+		return;
+	if (!devlink->default_esw_mode_apply_pending ||
+	    !__devl_is_registered(devlink))
+		return;
+	if (!devlink_try_get(devlink))
+		return;
+	if (!queue_work(devlink_default_esw_mode_wq,
+			&devlink->default_esw_mode_apply_work))
+		devlink_put(devlink);
+}
+
+static void devlink_default_esw_mode_apply_work(struct work_struct *work)
+{
+	struct devlink *devlink;
+
+	devlink = container_of(work, struct devlink,
+			       default_esw_mode_apply_work);
+

What happens if userspace eswitch mode set happens now? Any userspace
attempt should cancel the default apply. I don't see such mechanism in
your patches, did I miss it?

devlink_nl_eswitch_set_doit() calls
devlink_default_esw_mode_apply_pending_clear(), which clears the
pending bit.

So if a user sets the eswitch mode before the queued default
work applies it, the worker will see that the default is no longer
pending and will do nothing

Okay.

quoted

quoted

quoted

+	devl_lock(devlink);
+
+	if (devl_is_registered(devlink) &&
+	    devlink->default_esw_mode_apply_pending) {
+		devlink_default_esw_mode_apply_locked(devlink);
+		devlink->default_esw_mode_apply_pending = false;
+	}
+
+	devl_unlock(devlink);
+	devlink_put(devlink);
+}
+
+void devlink_default_esw_mode_instance_init(struct devlink *devlink)

Why "_instance_"? Care to drop?

Ack

quoted

quoted

+{
+	INIT_WORK(&devlink->default_esw_mode_apply_work,
+		  devlink_default_esw_mode_apply_work);
+	devlink->default_esw_mode_apply_pending = true;
+}
+
+void devlink_default_esw_mode_apply_pending_clear(struct devlink *devlink)
+{
+	devl_assert_locked(devlink);
+
+	devlink->default_esw_mode_apply_pending = false;
+}
+
+void devlink_default_esw_mode_instance_cleanup(struct devlink *devlink)

Why "_instance_"? Care to drop?

Ack

quoted

quoted

+{
+	if (cancel_work_sync(&devlink->default_esw_mode_apply_work))
+		devlink_put(devlink);
+}
+
static int __init devlink_default_esw_mode_setup(char *str)
{
	devlink_default_esw_mode_param = str;

@@ -228,10 +325,21 @@ int __init devlink_default_esw_mode_init(void)

		return err;
	}

+	devlink_default_esw_mode_wq = alloc_workqueue("devlink_default_esw_mode",
+						      WQ_UNBOUND | WQ_MEM_RECLAIM,
+						      0);
+	if (!devlink_default_esw_mode_wq) {
+		devlink_default_esw_mode_param = NULL;
+		devlink_default_esw_mode_nodes_clear();
+		pr_warn("devlink: devlink_eswitch_mode parameter ignored, failed to allocate workqueue\n");

Why you don't "return"  here? I think that we don't need to allow the
case wq is not allocated.

The function returns right after this block. It is not treated

What I ment was "return error".

quoted

as a valid “workqueue unavailable” mode, the parsed defaults are
cleared, the parameter is ignored, and no default eswitch mode will
be applied.

I kept it as a non critical failure so we do not abort the whole
devlink init just because the default-mode workqueue could not be
allocated.

Why to treat it like this? Is there any other example of such flow in
devlink? I don't see the benefit, only potential confusion in very
unlikely case the alloc_workqueue fails. Am I wrong? If not, just bail
out here.

As I'm dropping the work queue it doesn't matter. I'll treat failed
parsing as error then.

Mark

quoted

That said, I can make this more explicit by returning 0 directly
from this error path.

Mark

quoted

quoted

+	}
+
	return 0;
}

void __init devlink_default_esw_mode_cleanup(void)
{
+	if (devlink_default_esw_mode_wq)
+		destroy_workqueue(devlink_default_esw_mode_wq);
	devlink_default_esw_mode_nodes_clear();
}

[..]