[PATCH net-next 00/11] tools: ynl: prepare for wireguard

[PATCH net-next 00/11] tools: ynl: prepare for wireguard

[Asbjørn Sloth Tønnesen]

This series contains the last batch of YNL changes to support
the wireguard YNL conversion.

The next batch has been posted as an RFC series here:
https://lore.kernel.org/netdev/20250904-wg-ynl-rfc@fiberby.net/

Asbjørn Sloth Tønnesen (11):
  tools: ynl-gen: allow overriding name-prefix for constants
  tools: ynl-gen: generate nested array policies
  tools: ynl-gen: add sub-type check
  tools: ynl-gen: define count iterator in print_dump()
  tools: ynl-gen: define nlattr *array in a block scope
  tools: ynl-gen: don't validate nested array attribute types
  tools: ynl-gen: rename TypeArrayNest to TypeIndexedArray
  tools: ynl: move nest packing to a helper function
  tools: ynl: encode indexed-array
  tools: ynl: decode hex input
  tools: ynl: add ipv4-or-v6 display hint

 Documentation/netlink/genetlink-legacy.yaml |  2 +-
 tools/net/ynl/pyynl/lib/ynl.py              | 36 +++++++++++++++---
 tools/net/ynl/pyynl/ynl_gen_c.py            | 42 ++++++++++++++-------
 3 files changed, 59 insertions(+), 21 deletions(-)

-- 
2.51.0

[PATCH net-next 01/11] tools: ynl-gen: allow overriding name-prefix for constants

[Asbjørn Sloth Tønnesen]

Allow using custom name-prefix with constants,
just like it is for enum and flags declarations.

This is needed for generating WG_KEY_LEN in
include/uapi/linux/wireguard.h from a spec.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index fb7e03805a11..1543d4911bf5 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -3211,8 +3211,9 @@ def render_uapi(family, cw):
             cw.block_end(line=';')
             cw.nl()
         elif const['type'] == 'const':
+            name_pfx = const.get('name-prefix', f"{family.ident_name}-")
             defines.append([c_upper(family.get('c-define-name',
-                                               f"{family.ident_name}-{const['name']}")),
+                                               f"{name_pfx}{const['name']}")),
                             const['value']])
 
     if defines:
-- 
2.51.0

[PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Asbjørn Sloth Tønnesen]

This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.

Example spec (from future wireguard.yaml):
-
  name: wgpeer
  attributes:
    -
      name: allowedips
      type: indexed-array
      sub-type: nest
      nested-attributes: wgallowedip

yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).

This doesn't change any currently generated code, as it isn't
used in any specs currently used for generating code.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 1543d4911bf5..b7de7f6b1fc7 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -816,6 +816,11 @@ class TypeArrayNest(Type):
                     f'unsigned int n_{self.c_name}']
         return super().arg_member(ri)
 
+    def _attr_policy(self, policy):
+        if self.attr['sub-type'] == 'nest':
+            return f'NLA_POLICY_NESTED_ARRAY({self.nested_render_name}_nl_policy)'
+        return super()._attr_policy(policy)
+
     def _attr_typol(self):
         if self.attr['sub-type'] in scalars:
             return f'.type = YNL_PT_U{c_upper(self.sub_type[1:])}, '
-- 
2.51.0

[PATCH net-next 03/11] tools: ynl-gen: add sub-type check

[Asbjørn Sloth Tønnesen]

Add a check to verify that the sub-type is "nest", and throw an
exception if no policy could be generated, as a guard to prevent
against generating a bad policy.

This is a trivial patch with no behavioural changes intended.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index b7de7f6b1fc7..04c26ed92ca3 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -826,8 +826,10 @@ class TypeArrayNest(Type):
             return f'.type = YNL_PT_U{c_upper(self.sub_type[1:])}, '
         elif self.attr['sub-type'] == 'binary' and 'exact-len' in self.checks:
             return f'.type = YNL_PT_BINARY, .len = {self.checks["exact-len"]}, '
-        else:
+        elif self.attr['sub-type'] == 'nest':
             return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
+        else:
+            raise Exception(f"Typol for ArrayNest sub-type {self.attr['sub-type']} not supported, yet")
 
     def _attr_get(self, ri, var):
         local_vars = ['const struct nlattr *attr2;']
-- 
2.51.0

[PATCH net-next 04/11] tools: ynl-gen: define count iterator in print_dump()

[Asbjørn Sloth Tønnesen]

In wireguard_get_device_dump(), as generated by print_dump(),
it didn't generate a declaration of `unsigned int i`:

$ make -C tools/net/ynl/generated wireguard-user.o
-e      CC wireguard-user.o
wireguard-user.c: In function ‘wireguard_get_device_dump’:
wireguard-user.c:502:22: error: ‘i’ undeclared (first use in this fn)
  502 |                 for (i = 0; i < req->_count.peers; i++)
      |                      ^

Copy the logic from print_req() as it correctly generated the
iterator in wireguard_set_device().

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 04c26ed92ca3..b0eeedfca2f2 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -2425,6 +2425,11 @@ def print_dump(ri):
         local_vars += ['size_t hdr_len;',
                        'void *hdr;']
 
+    for _, attr in ri.struct['request'].member_list():
+        if attr.presence_type() == 'count':
+            local_vars += ['unsigned int i;']
+            break
+
     ri.cw.write_func_lvar(local_vars)
 
     ri.cw.p('yds.yarg.ys = ys;')
-- 
2.51.0

[PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Asbjørn Sloth Tønnesen]

Instead of trying to define "struct nlattr *array;" in the all
the right places, then simply define it in a block scope,
as it's only used here.

Before this patch it was generated for attribute set _put()
functions, like wireguard_wgpeer_put(), but missing and caused a
compile error for the command function wireguard_set_device().

$ make -C tools/net/ynl/generated wireguard-user.o
-e      CC wireguard-user.o
wireguard-user.c: In function ‘wireguard_set_device’:
wireguard-user.c:548:9: error: ‘array’ undeclared (first use in ..)
  548 |         array = ynl_attr_nest_start(nlh, WGDEVICE_A_PEERS);
      |         ^~~~~

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index b0eeedfca2f2..e6a84e13ec0a 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -842,6 +842,9 @@ class TypeArrayNest(Type):
         return get_lines, None, local_vars
 
     def attr_put(self, ri, var):
+        ri.cw.block_start()
+        ri.cw.p('struct nlattr *array;')
+        ri.cw.nl()
         ri.cw.p(f'array = ynl_attr_nest_start(nlh, {self.enum_name});')
         if self.sub_type in scalars:
             put_type = self.sub_type
@@ -857,6 +860,7 @@ class TypeArrayNest(Type):
         else:
             raise Exception(f"Put for ArrayNest sub-type {self.attr['sub-type']} not supported, yet")
         ri.cw.p('ynl_attr_nest_end(nlh, array);')
+        ri.cw.block_end()
 
     def _setter_lines(self, ri, member, presence):
         return [f"{member} = {self.c_name};",
@@ -2063,13 +2067,9 @@ def put_req_nested(ri, struct):
         init_lines.append(f"hdr = ynl_nlmsg_put_extra_header(nlh, {struct_sz});")
         init_lines.append(f"memcpy(hdr, &obj->_hdr, {struct_sz});")
 
-    has_anest = False
     has_count = False
     for _, arg in struct.member_list():
-        has_anest |= arg.type == 'indexed-array'
         has_count |= arg.presence_type() == 'count'
-    if has_anest:
-        local_vars.append('struct nlattr *array;')
     if has_count:
         local_vars.append('unsigned int i;')
 
-- 
2.51.0

[PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Asbjørn Sloth Tønnesen]

In nested arrays don't require that the intermediate
attribute type should be a valid attribute type, it
might just be an index or simple 0, it is often not
even used.

See include/net/netlink.h about NLA_NESTED_ARRAY:
> The difference to NLA_NESTED is the structure:
> NLA_NESTED has the nested attributes directly inside
> while an array has the nested attributes at another
> level down and the attribute types directly in the
> nesting don't matter.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index e6a84e13ec0a..3c0b158c4da8 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -834,11 +834,12 @@ class TypeArrayNest(Type):
     def _attr_get(self, ri, var):
         local_vars = ['const struct nlattr *attr2;']
         get_lines = [f'attr_{self.c_name} = attr;',
-                     'ynl_attr_for_each_nested(attr2, attr) {',
-                     '\tif (ynl_attr_validate(yarg, attr2))',
-                     '\t\treturn YNL_PARSE_CB_ERROR;',
-                     f'\tn_{self.c_name}++;',
-                     '}']
+                     'ynl_attr_for_each_nested(attr2, attr) {']
+        if self.attr['sub-type'] != 'nest':
+            get_lines.append('\tif (ynl_attr_validate(yarg, attr2))')
+            get_lines.append('\t\treturn YNL_PARSE_CB_ERROR;')
+        get_lines.append(f'\tn_{self.c_name}++;')
+        get_lines.append('}')
         return get_lines, None, local_vars
 
     def attr_put(self, ri, var):
-- 
2.51.0

[PATCH net-next 07/11] tools: ynl-gen: rename TypeArrayNest to TypeIndexedArray

[Asbjørn Sloth Tønnesen]

As TypeArrayNest can now be used with many other sub-types
than nest, then rename it to TypeIndexedArray, to reduce
confusion.

