[PATCH net-next v2 0/5] tools/net/ynl: Add support for nlctrl netlink family

[PATCH net-next v2 0/5] tools/net/ynl: Add support for nlctrl netlink family

[Donald Hunter]

This series adds a new YNL spec for the nlctrl family, plus some fixes
and enhancements for ynl.

Patch 1 fixes an extack decoding bug
Patch 2 gives cleaner netlink error reporting
Patch 3 fixes an array-nest codegen bug
Patch 4 adds nest-type-value support to ynl
Patch 5 contains the nlctrl spec

Changes from v1 -> v2:
 - Added patch 3 to fix codegen bug
 - Removed multi-level array-nest patch
 - Added nest-type-value patch
 - Updated nlctrl spec to align with headers and fix compilation

Donald Hunter (5):
  tools/net/ynl: Fix extack decoding for netlink-raw
  tools/net/ynl: Report netlink errors without stacktrace
  tools/net/ynl: Fix c codegen for array-nest
  tools/net/ynl: Add nest-type-value decoding
  doc/netlink/specs: Add spec for nlctrl netlink family

 Documentation/netlink/specs/nlctrl.yaml | 206 ++++++++++++++++++++++++
 tools/net/ynl/cli.py                    |  18 ++-
 tools/net/ynl/lib/__init__.py           |   4 +-
 tools/net/ynl/lib/ynl.py                |  19 ++-
 tools/net/ynl/ynl-gen-c.py              |   2 +-
 5 files changed, 238 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/netlink/specs/nlctrl.yaml

-- 
2.42.0

[PATCH net-next v2 1/5] tools/net/ynl: Fix extack decoding for netlink-raw

[Donald Hunter]

Extack decoding was using a hard-coded msg header size of 20 but
netlink-raw has a header size of 16.

Use a protocol specific msghdr_size() when decoding the attr offssets.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 239e22b7a85f..b810a478a304 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -353,6 +353,9 @@ class NetlinkProtocol:
             raise Exception(f'Multicast group "{mcast_name}" not present in the spec')
         return mcast_groups[mcast_name].value
 
+    def msghdr_size(self):
+        return 16
+
 
 class GenlProtocol(NetlinkProtocol):
     def __init__(self, family_name):
@@ -378,6 +381,8 @@ class GenlProtocol(NetlinkProtocol):
             raise Exception(f'Multicast group "{mcast_name}" not present in the family')
         return self.genl_family['mcast'][mcast_name]
 
+    def msghdr_size(self):
+        return super().msghdr_size() + 4
 
 
 class SpaceAttrs:
@@ -721,7 +726,7 @@ class YnlFamily(SpecFamily):
             return
 
         msg = self.nlproto.decode(self, NlMsg(request, 0, op.attr_set))
-        offset = 20 + self._struct_size(op.fixed_header)
+        offset = self.nlproto.msghdr_size() + self._struct_size(op.fixed_header)
         path = self._decode_extack_path(msg.raw_attrs, op.attr_set, offset,
                                         extack['bad-attr-offs'])
         if path:
-- 
2.42.0

[PATCH net-next v2 2/5] tools/net/ynl: Report netlink errors without stacktrace

[Donald Hunter]

ynl does not handle NlError exceptions so they get reported like program
failures. Handle the NlError exceptions and report the netlink errors
more cleanly.

Example now:

Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
	error: -2	extack: {'bad-attr': '.op'}

Example before:

Traceback (most recent call last):
  File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 81, in <module>
    main()
  File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 69, in main
    reply = ynl.dump(args.dump, attrs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 906, in dump
    return self._op(method, vals, [], dump=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 872, in _op
    raise NlError(nl_msg)
lib.ynl.NlError: Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
	error: -2	extack: {'bad-attr': '.op'}

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/cli.py          | 18 +++++++++++-------
 tools/net/ynl/lib/__init__.py |  4 ++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index e8a65fbc3698..f131e33ac3ee 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -6,7 +6,7 @@ import json
 import pprint
 import time
 
-from lib import YnlFamily, Netlink
+from lib import YnlFamily, Netlink, NlError
 
 
 class YnlEncoder(json.JSONEncoder):
@@ -66,12 +66,16 @@ def main():
     if args.sleep:
         time.sleep(args.sleep)
 
-    if args.do:
-        reply = ynl.do(args.do, attrs, args.flags)
-        output(reply)
-    if args.dump:
-        reply = ynl.dump(args.dump, attrs)
-        output(reply)
+    try:
+        if args.do:
+            reply = ynl.do(args.do, attrs, args.flags)
+            output(reply)
+        if args.dump:
+            reply = ynl.dump(args.dump, attrs)
+            output(reply)
+    except NlError as e:
+        print(e)
+        exit(1)
 
     if args.ntf:
         ynl.check_ntf()
diff --git a/tools/net/ynl/lib/__init__.py b/tools/net/ynl/lib/__init__.py
index f7eaa07783e7..9137b83e580a 100644
--- a/tools/net/ynl/lib/__init__.py
+++ b/tools/net/ynl/lib/__init__.py
@@ -2,7 +2,7 @@
 
 from .nlspec import SpecAttr, SpecAttrSet, SpecEnumEntry, SpecEnumSet, \
     SpecFamily, SpecOperation
-from .ynl import YnlFamily, Netlink
+from .ynl import YnlFamily, Netlink, NlError
 
 __all__ = ["SpecAttr", "SpecAttrSet", "SpecEnumEntry", "SpecEnumSet",
-           "SpecFamily", "SpecOperation", "YnlFamily", "Netlink"]
+           "SpecFamily", "SpecOperation", "YnlFamily", "Netlink", "NlError"]
-- 
2.42.0

[PATCH net-next v2 3/5] tools/net/ynl: Fix c codegen for array-nest

[Donald Hunter]

ynl-gen-c generates e.g. 'calloc(mcast_groups, sizeof(*dst->mcast_groups))'
for array-nest attrs when it should be 'n_mcast_groups'.

Add a 'n_' prefix in the generated code for array-nests.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/ynl-gen-c.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 2f5febfe66a1..67bfaff05154 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1667,7 +1667,7 @@ def _multi_parse(ri, struct, init_lines, local_vars):
         aspec = struct[anest]
 
         ri.cw.block_start(line=f"if (n_{aspec.c_name})")
-        ri.cw.p(f"dst->{aspec.c_name} = calloc({aspec.c_name}, sizeof(*dst->{aspec.c_name}));")
+        ri.cw.p(f"dst->{aspec.c_name} = calloc(n_{aspec.c_name}, sizeof(*dst->{aspec.c_name}));")
         ri.cw.p(f"dst->n_{aspec.c_name} = n_{aspec.c_name};")
         ri.cw.p('i = 0;')
         ri.cw.p(f"parg.rsp_policy = &{aspec.nested_render_name}_nest;")
-- 
2.42.0

[PATCH net-next v2 4/5] tools/net/ynl: Add nest-type-value decoding

[Donald Hunter]

The nlctrl genetlink-legacy family uses nest-type-value encoding as
described in Documentation/userspace-api/netlink/genetlink-legacy.rst

Add nest-type-value decoding to ynl.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index b810a478a304..2d7fdd903d9e 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -595,6 +595,16 @@ class YnlFamily(SpecFamily):
             decoded.append({ item.type: subattrs })
         return decoded
 
+    def _decode_nest_type_value(self, attr, attr_spec):
+        decoded = {}
+        value = attr
+        for name in attr_spec['type-value']:
+            value = NlAttr(value.raw, 0)
+            decoded[name] = value.type
+        subattrs = self._decode(NlAttrs(value.raw), attr_spec['nested-attributes'])
+        decoded.update(subattrs)
+        return decoded
+
     def _decode_unknown(self, attr):
         if attr.is_nest:
             return self._decode(NlAttrs(attr.raw), None)
@@ -686,6 +696,8 @@ class YnlFamily(SpecFamily):
                 decoded = {"value": value, "selector": selector}
             elif attr_spec["type"] == 'sub-message':
                 decoded = self._decode_sub_msg(attr, attr_spec, search_attrs)
+            elif attr_spec["type"] == 'nest-type-value':
+                decoded = self._decode_nest_type_value(attr, attr_spec)
             else:
                 if not self.process_unknown:
                     raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')
-- 
2.42.0

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

[Donald Hunter]

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

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
+
-- 
2.42.0

Re: [PATCH net-next v2 1/5] tools/net/ynl: Fix extack decoding for netlink-raw

[Jakub Kicinski]

On Wed,  6 Mar 2024 12:57:00 +0000 Donald Hunter wrote:
> Extack decoding was using a hard-coded msg header size of 20 but
> netlink-raw has a header size of 16.
> 
> Use a protocol specific msghdr_size() when decoding the attr offssets.
> 
> Signed-off-by: Donald Hunter <donald.hunter@gmail.com>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next v2 2/5] tools/net/ynl: Report netlink errors without stacktrace

[Jakub Kicinski]

On Wed,  6 Mar 2024 12:57:01 +0000 Donald Hunter wrote:
> ynl does not handle NlError exceptions so they get reported like program
> failures. Handle the NlError exceptions and report the netlink errors
> more cleanly.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next v2 3/5] tools/net/ynl: Fix c codegen for array-nest

[Jakub Kicinski]

On Wed,  6 Mar 2024 12:57:02 +0000 Donald Hunter wrote:
> ynl-gen-c generates e.g. 'calloc(mcast_groups, sizeof(*dst->mcast_groups))'
> for array-nest attrs when it should be 'n_mcast_groups'.
> 
> Add a 'n_' prefix in the generated code for array-nests.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next v2 4/5] tools/net/ynl: Add nest-type-value decoding

[Jakub Kicinski]

