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

[Mark Bloch]

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,