Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order
@ 2026-09-24 22:32 Jakub Kicinski
  2026-09-28  8:52 ` Nicolai Buchwitz
  2026-09-29  1:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-09-24 22:32 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
	donald.hunter

Family._load_nested_set_nest() collects the type-value names of a
nest-type-value attribute into a set and Struct.set_inherited() renders
them sorted, while TypeNestTypeValue._attr_get() passes them to the
parse helper in the order the spec lists them.  For nlctrl's

  type-value: [policy-id, attr-id]

that makes the generated helper take (attr_id, policy_id) and the caller
pass (policy_id, attr_id).  Both are __u32 so nothing complains, and
every C YNL user of the nlctrl policy dump gets the two swapped.

Two attributes nesting the same attribute set with the type values in
a different order now fail set_inherited() instead of comparing equal.
No spec does that today, but better safe than sorry.

Nesting an attribute set which is also used as a sub-message gets its own
check: it used to be caught by the list-versus-set type mismatch, which
the ordering fix removes, and without it the generator emits a call
to the sub-message parser with the selector argument missing.
Again, no spec does this today.

nlctrl is the only spec affected, discovered by an LLM scan of the spec.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Doesn't affect kernel code, just tooling, so not a fix.

CC: donald.hunter@gmail.com
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 2b3483db1b60..15c79849c609 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -986,7 +986,8 @@ from lib import SpecSubMessage
         self.family = family
         self.space_name = space_name
         self.attr_set = family.attr_sets[space_name]
-        # Use list to catch comparisons with empty sets
+        # Stored by reference, _load_nested_set_nest() fills the list in
+        # after constructing us.
         self._inherited = inherited if inherited is not None else []
         self.inherited = []
         self.fixed_header = None
@@ -1038,9 +1039,11 @@ from lib import SpecSubMessage
         return self.attr_list
 
     def set_inherited(self, new_inherited):
+        if self.submsg:
+            raise Exception("Nesting a sub-message as an attribute set not supported")
         if self._inherited != new_inherited:
-            raise Exception("Inheriting different members not supported")
-        self.inherited = [c_lower(x) for x in sorted(self._inherited)]
+            raise Exception("Inheriting different members, or a different order, not supported")
+        self.inherited = [c_lower(x) for x in self._inherited]
 
     def external_selectors(self):
         sels = []
@@ -1394,7 +1397,7 @@ from lib import SpecSubMessage
                 pns_key_list.append(name)
 
     def _load_nested_set_nest(self, spec):
-        inherit = set()
+        inherit = []
         nested = spec['nested-attributes']
         if nested not in self.root_sets:
             if nested not in self.pure_nested_structs:
@@ -1407,9 +1410,9 @@ from lib import SpecSubMessage
         if 'type-value' in spec:
             if nested in self.root_sets:
                 raise Exception("Inheriting members to a space used as root not supported")
-            inherit.update(set(spec['type-value']))
+            inherit.extend(spec['type-value'])
         elif spec['type'] == 'indexed-array':
-            inherit.add('idx')
+            inherit.append('idx')
         self.pure_nested_structs[nested].set_inherited(inherit)
 
         return nested
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order
  2026-09-24 22:32 [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order Jakub Kicinski
@ 2026-09-28  8:52 ` Nicolai Buchwitz
  2026-09-29  1:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Nicolai Buchwitz @ 2026-09-28  8:52 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	donald.hunter