This is a trivial patch with no behavioural changes intended.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 3c0b158c4da8..c4a6895ab5bb 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -792,7 +792,7 @@ class TypeMultiAttr(Type):
                 f"{presence} = n_{self.c_name};"]
 
 
-class TypeArrayNest(Type):
+class TypeIndexedArray(Type):
     def is_multi_val(self):
         return True
 
@@ -829,7 +829,7 @@ class TypeArrayNest(Type):
         elif self.attr['sub-type'] == 'nest':
             return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
         else:
-            raise Exception(f"Typol for ArrayNest sub-type {self.attr['sub-type']} not supported, yet")
+            raise Exception(f"Typol for IndexedArray sub-type {self.attr['sub-type']} not supported, yet")
 
     def _attr_get(self, ri, var):
         local_vars = ['const struct nlattr *attr2;']
@@ -859,7 +859,7 @@ class TypeArrayNest(Type):
             ri.cw.p(f'for (i = 0; i < {var}->_count.{self.c_name}; i++)')
             ri.cw.p(f"{self.nested_render_name}_put(nlh, i, &{var}->{self.c_name}[i]);")
         else:
-            raise Exception(f"Put for ArrayNest sub-type {self.attr['sub-type']} not supported, yet")
+            raise Exception(f"Put for IndexedArray sub-type {self.attr['sub-type']} not supported, yet")
         ri.cw.p('ynl_attr_nest_end(nlh, array);')
         ri.cw.block_end()
 
@@ -1137,7 +1137,7 @@ class AttrSet(SpecAttrSet):
             t = TypeNest(self.family, self, elem, value)
         elif elem['type'] == 'indexed-array' and 'sub-type' in elem:
             if elem["sub-type"] in ['binary', 'nest', 'u32']:
-                t = TypeArrayNest(self.family, self, elem, value)
+                t = TypeIndexedArray(self.family, self, elem, value)
             else:
                 raise Exception(f'new_attr: unsupported sub-type {elem["sub-type"]}')
         elif elem['type'] == 'nest-type-value':
-- 
2.51.0

[PATCH net-next 08/11] tools: ynl: move nest packing to a helper function

[Asbjørn Sloth Tønnesen]

This patch moves nest packing into a helper function,
that can also be used for packing indexed arrays.

No behavioural changes intended.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/lib/ynl.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index 8244a5f440b2..4928b41c636a 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -562,11 +562,8 @@ class YnlFamily(SpecFamily):
 
         if attr["type"] == 'nest':
             nl_type |= Netlink.NLA_F_NESTED
-            attr_payload = b''
             sub_space = attr['nested-attributes']
-            sub_attrs = SpaceAttrs(self.attr_sets[sub_space], value, search_attrs)
-            for subname, subvalue in value.items():
-                attr_payload += self._add_attr(sub_space, subname, subvalue, sub_attrs)
+            attr_payload = self._add_nest_attrs(value, sub_space, search_attrs)
         elif attr["type"] == 'flag':
             if not value:
                 # If value is absent or false then skip attribute creation.
@@ -623,6 +620,14 @@ class YnlFamily(SpecFamily):
         pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
         return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
 
+    def _add_nest_attrs(self, value, sub_space, search_attrs):
+        sub_attrs = SpaceAttrs(self.attr_sets[sub_space], value, search_attrs)
+        attr_payload = b''
+        for subname, subvalue in value.items():
+            attr_payload += self._add_attr(sub_space, subname, subvalue,
+                                           sub_attrs)
+        return attr_payload
+
     def _get_enum_or_unknown(self, enum, raw):
         try:
             name = enum.entries_by_val[raw].name
-- 
2.51.0

[PATCH net-next 09/11] tools: ynl: encode indexed-array

[Asbjørn Sloth Tønnesen]

This patch adds support for encoding indexed-array
attributes with sub-type nest in pyynl.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/lib/ynl.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index 4928b41c636a..a37294a751da 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -564,6 +564,11 @@ class YnlFamily(SpecFamily):
             nl_type |= Netlink.NLA_F_NESTED
             sub_space = attr['nested-attributes']
             attr_payload = self._add_nest_attrs(value, sub_space, search_attrs)
+        elif attr['type'] == 'indexed-array' and attr['sub-type'] == 'nest':
+            nl_type |= Netlink.NLA_F_NESTED
+            sub_space = attr['nested-attributes']
+            attr_payload = self._encode_indexed_array(value, sub_space,
+                                                      search_attrs)
         elif attr["type"] == 'flag':
             if not value:
                 # If value is absent or false then skip attribute creation.
@@ -617,6 +622,9 @@ class YnlFamily(SpecFamily):
         else:
             raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
 
+        return self._add_attr_raw(nl_type, attr_payload)
+
+    def _add_attr_raw(self, nl_type, attr_payload):
         pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
         return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
 
@@ -628,6 +636,15 @@ class YnlFamily(SpecFamily):
                                            sub_attrs)
         return attr_payload
 
+    def _encode_indexed_array(self, vals, sub_space, search_attrs):
+        attr_payload = b''
+        nested_flag = Netlink.NLA_F_NESTED
+        for i, val in enumerate(vals):
+            idx = i | Netlink.NLA_F_NESTED
+            val_payload = self._add_nest_attrs(val, sub_space, search_attrs)
+            attr_payload += self._add_attr_raw(idx, val_payload)
+        return attr_payload
+
     def _get_enum_or_unknown(self, enum, raw):
         try:
             name = enum.entries_by_val[raw].name
-- 
2.51.0

[PATCH net-next 10/11] tools: ynl: decode hex input

[Asbjørn Sloth Tønnesen]

This patch add support for decoding hex input, so
that binary attributes can be read through --json.

Example (using future wireguard.yaml):
 $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
   --do set-device --json '{"ifindex":3,
     "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/lib/ynl.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index a37294a751da..78c0245ca587 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -973,6 +973,8 @@ class YnlFamily(SpecFamily):
                 raw = ip.packed
             else:
                 raw = int(ip)
+        elif attr_spec.display_hint == 'hex':
+            raw = bytes.fromhex(string)
         else:
             raise Exception(f"Display hint '{attr_spec.display_hint}' not implemented"
                             f" when parsing '{attr_spec['name']}'")
-- 
2.51.0

[PATCH net-next 11/11] tools: ynl: add ipv4-or-v6 display hint

[Asbjørn Sloth Tønnesen]

The attribute WGALLOWEDIP_A_IPADDR can contain either an IPv4
or an IPv6 address depending on WGALLOWEDIP_A_FAMILY, however
in practice it is enough to look at the attribute length.

This patch implements an ipv4-or-v6 display hint, that can
deal with this kind of attribute.

It only implements this display hint for genetlink-legacy, it
can be added to other protocol variants if needed, but we don't
want to encourage it's use.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 Documentation/netlink/genetlink-legacy.yaml | 2 +-
 tools/net/ynl/pyynl/lib/ynl.py              | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index b29d62eefa16..66fb8653a344 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -154,7 +154,7 @@ properties:
                   Optional format indicator that is intended only for choosing
                   the right formatting mechanism when displaying values of this
                   type.
-                enum: [ hex, mac, fddi, ipv4, ipv6, uuid ]
+                enum: [ hex, mac, fddi, ipv4, ipv6, ipv4-or-v6, uuid ]
               struct:
                 description: Name of the nested struct type.
                 type: string
diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index 78c0245ca587..6d2f12d43a08 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -958,7 +958,7 @@ class YnlFamily(SpecFamily):
                 formatted = hex(raw)
             else:
                 formatted = bytes.hex(raw, ' ')
-        elif display_hint in [ 'ipv4', 'ipv6' ]:
+        elif display_hint in [ 'ipv4', 'ipv6', 'ipv4-or-v6' ]:
             formatted = format(ipaddress.ip_address(raw))
         elif display_hint == 'uuid':
             formatted = str(uuid.UUID(bytes=raw))
@@ -967,7 +967,7 @@ class YnlFamily(SpecFamily):
         return formatted
 
     def _from_string(self, string, attr_spec):
-        if attr_spec.display_hint in ['ipv4', 'ipv6']:
+        if attr_spec.display_hint in ['ipv4', 'ipv6', 'ipv4-or-v6']:
             ip = ipaddress.ip_address(string)
             if attr_spec['type'] == 'binary':
                 raw = ip.packed
-- 
2.51.0

