[PATCH net-next 0/3] tools: ynl: decode link types present in tests

[PATCH net-next 0/3] tools: ynl: decode link types present in tests

[Jakub Kicinski]

Using a kernel built for the net selftest target to run drivers/net
tests currently fails, because the net kernel automatically spawns
a handful of tunnel devices which YNL can't decode.

Fill in those missing link types in rt_link. We need to extend subset
support a bit for it to work.

Jakub Kicinski (3):
  tools: ynl: correctly handle overrides of fields in subset
  tools: ynl: print some information about attribute we can't parse
  netlink: specs: rt_link: decode ip6tnl, vti and vti6 link attrs

 Documentation/netlink/specs/rt_link.yaml | 87 ++++++++++++++++++++++++
 tools/net/ynl/lib/nlspec.py              |  5 +-
 tools/net/ynl/lib/ynl.py                 | 72 +++++++++++---------
 3 files changed, 129 insertions(+), 35 deletions(-)

-- 
2.47.1

[PATCH net-next 1/3] tools: ynl: correctly handle overrides of fields in subset

[Jakub Kicinski]

We stated in documentation [1] and previous discussions [2]
that the need for overriding fields in members of subsets
is anticipated. Implement it.

[1] https://docs.kernel.org/next/userspace-api/netlink/specs.html#subset-of
[2] https://lore.kernel.org/netdev/20231004171350.1f59cd1d@kernel.org/

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/nlspec.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index a745739655ad..314ec8007496 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -219,7 +219,10 @@ jsonschema = None
         else:
             real_set = family.attr_sets[self.subset_of]
             for elem in self.yaml['attributes']:
-                attr = real_set[elem['name']]
+                real_attr = real_set[elem['name']]
+                combined_elem = real_attr.yaml | elem
+                attr = self.new_attr(combined_elem, real_attr.value)
+
                 self.attrs[attr.name] = attr
                 self.attrs_by_val[attr.value] = attr
 
-- 
2.47.1

Re: [PATCH net-next 1/3] tools: ynl: correctly handle overrides of fields in subset

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> We stated in documentation [1] and previous discussions [2]
> that the need for overriding fields in members of subsets
> is anticipated. Implement it.
>
> [1] https://docs.kernel.org/next/userspace-api/netlink/specs.html#subset-of
> [2] https://lore.kernel.org/netdev/20231004171350.1f59cd1d@kernel.org/
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

I guess we're okay with requiring Python >= 3.9 for combining
dicts with |

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

> ---
>  tools/net/ynl/lib/nlspec.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
> index a745739655ad..314ec8007496 100644
> --- a/tools/net/ynl/lib/nlspec.py
> +++ b/tools/net/ynl/lib/nlspec.py
> @@ -219,7 +219,10 @@ jsonschema = None
>          else:
>              real_set = family.attr_sets[self.subset_of]
>              for elem in self.yaml['attributes']:
> -                attr = real_set[elem['name']]
> +                real_attr = real_set[elem['name']]
> +                combined_elem = real_attr.yaml | elem
> +                attr = self.new_attr(combined_elem, real_attr.value)
> +
>                  self.attrs[attr.name] = attr
>                  self.attrs_by_val[attr.value] = attr

Re: [PATCH net-next 1/3] tools: ynl: correctly handle overrides of fields in subset

[Jakub Kicinski]

On Mon, 06 Jan 2025 13:27:49 +0000 Donald Hunter wrote:
> > We stated in documentation [1] and previous discussions [2]
> > that the need for overriding fields in members of subsets
> > is anticipated. Implement it.
> >
> > [1] https://docs.kernel.org/next/userspace-api/netlink/specs.html#subset-of
> > [2] https://lore.kernel.org/netdev/20231004171350.1f59cd1d@kernel.org/
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>  
> 
> I guess we're okay with requiring Python >= 3.9 for combining
> dicts with |

Ah, I didn't realize. Does YNL work on older versions today?
I thought we already narrowed down to 3.9+. That may have
been tests not YNL itself.

The "oldest" OS I have is CentOS 9(-derived) and has 3.9,
so from my selfish perspective 3.9+ is perfectly fine :)

