Re: [PATCH net-next V4 2/2] net/mlx5: implement max_sfs parameter

[Nikolay Aleksandrov <razor@blackwall.org>]

On Thu, Jul 23, 2026 at 11:26:43AM +0300, Tariq Toukan wrote:
> From: Nikolay Aleksandrov <nikolay@nvidia.com>
> 
> Implement max_sfs generic parameter to allow users to control the total
> light-weight NIC subfunctions that can be created using devlink instead
> of external vendor tools. A value of 0 will effectively disable creation
> of new subfunction devices. A warning is sent to user-space via extack
> (returning extack without error code is interpreted as a warning by
> user-space tools). The maximum value is capped at U16_MAX.
> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  Documentation/networking/devlink/mlx5.rst     |   7 +-
>  .../mellanox/mlx5/core/lib/nv_param.c         | 118 +++++++++++++++++-
>  2 files changed, 121 insertions(+), 4 deletions(-)
> 
 
Sashiko has 2 comments:
> +	data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
> +	MLX5_SET(nv_global_pci_conf, data, per_pf_num_sf, !!ctx->val.vu32);
 If max_sfs is set to 0 for a single Physical Function (PF), does this
 unintentionally clear the global per_pf_num_sf bit?
 The commit message and documentation indicate max_sfs applies to each PF
 independently. However, clearing this global device configuration bit would
 silently disable Subfunction capabilities for all other PFs on the entire
 device.
 Could this break Subfunction configurations on independent ports?

Answer: Yes, it does. I just tested it and it switches the config for both
        PFs on a device. Somehow I missed this, and it was in earlier
        reviews as well. The !!ctx->val.vu32 should just be 1.
        I'm sorry for the noise, I'll fix it and queue v5.
and

Does this unconditionally overwrite any existing configuration for the
SubFunction BAR size?
When a user configures a custom log_sf_bar_size via firmware tools like
mlxconfig, and then later uses devlink to change max_sfs, it appears this
line discards any non-zero value already present in the configuration data
and forcibly replaces it with MLX5_DEFAULT_LOG_SF_BAR_SIZE.
Should this code preserve the existing log_sf_bar_size if it was already
configured?

Answer: Yes, it does and it is okay. That was intentional.