Re: [PATCH net-next 01/11] tools: ynl-gen: allow overriding name-prefix for constants

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> Allow using custom name-prefix with constants,
> just like it is for enum and flags declarations.
>
> This is needed for generating WG_KEY_LEN in
> include/uapi/linux/wireguard.h from a spec.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
>
> Example spec (from future wireguard.yaml):
> -
>   name: wgpeer
>   attributes:
>     -
>       name: allowedips
>       type: indexed-array
>       sub-type: nest
>       nested-attributes: wgallowedip
>
> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
>
> This doesn't change any currently generated code, as it isn't
> used in any specs currently used for generating code.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 03/11] tools: ynl-gen: add sub-type check

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> Add a check to verify that the sub-type is "nest", and throw an
> exception if no policy could be generated, as a guard to prevent
> against generating a bad policy.
>
> This is a trivial patch with no behavioural changes intended.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 04/11] tools: ynl-gen: define count iterator in print_dump()

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> In wireguard_get_device_dump(), as generated by print_dump(),
> it didn't generate a declaration of `unsigned int i`:
>
> $ make -C tools/net/ynl/generated wireguard-user.o
> -e      CC wireguard-user.o
> wireguard-user.c: In function ‘wireguard_get_device_dump’:
> wireguard-user.c:502:22: error: ‘i’ undeclared (first use in this fn)
>   502 |                 for (i = 0; i < req->_count.peers; i++)
>       |                      ^
>
> Copy the logic from print_req() as it correctly generated the
> iterator in wireguard_set_device().
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> Instead of trying to define "struct nlattr *array;" in the all
> the right places, then simply define it in a block scope,
> as it's only used here.
>
> Before this patch it was generated for attribute set _put()
> functions, like wireguard_wgpeer_put(), but missing and caused a
> compile error for the command function wireguard_set_device().
>
> $ make -C tools/net/ynl/generated wireguard-user.o
> -e      CC wireguard-user.o
> wireguard-user.c: In function ‘wireguard_set_device’:
> wireguard-user.c:548:9: error: ‘array’ undeclared (first use in ..)
>   548 |         array = ynl_attr_nest_start(nlh, WGDEVICE_A_PEERS);
>       |         ^~~~~
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 07/11] tools: ynl-gen: rename TypeArrayNest to TypeIndexedArray

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> As TypeArrayNest can now be used with many other sub-types
> than nest, then rename it to TypeIndexedArray, to reduce
> confusion.
>
> This is a trivial patch with no behavioural changes intended.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 08/11] tools: ynl: move nest packing to a helper function

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> This patch moves nest packing into a helper function,
> that can also be used for packing indexed arrays.
>
> No behavioural changes intended.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next 09/11] tools: ynl: encode indexed-array

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> This patch adds support for encoding indexed-array
> attributes with sub-type nest in pyynl.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  tools/net/ynl/pyynl/lib/ynl.py | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index 4928b41c636a..a37294a751da 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -564,6 +564,11 @@ class YnlFamily(SpecFamily):
>              nl_type |= Netlink.NLA_F_NESTED
>              sub_space = attr['nested-attributes']
>              attr_payload = self._add_nest_attrs(value, sub_space, search_attrs)
> +        elif attr['type'] == 'indexed-array' and attr['sub-type'] == 'nest':
> +            nl_type |= Netlink.NLA_F_NESTED
> +            sub_space = attr['nested-attributes']
> +            attr_payload = self._encode_indexed_array(value, sub_space,
> +                                                      search_attrs)
>          elif attr["type"] == 'flag':
>              if not value:
>                  # If value is absent or false then skip attribute creation.
> @@ -617,6 +622,9 @@ class YnlFamily(SpecFamily):
>          else:
>              raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
>  
> +        return self._add_attr_raw(nl_type, attr_payload)
> +
> +    def _add_attr_raw(self, nl_type, attr_payload):
>          pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
>          return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
>  
> @@ -628,6 +636,15 @@ class YnlFamily(SpecFamily):
>                                             sub_attrs)
>          return attr_payload
>  
> +    def _encode_indexed_array(self, vals, sub_space, search_attrs):
> +        attr_payload = b''
> +        nested_flag = Netlink.NLA_F_NESTED

This line is not doing anything, right?

> +        for i, val in enumerate(vals):
> +            idx = i | Netlink.NLA_F_NESTED
> +            val_payload = self._add_nest_attrs(val, sub_space, search_attrs)
> +            attr_payload += self._add_attr_raw(idx, val_payload)
> +        return attr_payload
> +
>      def _get_enum_or_unknown(self, enum, raw):
>          try:
>              name = enum.entries_by_val[raw].name

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> This patch add support for decoding hex input, so
> that binary attributes can be read through --json.
>
> Example (using future wireguard.yaml):
>  $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>    --do set-device --json '{"ifindex":3,
>      "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

FWIW, the hex can include spaces or not when using bytes.fromhex(). When
formatting hex for output, I chose to include spaces, but I don't really
know if that was a good choice or not.

Re: [PATCH net-next 11/11] tools: ynl: add ipv4-or-v6 display hint

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> The attribute WGALLOWEDIP_A_IPADDR can contain either an IPv4
> or an IPv6 address depending on WGALLOWEDIP_A_FAMILY, however
> in practice it is enough to look at the attribute length.
>
> This patch implements an ipv4-or-v6 display hint, that can
> deal with this kind of attribute.
>
> It only implements this display hint for genetlink-legacy, it
> can be added to other protocol variants if needed, but we don't
> want to encourage it's use.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

I suspect there are occurrences of ipv4 or ipv6 in the existing specs
that really should be ipv4-or-ipv6 but the python code doesn't care.

Re: [PATCH net-next 09/11] tools: ynl: encode indexed-array

[Asbjørn Sloth Tønnesen]

Hi Donald,

Thanks for the reviews.

On 9/5/25 10:49 AM, Donald Hunter wrote:
> Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:
> 
>> This patch adds support for encoding indexed-array
>> attributes with sub-type nest in pyynl.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>> ---
>>   tools/net/ynl/pyynl/lib/ynl.py | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
>> index 4928b41c636a..a37294a751da 100644
>> --- a/tools/net/ynl/pyynl/lib/ynl.py
>> +++ b/tools/net/ynl/pyynl/lib/ynl.py
>> @@ -564,6 +564,11 @@ class YnlFamily(SpecFamily):
>>               nl_type |= Netlink.NLA_F_NESTED
>>               sub_space = attr['nested-attributes']
>>               attr_payload = self._add_nest_attrs(value, sub_space, search_attrs)
>> +        elif attr['type'] == 'indexed-array' and attr['sub-type'] == 'nest':
>> +            nl_type |= Netlink.NLA_F_NESTED
>> +            sub_space = attr['nested-attributes']
>> +            attr_payload = self._encode_indexed_array(value, sub_space,
>> +                                                      search_attrs)
>>           elif attr["type"] == 'flag':
>>               if not value:
>>                   # If value is absent or false then skip attribute creation.
>> @@ -617,6 +622,9 @@ class YnlFamily(SpecFamily):
>>           else:
>>               raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
>>   
>> +        return self._add_attr_raw(nl_type, attr_payload)
>> +
>> +    def _add_attr_raw(self, nl_type, attr_payload):
>>           pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
>>           return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
>>   
>> @@ -628,6 +636,15 @@ class YnlFamily(SpecFamily):
>>                                              sub_attrs)
>>           return attr_payload
>>   
>> +    def _encode_indexed_array(self, vals, sub_space, search_attrs):
>> +        attr_payload = b''
>> +        nested_flag = Netlink.NLA_F_NESTED
> 
> This line is not doing anything, right?

Right, that line shouldn't be there, it is a remain of an early version, where
I didn't add the indexes, as NLA_NESTED_ARRAY is actually an unindexed-array.

The wireguard kernel code only sends zero types, and it doesn't care that user-
space sends an indexed array back, eg. when setting multiple allowed ips.

>> +        for i, val in enumerate(vals):
>> +            idx = i | Netlink.NLA_F_NESTED
>> +            val_payload = self._add_nest_attrs(val, sub_space, search_attrs)
>> +            attr_payload += self._add_attr_raw(idx, val_payload)
>> +        return attr_payload
>> +
>>       def _get_enum_or_unknown(self, enum, raw):
>>           try:
>>               name = enum.entries_by_val[raw].name

-- 
pw-bot: cr

Re: [PATCH net-next 01/11] tools: ynl-gen: allow overriding name-prefix for constants

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:24 +0000 Asbjørn Sloth Tønnesen wrote:
> Allow using custom name-prefix with constants,
> just like it is for enum and flags declarations.
> 
> This is needed for generating WG_KEY_LEN in
> include/uapi/linux/wireguard.h from a spec.

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

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:25 +0000 Asbjørn Sloth Tønnesen wrote:
> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
> 
> Example spec (from future wireguard.yaml):
> -
>   name: wgpeer
>   attributes:
>     -
>       name: allowedips
>       type: indexed-array
>       sub-type: nest
>       nested-attributes: wgallowedip
> 
> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
> 
> This doesn't change any currently generated code, as it isn't
> used in any specs currently used for generating code.


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

Re: [PATCH net-next 03/11] tools: ynl-gen: add sub-type check

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:26 +0000 Asbjørn Sloth Tønnesen wrote:
> Add a check to verify that the sub-type is "nest", and throw an
> exception if no policy could be generated, as a guard to prevent
> against generating a bad policy.
> 
> This is a trivial patch with no behavioural changes intended.

I _think_ the expectation was that one of the other methods which
validate the types more thoroughly has to be called if this one is.
But either way:

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

Re: [PATCH net-next 04/11] tools: ynl-gen: define count iterator in print_dump()

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:27 +0000 Asbjørn Sloth Tønnesen wrote:
> In wireguard_get_device_dump(), as generated by print_dump(),
> it didn't generate a declaration of `unsigned int i`:
> 
> $ make -C tools/net/ynl/generated wireguard-user.o
> -e      CC wireguard-user.o
> wireguard-user.c: In function ‘wireguard_get_device_dump’:
> wireguard-user.c:502:22: error: ‘i’ undeclared (first use in this fn)
>   502 |                 for (i = 0; i < req->_count.peers; i++)
>       |                      ^
> 
> Copy the logic from print_req() as it correctly generated the
> iterator in wireguard_set_device().

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

Re: [PATCH net-next 01/11] tools: ynl-gen: allow overriding name-prefix for constants

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1411 bytes --]



