Re: [PATCH v7 net-next 2/5] devlink: Implement devlink param multi attribute nested data values

[Simon Horman]

On Mon, Mar 23, 2026 at 09:21:07AM +0530, Ratheesh Kannoth wrote:
> From: Saeed Mahameed <saeedm@xxxxxxxxxx>
> 
> Devlink param value attribute is not defined since devlink is handling
> the value validating and parsing internally, this allows us to implement
> multi attribute values without breaking any policies.
> 
> Devlink param multi-attribute values are considered to be dynamically
> sized arrays of u64 values, by introducing a new devlink param type
> DEVLINK_PARAM_TYPE_U64_ARRAY, driver and user space can set a variable
> count of u32 values into the DEVLINK_ATTR_PARAM_VALUE_DATA attribute.
> 
> Implement get/set parsing and add to the internal value structure passed
> to drivers.
> 
> This is useful for devices that need to configure a list of values for
> a specific configuration.
> 
> example:
> $ devlink dev param show pci/... name multi-value-param
> name multi-value-param type driver-specific
> values:
> cmode permanent value: 0,1,2,3,4,5,6,7
> 
> $ devlink dev param set pci/... name multi-value-param \
> 		value 4,5,6,7,0,1,2,3 cmode permanent
> 
> Signed-off-by: Saeed Mahameed <saeedm@xxxxxxxxxx>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

...

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> index 2eb666a46f39..e96dfe322c14 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> @@ -43,7 +43,6 @@ struct mlx5e_pcie_cong_event {
>  	struct mlx5e_pcie_cong_stats stats;
>  };
>  
> -
>  static const struct counter_desc mlx5e_pcie_cong_stats_desc[] = {
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_pcie_cong_stats,
>  			     pci_bw_inbound_high) },

The above hunk seems to have been included by mistake.

> @@ -259,7 +258,11 @@ mlx5e_pcie_cong_get_thresh_config(struct mlx5_core_dev *dev,
>  		MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH,
>  	};
>  	struct devlink *devlink = priv_to_devlink(dev);
> -	union devlink_param_value val[4];
> +	union devlink_param_value *val;
> +
> +	val = kcalloc(4, sizeof(*val), GFP_KERNEL);
> +	if (!val)
> +		return -ENOMEM;
>  
>  	for (int i = 0; i < 4; i++) {
>  		u32 id = ids[i];

The lines following this hunk are:

		int err;

		err = devl_param_driverinit_value_get(devlink, id, &val[i]);
		if (err)
			return err;

AI review points out that if the error condition above is reached,
then val will be leaked.

> @@ -275,6 +278,7 @@ mlx5e_pcie_cong_get_thresh_config(struct mlx5_core_dev *dev,
>  	config->outbound_low = val[2].vu16;
>  	config->outbound_high = val[3].vu16;
>  
> +	kfree(val);
>  	return 0;
>  }
>  

Overall I would suggest breaking the mlx5 driver changes out into a
separate preparatory patch, which precedes the devlink patch in the
patch-set.

...

-- 
pw-bot: changes-requested