Re: [PATCH iproute2-next v2 2/2] devlink: support u64-array values in devlink param show/set

[David Ahern <dsahern@kernel.org>]

On 6/29/26 7:50 PM, Ratheesh Kannoth wrote:
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index 9372e92f..3c29601d 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -3496,13 +3496,115 @@ static const struct param_val_conv param_val_conv[] = {
>  };
>  
>  #define PARAM_VAL_CONV_LEN ARRAY_SIZE(param_val_conv)
> +#define DEVLINK_PARAM_MAX_ARRAY_SIZE 32

Why 32? Is that based on current code? How does the kernel side handle
the number of parameters? What happens if the kernel sends more than 32
parameters - from a user's perspective, not this code and processing the
output?

> +
> +struct devlink_param_u64_array {
> +	uint64_t size;
> +	uint64_t val[DEVLINK_PARAM_MAX_ARRAY_SIZE];
> +};
> +
> +static int param_value_nested_u64_attr_cb(const struct nlattr *attr, void *data)
> +{
> +	struct devlink_param_u64_array *arr = data;
> +	unsigned int len;
> +
> +	if (mnl_attr_get_type(attr) != DEVLINK_ATTR_PARAM_VALUE_DATA)
> +		return MNL_CB_OK;
> +
> +	if (arr->size >= DEVLINK_PARAM_MAX_ARRAY_SIZE)
> +		return MNL_CB_ERROR;
> +
> +	len = mnl_attr_get_payload_len(attr);
> +	if (len == sizeof(uint32_t))
> +		arr->val[arr->size++] = mnl_attr_get_u32(attr);
> +	else if (len == sizeof(uint64_t))
> +		arr->val[arr->size++] = mnl_attr_get_u64(attr);
> +	else
> +		return MNL_CB_ERROR;
> +
> +	return MNL_CB_OK;
> +}
> +
> +static int param_value_u64_array_fill(struct nlattr *nl,
> +				      struct devlink_param_u64_array *arr)
> +{
> +	int err;
> +
> +	arr->size = 0;
> +	err = mnl_attr_parse_nested(nl, param_value_nested_u64_attr_cb, arr);
> +	if (err != MNL_CB_OK)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static bool param_value_u64_array_equal(const struct devlink_param_u64_array *a,
> +					const struct devlink_param_u64_array *b)
> +{
> +	uint64_t i;
> +
> +	if (a->size != b->size)
> +		return false;
> +
> +	for (i = 0; i < a->size; i++) {
> +		if (a->val[i] != b->val[i])
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +static int param_value_u64_array_put_from_str(struct nlmsghdr *nlh,
> +					      const char *param_value,
> +					      const struct devlink_param_u64_array *cur)
> +{
> +	struct devlink_param_u64_array new_arr = {};
> +	char *copy, *token, *saveptr = NULL;
> +	char delim[] = " ,";
> +	uint64_t val;
> +	int err;
> +
> +	copy = strdup(param_value);
> +	if (!copy)
> +		return -ENOMEM;
> +
> +	token = strtok_r(copy, delim, &saveptr);
> +	while (token) {
> +		if (new_arr.size >= DEVLINK_PARAM_MAX_ARRAY_SIZE) {
> +			free(copy);
> +			pr_err("Too many array elements (max %d)\n",
> +			       DEVLINK_PARAM_MAX_ARRAY_SIZE);
> +			return -EINVAL;
> +		}
> +		err = get_u64((__u64 *)&val, token, 10);
> +		if (err) {
> +			free(copy);
> +			pr_err("Value \"%s\" is not a number or not within range\n",
> +			       token);
> +			return err;
> +		}
> +		new_arr.val[new_arr.size++] = val;
> +		token = strtok_r(NULL, delim, &saveptr);
> +	}
> +	free(copy);
> +
> +	if (cur && param_value_u64_array_equal(&new_arr, cur))
> +		return 1;
> +
> +	for (uint64_t i = 0; i < new_arr.size; i++)

put the declaration at the top of the function with the rest of them.
global comment; fix all of them.

> +		mnl_attr_put_u64(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, new_arr.val[i]);

Why can't this put be done in the loop above as the string is processed?

> +
> +	return 0;
> +}
>  
>  static int pr_out_param_value_print(const char *nla_name, int nla_type,
>  				     struct nlattr *val_attr, bool conv_exists,
> -				     const char *label, bool flag_as_u8)
> +				     const char *label, bool flag_as_u8, struct nlattr *nl)
>  {
> +	struct devlink_param_u64_array u64_arr = { };
>  	const char *vstr;
> -	int err;
> +	char buffer[1024];
> +	int err, cnt = 0;
>  
>  	print_string(PRINT_FP, NULL, " %s ", label);
>