Re: [PATCH net-next] tools: ynl-gen-c: keep the type values of a nest in spec order

[Nicolai Buchwitz <nb@tipi-net.de>]

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