[PATCH net v2 0/3] netlink: specs: rt_addr: fix problems revealed by C codegen

[PATCH net v2 0/3] netlink: specs: rt_addr: fix problems revealed by C codegen

[Jakub Kicinski]

I put together basic YNL C support for classic netlink. This revealed
a few problems in the rt_addr spec.

v2:
 - fix the Fixes tag on patch 1
 - add 2 more patches
v1: https://lore.kernel.org/20250401012939.2116915-1-kuba@kernel.org

Jakub Kicinski (3):
  netlink: specs: rt_addr: fix the spec format / schema failures
  netlink: specs: rt_addr: fix get multi command name
  netlink: specs: rt_addr: pull the ifa- prefix out of the names

 Documentation/netlink/specs/rt_addr.yaml | 44 +++++++++++++-----------
 tools/testing/selftests/net/rtnetlink.py |  2 +-
 2 files changed, 24 insertions(+), 22 deletions(-)

-- 
2.49.0

[PATCH net v2 1/3] netlink: specs: rt_addr: fix the spec format / schema failures

[Jakub Kicinski]

The spec is mis-formatted, schema validation says:

  Failed validating 'type' in schema['properties']['operations']['properties']['list']['items']['properties']['dump']['properties']['request']['properties']['value']:
    {'minimum': 0, 'type': 'integer'}

  On instance['operations']['list'][3]['dump']['request']['value']:
    '58 - ifa-family'

The ifa-family clearly wants to be part of an attribute list.

Reviewed-by: Yuyang Huang <yuyanghuang@google.com>
Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: donald.hunter@gmail.com
CC: yuyanghuang@google.com
CC: jacob.e.keller@intel.com
---
 Documentation/netlink/specs/rt_addr.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
index 5dd5469044c7..3bc9b6f9087e 100644
--- a/Documentation/netlink/specs/rt_addr.yaml
+++ b/Documentation/netlink/specs/rt_addr.yaml
@@ -187,6 +187,7 @@ protonum: 0
       dump:
         request:
           value: 58
+          attributes:
             - ifa-family
         reply:
           value: 58
-- 
2.49.0

[PATCH net v2 2/3] netlink: specs: rt_addr: fix get multi command name

[Jakub Kicinski]

Command names should match C defines, codegens may depend on it.

Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/rt_addr.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
index 3bc9b6f9087e..1650dc3f091a 100644
--- a/Documentation/netlink/specs/rt_addr.yaml
+++ b/Documentation/netlink/specs/rt_addr.yaml
@@ -169,7 +169,7 @@ protonum: 0
           value: 20
           attributes: *ifaddr-all
     -
-      name: getmaddrs
+      name: getmulticast
       doc: Get / dump IPv4/IPv6 multicast addresses.
       attribute-set: addr-attrs
       fixed-header: ifaddrmsg
-- 
2.49.0

[PATCH net v2 3/3] netlink: specs: rt_addr: pull the ifa- prefix out of the names

[Jakub Kicinski]

YAML specs don't normally include the C prefix name in the name
of the YAML attr. Remove the ifa- prefix from all attributes
in addr-attrs and specify name-prefix instead.

This is a bit risky, hopefully there aren't many users out there.

Fixes: dfb0f7d9d979 ("doc/netlink: Add spec for rt addr messages")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/rt_addr.yaml | 41 ++++++++++++------------
 tools/testing/selftests/net/rtnetlink.py |  2 +-
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
index 1650dc3f091a..d032562d1240 100644
--- a/Documentation/netlink/specs/rt_addr.yaml
+++ b/Documentation/netlink/specs/rt_addr.yaml
@@ -78,45 +78,46 @@ protonum: 0
 attribute-sets:
   -
     name: addr-attrs
+    name-prefix: ifa-
     attributes:
       -
-        name: ifa-address
+        name: address
         type: binary
         display-hint: ipv4
       -
-        name: ifa-local
+        name: local
         type: binary
         display-hint: ipv4
       -
-        name: ifa-label
+        name: label
         type: string
       -
-        name: ifa-broadcast
+        name: broadcast
         type: binary
         display-hint: ipv4
       -
-        name: ifa-anycast
+        name: anycast
         type: binary
       -
