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

[David Ahern <dsahern@kernel.org>]

On 6/28/26 8:22 PM, Ratheesh Kannoth wrote:
> On 2026-06-28 at 22:49:48, David Ahern (dsahern@kernel.org) wrote:
>> On 6/14/26 10:10 PM, Ratheesh Kannoth wrote:
>>> @@ -3904,6 +3935,14 @@ static int cmd_dev_param_set(struct dl *dl)
>>>  		if (!strcmp(dl->opts.param_value, ctx.value.vstr))
>>>  			return 0;
>>>  		break;
>>> +	case 129:
>>
>> no magic numbers. What does 129 represent? Is there a named macro for
>> it? If not, why not if this is part of a UAPI?
> 
> The magic number 129 actually represents DEVLINK_PARAM_TYPE_U64_ARRAY from the kernel UAPI (include/uapi/linux/devlink.h).

The uapi is actually devlink_var_attr_type and in this case
DEVLINK_VAR_ATTR_TYPE_U64_ARRAY.

> 
> The other cases in this switch block utilize MNL_TYPE_* constants from libmnl. I previously tried to
> patch libmnl to add a matching MNL_TYPE_UARR = 129 macro, but the netfilter maintainers declined it,
> noting that the enum is internal to libmnl (thread: https://lore.kernel.org/netfilter-devel/20260623043755.2435685-1-rkannoth@marvell.com/).

I agree with them.

Looking at the use of devlink_var_attr_type it is duplicating MNL values
for the early entries, but it is not nla_type as part of the nla_attr
uapi. Seems to me the 'nla_type' reference in the code is misleading.

I think cmd_dev_param_set in iproute2 needs to be changed to reference
devlink_var_attr_type instead of MNL_TYPE_*. With that as a prep patch,
add in the new use of DEVLINK_VAR_ATTR_TYPE_U64_ARRAY for this patch.
@Jiri: agree?

And then your subject line and commit message need to be updated to
reference a u64 array, not u32, correct?

> 
> To resolve this without magic numbers, I can use the existing kernel macro directly in
> the case statement with an explanatory comment, like so:
> 
> 	/* DEVLINK_PARAM_TYPE_U64_ARRAY maps to 129 */
> 	case DEVLINK_PARAM_TYPE_U64_ARRAY:
> 
> Please let me know if this approach works for you, or if you prefer a different handling.
> 
>>