netdev - Re: [PATCH net-next v2 5/5] doc/netlink/specs: Add spec for nlctrl netlink family

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: <CAD4GDZwtD7v_zQzeGDu93sropHbRsRANUMJ-MAB1w+CZCMyXuQ@mail.gmail.com>
Date: Wed, 6 Mar 2024 22:54:08 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, 
	Jacob Keller <jacob.e.keller@...el.com>, Jiri Pirko <jiri@...nulli.us>, 
	Stanislav Fomichev <sdf@...gle.com>, donald.hunter@...hat.com
Subject: Re: [PATCH net-next v2 5/5] doc/netlink/specs: Add spec for nlctrl
 netlink family

On Wed, 6 Mar 2024 at 18:31, Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed,  6 Mar 2024 12:57:04 +0000 Donald Hunter wrote:
> > +doc: |
> > +  Genetlink control.
>
> How about:
>
>   genetlink meta-family, exposes information about all
>   genetlink families registered in the kernel (including itself).

Ack. Much better than my lazy boilerplate.

> > +definitions:
> > +  -
> > +    name: op-flags
> > +    type: flags
> > +    enum-name: ''
>
> I've used
>         enum-name:
> i.e. empty value in other places.
> Is using empty string more idiomatic?

I got this when I tried to use an empty value, so I used '' everywhere instead.

jsonschema.exceptions.ValidationError: None is not of type 'string'

Failed validating 'type' in
schema['properties']['attribute-sets']['items']['properties']['enum-name']:
    {'description': 'Name for the enum type of the attribute.',
     'type': 'string'}

On instance['attribute-sets'][1]['enum-name']:
    None

It turns out that the fix for that is a schema change:

--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -169,7 +169,7 @@ properties:
           type: string
         enum-name:
           description: Name for the enum type of the attribute.
-          type: string
+          type: [ string, "null" ]
         doc:
           description: Documentation of the space.
           type: string

I'll respin with a cleaned up nlctrl spec and fixes for the schemas.

> Unnamed enums are kinda special in my mind, because we will use normal
> integer types to store the values in code gen.

Yeah, unfortunately the genetlink uapi uses unnamed enums for everything.

https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/genetlink.h#L40

[...]

> s/_/-/
> The codegen will convert them back

Ack.

> > +    name: ctrl-attrs
> > +    name-prefix: CTRL_ATTR_
>
> also: s/_/-/ and lower case, code-gen will take care of the exact
> formatting.

Ack.

> With those nits:
>
> Acked-by: Jakub Kicinski <kuba@...nel.org>
>
> I haven't checked the exact formatting, but off the top of my head
> the contents look good :)

Thx.