netdev - Re: [PATCH net-next 01/14] devlink: define enum for attr types of dynamic attributes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9tJEDfPK0HecSR5@x130>
Date: Wed, 19 Mar 2025 15:45:36 -0700
From: Saeed Mahameed <saeed@...nel.org>
To: Simon Horman <horms@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
	Tariq Toukan <tariqt@...dia.com>, Gal Pressman <gal@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>, Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net-next 01/14] devlink: define enum for attr types of
 dynamic attributes

On 06 Mar 12:05, Simon Horman wrote:
>On Thu, Feb 27, 2025 at 06:12:14PM -0800, Saeed Mahameed wrote:
>> From: Jiri Pirko <jiri@...dia.com>
>>
>> Devlink param and health reporter fmsg use attributes with dynamic type
>> which is determined according to a different type. Currently used values
>> are NLA_*. The problem is, they are not part of UAPI. They may change
>> which would cause a break.
>>
>> To make this future safe, introduce a enum that shadows NLA_* values in
>> it and is part of UAPI.
>>
>> Also, this allows to possibly carry types that are unrelated to NLA_*
>> values.
>>
>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
>> ---
>>  Documentation/netlink/specs/devlink.yaml | 23 ++++++++++++++++++
>>  include/uapi/linux/devlink.h             | 17 ++++++++++++++
>>  net/devlink/health.c                     | 17 ++++++++++++--
>>  net/devlink/param.c                      | 30 ++++++++++++------------
>>  4 files changed, 70 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
>> index 09fbb4c03fc8..e99fc51856c5 100644
>> --- a/Documentation/netlink/specs/devlink.yaml
>> +++ b/Documentation/netlink/specs/devlink.yaml
>> @@ -202,6 +202,29 @@ definitions:
>>          name: exception
>>        -
>>          name: control
>> +  -
>> +    type: enum
>> +    name: dyn_attr_type
>> +    entries:
>> +      -
>> +        name: u8
>> +        value: 1
>> +      -
>> +        name: u16
>> +      -
>> +        name: u32
>> +      -
>> +        name: u64
>> +      -
>> +        name: string
>> +      -
>> +        name: flag
>> +      -
>> +        name: nul_string
>> +        value: 10
>> +      -
>> +        name: binary
>> +  -
^^^^^^^ extra empty type.. 

>
>Hi Saeed,
>
>Thanks for these patches.
>
>Unfortunately the above seems to cause ynl-regen to blow up.

Thanks Simon, good catch, there was an extra empty type added by mistake,
see above, will remove in V2.

>I don't know why, but I see:
>
>  $ ./tools/net/ynl/ynl-regen.sh
>  Traceback (most recent call last):
>    File "/home/nipa/net-next/wt-1/tools/net/ynl/pyynl/ynl_gen_c.py", line 3074, in <module>
>      main()
>      ~~~~^^
>    File "/home/nipa/net-next/wt-1/tools/net/ynl/pyynl/ynl_gen_c.py", line 2783, in main
>      parsed = Family(args.spec, exclude_ops)
>    File "/home/nipa/net-next/wt-1/tools/net/ynl/pyynl/ynl_gen_c.py", line 954, in __init__
>      super().__init__(file_name, exclude_ops=exclude_ops)
>      ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    File "/home/nipa/net-next/wt-1/tools/net/ynl/pyynl/lib/nlspec.py", line 462, in __init__
>      jsonschema.validate(self.yaml, schema)
>      ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
>    File "/usr/lib/python3.13/site-packages/jsonschema/validators.py", line 1307, in validate
>      raise error
>  jsonschema.exceptions.ValidationError: None is not of type 'object'
>
>  ...
>
>Perhaps I need a newer version of jsonschema?
>
>
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 9401aa343673..8cdd60eb3c43 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -385,6 +385,23 @@ enum devlink_linecard_state {
>>  	DEVLINK_LINECARD_STATE_MAX = __DEVLINK_LINECARD_STATE_MAX - 1
>>  };
>>
>> +/**
>> + * enum devlink_dyn_attr_type - Dynamic attribute type type.
>
>Perhaps this relates to auto-generation.
>But I think the Kernel doc should also document the enum values:
>DEVLINK_DYN_ATTR_TYPE_U8, ...
>
>> + */
>> +enum devlink_dyn_attr_type {
>> +	/* Following values relate to the internal NLA_* values */
>> +	DEVLINK_DYN_ATTR_TYPE_U8 = 1,
>> +	DEVLINK_DYN_ATTR_TYPE_U16,
>> +	DEVLINK_DYN_ATTR_TYPE_U32,
>> +	DEVLINK_DYN_ATTR_TYPE_U64,
>> +	DEVLINK_DYN_ATTR_TYPE_STRING,
>> +	DEVLINK_DYN_ATTR_TYPE_FLAG,
>> +	DEVLINK_DYN_ATTR_TYPE_NUL_STRING = 10,
>> +	DEVLINK_DYN_ATTR_TYPE_BINARY,
>> +	__DEVLINK_DYN_ATTR_TYPE_CUSTOM_BASE = 0x80,
>> +	/* Any possible custom types, unrelated to NLA_* values go below */
>> +};
>> +
>>  enum devlink_attr {
>>  	/* don't change the order or add anything between, this is ABI! */
>>  	DEVLINK_ATTR_UNSPEC,
>
>...