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] [day] [month] [year] [list]

Message-ID: <20250306120545.GY3666230@kernel.org>
Date: Thu, 6 Mar 2025 12:05:45 +0000
From: Simon Horman <horms@...nel.org>
To: Saeed Mahameed <saeed@...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 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
> +  -
>  
>  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,

...