Re: [PATCH net-next] net/mlx5: Remove max_external_SFs device resource

[Or Har-Toov <ohartoov@nvidia.com>]

On 12/10/2026 12:40, netdev-bot+sashiko@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The patch removes the device-scope devlink resource
>    `max_external_SFs` (resource id 2, MLX5_DL_RES_MAX_EXTERNAL_SFS) from…
> - [Low] The commit message says "Users can look up SF limits per-port
>    directly via 'devlink port show'".
> 
> --
> 
> net/mlx5: Remove max_external_SFs device resource
> 
> This commit removes the device-level max_external_SFs devlink resource
> from mlx5. The reason given is that the per-port max_SFs resources on
> each PF port representor make it redundant. The devlink-resource.rst
> examples are updated to match.
> 
>> Users can look up SF limits per-port directly via 'devlink port show'
>> to correlate with controller number and pfnum.
> 
> [Severity: Low]
> Is 'devlink port show' the right command here? Port-scoped resources are
> returned by DEVLINK_CMD_RESOURCE_DUMP in net/devlink/resource.c. The port
> get reply does not include them:
> 
> net/devlink/resource.c:devlink_nl_resource_dump_doit() {
>      ...
>          resource_list = devlink_port ?
>                  &devlink_port->resource_list : &devlink->resource_list;
>      ...
> }
> 
> The devlink-resource.rst updated by this patch lists port resources with
> 'devlink resource show pci/0000:03:00.0/196608' and 'devlink resource
> show scope port'.
> 
> Could the commit message say that the max_SFs values come from devlink
> resource show, and that devlink port show only maps the port index to
> controller and pfnum?
> 
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/hw_table.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/hw_table.c
>> index 1724bf7e0e7af..c36dbd7de8778 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/hw_table.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/hw_table.c
>> @@ -262,24 +262,17 @@ static void mlx5_sf_hw_table_res_unregister(struct mlx5_core_dev *dev)
>>        devl_resources_unregister(priv_to_devlink(dev));
>>   }
>>
>> -static int mlx5_sf_hw_table_res_register(struct mlx5_core_dev *dev, u16 max_fn,
>> -                                      u16 max_ext_fn)
>> +static int mlx5_sf_hw_table_res_register(struct mlx5_core_dev *dev,
>> +                                      u16 max_fn)
>>   {
>>        struct devlink_resource_size_params size_params;
>>        struct devlink *devlink = priv_to_devlink(dev);
>> -     int err;
>>
>>        devlink_resource_size_params_init(&size_params, max_fn, max_fn, 1,
>>                                          DEVLINK_RESOURCE_UNIT_ENTRY);
>> -     err = devl_resource_register(devlink, "max_local_SFs", max_fn, MLX5_DL_RES_MAX_LOCAL_SFS,
>> -                                  DEVLINK_RESOURCE_ID_PARENT_TOP, &size_params);
>> -     if (err)
>> -             return err;
>> -
>> -     devlink_resource_size_params_init(&size_params, max_ext_fn, max_ext_fn, 1,
>> -                                       DEVLINK_RESOURCE_UNIT_ENTRY);
>> -     return devl_resource_register(devlink, "max_external_SFs", max_ext_fn,
>> -                                   MLX5_DL_RES_MAX_EXTERNAL_SFS, DEVLINK_RESOURCE_ID_PARENT_TOP,
>> +     return devl_resource_register(devlink, "max_local_SFs", max_fn,
>> +                                   MLX5_DL_RES_MAX_LOCAL_SFS,
>> +                                   DEVLINK_RESOURCE_ID_PARENT_TOP,
>>                                      &size_params);
>>   }
> 
> [Severity: Medium]
> Does the per-port max_SFs fully replace max_external_SFs?
> 
> max_external_SFs has been visible on pci/<bdf> since commit 6486c0f44ed8
> ("net/mlx5: Expose max possible SFs via devlink resource"). It was
> registered at probe, gated only by mlx5_vhca_event_supported(). Anything
> that looks it up by name, or by resource id 2, will no longer find it in
> any eswitch mode.
> 
> The replacement is registered only on this path:
> 
> mlx5_esw_offloads_load_rep()
>    mlx5_esw_offloads_devlink_port_register()
>      mlx5_esw_devlink_port_res_register()
> 
> Both ends of that path can skip it:
> 
> eswitch_offloads.c:mlx5_esw_offloads_load_rep() {
>          if (esw->mode != MLX5_ESWITCH_OFFLOADS)
>                  return 0;
>      ...
> }
> 
> esw/devlink_port.c:mlx5_esw_devlink_port_res_register() {
>      ...
>          if (vport_num != MLX5_VPORT_HOST_PF &&
>              !mlx5_esw_is_spf_vport(esw, vport_num))
>                  return 0;
>      ...
> }
> 
> In legacy mode (the default at probe), or when the host PF representor
> isn't loaded, does any devlink resource still report the external host
> SF capacity?
> 
> mlx5_sf_hw_table_init() still computes max_ext_fn through
> mlx5_esw_sf_max_hpf_functions() and uses it for MLX5_SF_HWC_EXT_HOST. So
> the driver still has the value, but no longer exposes it.
> 
> Also, mlx5_esw_offloads_devlink_port_register() only reports a failure
> from mlx5_esw_devlink_port_res_register() through mlx5_core_dbg(). If
> that registration fails, nothing exposes the value at all.
> 
> max_local_SFs stays device-level and is always visible, so the two
> limits are now exposed in different ways.
> 
> SF port creation already requires switchdev mode, so in legacy mode the
> missing value is planning information rather than something needed at
> runtime. Even so, neither the commit message nor the devlink-resource.rst
> update says that a user-visible resource is being removed. Neither says
> that the port-level max_SFs entries exist only in switchdev mode.
> 
> Could this be stated, or could the device-level entry be kept for
> compatibility?
> 

legacy mode is not relevant for SFs and when no host PF rep is loaded, 
the external max SF showed 0 anyway.

> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011093828.784454-1-tariqt%40nvidia.com