On 25.9.2026 00:32, Jakub Kicinski wrote:
> Family._load_nested_set_nest() collects the type-value names of a
> nest-type-value attribute into a set and Struct.set_inherited() renders
> them sorted, while TypeNestTypeValue._attr_get() passes them to the
> parse helper in the order the spec lists them.  For nlctrl's
> 
>   type-value: [policy-id, attr-id]
> 
> that makes the generated helper take (attr_id, policy_id) and the 
> caller
> pass (policy_id, attr_id).  Both are __u32 so nothing complains, and
> every C YNL user of the nlctrl policy dump gets the two swapped.
> 
> Two attributes nesting the same attribute set with the type values in
> a different order now fail set_inherited() instead of comparing equal.
> No spec does that today, but better safe than sorry.
> 
> Nesting an attribute set which is also used as a sub-message gets its 
> own
> check: it used to be caught by the list-versus-set type mismatch, which
> the ordering fix removes, and without it the generator emits a call
> to the sub-message parser with the selector argument missing.
> Again, no spec does this today.
> 
> nlctrl is the only spec affected, discovered by an LLM scan of the 
> spec.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> Doesn't affect kernel code, just tooling, so not a fix.
> 
> CC: donald.hunter@gmail.com
> ---
>  tools/net/ynl/pyynl/ynl_gen_c.py | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py 
> b/tools/net/ynl/pyynl/ynl_gen_c.py
> index 2b3483db1b60..15c79849c609 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -986,7 +986,8 @@ from lib import SpecSubMessage
>          self.family = family
>          self.space_name = space_name
>          self.attr_set = family.attr_sets[space_name]
> -        # Use list to catch comparisons with empty sets
> +        # Stored by reference, _load_nested_set_nest() fills the list 
> in
> +        # after constructing us.
>          self._inherited = inherited if inherited is not None else []
>          self.inherited = []
>          self.fixed_header = None
> @@ -1038,9 +1039,11 @@ from lib import SpecSubMessage
>          return self.attr_list
> 
>      def set_inherited(self, new_inherited):
> +        if self.submsg:
> +            raise Exception("Nesting a sub-message as an attribute set 
> not supported")
>          if self._inherited != new_inherited:
> -            raise Exception("Inheriting different members not 
> supported")
> -        self.inherited = [c_lower(x) for x in sorted(self._inherited)]
> +            raise Exception("Inheriting different members, or a 
> different order, not supported")
> +        self.inherited = [c_lower(x) for x in self._inherited]
> 
>      def external_selectors(self):
>          sels = []
> @@ -1394,7 +1397,7 @@ from lib import SpecSubMessage
>                  pns_key_list.append(name)
> 
>      def _load_nested_set_nest(self, spec):
> -        inherit = set()
> +        inherit = []
>          nested = spec['nested-attributes']
>          if nested not in self.root_sets:
>              if nested not in self.pure_nested_structs:
> @@ -1407,9 +1410,9 @@ from lib import SpecSubMessage
>          if 'type-value' in spec:
>              if nested in self.root_sets:
>                  raise Exception("Inheriting members to a space used as 
> root not supported")
> -            inherit.update(set(spec['type-value']))
> +            inherit.extend(spec['type-value'])
>          elif spec['type'] == 'indexed-array':
> -            inherit.add('idx')
> +            inherit.append('idx')
>          self.pure_nested_structs[nested].set_inherited(inherit)
> 
>          return nested

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order
  2026-09-24 22:32 [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order Jakub Kicinski
  2026-09-28  8:52 ` Nicolai Buchwitz
@ 2026-09-29  1:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-29  1:30 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	donald.hunter

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 24 Sep 2026 15:32:45 -0700 you wrote:
> Family._load_nested_set_nest() collects the type-value names of a
> nest-type-value attribute into a set and Struct.set_inherited() renders
> them sorted, while TypeNestTypeValue._attr_get() passes them to the
> parse helper in the order the spec lists them.  For nlctrl's
> 
>   type-value: [policy-id, attr-id]
> 
> [...]

Here is the summary with links:
  - [net-next] tools: ynl-gen-c: keep the type values of a nest in spec order
    https://git.kernel.org/netdev/net-next/c/c20d84c6dec8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-29  1:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 22:32 [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order Jakub Kicinski
2026-09-28  8:52 ` Nicolai Buchwitz
2026-09-29  1:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox