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

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-06-30 at 20:06:17, David Ahern (dsahern@kernel.org) wrote:
> 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?
Yes, this aligns with the current kernel-side limits. See:
https://lore.kernel.org/all/20260609040453.711932-5-rkannoth@marvell.com/

>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?
The kernel strictly validates and restricts the number of parameters. To be safe, this patch
adds an explicit bounds check to prevent userspace issues if that threshold is ever crossed.

Ideally, since "union devlink_param_value" is omitted from the UAPI, we have to define
DEVLINK_PARAM_MAX_ARRAY_SIZE here. Moving the underlying structures to the UAPI in the
future would allow us to share a single definition and avoid this hardcoded value in userspace.

>
> > +
> > +struct devlink_param_u64_array {
> > +	uint64_t size;
> > +	uint64_t val[DEVLINK_PARAM_MAX_ARRAY_SIZE];
> > +};
> > +
> > +
> > +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.
ACK.
>
> > +		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?
ACK.
>