Re: [PATCH net-next 1/3] tools: ynl: correctly handle overrides of fields in subset

[Donald Hunter]

On Mon, 6 Jan 2025 at 15:36, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 06 Jan 2025 13:27:49 +0000 Donald Hunter wrote:
> > > We stated in documentation [1] and previous discussions [2]
> > > that the need for overriding fields in members of subsets
> > > is anticipated. Implement it.
> > >
> > > [1] https://docs.kernel.org/next/userspace-api/netlink/specs.html#subset-of
> > > [2] https://lore.kernel.org/netdev/20231004171350.1f59cd1d@kernel.org/
> > >
> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> >
> > I guess we're okay with requiring Python >= 3.9 for combining
> > dicts with |
>
> Ah, I didn't realize. Does YNL work on older versions today?
> I thought we already narrowed down to 3.9+. That may have
> been tests not YNL itself.

You're right, we may already be committed to 3.9+

> The "oldest" OS I have is CentOS 9(-derived) and has 3.9,
> so from my selfish perspective 3.9+ is perfectly fine :)

To be fair, on a previous commit you mentioned that it affected CentOS
8 which EOLed back in May 2024 so we shouldn't feel compelled to
support it any more.

https://lore.kernel.org/all/20230524170712.2036128-1-kuba@kernel.org/

[PATCH net-next 2/3] tools: ynl: print some information about attribute we can't parse

[Jakub Kicinski]

When parsing throws an exception one often has to figure out which
attribute couldn't be parsed from first principles. For families
with large message parsing trees like rtnetlink guessing the
attribute can be hard.

Print a bit of information as the exception travels out, e.g.:

  # when dumping rt links
  Error decoding 'flags' from 'linkinfo-ip6tnl-attrs'
  Error decoding 'data' from 'linkinfo-attrs'
  Error decoding 'linkinfo' from 'link-attrs'
  Traceback (most recent call last):
    File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 119, in <module>
      main()
    File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 100, in main
      reply = ynl.dump(args.dump, attrs)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1064, in dump
      return self._op(method, vals, dump=True)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1058, in _op
      return self._ops(ops)[0]
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1045, in _ops
      rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 738, in _decode
      subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 763, in _decode
      decoded = self._decode_sub_msg(attr, attr_spec, search_attrs)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 714, in _decode_sub_msg
      subdict = self._decode(NlAttrs(attr.raw, offset), msg_format.attr_set)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 749, in _decode
      decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order)
    File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 147, in as_scalar
      return format.unpack(self.raw)[0]
  struct.error: unpack requires a buffer of 2 bytes

The Traceback is what we would previously see, the "Error..."
messages are new. We print a message per level (in the stack
order). Printing single combined message gets tricky quickly
given sub-messages etc.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/ynl.py | 72 +++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index eea29359a899..08f8bf89cfc2 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -733,41 +733,45 @@ genl_family_name_to_id = None
                 self._rsp_add(rsp, attr_name, None, self._decode_unknown(attr))
                 continue
 