On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
> Allow using custom name-prefix with constants,
> just like it is for enum and flags declarations.
> 
> This is needed for generating WG_KEY_LEN in
> include/uapi/linux/wireguard.h from a spec.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  tools/net/ynl/pyynl/ynl_gen_c.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index fb7e03805a11..1543d4911bf5 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -3211,8 +3211,9 @@ def render_uapi(family, cw):
>              cw.block_end(line=';')
>              cw.nl()
>          elif const['type'] == 'const':
> +            name_pfx = const.get('name-prefix', f"{family.ident_name}-")

Previously we always used "{family.ident_name}-", but now we get the
name-prefix and use that, falling back to the default if it doesn't
exist. Good.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>              defines.append([c_upper(family.get('c-define-name',
> -                                               f"{family.ident_name}-{const['name']}")),
> +                                               f"{name_pfx}{const['name']}")),
>                              const['value']])
>  
>      if defines:


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:28 +0000 Asbjørn Sloth Tønnesen wrote:
> Instead of trying to define "struct nlattr *array;" in the all
> the right places, then simply define it in a block scope,
> as it's only used here.
> 
> Before this patch it was generated for attribute set _put()
> functions, like wireguard_wgpeer_put(), but missing and caused a
> compile error for the command function wireguard_set_device().
> 
> $ make -C tools/net/ynl/generated wireguard-user.o
> -e      CC wireguard-user.o
> wireguard-user.c: In function ‘wireguard_set_device’:
> wireguard-user.c:548:9: error: ‘array’ undeclared (first use in ..)
>   548 |         array = ynl_attr_nest_start(nlh, WGDEVICE_A_PEERS);
>       |         ^~~~~

Dunno about this one. In patch 4 you basically add another instance of
the "let's declare local vars at function level" approach. And here
you're going the other way. This patch will certainly work, but I felt
like I wouldn't have written it this way if I was typing in the parsers
by hand.

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1647 bytes --]



On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
> 
> Example spec (from future wireguard.yaml):
> -
>   name: wgpeer
>   attributes:
>     -
>       name: allowedips
>       type: indexed-array
>       sub-type: nest
>       nested-attributes: wgallowedip
> 
> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
> 
> This doesn't change any currently generated code, as it isn't
> used in any specs currently used for generating code.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---

Is this keyed of off the sub-type? Does you mean that all the existing
uses of 'sub-type: nest' don't generate code today? Or that this
_attr_policy implementation is not called yet?

I checked and we have quite a number of uses:

> $ rg 'sub-type: nest'
> Documentation/netlink/specs/nlctrl.yaml
> 69:        sub-type: nest
> 74:        sub-type: nest
> 
> Documentation/netlink/specs/tc.yaml
> 2045:        sub-type: nest
> 2065:        sub-type: nest
> 2190:        sub-type: nest
> 2304:        sub-type: nest
> 2494:        sub-type: nest
> 3021:        sub-type: nest
> 3181:        sub-type: nest
> 3567:        sub-type: nest
> 3799:        sub-type: nest
> 
> Documentation/netlink/specs/rt-link.yaml
> 2203:        sub-type: nest
> 
> Documentation/netlink/specs/nl80211.yaml
> 610:        sub-type: nest
> 1309:        sub-type: nest
> 1314:        sub-type: nest
> 1337:        sub-type: nest
> 1420:        sub-type: nest
> 1476:        sub-type: nest
> 1615:        sub-type: nest
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 03/11] tools: ynl-gen: add sub-type check

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1415 bytes --]



On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
> Add a check to verify that the sub-type is "nest", and throw an
> exception if no policy could be generated, as a guard to prevent
> against generating a bad policy.
> 
> This is a trivial patch with no behavioural changes intended.
> > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  tools/net/ynl/pyynl/ynl_gen_c.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index b7de7f6b1fc7..04c26ed92ca3 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -826,8 +826,10 @@ class TypeArrayNest(Type):
>              return f'.type = YNL_PT_U{c_upper(self.sub_type[1:])}, '
>          elif self.attr['sub-type'] == 'binary' and 'exact-len' in self.checks:
>              return f'.type = YNL_PT_BINARY, .len = {self.checks["exact-len"]}, '
> -        else:
> +        elif self.attr['sub-type'] == 'nest':
>              return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
> +        else:
> +            raise Exception(f"Typol for ArrayNest sub-type {self.attr['sub-type']} not supported, yet")
>  
>      def _attr_get(self, ri, var):
>          local_vars = ['const struct nlattr *attr2;']


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 04/11] tools: ynl-gen: define count iterator in print_dump()

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1457 bytes --]



On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
> In wireguard_get_device_dump(), as generated by print_dump(),
> it didn't generate a declaration of `unsigned int i`:
> 
> $ make -C tools/net/ynl/generated wireguard-user.o
> -e      CC wireguard-user.o
> wireguard-user.c: In function ‘wireguard_get_device_dump’:
> wireguard-user.c:502:22: error: ‘i’ undeclared (first use in this fn)
>   502 |                 for (i = 0; i < req->_count.peers; i++)
>       |                      ^
> 
> Copy the logic from print_req() as it correctly generated the
> iterator in wireguard_set_device().
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  tools/net/ynl/pyynl/ynl_gen_c.py | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index 04c26ed92ca3..b0eeedfca2f2 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -2425,6 +2425,11 @@ def print_dump(ri):
>          local_vars += ['size_t hdr_len;',
>                         'void *hdr;']
>  
> +    for _, attr in ri.struct['request'].member_list():
> +        if attr.presence_type() == 'count':
> +            local_vars += ['unsigned int i;']
> +            break
> +
>      ri.cw.write_func_lvar(local_vars)
>  
>      ri.cw.p('yds.yarg.ys = ys;')


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:29 +0000 Asbjørn Sloth Tønnesen wrote:
> In nested arrays don't require that the intermediate
> attribute type should be a valid attribute type, it
> might just be an index or simple 0, it is often not
> even used.
> 
> See include/net/netlink.h about NLA_NESTED_ARRAY:
> > The difference to NLA_NESTED is the structure:
> > NLA_NESTED has the nested attributes directly inside
> > while an array has the nested attributes at another
> > level down and the attribute types directly in the
> > nesting don't matter.  

I don't understand, please provide more details.
This is an ArrayNest, right?

[ARRAY-ATTR]
  [ENTRY]
    [MEMBER1]
    [MEMBER2]
  [ENTRY]
    [MEMBER1]
    [MEMBER2]

Which level are you saying doesn't matter?
If entry is a nest it must be a valid nest.
What the comment you're quoting is saying is that the nla_type of ENTRY
doesn't matter.

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 2001 bytes --]



On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
> In nested arrays don't require that the intermediate
> attribute type should be a valid attribute type, it
> might just be an index or simple 0, it is often not
> even used.
> 
> See include/net/netlink.h about NLA_NESTED_ARRAY:
>> The difference to NLA_NESTED is the structure:
>> NLA_NESTED has the nested attributes directly inside
>> while an array has the nested attributes at another
>> level down and the attribute types directly in the
>> nesting don't matter.
> 

To me, it would seem like it makes more sense to define these (even if
thats defined per family?) than to just say they aren't defined at all?

Hm.

> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  tools/net/ynl/pyynl/ynl_gen_c.py | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index e6a84e13ec0a..3c0b158c4da8 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -834,11 +834,12 @@ class TypeArrayNest(Type):
>      def _attr_get(self, ri, var):
>          local_vars = ['const struct nlattr *attr2;']
>          get_lines = [f'attr_{self.c_name} = attr;',
> -                     'ynl_attr_for_each_nested(attr2, attr) {',
> -                     '\tif (ynl_attr_validate(yarg, attr2))',
> -                     '\t\treturn YNL_PARSE_CB_ERROR;',
> -                     f'\tn_{self.c_name}++;',
> -                     '}']
> +                     'ynl_attr_for_each_nested(attr2, attr) {']
> +        if self.attr['sub-type'] != 'nest':
> +            get_lines.append('\tif (ynl_attr_validate(yarg, attr2))')
> +            get_lines.append('\t\treturn YNL_PARSE_CB_ERROR;')
> +        get_lines.append(f'\tn_{self.c_name}++;')
> +        get_lines.append('}')
>          return get_lines, None, local_vars
>  
>      def attr_put(self, ri, var):


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 07/11] tools: ynl-gen: rename TypeArrayNest to TypeIndexedArray

[Jakub Kicinski]

On Thu,  4 Sep 2025 22:01:30 +0000 Asbjørn Sloth Tønnesen wrote:
> As TypeArrayNest can now be used with many other sub-types
> than nest, then rename it to TypeIndexedArray, to reduce
> confusion.

You need to also find all the local comments and variables which allude
to the name.

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 803 bytes --]



On 9/5/2025 3:51 AM, Donald Hunter wrote:
> Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:
> 
>> This patch add support for decoding hex input, so
>> that binary attributes can be read through --json.
>>
>> Example (using future wireguard.yaml):
>>  $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>>    --do set-device --json '{"ifindex":3,
>>      "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> 
> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
> 
> FWIW, the hex can include spaces or not when using bytes.fromhex(). When
> formatting hex for output, I chose to include spaces, but I don't really
> know if that was a good choice or not.

I also prefer the spaces for readability.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Asbjørn Sloth Tønnesen]

