Re: [PATCH v2 net-next 3/3] tools: ynl: add support for encoding multi-attr

[Jakub Kicinski <kuba@kernel.org>]

On Thu,  1 Feb 2024 16:12:51 +0100 Alessandro Marcolini wrote:
> Multi-attr elements could not be encoded because of missing logic in the
> ynl code. Enable encoding of these attributes by checking if the nest
> attribute in the spec contains multi-attr attributes and if the value to
> be processed is a list.
> 
> This has been tested both with the taprio and ets qdisc which contain
> this kind of attributes.
> 
> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
> ---
>  tools/net/ynl/lib/ynl.py | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 0f4193cc2e3b..e4e6a3fe0f23 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -447,10 +447,19 @@ class YnlFamily(SpecFamily):
>          if attr["type"] == 'nest':
>              nl_type |= Netlink.NLA_F_NESTED
>              attr_payload = b''
> -            sub_attrs = SpaceAttrs(self.attr_sets[space], value, search_attrs)
> -            for subname, subvalue in value.items():
> -                attr_payload += self._add_attr(attr['nested-attributes'],
> -                                               subname, subvalue, sub_attrs)
> +            nested_attrs = self.attr_sets[attr["nested-attributes"]]
> +
> +            if any(v.is_multi for _,v in nested_attrs.items()) and isinstance(value, list):

I think you're trying to handle this at the wrong level. The main
message can also contain multi-attr, so looping inside nests won't
cut it.

Early in the function check if attr.is_multi and isinstance(value,
list), and if so do:

	attr_payload = b''
	for subvalue in value:
		attr_payload += self._add_attr(space, name, subvalue,
					       search_attrs) 
	return attr_payload

IOW all you need to do is recursively call _add_attr() with the
subvalues stripped. You don't have to descend into a nest.

> +                for item in value:
> +                    sub_attrs = SpaceAttrs(self.attr_sets[space], item, search_attrs)
> +                    for subname, subvalue in item.items():
> +                        attr_payload += self._add_attr(attr['nested-attributes'],
> +                                                       subname, subvalue, sub_attrs)
> +            else:
> +                sub_attrs = SpaceAttrs(self.attr_sets[space], value, search_attrs)
> +                for subname, subvalue in value.items():
> +                    attr_payload += self._add_attr(attr['nested-attributes'],
> +                                                   subname, subvalue, sub_attrs)
>          elif attr["type"] == 'flag':
>              attr_payload = b''
>          elif attr["type"] == 'string':