| Message ID | 20240306125704.63934-6-donald.hunter@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tools/net/ynl: Add support for nlctrl netlink family | expand |
On Wed, 6 Mar 2024 12:57:04 +0000 Donald Hunter wrote: > diff --git a/Documentation/netlink/specs/nlctrl.yaml b/Documentation/netlink/specs/nlctrl.yaml > new file mode 100644 > index 000000000000..2e55e61aea11 > --- /dev/null > +++ b/Documentation/netlink/specs/nlctrl.yaml > @@ -0,0 +1,206 @@ > +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) > + > +name: nlctrl > +protocol: genetlink-legacy > +uapi-header: linux/genetlink.h > + > +doc: | > + Genetlink control. How about: genetlink meta-family, exposes information about all genetlink families registered in the kernel (including itself). > +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? Unnamed enums are kinda special in my mind, because we will use normal integer types to store the values in code gen. > + entries: > + - admin-perm > + - cmd-cap-do > + - cmd-cap-dump > + - cmd-cap-haspol > + - uns-admin-perm > + - > + name: attr-type > + enum-name: netlink_attribute_type s/_/-/ The codegen will convert them back > + type: enum > + entries: > + - invalid > + - flag > + - u8 > + - u16 > + - u32 > + - u64 > + - s8 > + - s16 > + - s32 > + - s64 > + - binary > + - string > + - nul-string > + - nested > + - nested-array > + - bitfield32 > + - sint > + - uint > + > +attribute-sets: > + - > + name: ctrl-attrs > + name-prefix: CTRL_ATTR_ also: s/_/-/ and lower case, code-gen will take care of the exact formatting. With those nits: Acked-by: Jakub Kicinski <kuba@kernel.org> I haven't checked the exact formatting, but off the top of my head the contents look good :)
On Wed, 6 Mar 2024 at 18:31, Jakub Kicinski <kuba@kernel.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@kernel.org> > > I haven't checked the exact formatting, but off the top of my head > the contents look good :) Thx.
On Wed, 6 Mar 2024 22:54:08 +0000 Donald Hunter wrote: > > 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. Hm, is this some new version of jsonschema perhaps? We use empty values all over the place: $ git grep 'enum-name:$' -- Documentation/netlink/specs/ Documentation/netlink/specs/ethtool.yaml: enum-name: Documentation/netlink/specs/fou.yaml: enum-name: Documentation/netlink/specs/ovs_datapath.yaml: enum-name: Documentation/netlink/specs/ovs_flow.yaml: enum-name: Documentation/netlink/specs/ovs_flow.yaml: enum-name:
Jakub Kicinski <kuba@kernel.org> writes: > On Wed, 6 Mar 2024 22:54:08 +0000 Donald Hunter wrote: >> > 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. > > Hm, is this some new version of jsonschema perhaps? > We use empty values all over the place: > > $ git grep 'enum-name:$' -- Documentation/netlink/specs/ > Documentation/netlink/specs/ethtool.yaml: enum-name: > Documentation/netlink/specs/fou.yaml: enum-name: > Documentation/netlink/specs/ovs_datapath.yaml: enum-name: > Documentation/netlink/specs/ovs_flow.yaml: enum-name: > Documentation/netlink/specs/ovs_flow.yaml: enum-name: Ah, sorry I should have said that enum-name in definitions already had 'type: [ string, "null" ]'. It was missing from enum-name in attribute sets and operations. It turns out that all the existing usage was for definitions.
diff --git a/Documentation/netlink/specs/nlctrl.yaml b/Documentation/netlink/specs/nlctrl.yaml new file mode 100644 index 000000000000..2e55e61aea11 --- /dev/null +++ b/Documentation/netlink/specs/nlctrl.yaml @@ -0,0 +1,206 @@ +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) + +name: nlctrl +protocol: genetlink-legacy +uapi-header: linux/genetlink.h + +doc: | + Genetlink control. + +definitions: + - + name: op-flags + type: flags + enum-name: '' + entries: + - admin-perm + - cmd-cap-do + - cmd-cap-dump + - cmd-cap-haspol + - uns-admin-perm + - + name: attr-type + enum-name: netlink_attribute_type + type: enum + entries: + - invalid + - flag + - u8 + - u16 + - u32 + - u64 + - s8 + - s16 + - s32 + - s64 + - binary + - string + - nul-string + - nested + - nested-array + - bitfield32 + - sint + - uint + +attribute-sets: + - + name: ctrl-attrs + name-prefix: CTRL_ATTR_ + attributes: + - + name: family-id + type: u16 + - + name: family-name + type: string + - + name: version + type: u32 + - + name: hdrsize + type: u32 + - + name: maxattr + type: u32 + - + name: ops + type: array-nest + nested-attributes: op-attrs + - + name: mcast-groups + type: array-nest + nested-attributes: mcast-group-attrs + - + name: policy + type: nest-type-value + type-value: [ policy-id, attr-id ] + nested-attributes: policy-attrs + - + name: op-policy + type: nest-type-value + type-value: [ op-id ] + nested-attributes: op-policy-attrs + - + name: op + type: u32 + - + name: mcast-group-attrs + name-prefix: CTRL_ATTR_MCAST_GRP_ + enum-name: '' + attributes: + - + name: name + type: string + - + name: id + type: u32 + - + name: op-attrs + name-prefix: CTRL_ATTR_OP_ + enum-name: '' + attributes: + - + name: id + type: u32 + - + name: flags + type: u32 + enum: op-flags + enum-as-flags: true + - + name: policy-attrs + name-prefix: NL_POLICY_TYPE_ATTR_ + enum-name: '' + attributes: + - + name: type + type: u32 + enum: attr-type + - + name: min-value-s + type: s64 + - + name: max-value-s + type: s64 + - + name: min-value-u + type: u64 + - + name: max-value-u + type: u64 + - + name: min-length + type: u32 + - + name: max-length + type: u32 + - + name: policy-idx + type: u32 + - + name: policy-maxtype + type: u32 + - + name: bitfield32-mask + type: u32 + - + name: mask + type: u64 + - + name: pad + type: pad + - + name: op-policy-attrs + name-prefix: CTRL_ATTR_POLICY_ + enum-name: '' + attributes: + - + name: do + type: u32 + - + name: dump + type: u32 + +operations: + enum-model: directional + name-prefix: CTRL_CMD_ + list: + - + name: getfamily + doc: Get / dump genetlink families + attribute-set: ctrl-attrs + do: + request: + value: 3 + attributes: + - family-name + reply: &all_attrs + value: 1 + attributes: + - family-id + - family-name + - hdrsize + - maxattr + - mcast-groups + - ops + - version + dump: + reply: *all_attrs + - + name: getpolicy + doc: Get / dump genetlink policies + attribute-set: ctrl-attrs + dump: + request: + value: 10 + attributes: + - family-name + - family-id + - op + reply: + value: 10 + attributes: + - family-id + - op-policy + - policy +
Add a spec for the nlctrl family. Example usage: ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/nlctrl.yaml \ --do getfamily --json '{"family-name": "nlctrl"}' ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/nlctrl.yaml \ --dump getpolicy --json '{"family-name": "nlctrl"}' Signed-off-by: Donald Hunter <donald.hunter@gmail.com> --- Documentation/netlink/specs/nlctrl.yaml | 206 ++++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 Documentation/netlink/specs/nlctrl.yaml