On 9/6/25 12:18 AM, Jakub Kicinski wrote:
> On Thu,  4 Sep 2025 22:01:28 +0000 Asbjørn Sloth Tønnesen wrote:
>> Instead of trying to define "struct nlattr *array;" in the all
>> the right places, then simply define it in a block scope,
>> as it's only used here.
>>
>> Before this patch it was generated for attribute set _put()
>> functions, like wireguard_wgpeer_put(), but missing and caused a
>> compile error for the command function wireguard_set_device().
>>
>> $ make -C tools/net/ynl/generated wireguard-user.o
>> -e      CC wireguard-user.o
>> wireguard-user.c: In function ‘wireguard_set_device’:
>> wireguard-user.c:548:9: error: ‘array’ undeclared (first use in ..)
>>    548 |         array = ynl_attr_nest_start(nlh, WGDEVICE_A_PEERS);
>>        |         ^~~~~
> 
> Dunno about this one. In patch 4 you basically add another instance of
> the "let's declare local vars at function level" approach. And here
> you're going the other way. This patch will certainly work, but I felt
> like I wouldn't have written it this way if I was typing in the parsers
> by hand.

Thanks for the reviews.

In patch 4, it is about a variable used by multiple Type classes having
presence_type() = 'count', which is currently 3 classes:
- TypeBinaryScalarArray
- TypeMultiAttr
- TypeArrayNest (later renamed to TypeIndexedArray)

In patch 5, I move code for a special variable used by one Type class,
to be contained within that class. It makes it easier to ensure that the
variable is only defined, when used, and vice versa. This comes at the
cost of the generated code looking generated.

If we should make the generated code look like it was written by humans,
then I would move the definition of these local variables into a class
method, so `i` can be generated by the generic implementation, and `array`
can be implemented in it's class. I will take a stab at this, but it might
be too much refactoring for this series, eg. `len` is also defined local
to conditional blocks multiple branches in a row.

tools/net/ynl/generated/nl80211-user.c:
nl80211_iftype_data_attrs_parse(..) {
   [..]
   ynl_attr_for_each_nested(attr, nested) {
     unsigned int type = ynl_attr_type(attr);

     if (type == NL80211_BAND_IFTYPE_ATTR_IFTYPES) {
       unsigned int len;
       [..]
     } else if (type == NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC) {
       unsigned int len;
       [..]
     [same pattern 8 times, so 11 times in total]
     } else if (type == NL80211_BAND_IFTYPE_ATTR_EHT_CAP_PPE) {
       unsigned int len;
       [..]
     }
   }
   return 0;
}

(I didn't have to search for this, I saw the pattern in wireguard-user.c,
looked for it in nl80211-user.c and this was the first `len` usage there.)

That looks very generated, I would have `len` defined together with `type`,
and a switch statement would also look a lot more natural, but maybe leave
the if->switch conversion for the compiler to detect.

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Asbjørn Sloth Tønnesen]

On 9/6/25 12:23 AM, Jakub Kicinski wrote:
> On Thu,  4 Sep 2025 22:01:29 +0000 Asbjørn Sloth Tønnesen wrote:
>> In nested arrays don't require that the intermediate
>> attribute type should be a valid attribute type, it
>> might just be an index or simple 0, it is often not
>> even used.
>>
>> See include/net/netlink.h about NLA_NESTED_ARRAY:
>>> The difference to NLA_NESTED is the structure:
>>> NLA_NESTED has the nested attributes directly inside
>>> while an array has the nested attributes at another
>>> level down and the attribute types directly in the
>>> nesting don't matter.
> 
> I don't understand, please provide more details.
> This is an ArrayNest, right?
> 
> [ARRAY-ATTR]
>    [ENTRY]
>      [MEMBER1]
>      [MEMBER2]
>    [ENTRY]
>      [MEMBER1]
>      [MEMBER2]
> 
> Which level are you saying doesn't matter?
> If entry is a nest it must be a valid nest.
> What the comment you're quoting is saying is that the nla_type of ENTRY
> doesn't matter.

I will expand this in v2, but the gist of it is that this is part of the
"split attribute counting, and later allocating an array to hold them" code.

The check that I remove for nested arrays, is an early exit during the
counting phase. Later in the allocation and parse phase it validates the
nested payload.

In include/uapi/linux/wireguard.h:
 > WGDEVICE_A_PEERS: NLA_NESTED
 >   0: NLA_NESTED
 >     WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
 >     [..]
 >   0: NLA_NESTED
 >     ...
 >   ...

The current check requires that the nested type is valid in the nested
attribute set, which in this case resolves to WGDEVICE_A_UNSPEC, which is
YNL_PT_REJECT, and it takes the early exit and returns YNL_PARSE_CB_ERROR.

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Asbjørn Sloth Tønnesen]

CC: Johannes

On 9/6/25 12:19 AM, Jacob Keller wrote:
> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
>>
>> Example spec (from future wireguard.yaml):
>> -
>>    name: wgpeer
>>    attributes:
>>      -
>>        name: allowedips
>>        type: indexed-array
>>        sub-type: nest
>>        nested-attributes: wgallowedip
>>
>> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
>>
>> This doesn't change any currently generated code, as it isn't
>> used in any specs currently used for generating code.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>> ---
> 
> Is this keyed of off the sub-type? Does you mean that all the existing
> uses of 'sub-type: nest' don't generate code today? Or that this
> _attr_policy implementation is not called yet?

Thanks for the reviews. Yeah, it is a careful wording, because we have
specs matching it, but there aren't any source files that triggers
ynl-gen to generate code based on those specs.

Therefore this patch, doesn't result in any code changes when running:
$ ./tools/net/ynl/ynl-regen.sh -f

Actually ynl-gen generates a fictive "{ .type = NLA_INDEXED_ARRAY, }"
policy, without this patch, leading to a build failure.

> I checked and we have quite a number of uses:
> 
>> $ rg 'sub-type: nest'
>> Documentation/netlink/specs/nlctrl.yaml
 >> [..]
>> Documentation/netlink/specs/tc.yaml
>> [..]
>> Documentation/netlink/specs/rt-link.yaml
>> [..]
>> Documentation/netlink/specs/nl80211.yaml
>> [..]

None of those currently have a generated netlink policy.

These are the netlink policies currently generated by ynl-gen:
$ git grep -h -B1 'YNL-GEN kernel source' | grep '^/\*[^ ]'
/*      Documentation/netlink/specs/dpll.yaml */
/*      Documentation/netlink/specs/ovpn.yaml */
/*      Documentation/netlink/specs/team.yaml */
/*      Documentation/netlink/specs/lockd.yaml */
/*      Documentation/netlink/specs/nfsd.yaml */
/*      Documentation/netlink/specs/netdev.yaml */
/*      Documentation/netlink/specs/devlink.yaml */
/*      Documentation/netlink/specs/handshake.yaml */
/*      Documentation/netlink/specs/fou.yaml */
/*      Documentation/netlink/specs/mptcp_pm.yaml */
/*      Documentation/netlink/specs/net_shaper.yaml */

Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY()
macro in commit 1501d13596b9 for use in nl80211, and it's therefore
used in net/wireless/nl80211.c, but outside of that the macro is
only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c).

Wireguard adopts the macro in this RFC patch:
https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Asbjørn Sloth Tønnesen]

On 9/6/25 12:27 AM, Jacob Keller wrote:
> On 9/5/2025 3:51 AM, Donald Hunter wrote:
>> Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:
>>
>>> This patch add support for decoding hex input, so
>>> that binary attributes can be read through --json.
>>>
>>> Example (using future wireguard.yaml):
>>>   $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>>>     --do set-device --json '{"ifindex":3,
>>>       "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
>>>
>>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>>
>> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
>>
>> FWIW, the hex can include spaces or not when using bytes.fromhex(). When
>> formatting hex for output, I chose to include spaces, but I don't really
>> know if that was a good choice or not.
> 
> I also prefer the spaces for readability.
I formatted it with spaces for clarity, even without spaces it was a bit
long for one line. Spaces also has the advantage that you don't have to
think about endianness.

Should we define the display hints a bit more in a .rst, or is it OK that
they end up being implementation specific for each language library? Do we
want them to behave the same in a Rust YNL library, as they do in Python?

BTW: The rest of the key used in the example can be found with this key-gen:
$ printf "hello world" | sha1sum
[redacted key material]

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Asbjørn Sloth Tønnesen]

CC: Johannes

On 9/6/25 12:24 AM, Jacob Keller wrote:
> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>> In nested arrays don't require that the intermediate
>> attribute type should be a valid attribute type, it
>> might just be an index or simple 0, it is often not
>> even used.
>>
>> See include/net/netlink.h about NLA_NESTED_ARRAY:
>>> The difference to NLA_NESTED is the structure:
>>> NLA_NESTED has the nested attributes directly inside
>>> while an array has the nested attributes at another
>>> level down and the attribute types directly in the
>>> nesting don't matter.
>>
> 
> To me, it would seem like it makes more sense to define these (even if
> thats defined per family?) than to just say they aren't defined at all?
> 
> Hm.

I considered adding some of that metadata too, as I am actually removing
it for wireguard (in comment form, but still).

In include/uapi/linux/wireguard.h in the comment block at the top, it is
very clear that wireguard only used type 0 for all the nested array
entries, however the truth is that it doesn't care. It therefore doesn't
matter if the generated -user.* keeps track of the index in .idx, or that
cli.py decodes a JSON array and sends it with indexes, it's not needed,
but it still works.

