Re: [PATCH v16 net-next 4/9] devlink: Implement devlink param multi attribute nested data values

[Jakub Kicinski]

On Thu, 21 May 2026 15:22:58 +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 u64 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.

> diff --git a/net/devlink/param.c b/net/devlink/param.c
> index 1a196d3a843d..6e0e48696f4a 100644
> --- a/net/devlink/param.c
> +++ b/net/devlink/param.c
> @@ -252,6 +252,15 @@ devlink_nl_param_value_put(struct sk_buff *msg, enum devlink_param_type type,
>  				return -EMSGSIZE;
>  		}
>  		break;
> +	case DEVLINK_PARAM_TYPE_U64_ARRAY:
> +		for (int i = 0; i < val->u64arr.size; i++) {
> +			if (i >= __DEVLINK_PARAM_MAX_ARRAY_SIZE)
> +				break;

Why not check this before the loop and return an error if someone tries
to dump too much data?

> +			if (nla_put_uint(msg, nla_type, val->u64arr.val[i]))
> +				return -EMSGSIZE;
> +		}
> +		break;
>  	}
>  	return 0;
>  }
> @@ -304,56 +313,78 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
>  				 u32 portid, u32 seq, int flags,
>  				 struct netlink_ext_ack *extack)
>  {
> -	union devlink_param_value default_value[DEVLINK_PARAM_CMODE_MAX + 1];
> -	union devlink_param_value param_value[DEVLINK_PARAM_CMODE_MAX + 1];
>  	bool default_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
>  	bool param_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
>  	const struct devlink_param *param = param_item->param;
> -	struct devlink_param_gset_ctx ctx;
> +	union devlink_param_value *default_value;
> +	union devlink_param_value *param_value;
> +	struct devlink_param_gset_ctx *ctx;
>  	struct nlattr *param_values_list;
>  	struct nlattr *param_attr;
>  	void *hdr;
>  	int err;
>  	int i;
>  
> +	default_value = kcalloc(DEVLINK_PARAM_CMODE_MAX + 1,
> +				sizeof(*default_value), GFP_KERNEL);
> +	if (!default_value)
> +		return -ENOMEM;
> +
> +	param_value = kcalloc(DEVLINK_PARAM_CMODE_MAX + 1,
> +			      sizeof(*param_value), GFP_KERNEL);
> +	if (!param_value) {
> +		kfree(default_value);
> +		return -ENOMEM;
> +	}
> +
> +	ctx = kzalloc_obj(*ctx);
> +	if (!ctx) {
> +		kfree(param_value);
> +		kfree(default_value);
> +		return -ENOMEM;
> +	}

The mem alloc should preferably be a separate patch for ease of review.

>  static void devlink_param_notify(struct devlink *devlink,
> @@ -507,7 +545,7 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
>  				  union devlink_param_value *value)
>  {
>  	struct nlattr *param_data;
> -	int len;
> +	int len, cnt, rem;
>  
>  	param_data = info->attrs[DEVLINK_ATTR_PARAM_VALUE_DATA];
>  
> @@ -547,6 +585,26 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
>  			return -EINVAL;
>  		value->vbool = nla_get_flag(param_data);
>  		break;
> +
> +	case DEVLINK_PARAM_TYPE_U64_ARRAY:
> +		cnt = 0;
> +		nla_for_each_attr_type(param_data,
> +				       DEVLINK_ATTR_PARAM_VALUE_DATA,
> +				       genlmsg_data(info->genlhdr),
> +				       genlmsg_len(info->genlhdr), rem) {
> +			if (cnt >= __DEVLINK_PARAM_MAX_ARRAY_SIZE)
> +				return -EMSGSIZE;
> +
> +			if ((nla_len(param_data) != sizeof(u64)) &&
> +			    (nla_len(param_data) != sizeof(u32)))

NL_SET_BAD_ATTR() would be good here

> +				return -EINVAL;
> +
> +			value->u64arr.val[cnt] = (u64)nla_get_uint(param_data);

Why the cast? Looks like a leftover..

> +			cnt++;
> +		}
> +
> +		value->u64arr.size = cnt;
> +		break;
>  	}
>  	return 0;
>  }