Re: [PATCH net-next V2 6/7] net/mlx5: E-switch, load reps via work queue after registration

Mark Bloch
2026-05-03T08:01:23Z

On 03/05/2026 4:42, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net/mlx5: E-switch, load reps via work queue after registration
>
> This change defers loading of newly registered representors to the
> E-Switch work queue so the handler can acquire the devlink lock that is
> not held by the auxiliary driver registration context. The unregister
> path continues to unload representors synchronously.
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> index 8f656253981b..f26d1652dd05 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> @@ -4563,6 +4563,38 @@ mlx5_eswitch_register_vport_reps_blocked(struct mlx5_eswitch *esw,
>
> [ ... ]
>
>> +static void mlx5_eswitch_reload_reps(struct mlx5_eswitch *esw)
>> +{
>> + mlx5_esw_reps_block(esw);
>> + mlx5_eswitch_reload_reps_blocked(esw);
>> + mlx5_esw_reps_unblock(esw);
>> +}
>> +
>> static void
>> mlx5_eswitch_register_vport_reps_locked(struct mlx5_eswitch *esw,
>> const struct mlx5_eswitch_rep_ops *ops,
>> @@ -4574,6 +4606,8 @@ mlx5_eswitch_register_vport_reps_locked(struct mlx5_eswitch *esw,
>> mlx5_esw_reps_block(esw);
>> mlx5_eswitch_register_vport_reps_blocked(esw, ops, rep_type);
>> mlx5_esw_reps_unblock(esw);
>> +
>> + mlx5_esw_add_work(esw, mlx5_eswitch_reload_reps);
>
> The return value of mlx5_esw_add_work() is discarded here.
>
> Looking at mlx5_esw_add_work():
>
> static int mlx5_esw_add_work(struct mlx5_eswitch *esw,
> void (*func)(struct mlx5_eswitch *esw))
> {
> struct mlx5_host_work *host_work;
>
> host_work = kzalloc_obj(*host_work, GFP_ATOMIC);
> if (!host_work)
> return -ENOMEM;
> ...
> }
>
> the allocation is GFP_ATOMIC and can fail, in which case no work is
> queued.
>
> If mlx5_esw_add_work() returns -ENOMEM while the E-Switch is already in
> MLX5_ESWITCH_OFFLOADS mode, the reload that mlx5_eswitch_reload_reps()
> was meant to perform never runs, so the newly registered rep type will
> not be loaded for the already-enabled vports — which is the exact case
> this commit is introducing infrastructure to handle.
>
> Registration still returns success to the auxiliary driver
> (mlx5e_rep_probe / mlx5r_rep_probe), with no log message, no retry, and
> no way for the caller to notice the dropped reload.
>
> The other existing caller, mlx5_esw_funcs_changed_handler(), does check
> the return value of mlx5_esw_add_work(), suggesting allocation failure
> is expected to be handled.
>
> Should this call check the return value and at least emit a warning or
> retry, so that a GFP_ATOMIC allocation failure during switchdev-mode
> registration does not silently leave the representors unloaded?
>
>> }

I don’t think this needs a warning. This reload is a best effort path
for late rep_ops registration, REP_REGISTERED only means the ops are
installed, while users still key off REP_LOADED before using rep
private data.

The work can also legitimately do nothing because the E-Switch
mode changed or the generation check made it stale. When registration
happens during an E-Switch rescan, the normal E-Switch path will try
to load the reps and report any real failure back to the user.

I’ll still make mlx5_esw_add_work() take a gfp_t, so this path can
use GFP_KERNEL instead of forcing GFP_ATOMIC reduce allocation failure
risk.

Mark

>>
>> void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,