-        name: ifa-cacheinfo
+        name: cacheinfo
         type: binary
-        struct: ifa-cacheinfo
+        struct: cacheinfo
       -
-        name: ifa-multicast
+        name: multicast
         type: binary
       -
-        name: ifa-flags
+        name: flags
         type: u32
         enum: ifa-flags
         enum-as-flags: true
       -
-        name: ifa-rt-priority
+        name: rt-priority
         type: u32
       -
-        name: ifa-target-netnsid
+        name: target-netnsid
         type: binary
       -
-        name: ifa-proto
+        name: proto
         type: u8
 
 
@@ -137,10 +138,10 @@ protonum: 0
             - ifa-prefixlen
             - ifa-scope
             - ifa-index
-            - ifa-address
-            - ifa-label
-            - ifa-local
-            - ifa-cacheinfo
+            - address
+            - label
+            - local
+            - cacheinfo
     -
       name: deladdr
       doc: Remove address
@@ -154,8 +155,8 @@ protonum: 0
             - ifa-prefixlen
             - ifa-scope
             - ifa-index
-            - ifa-address
-            - ifa-local
+            - address
+            - local
     -
       name: getaddr
       doc: Dump address information.
@@ -182,8 +183,8 @@ protonum: 0
         reply:
           value: 58
           attributes: &mcaddr-attrs
-            - ifa-multicast
-            - ifa-cacheinfo
+            - multicast
+            - cacheinfo
       dump:
         request:
           value: 58
