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

Simon Horman
2025-03-06T12:05:50Z

On Thu, Feb 27, 2025 at 06:12:14PM -0800, Saeed Mahameed wrote:
> From: Jiri Pirko <jiri@nvidia.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@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.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
> + -
>
> attribute-sets:
> -

Hi Saeed,

Thanks for these patches.

Unfortunately the above seems to cause ynl-regen to blow up.
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,

...