On Wed,  6 Mar 2024 12:57:03 +0000 Donald Hunter wrote:
> The nlctrl genetlink-legacy family uses nest-type-value encoding as
> described in Documentation/userspace-api/netlink/genetlink-legacy.rst
> 
> Add nest-type-value decoding to ynl.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

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

[Jakub Kicinski]

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 :)

Re: [PATCH net-next v2 0/5] tools/net/ynl: Add support for nlctrl netlink family

[Jakub Kicinski]

On Wed,  6 Mar 2024 12:56:59 +0000 Donald Hunter wrote:
> This series adds a new YNL spec for the nlctrl family, plus some fixes
> and enhancements for ynl.
> 
> Patch 1 fixes an extack decoding bug
> Patch 2 gives cleaner netlink error reporting
> Patch 3 fixes an array-nest codegen bug
> Patch 4 adds nest-type-value support to ynl
> Patch 5 contains the nlctrl spec

Somewhat incredibly the C seems to work, I tested with this sample:

// SPDX-License-Identifier: GPL-2.0
#include <stdio.h>
#include <string.h>

#include <ynl.h>

#include "nlctrl-user.h"

int main(int argc, char **argv)
{
	struct nlctrl_getfamily_list *fams;
	struct ynl_error yerr;
	struct ynl_sock *ys;
	char *name;

	ys = ynl_sock_create(&ynl_nlctrl_family, &yerr);
	if (!ys) {
		fprintf(stderr, "YNL: %s\n", yerr.msg);
		return 1;
	}

	printf("Select family ($name; or 0 = dump): ");
	scanf("%ms", &name);

	if (!name || !strcmp(name, "0")) {
		fams = nlctrl_getfamily_dump(ys);
		if (!fams)
			goto err_close;

		ynl_dump_foreach(fams, f)
			printf("%d: %s\n", f->family_id, f->family_name);
		nlctrl_getfamily_list_free(fams);
	} else {
		struct nlctrl_getfamily_req *req;
		struct nlctrl_getfamily_rsp *f;

		req = nlctrl_getfamily_req_alloc();
		nlctrl_getfamily_req_set_family_name(req, name);

		f = nlctrl_getfamily(ys, req);
		nlctrl_getfamily_req_free(req);
		if (!f)
			goto err_close;

		printf("%d: %s\n", f->family_id, f->family_name);
		nlctrl_getfamily_rsp_free(f);
	}
	free(name);

	ynl_sock_destroy(ys);
	return 0;

err_close:
	fprintf(stderr, "YNL: %s\n", ys->err.msg);
	ynl_sock_destroy(ys);
	return 2;
}

Re: [PATCH net-next v2 0/5] tools/net/ynl: Add support for nlctrl netlink family

[Donald Hunter]

On Wed, 6 Mar 2024 at 18:32, Jakub Kicinski <kuba@kernel.org> wrote:
>
> Somewhat incredibly the C seems to work, I tested with this sample:

Wow, above and beyond for writing a sample. Was it ever in any doubt
that it would work ;-)

> // SPDX-License-Identifier: GPL-2.0
> #include <stdio.h>
> #include <string.h>
>
> #include <ynl.h>
>
> #include "nlctrl-user.h"
>
> int main(int argc, char **argv)
> {
>         struct nlctrl_getfamily_list *fams;
>         struct ynl_error yerr;
>         struct ynl_sock *ys;
>         char *name;
>
>         ys = ynl_sock_create(&ynl_nlctrl_family, &yerr);
>         if (!ys) {
>                 fprintf(stderr, "YNL: %s\n", yerr.msg);
>                 return 1;
>         }
>
>         printf("Select family ($name; or 0 = dump): ");
>         scanf("%ms", &name);
>
>         if (!name || !strcmp(name, "0")) {
>                 fams = nlctrl_getfamily_dump(ys);
>                 if (!fams)
>                         goto err_close;
>
>                 ynl_dump_foreach(fams, f)
>                         printf("%d: %s\n", f->family_id, f->family_name);
>                 nlctrl_getfamily_list_free(fams);
>         } else {
>                 struct nlctrl_getfamily_req *req;
>                 struct nlctrl_getfamily_rsp *f;
>
>                 req = nlctrl_getfamily_req_alloc();
>                 nlctrl_getfamily_req_set_family_name(req, name);
>
>                 f = nlctrl_getfamily(ys, req);
>                 nlctrl_getfamily_req_free(req);
>                 if (!f)
>                         goto err_close;
>
>                 printf("%d: %s\n", f->family_id, f->family_name);
>                 nlctrl_getfamily_rsp_free(f);
>         }
>         free(name);
>
>         ynl_sock_destroy(ys);
>         return 0;
>
> err_close:
>         fprintf(stderr, "YNL: %s\n", ys->err.msg);
>         ynl_sock_destroy(ys);
>         return 2;
> }

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

[Donald Hunter]

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.

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

[Jakub Kicinski]

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:

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

[Donald Hunter]

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.