diff --git a/tools/testing/selftests/net/rtnetlink.py b/tools/testing/selftests/net/rtnetlink.py
index 80950888800b..cadd7bc69241 100755
--- a/tools/testing/selftests/net/rtnetlink.py
+++ b/tools/testing/selftests/net/rtnetlink.py
@@ -15,7 +15,7 @@ IPV4_ALL_HOSTS_MULTICAST = b'\xe0\x00\x00\x01'
     addresses = rtnl.getmaddrs({"ifa-family": socket.AF_INET}, dump=True)
 
     all_host_multicasts = [
-        addr for addr in addresses if addr['ifa-multicast'] == IPV4_ALL_HOSTS_MULTICAST
+        addr for addr in addresses if addr['multicast'] == IPV4_ALL_HOSTS_MULTICAST
     ]
 
     ksft_ge(len(all_host_multicasts), 1,
-- 
2.49.0

Re: [PATCH net v2 1/3] netlink: specs: rt_addr: fix the spec format / schema failures

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> The spec is mis-formatted, schema validation says:
>
>   Failed validating 'type' in schema['properties']['operations']['properties']['list']['items']['properties']['dump']['properties']['request']['properties']['value']:
>     {'minimum': 0, 'type': 'integer'}
>
>   On instance['operations']['list'][3]['dump']['request']['value']:
>     '58 - ifa-family'
>
> The ifa-family clearly wants to be part of an attribute list.
>
> Reviewed-by: Yuyang Huang <yuyanghuang@google.com>
> Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

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

Re: [PATCH net v2 2/3] netlink: specs: rt_addr: fix get multi command name

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> Command names should match C defines, codegens may depend on it.
>
> Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  Documentation/netlink/specs/rt_addr.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
> index 3bc9b6f9087e..1650dc3f091a 100644
> --- a/Documentation/netlink/specs/rt_addr.yaml
> +++ b/Documentation/netlink/specs/rt_addr.yaml
> @@ -169,7 +169,7 @@ protonum: 0
>            value: 20
>            attributes: *ifaddr-all
>      -
> -      name: getmaddrs
> +      name: getmulticast
>        doc: Get / dump IPv4/IPv6 multicast addresses.
>        attribute-set: addr-attrs
>        fixed-header: ifaddrmsg

The op name is referenced in tools/testing/selftests/net/rtnetlink.py so
it needs to change as well:

     addresses = rtnl.getmaddrs({"ifa-family": socket.AF_INET}, dump=True)

Re: [PATCH net v2 3/3] netlink: specs: rt_addr: pull the ifa- prefix out of the names

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> YAML specs don't normally include the C prefix name in the name
> of the YAML attr. Remove the ifa- prefix from all attributes
> in addr-attrs and specify name-prefix instead.
>
> This is a bit risky, hopefully there aren't many users out there.
>
> Fixes: dfb0f7d9d979 ("doc/netlink: Add spec for rt addr messages")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  Documentation/netlink/specs/rt_addr.yaml | 41 ++++++++++++------------
>  tools/testing/selftests/net/rtnetlink.py |  2 +-
>  2 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
> index 1650dc3f091a..d032562d1240 100644
> --- a/Documentation/netlink/specs/rt_addr.yaml
> +++ b/Documentation/netlink/specs/rt_addr.yaml
> @@ -78,45 +78,46 @@ protonum: 0
>  attribute-sets:
>    -
>      name: addr-attrs
> +    name-prefix: ifa-
>      attributes:
>        -
> -        name: ifa-address
> +        name: address
>          type: binary
>          display-hint: ipv4
>        -
> -        name: ifa-local
> +        name: local
>          type: binary
>          display-hint: ipv4
>        -
> -        name: ifa-label
> +        name: label
>          type: string
>        -
> -        name: ifa-broadcast
> +        name: broadcast
>          type: binary
>          display-hint: ipv4
>        -
> -        name: ifa-anycast
> +        name: anycast
>          type: binary
>        -
> -        name: ifa-cacheinfo
> +        name: cacheinfo
>          type: binary
> -        struct: ifa-cacheinfo
> +        struct: cacheinfo

The struct is still called ifa-cacheinfo so this breaks:

Error decoding 'cacheinfo' from 'addr-attrs'
Traceback (most recent call last):
...
  File "/home/donaldh/net-next/tools/net/ynl/pyynl/lib/ynl.py", line 846, in _decode_struct
    members = self.consts[name].members
              ~~~~~~~~~~~^^^^^^
KeyError: 'cacheinfo'

Re: [PATCH net v2 1/3] netlink: specs: rt_addr: fix the spec format / schema failures

[Jacob Keller]

On 4/1/2025 6:02 PM, Jakub Kicinski wrote:
> The spec is mis-formatted, schema validation says:
> 
>   Failed validating 'type' in schema['properties']['operations']['properties']['list']['items']['properties']['dump']['properties']['request']['properties']['value']:
>     {'minimum': 0, 'type': 'integer'}
> 
>   On instance['operations']['list'][3]['dump']['request']['value']:
>     '58 - ifa-family'
> 
> The ifa-family clearly wants to be part of an attribute list.
> 
> Reviewed-by: Yuyang Huang <yuyanghuang@google.com>
> Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: donald.hunter@gmail.com
> CC: yuyanghuang@google.com
> CC: jacob.e.keller@intel.com
> ---
>  Documentation/netlink/specs/rt_addr.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
> index 5dd5469044c7..3bc9b6f9087e 100644
> --- a/Documentation/netlink/specs/rt_addr.yaml
> +++ b/Documentation/netlink/specs/rt_addr.yaml
> @@ -187,6 +187,7 @@ protonum: 0
>        dump:
>          request:
>            value: 58
> +          attributes:
>              - ifa-family
>          reply:
>            value: 58

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

Re: [PATCH net v2 2/3] netlink: specs: rt_addr: fix get multi command name

[Jacob Keller]

On 4/1/2025 6:02 PM, Jakub Kicinski wrote:
> Command names should match C defines, codegens may depend on it.
> 
> Fixes: 4f280376e531 ("selftests/net: Add selftest for IPv4 RTM_GETMULTICAST support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  Documentation/netlink/specs/rt_addr.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/netlink/specs/rt_addr.yaml b/Documentation/netlink/specs/rt_addr.yaml
> index 3bc9b6f9087e..1650dc3f091a 100644
> --- a/Documentation/netlink/specs/rt_addr.yaml
> +++ b/Documentation/netlink/specs/rt_addr.yaml
> @@ -169,7 +169,7 @@ protonum: 0
>            value: 20
>            attributes: *ifaddr-all
>      -
> -      name: getmaddrs
> +      name: getmulticast
>        doc: Get / dump IPv4/IPv6 multicast addresses.
>        attribute-set: addr-attrs
>        fixed-header: ifaddrmsg

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