-            if attr_spec["type"] == 'nest':
-                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs)
-                decoded = subdict
-            elif attr_spec["type"] == 'string':
-                decoded = attr.as_strz()
-            elif attr_spec["type"] == 'binary':
-                decoded = self._decode_binary(attr, attr_spec)
-            elif attr_spec["type"] == 'flag':
-                decoded = True
-            elif attr_spec.is_auto_scalar:
-                decoded = attr.as_auto_scalar(attr_spec['type'], attr_spec.byte_order)
-            elif attr_spec["type"] in NlAttr.type_formats:
-                decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order)
-                if 'enum' in attr_spec:
-                    decoded = self._decode_enum(decoded, attr_spec)
-                elif attr_spec.display_hint:
-                    decoded = self._formatted_string(decoded, attr_spec.display_hint)
-            elif attr_spec["type"] == 'indexed-array':
-                decoded = self._decode_array_attr(attr, attr_spec)
-            elif attr_spec["type"] == 'bitfield32':
-                value, selector = struct.unpack("II", attr.raw)
-                if 'enum' in attr_spec:
-                    value = self._decode_enum(value, attr_spec)
-                    selector = self._decode_enum(selector, attr_spec)
-                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"]}')
-                decoded = self._decode_unknown(attr)
+            try:
+                if attr_spec["type"] == 'nest':
+                    subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs)
+                    decoded = subdict
+                elif attr_spec["type"] == 'string':
+                    decoded = attr.as_strz()
+                elif attr_spec["type"] == 'binary':
+                    decoded = self._decode_binary(attr, attr_spec)
+                elif attr_spec["type"] == 'flag':
+                    decoded = True
+                elif attr_spec.is_auto_scalar:
+                    decoded = attr.as_auto_scalar(attr_spec['type'], attr_spec.byte_order)
+                elif attr_spec["type"] in NlAttr.type_formats:
+                    decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order)
+                    if 'enum' in attr_spec:
+                        decoded = self._decode_enum(decoded, attr_spec)
+                    elif attr_spec.display_hint:
+                        decoded = self._formatted_string(decoded, attr_spec.display_hint)
+                elif attr_spec["type"] == 'indexed-array':
+                    decoded = self._decode_array_attr(attr, attr_spec)
+                elif attr_spec["type"] == 'bitfield32':
+                    value, selector = struct.unpack("II", attr.raw)
+                    if 'enum' in attr_spec:
+                        value = self._decode_enum(value, attr_spec)
+                        selector = self._decode_enum(selector, attr_spec)
+                    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"]}')
+                    decoded = self._decode_unknown(attr)
 
-            self._rsp_add(rsp, attr_spec["name"], attr_spec.is_multi, decoded)
+                self._rsp_add(rsp, attr_spec["name"], attr_spec.is_multi, decoded)
+            except:
+                print(f"Error decoding '{attr_spec.name}' from '{space}'")
+                raise
 
         return rsp
 
-- 
2.47.1

Re: [PATCH net-next 2/3] tools: ynl: print some information about attribute we can't parse

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> When parsing throws an exception one often has to figure out which
> attribute couldn't be parsed from first principles. For families
> with large message parsing trees like rtnetlink guessing the
> attribute can be hard.
>
> Print a bit of information as the exception travels out, e.g.:
>
>   # when dumping rt links
>   Error decoding 'flags' from 'linkinfo-ip6tnl-attrs'
>   Error decoding 'data' from 'linkinfo-attrs'
>   Error decoding 'linkinfo' from 'link-attrs'
>   Traceback (most recent call last):
>     File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 119, in <module>
>       main()
>     File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 100, in main
>       reply = ynl.dump(args.dump, attrs)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1064, in dump
>       return self._op(method, vals, dump=True)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1058, in _op
>       return self._ops(ops)[0]
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1045, in _ops
>       rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 738, in _decode
>       subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 763, in _decode
>       decoded = self._decode_sub_msg(attr, attr_spec, search_attrs)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 714, in _decode_sub_msg
>       subdict = self._decode(NlAttrs(attr.raw, offset), msg_format.attr_set)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 749, in _decode
>       decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order)
>     File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 147, in as_scalar
>       return format.unpack(self.raw)[0]
>   struct.error: unpack requires a buffer of 2 bytes
>
> The Traceback is what we would previously see, the "Error..."
> messages are new. We print a message per level (in the stack
> order). Printing single combined message gets tricky quickly
> given sub-messages etc.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

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

[PATCH net-next 3/3] netlink: specs: rt_link: decode ip6tnl, vti and vti6 link attrs

[Jakub Kicinski]

Some of our tests load vti and ip6tnl so not being able to decode
the link attrs gets in the way of using Python YNL for testing.

Decode link attributes for ip6tnl, vti and vti6.

ip6tnl uses IFLA_IPTUN_FLAGS as u32, while ipv4 and sit expect
a u16 attribute, so we have a (first?) subset type override...

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/rt_link.yaml | 87 ++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/Documentation/netlink/specs/rt_link.yaml b/Documentation/netlink/specs/rt_link.yaml
index 96465376d6fe..363c4d4f0779 100644
--- a/Documentation/netlink/specs/rt_link.yaml
+++ b/Documentation/netlink/specs/rt_link.yaml
@@ -1825,6 +1825,48 @@ protonum: 0
       -
         name: erspan-hwid
         type: u16
+  -
+    name: linkinfo-vti-attrs
+    name-prefix: ifla-vti-
+    attributes:
+      -
+        name: link
+        type: u32
+      -
+        name: ikey
+        type: u32
+      -
+        name: okey
+        type: u32
+      -
+        name: local
+        type: binary
+        display-hint: ipv4
+      -
+        name: remote
+        type: binary
+        display-hint: ipv4
+      -
+        name: fwmark
+        type: u32
+  -
+    name: linkinfo-vti6-attrs
+    subset-of: linkinfo-vti-attrs
+    attributes:
+      -
+        name: link
+      -
+        name: ikey
+      -
+        name: okey
+      -
+        name: local
+        display-hint: ipv6
+      -
+        name: remote
+        display-hint: ipv6
+      -
+        name: fwmark
   -
     name: linkinfo-geneve-attrs
     name-prefix: ifla-geneve-
@@ -1941,6 +1983,42 @@ protonum: 0
       -
         name: fwmark
         type: u32
+  -
+    name: linkinfo-ip6tnl-attrs
+    subset-of: linkinfo-iptun-attrs
+    attributes:
+      -
+        name: link
+      -
+        name: local
+        display-hint: ipv6
+      -
+        name: remote
+        display-hint: ipv6
+      -
+        name: ttl
+      -
+        name: encap-limit
+      -
+        name: flowinfo
+      -
+        name: flags
+        # ip6tnl unlike ipip and sit has 32b flags
+        type: u32
+      -
+        name: proto
+      -
+        name: encap-type
+      -
+        name: encap-flags
+      -
+        name: encap-sport
+      -
+        name: encap-dport
+      -
+        name: collect-metadata
+      -
+        name: fwmark
   -
     name: linkinfo-tun-attrs
     name-prefix: ifla-tun-
@@ -2195,6 +2273,9 @@ protonum: 0
       -
         value: ipip
         attribute-set: linkinfo-iptun-attrs
+      -
+        value: ip6tnl
+        attribute-set: linkinfo-ip6tnl-attrs
       -
         value: sit
         attribute-set: linkinfo-iptun-attrs
@@ -2207,6 +2288,12 @@ protonum: 0
       -
         value: vrf
         attribute-set: linkinfo-vrf-attrs
+      -
+        value: vti
+        attribute-set: linkinfo-vti-attrs
+      -
+        value: vti6
+        attribute-set: linkinfo-vti6-attrs
       -
         value: netkit
         attribute-set: linkinfo-netkit-attrs
-- 
2.47.1

Re: [PATCH net-next 3/3] netlink: specs: rt_link: decode ip6tnl, vti and vti6 link attrs

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> Some of our tests load vti and ip6tnl so not being able to decode
> the link attrs gets in the way of using Python YNL for testing.
>
> Decode link attributes for ip6tnl, vti and vti6.
>
> ip6tnl uses IFLA_IPTUN_FLAGS as u32, while ipv4 and sit expect
> a u16 attribute, so we have a (first?) subset type override...
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

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

Re: [PATCH net-next 0/3] tools: ynl: decode link types present in tests

[Stanislav Fomichev]

On 01/04, Jakub Kicinski wrote:
> Using a kernel built for the net selftest target to run drivers/net
> tests currently fails, because the net kernel automatically spawns
> a handful of tunnel devices which YNL can't decode.
> 
> Fill in those missing link types in rt_link. We need to extend subset
> support a bit for it to work.
> 
> Jakub Kicinski (3):
>   tools: ynl: correctly handle overrides of fields in subset
>   tools: ynl: print some information about attribute we can't parse
>   netlink: specs: rt_link: decode ip6tnl, vti and vti6 link attrs
> 
>  Documentation/netlink/specs/rt_link.yaml | 87 ++++++++++++++++++++++++
>  tools/net/ynl/lib/nlspec.py              |  5 +-
>  tools/net/ynl/lib/ynl.py                 | 72 +++++++++++---------
>  3 files changed, 129 insertions(+), 35 deletions(-)

I've seen similar issues with (builtin) ip6tnl in my VMs, thanks for
fixing!

Acked-by: Stanislav Fomichev <sdf@fomichev.me>