In practice I don't think we will break any clients if we enforced it, and
validated that wireguard only accepts type 0 entries, in it's nested arrays.

For the other families, I don't know how well defined it is, Johannes have
stated that nl80211 doesn't care which types are used, but I have no idea
how consistent clients have abused that statement to send random data,
or do they all just send zeros?

This would make a lot more sense if 'array-nest' hadn't been renamed to
'indexed-array' in ynl, because it feels wrong to add 'unindexed: true' now.
We could also call it 'all-zero-indexed: true'.

In cli.py this gives some extra issues, as seen in [1], the nested arrays
are outputted as '[{0: {..}}, {0: {..}}, ..]', but on input has the format
'[{..},{..}, ..]' because it has to be JSON-compatible on input.

If we had an attribute like 'all-zero-indexed' then cli.py, could also output
'[{..},{..}, ..]'.

[1] https://lore.kernel.org/netdev/20250904220255.1006675-3-ast@fiberby.net/

Re: [PATCH net-next 11/11] tools: ynl: add ipv4-or-v6 display hint

[Asbjørn Sloth Tønnesen]

On 9/5/25 10:53 AM, Donald Hunter wrote:
> Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:
>> The attribute WGALLOWEDIP_A_IPADDR can contain either an IPv4
>> or an IPv6 address depending on WGALLOWEDIP_A_FAMILY, however
>> in practice it is enough to look at the attribute length.
>>
>> This patch implements an ipv4-or-v6 display hint, that can
>> deal with this kind of attribute.
>>
>> It only implements this display hint for genetlink-legacy, it
>> can be added to other protocol variants if needed, but we don't
>> want to encourage it's use.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> 
> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
> 
> I suspect there are occurrences of ipv4 or ipv6 in the existing specs
> that really should be ipv4-or-ipv6 but the python code doesn't care.

I haven't been able to find any, containing 'ip' or 'addr'.

Speaking of display hints, then WGPEER_A_ENDPOINT is another interesting
case, it is struct sockaddr_in or struct sockaddr_in6, I have left that
as a plain binary for now, but maybe that could be a struct map, based
on struct length.

attribute-sets:
   -
     name: wgpeer
     [..]
     attributes:
       [..]
       -
         name: endpoint
         type: binary
         struct-map:
           - sockaddr_in
           - sockaddr_in6

With the requirement being, that all structs must have a unique length.

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Jakub Kicinski]

On Sat, 6 Sep 2025 13:13:29 +0000 Asbjørn Sloth Tønnesen wrote:
> In patch 4, it is about a variable used by multiple Type classes having
> presence_type() = 'count', which is currently 3 classes:
> - TypeBinaryScalarArray
> - TypeMultiAttr
> - TypeArrayNest (later renamed to TypeIndexedArray)
> 
> In patch 5, I move code for a special variable used by one Type class,
> to be contained within that class. It makes it easier to ensure that the
> variable is only defined, when used, and vice versa. This comes at the
> cost of the generated code looking generated.

So you're agreeing?

> If we should make the generated code look like it was written by humans,
> then I would move the definition of these local variables into a class
> method, so `i` can be generated by the generic implementation, and `array`
> can be implemented in it's class. I will take a stab at this, but it might
> be too much refactoring for this series, eg. `len` is also defined local
> to conditional blocks multiple branches in a row.
> 
> tools/net/ynl/generated/nl80211-user.c:
> nl80211_iftype_data_attrs_parse(..) {
>    [..]
>    ynl_attr_for_each_nested(attr, nested) {
>      unsigned int type = ynl_attr_type(attr);
> 
>      if (type == NL80211_BAND_IFTYPE_ATTR_IFTYPES) {
>        unsigned int len;
>        [..]
>      } else if (type == NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC) {
>        unsigned int len;
>        [..]
>      [same pattern 8 times, so 11 times in total]
>      } else if (type == NL80211_BAND_IFTYPE_ATTR_EHT_CAP_PPE) {
>        unsigned int len;
>        [..]
>      }
>    }
>    return 0;
> }

It's pretty easily doable, I already gave up on not calling _attr_get()
for sub-messages.

> That looks very generated, I would have `len` defined together with `type`,
> and a switch statement would also look a lot more natural, but maybe leave
> the if->switch conversion for the compiler to detect.

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index fb7e03805a11..8a1f8a477566 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -243,7 +243,7 @@ from lib import SpecSubMessage, SpecSubMessageFormat
         raise Exception(f"Attr get not implemented for class type {self.type}")
 
     def attr_get(self, ri, var, first):
-        lines, init_lines, local_vars = self._attr_get(ri, var)
+        lines, init_lines, _ = self._attr_get(ri, var)
         if type(lines) is str:
             lines = [lines]
         if type(init_lines) is str:
@@ -251,10 +251,6 @@ from lib import SpecSubMessage, SpecSubMessageFormat
 
         kw = 'if' if first else 'else if'
         ri.cw.block_start(line=f"{kw} (type == {self.enum_name})")
-        if local_vars:
-            for local in local_vars:
-                ri.cw.p(local)
-            ri.cw.nl()
 
         if not self.is_multi_val():
             ri.cw.p("if (ynl_attr_validate(yarg, attr))")
@@ -2101,6 +2097,7 @@ _C_KW = {
             else:
                 raise Exception(f"Per-op fixed header not supported, yet")
 
+    var_set = set()
     array_nests = set()
     multi_attrs = set()
     needs_parg = False
@@ -2118,6 +2115,13 @@ _C_KW = {
             multi_attrs.add(arg)
         needs_parg |= 'nested-attributes' in aspec
         needs_parg |= 'sub-message' in aspec
+
+        try:
+            _, _, l_vars = aspec._attr_get(ri, '')
+            var_set |= set(l_vars) if l_vars else set()
+        except:
+            pass  # _attr_get() not implemented by simple types, ignore
+    local_vars += list(var_set)
     if array_nests or multi_attrs:
         local_vars.append('int i;')
     if needs_parg:

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jakub Kicinski]

On Sat, 6 Sep 2025 13:22:01 +0000 Asbjørn Sloth Tønnesen wrote:
> > I don't understand, please provide more details.
> > This is an ArrayNest, right?
> > 
> > [ARRAY-ATTR]
> >    [ENTRY]
> >      [MEMBER1]
> >      [MEMBER2]
> >    [ENTRY]
> >      [MEMBER1]
> >      [MEMBER2]
> > 
> > Which level are you saying doesn't matter?
> > If entry is a nest it must be a valid nest.
> > What the comment you're quoting is saying is that the nla_type of ENTRY
> > doesn't matter.  
> 
> I will expand this in v2, but the gist of it is that this is part of the
> "split attribute counting, and later allocating an array to hold them" code.
> 
> The check that I remove for nested arrays, is an early exit during the
> counting phase. Later in the allocation and parse phase it validates the
> nested payload.
> 
> In include/uapi/linux/wireguard.h:
>  > WGDEVICE_A_PEERS: NLA_NESTED
>  >   0: NLA_NESTED
>  >     WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
>  >     [..]
>  >   0: NLA_NESTED
>  >     ...
>  >   ...  
> 
> The current check requires that the nested type is valid in the nested
> attribute set, which in this case resolves to WGDEVICE_A_UNSPEC, which is
> YNL_PT_REJECT, and it takes the early exit and returns YNL_PARSE_CB_ERROR.

I see your point now. We're validating ENTRY as an attribute in the
parent attribute set, but it's just a meaningless id.

I think we need more fixing here. The real parsing loop will only
validate what's _inside_ the [MEMBER]. Which doesn't matter all
that much to nests, but look at what happens if subtype is a scalar.
We'll just call ynl_attr_get_u32(), type is never really validate.

I think we need this, and make the codegen feed in the ARRAY-ATTR type
to validate ENTRY?

diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
index 2a169c3c0797..e43167398c69 100644
--- a/tools/net/ynl/lib/ynl.c
+++ b/tools/net/ynl/lib/ynl.c
@@ -360,15 +360,15 @@ static int ynl_cb_done(const struct nlmsghdr *nlh, struct ynl_parse_arg *yarg)
 
 /* Attribute validation */
 
-int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
+int __ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr,
+			unsigned int type)
 {
 	const struct ynl_policy_attr *policy;
-	unsigned int type, len;
 	unsigned char *data;
+	unsigned int len;
 
 	data = ynl_attr_data(attr);
 	len = ynl_attr_data_len(attr);
-	type = ynl_attr_type(attr);
 	if (type > yarg->rsp_policy->max_attr) {
 		yerr(yarg->ys, YNL_ERROR_INTERNAL,
 		     "Internal error, validating unknown attribute");
@@ -450,6 +450,11 @@ int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
 	return 0;
 }
 
+int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
+{
+	return __ynl_attr_validate(yarg, attr, ynl_attr_type(attr));
+}
+
 int ynl_submsg_failed(struct ynl_parse_arg *yarg, const char *field_name,
 		      const char *sel_name)
 {

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Johannes Berg]

On Sat, 2025-09-06 at 14:13 +0000, Asbjørn Sloth Tønnesen wrote:
> 
> Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY()
> macro in commit 1501d13596b9 for use in nl80211, and it's therefore
> used in net/wireless/nl80211.c, but outside of that the macro is
> only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c).
> 
> Wireguard adopts the macro in this RFC patch:
> https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/

I think the general consensus now is that preference should be towards
arrays being expressed by giving the attribute holding the array
multiple times, i.e. each occurrence of an attribute holds a single
entry of the array:

[header][type1:a1][type2:b][type1:a2][type1:a3]

resulting in an array

[a1, a2, a3] and a separate value "b",

rather than a nested array:

[header][type1:[1:a1][2:a2][3:a3]][type2:b]


Of course if each entry has multiple values, then you'd still need
nesting:

[header][type1:[subtype1:x1][subtype2:x2]][type1:[subtype1:y1][subtype2:y2]]

would be an array

[[x1, x2], [y1, y2]].


I can't get rid of the nested array types in nl80211 though, of course.

I'm not sure the nl80211 ynl code was ever merged, but it wasn't
authoritative anyway, just for some limited userspace generation, so I'm
not sure the whole ynl handling this is needed at all?


johannes

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Johannes Berg]

On Sat, 2025-09-06 at 15:10 +0000, Asbjørn Sloth Tønnesen wrote:
> 
> For the other families, I don't know how well defined it is, Johannes have
> stated that nl80211 doesn't care which types are used, but I have no idea
> how consistent clients have abused that statement to send random data,
> or do they all just send zeros?

I think most clients probably send incrementing numbers (1, 2, 3, ...),
but maybe some start at 0, some sometimes might use band numbers and
thus have sparse values, etc.

But as I just wrote, I'm not really sure it should be used at all.

joahnnes

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Donald Hunter]

Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:

> On 9/6/25 12:27 AM, Jacob Keller wrote:
>> On 9/5/2025 3:51 AM, Donald Hunter wrote:
>>> Asbjørn Sloth Tønnesen <ast@fiberby.net> writes:
>>>
>>>> This patch add support for decoding hex input, so
>>>> that binary attributes can be read through --json.
>>>>
>>>> Example (using future wireguard.yaml):
>>>>   $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>>>>     --do set-device --json '{"ifindex":3,
>>>>       "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
>>>>
>>>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>>>
>>> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
>>>
>>> FWIW, the hex can include spaces or not when using bytes.fromhex(). When
>>> formatting hex for output, I chose to include spaces, but I don't really
>>> know if that was a good choice or not.
>> I also prefer the spaces for readability.
> I formatted it with spaces for clarity, even without spaces it was a bit
> long for one line. Spaces also has the advantage that you don't have to
> think about endianness.
>
> Should we define the display hints a bit more in a .rst, or is it OK that
> they end up being implementation specific for each language library? Do we
> want them to behave the same in a Rust YNL library, as they do in Python?

Yes we should probably extend the existing doc to at least describe some
of the defacto behaviour.

https://docs.kernel.org/userspace-api/netlink/specs.html#display-hint

> BTW: The rest of the key used in the example can be found with this key-gen:
> $ printf "hello world" | sha1sum
> [redacted key material]

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Asbjørn Sloth Tønnesen]

On 9/8/25 7:54 AM, Johannes Berg wrote:
> On Sat, 2025-09-06 at 14:13 +0000, Asbjørn Sloth Tønnesen wrote:
>> Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY()
>> macro in commit 1501d13596b9 for use in nl80211, and it's therefore
>> used in net/wireless/nl80211.c, but outside of that the macro is
>> only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c).
>>
>> Wireguard adopts the macro in this RFC patch:
>> https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/
> 
 > I think the general consensus now is that preference should be towards
 > arrays being expressed by giving the attribute holding the array
 > multiple times, i.e. each occurrence of an attribute holds a single
 > entry of the array:
 >
 > [header][type1:a1][type2:b][type1:a2][type1:a3]
 >
 > resulting in an array
 >
 > [a1, a2, a3] and a separate value "b",
 >
 > rather than a nested array:
 >
 > [header][type1:[1:a1][2:a2][3:a3]][type2:b]
 >
 >
 > Of course if each entry has multiple values, then you'd still need
 > nesting:
 >
 > [header][type1:[subtype1:x1][subtype2:x2]][type1:[subtype1:y1][subtype2:y2]]
 >
 > would be an array
 >
 > [[x1, x2], [y1, y2]].

Thank you for the consensus write up. Should we prohibit indexed-array with sub-type
nest for families with a genetlink protocol?

It is currently only used in families with a netlink-raw or genetlink-legacy protocol.

> I can't get rid of the nested array types in nl80211 though, of course.

Wireguard is already in the same boat. It is not using the term, nor the policy,
but it is doing the validation in the handler through, so it can adopt a
NLA_POLICY_NESTED_ARRAY() policy, instead of a plain '{ .type = NLA_NESTED }'.

Comments on the protocol in include/uapi/linux/wireguard.h:
 > WGDEVICE_A_PEERS: NLA_NESTED
 >   0: NLA_NESTED
 >     WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
 >     [..]
 >   0: NLA_NESTED
 >     ...
 >   ...

Given that, as Jacob pointed out, there are more families with nested arrays in
their YNL spec, than those using NLA_NESTED_ARRAY, then it appears that there
are more families already in the boat.

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Johannes Berg]

On Mon, 2025-09-08 at 09:08 +0000, Asbjørn Sloth Tønnesen wrote:
> 
> Thank you for the consensus write up. Should we prohibit indexed-array with sub-type
> nest for families with a genetlink protocol?
> 
> It is currently only used in families with a netlink-raw or genetlink-legacy protocol.

I have no strong opinion on that, but I guess maybe so? At least print
out a warning for anyone who's trying to add such a new thing perhaps,
so that new stuff that isn't just a port (to ynl) or annotation of
existing APIs doesn't add it.

> > I can't get rid of the nested array types in nl80211 though, of course.
> 
> Wireguard is already in the same boat. [...]

Oh, sorry. I didn't look at the linked patch and thought it was adding
such a new thing. Looking now, I see it just makes the policy validate
it instead of (only) doing it in the code. (FWIW, in the code you could
then also set the policy argument for nla_parse_nested() calls to NULL.)

> Given that, as Jacob pointed out, there are more families with nested arrays in
> their YNL spec, than those using NLA_NESTED_ARRAY, then it appears that there
> are more families already in the boat.

Right.

johannes

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Sabrina Dubroca]

2025-09-04, 22:01:33 +0000, Asbjørn Sloth Tønnesen wrote:
> This patch add support for decoding hex input, so
> that binary attributes can be read through --json.
> 
> Example (using future wireguard.yaml):
>  $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>    --do set-device --json '{"ifindex":3,
>      "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  tools/net/ynl/pyynl/lib/ynl.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index a37294a751da..78c0245ca587 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -973,6 +973,8 @@ class YnlFamily(SpecFamily):
>                  raw = ip.packed
>              else:
>                  raw = int(ip)
> +        elif attr_spec.display_hint == 'hex':
> +            raw = bytes.fromhex(string)

I'm working on a spec for macsec and ended up with a similar change,
but doing instead:

+        elif attr_spec.display_hint == 'hex':
+            raw = int(string, 16)

since the destination attribute is u32/u64 and not binary for macsec.

So maybe this should be:

+            if attr_spec['type'] == 'binary':
+                raw = bytes.fromhex(string)
+            else:
+                raw = int(string, 16)

to cover both cases?

I think it matches better what's already in _formatted_string.

(I don't mind having the current patch go in and making this change
together with the macsec spec when it's ready)

>          else:
>              raise Exception(f"Display hint '{attr_spec.display_hint}' not implemented"
>                              f" when parsing '{attr_spec['name']}'")
> -- 
> 2.51.0
> 
> 

-- 
Sabrina

Re: [PATCH net-next 10/11] tools: ynl: decode hex input

[Asbjørn Sloth Tønnesen]

On 9/9/25 6:10 PM, Sabrina Dubroca wrote:
> 2025-09-04, 22:01:33 +0000, Asbjørn Sloth Tønnesen wrote:
>> This patch add support for decoding hex input, so
>> that binary attributes can be read through --json.
>>
>> Example (using future wireguard.yaml):
>>   $ sudo ./tools/net/ynl/pyynl/cli.py --family wireguard \
>>     --do set-device --json '{"ifindex":3,
>>       "private-key":"2a ae 6c 35 c9 4f cf <... to 32 bytes>"}'
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>> ---
>>   tools/net/ynl/pyynl/lib/ynl.py | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
>> index a37294a751da..78c0245ca587 100644
>> --- a/tools/net/ynl/pyynl/lib/ynl.py
>> +++ b/tools/net/ynl/pyynl/lib/ynl.py
>> @@ -973,6 +973,8 @@ class YnlFamily(SpecFamily):
>>                   raw = ip.packed
>>               else:
>>                   raw = int(ip)
>> +        elif attr_spec.display_hint == 'hex':
>> +            raw = bytes.fromhex(string)
> 
> I'm working on a spec for macsec and ended up with a similar change,
> but doing instead:
> 
> +        elif attr_spec.display_hint == 'hex':
> +            raw = int(string, 16)
> 
> since the destination attribute is u32/u64 and not binary for macsec.
> 
> So maybe this should be:
> 
> +            if attr_spec['type'] == 'binary':
> +                raw = bytes.fromhex(string)
> +            else:
> +                raw = int(string, 16)
> 
> to cover both cases?
> 
> I think it matches better what's already in _formatted_string.
> 
> (I don't mind having the current patch go in and making this change
> together with the macsec spec when it's ready)

Cool, I will include it in v2, which I hope to get out tomorrow.

Re: [PATCH net-next 02/11] tools: ynl-gen: generate nested array policies

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1295 bytes --]



On 9/6/2025 7:13 AM, Asbjørn Sloth Tønnesen wrote:
> CC: Johannes
> 
> On 9/6/25 12:19 AM, Jacob Keller wrote:
>> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>>> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
>>>
>>> Example spec (from future wireguard.yaml):
>>> -
>>>    name: wgpeer
>>>    attributes:
>>>      -
>>>        name: allowedips
>>>        type: indexed-array
>>>        sub-type: nest
>>>        nested-attributes: wgallowedip
>>>
>>> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
>>>
>>> This doesn't change any currently generated code, as it isn't
>>> used in any specs currently used for generating code.
>>>
>>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>>> ---
>>
>> Is this keyed of off the sub-type? Does you mean that all the existing
>> uses of 'sub-type: nest' don't generate code today? Or that this
>> _attr_policy implementation is not called yet?
> 
> Thanks for the reviews. Yeah, it is a careful wording, because we have
> specs matching it, but there aren't any source files that triggers
> ynl-gen to generate code based on those specs.
> 
Ok. That's what I thought and just wanted to be certain, since I saw
several uses in the code. Thanks for explaining!

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 2876 bytes --]



On 9/6/2025 8:10 AM, Asbjørn Sloth Tønnesen wrote:
> CC: Johannes
> 
> On 9/6/25 12:24 AM, Jacob Keller wrote:
>> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>>> In nested arrays don't require that the intermediate
>>> attribute type should be a valid attribute type, it
>>> might just be an index or simple 0, it is often not
>>> even used.
>>>
>>> See include/net/netlink.h about NLA_NESTED_ARRAY:
>>>> The difference to NLA_NESTED is the structure:
>>>> NLA_NESTED has the nested attributes directly inside
>>>> while an array has the nested attributes at another
>>>> level down and the attribute types directly in the
>>>> nesting don't matter.
>>>
>>
>> To me, it would seem like it makes more sense to define these (even if
>> thats defined per family?) than to just say they aren't defined at all?
>>
>> Hm.
> 
> I considered adding some of that metadata too, as I am actually removing
> it for wireguard (in comment form, but still).
> 
> In include/uapi/linux/wireguard.h in the comment block at the top, it is
> very clear that wireguard only used type 0 for all the nested array
> entries, however the truth is that it doesn't care. It therefore doesn't
> matter if the generated -user.* keeps track of the index in .idx, or that
> cli.py decodes a JSON array and sends it with indexes, it's not needed,
> but it still works.
> 
> In practice I don't think we will break any clients if we enforced it, and
> validated that wireguard only accepts type 0 entries, in it's nested arrays.
> 
> For the other families, I don't know how well defined it is, Johannes have
> stated that nl80211 doesn't care which types are used, but I have no idea
> how consistent clients have abused that statement to send random data,
> or do they all just send zeros?
> 

Changing it at this point could be a significant backwards compat break,
as some clients might somehow send data that wasn't zero-initialized,
and checking would break them. At this point I guess it makes sense to
leave it as is... It would increase code cost and complexity for no gain.

> This would make a lot more sense if 'array-nest' hadn't been renamed to
> 'indexed-array' in ynl, because it feels wrong to add 'unindexed: true' now.
> We could also call it 'all-zero-indexed: true'.
> 
> In cli.py this gives some extra issues, as seen in [1], the nested arrays
> are outputted as '[{0: {..}}, {0: {..}}, ..]', but on input has the format
> '[{..},{..}, ..]' because it has to be JSON-compatible on input.
> 
> If we had an attribute like 'all-zero-indexed' then cli.py, could also output
> '[{..},{..}, ..]'.
> 

This part would be cool. If we know the index is "uninteresting",
eliding it so that the input and output formats match is good.

> [1] https://lore.kernel.org/netdev/20250904220255.1006675-3-ast@fiberby.net/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Asbjørn Sloth Tønnesen]

On 9/6/25 7:07 PM, Jakub Kicinski wrote:
> On Sat, 6 Sep 2025 13:13:29 +0000 Asbjørn Sloth Tønnesen wrote:
>> In patch 4, it is about a variable used by multiple Type classes having
>> presence_type() = 'count', which is currently 3 classes:
>> - TypeBinaryScalarArray
>> - TypeMultiAttr
>> - TypeArrayNest (later renamed to TypeIndexedArray)
>>
>> In patch 5, I move code for a special variable used by one Type class,
>> to be contained within that class. It makes it easier to ensure that the
>> variable is only defined, when used, and vice versa. This comes at the
>> cost of the generated code looking generated.
> 
> So you're agreeing?
> 
>> If we should make the generated code look like it was written by humans,
>> then I would move the definition of these local variables into a class
>> method, so `i` can be generated by the generic implementation, and `array`
>> can be implemented in it's class. I will take a stab at this, but it might
>> be too much refactoring for this series, eg. `len` is also defined local
>> to conditional blocks multiple branches in a row.
>>
>> tools/net/ynl/generated/nl80211-user.c:
>> nl80211_iftype_data_attrs_parse(..) {
>>     [..]
>>     ynl_attr_for_each_nested(attr, nested) {
>>       unsigned int type = ynl_attr_type(attr);
>>
>>       if (type == NL80211_BAND_IFTYPE_ATTR_IFTYPES) {
>>         unsigned int len;
>>         [..]
>>       } else if (type == NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC) {
>>         unsigned int len;
>>         [..]
>>       [same pattern 8 times, so 11 times in total]
>>       } else if (type == NL80211_BAND_IFTYPE_ATTR_EHT_CAP_PPE) {
>>         unsigned int len;
>>         [..]
>>       }
>>     }
>>     return 0;
>> }
> 
> It's pretty easily doable, I already gave up on not calling _attr_get()
> for sub-messages.
> 
>> That looks very generated, I would have `len` defined together with `type`,
>> and a switch statement would also look a lot more natural, but maybe leave
>> the if->switch conversion for the compiler to detect.
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index fb7e03805a11..8a1f8a477566 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -243,7 +243,7 @@ from lib import SpecSubMessage, SpecSubMessageFormat
>           raise Exception(f"Attr get not implemented for class type {self.type}")
>   
>       def attr_get(self, ri, var, first):
> -        lines, init_lines, local_vars = self._attr_get(ri, var)
> +        lines, init_lines, _ = self._attr_get(ri, var)
>           if type(lines) is str:
>               lines = [lines]
>           if type(init_lines) is str:
> @@ -251,10 +251,6 @@ from lib import SpecSubMessage, SpecSubMessageFormat
>   
>           kw = 'if' if first else 'else if'
>           ri.cw.block_start(line=f"{kw} (type == {self.enum_name})")
> -        if local_vars:
> -            for local in local_vars:
> -                ri.cw.p(local)
> -            ri.cw.nl()
>   
>           if not self.is_multi_val():
>               ri.cw.p("if (ynl_attr_validate(yarg, attr))")
> @@ -2101,6 +2097,7 @@ _C_KW = {
>               else:
>                   raise Exception(f"Per-op fixed header not supported, yet")
>   
> +    var_set = set()
>       array_nests = set()
>       multi_attrs = set()
>       needs_parg = False
> @@ -2118,6 +2115,13 @@ _C_KW = {
>               multi_attrs.add(arg)
>           needs_parg |= 'nested-attributes' in aspec
>           needs_parg |= 'sub-message' in aspec
> +
> +        try:
> +            _, _, l_vars = aspec._attr_get(ri, '')
> +            var_set |= set(l_vars) if l_vars else set()
> +        except:
> +            pass  # _attr_get() not implemented by simple types, ignore
> +    local_vars += list(var_set)
>       if array_nests or multi_attrs:
>           local_vars.append('int i;')
>       if needs_parg:
I left this for you to submit, there is a trivial conflict with patch 8
in my v2 posting.

It gives a pretty nice diffstat when comparing the generated code:

  devlink-user.c      |  187 +++----------------
  dpll-user.c         |   10 -
  ethtool-user.c      |   49 +----
  fou-user.c          |    5
  handshake-user.c    |    3
  mptcp_pm-user.c     |    3
  nfsd-user.c         |   16 -
  nl80211-user.c      |  159 +---------------
  nlctrl-user.c       |   21 --
  ovpn-user.c         |    7
  ovs_datapath-user.c |    9
  ovs_flow-user.c     |   89 ---------
  ovs_vport-user.c    |    7
  rt-addr-user.c      |   14 -
  rt-link-user.c      |  183 ++----------------
  rt-neigh-user.c     |   14 -
  rt-route-user.c     |   26 --
  rt-rule-user.c      |   11 -
  tc-user.c           |  380 +++++----------------------------------
  tcp_metrics-user.c  |    7
  team-user.c         |    5
  21 files changed, 175 insertions(+), 1030 deletions(-)

Re: [PATCH net-next 05/11] tools: ynl-gen: define nlattr *array in a block scope

[Jakub Kicinski]

On Thu, 11 Sep 2025 00:01:48 +0000 Asbjørn Sloth Tønnesen wrote:
> I left this for you to submit, there is a trivial conflict with patch 8
> in my v2 posting.

Hah, no, please take this and submit as yours if you can. You can add
me as Suggested-by. I already dropped it form my tree.
Maintainers tend to throw random snippets of code at people, it's just
quicker than explaining but we have enough commits in our name..