[PATCH net-next 0/3] tools: ynl: Add sub-message and multi-attr encoding support

[PATCH net-next 0/3] tools: ynl: Add sub-message and multi-attr encoding support

[Alessandro Marcolini]

This patchset adds the encoding support for sub-message attributes and
multi-attr objects.

Patch 1 corrects a typo and the docstring for SpecSubMessageFormat
Patch 2 adds the multi-attr attribute to the entry object for taprio
Patch 3 updates the _add_attr method to support sub-message encoding

It is now possible to add a taprio qdisc using ynl:
# /tools/net/ynl/cli.py --spec Documentation/netlink/specs/tc.yaml --do newqdisc --create --json '{"family":1, "ifindex":4, "handle":65536, "parent":4294967295, "info":0, "kind":"taprio", "stab":{"base":"000000000000001f00000000000000000000000000000000"}, "options":{"priomap":"03010101010101010101010101010101010001000100020000000000000000000000000000000000000000000000000000000100020003000000000000000000000000000000000000000000000000000000", "sched-clockid":11, "sched-entry-list":[{"entry":{"index":0, "cmd":0, "gate-mask":1, "interval":300000}}, {"entry":{"index":1, "cmd":0, "gate-mask":2, "interval":300000}}, {"entry":{"index":2, "cmd":0, "gate-mask":4, "interval":400000}}], "sched-base-time":1528743495910289987, "flags": 1}}'

Alessandro Marcolini (3):
  tools: ynl: correct typo and docstring
  doc: netlink: specs: tc: add multi-attr to tc-taprio-sched-entry
  tools: ynl: add encoding support for 'sub-message' to ynl

 Documentation/netlink/specs/tc.yaml |  3 +-
 tools/net/ynl/lib/nlspec.py         |  7 ++--
 tools/net/ynl/lib/ynl.py            | 54 +++++++++++++++++++++++++----
 3 files changed, 53 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH net-next 1/3] tools: ynl: correct typo and docstring

[Alessandro Marcolini]

Correct typo in SpecAttr docstring. Changed SpecSubMessageFormat
docstring.

Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
---
 tools/net/ynl/lib/nlspec.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index 44f13e383e8a..f8feae363970 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -144,7 +144,7 @@ class SpecEnumSet(SpecElement):
 
 
 class SpecAttr(SpecElement):
-    """ Single Netlink atttribute type
+    """ Single Netlink attribute type
 
     Represents a single attribute type within an attr space.
 
@@ -306,10 +306,9 @@ class SpecSubMessage(SpecElement):
 
 
 class SpecSubMessageFormat(SpecElement):
-    """ Netlink sub-message definition
+    """ Netlink sub-message format definition
 
-    Represents a set of sub-message formats for polymorphic nlattrs
-    that contain type-specific sub messages.
+    Represents a single format for a sub-message.
 
     Attributes:
         value         attribute value to match against type selector
-- 
2.43.0

[PATCH net-next 2/3] doc: netlink: specs: tc: add multi-attr to tc-taprio-sched-entry

[Alessandro Marcolini]

Add multi-attr attribute to tc-taprio-sched-entry to specify multiple
entries.
Also remove the TODO that will be fixed by the next commit.

Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
---
 Documentation/netlink/specs/tc.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
index 4346fa402fc9..5e520d3125b6 100644
--- a/Documentation/netlink/specs/tc.yaml
+++ b/Documentation/netlink/specs/tc.yaml
@@ -1573,6 +1573,7 @@ attribute-sets:
         name: entry
         type: nest
         nested-attributes: tc-taprio-sched-entry
+        multi-attr: true
   -
     name: tc-taprio-sched-entry
     attributes:
@@ -1667,7 +1668,7 @@ attribute-sets:
         type: binary
       -
         name: app
-        type: binary # TODO sub-message needs 2+ level deep lookup
+        type: binary
         sub-message: tca-stats-app-msg
         selector: kind
       -
-- 
2.43.0

[PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Alessandro Marcolini]

Add encoding support for 'sub-message' attribute and for resolving
sub-message selectors at different nesting level from the key
attribute.

Also, add encoding support for multi-attr attributes.

Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 54 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 1e10512b2117..f8c56944f7e7 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
         self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
                              mcast_id)
 
-    def _add_attr(self, space, name, value):
+    def _add_attr(self, space, name, value, vals):
         try:
             attr = self.attr_sets[space][name]
         except KeyError:
@@ -458,8 +458,13 @@ class YnlFamily(SpecFamily):
         if attr["type"] == 'nest':
             nl_type |= Netlink.NLA_F_NESTED
             attr_payload = b''
-            for subname, subvalue in value.items():
-                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
+            # Check if it's a list of values (i.e. it contains multi-attr elements)
+            for subname, subvalue in (
+                ((k, v) for item in value for k, v in item.items())
+                if isinstance(value, list)
+                else value.items()
+            ):
+                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
         elif attr["type"] == 'flag':
             attr_payload = b''
         elif attr["type"] == 'string':
@@ -481,6 +486,12 @@ class YnlFamily(SpecFamily):
             attr_payload = format.pack(int(value))
         elif attr['type'] in "bitfield32":
             attr_payload = struct.pack("II", int(value["value"]), int(value["selector"]))
+        elif attr['type'] == "sub-message":
+            spec = self._resolve_selector(attr, vals)
+            attr_spec = spec["attribute-set"]
+            attr_payload = b''
+            for subname, subvalue in value.items():
+                attr_payload += self._add_attr(attr_spec, subname, subvalue, vals)
         else:
             raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
 
@@ -555,9 +566,40 @@ class YnlFamily(SpecFamily):
         sub_msg_spec = self.sub_msgs[sub_msg]
 
         selector = attr_spec.selector
-        if selector not in vals:
+
+        def _find_attr_path(attr, vals, path=None):
+            if path is None:
+                path = []
+            if isinstance(vals, dict):
+                if attr in vals:
+                    return path
+                for k, v in vals.items():
+                    result = _find_attr_path(attr, v, path + [k])
+                    if result is not None:
+                        return result
+            elif isinstance(vals, list):
+                for idx, v in enumerate(vals):
+                    result = _find_attr_path(attr, v, path + [idx])
+                    if result is not None:
+                        return result
+            return None
+
+        def _find_selector_val(sel, vals, path):
+            while path != []:
+                v = vals.copy()
+                for step in path:
+                    v = v[step]
+                if sel in v:
+                    return v[sel]
+                path.pop()
+            return vals[sel] if sel in vals else None
+
+        attr_path = _find_attr_path(attr_spec.name, vals)
+        value = _find_selector_val(selector, vals, attr_path)
+
+        if value is None:
             raise Exception(f"There is no value for {selector} to resolve '{attr_spec.name}'")
-        value = vals[selector]
+
         if value not in sub_msg_spec.formats:
             raise Exception(f"No message format for '{value}' in sub-message spec '{sub_msg}'")
 
@@ -772,7 +814,7 @@ class YnlFamily(SpecFamily):
                     format = NlAttr.get_format(m.type, m.byte_order)
                     msg += format.pack(value)
         for name, value in vals.items():
-            msg += self._add_attr(op.attr_set.name, name, value)
+            msg += self._add_attr(op.attr_set.name, name, value, vals)
         msg = _genl_msg_finalize(msg)
 
         self.sock.send(msg, 0)
-- 
2.43.0

Re: [PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Breno Leitao]

On Mon, Jan 22, 2024 at 08:19:41PM +0100, Alessandro Marcolini wrote:
> Add encoding support for 'sub-message' attribute and for resolving
> sub-message selectors at different nesting level from the key
> attribute.
> 
> Also, add encoding support for multi-attr attributes.
> 
> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
> ---
>  tools/net/ynl/lib/ynl.py | 54 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 1e10512b2117..f8c56944f7e7 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
>          self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
>                               mcast_id)
>  
> -    def _add_attr(self, space, name, value):
> +    def _add_attr(self, space, name, value, vals):
>          try:
>              attr = self.attr_sets[space][name]
>          except KeyError:
> @@ -458,8 +458,13 @@ class YnlFamily(SpecFamily):
>          if attr["type"] == 'nest':
>              nl_type |= Netlink.NLA_F_NESTED
>              attr_payload = b''
> -            for subname, subvalue in value.items():
> -                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
> +            # Check if it's a list of values (i.e. it contains multi-attr elements)
> +            for subname, subvalue in (
> +                ((k, v) for item in value for k, v in item.items())
> +                if isinstance(value, list)
> +                else value.items()
> +            ):

This is a bit hard to read.

Is it possible to make it a bit easier to read?

Re: [PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Alessandro Marcolini]

On 1/22/24 20:46, Breno Leitao wrote:
> This is a bit hard to read.
>
> Is it possible to make it a bit easier to read?

Hi! Yes, an easier to read version could be:

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index f8c56944f7e7..d837e769c5bf 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -459,12 +459,13 @@ class YnlFamily(SpecFamily):
             nl_type |= Netlink.NLA_F_NESTED
             attr_payload = b''
             # Check if it's a list of values (i.e. it contains multi-attr elements)
-            for subname, subvalue in (
-                ((k, v) for item in value for k, v in item.items())
-                if isinstance(value, list)
-                else value.items()
-            ):
-                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
+            if isinstance(value, list):
+                for item in value:
+                    for subname, subvalue in item.items():
+                        attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
+            else:
+                for subname, subvalue in value.items():
+                    attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
         elif attr["type"] == 'flag':
             attr_payload = b''
         elif attr["type"] == 'string':

Re: [PATCH net-next 0/3] tools: ynl: Add sub-message and multi-attr encoding support

[Donald Hunter]

> On 22 Jan 2024, at 20:19, Alessandro Marcolini <alessandromarcolini99@gmail.com> wrote:
> 
> This patchset adds the encoding support for sub-message attributes and
> multi-attr objects.

I have a longer patchset that covers this plus some refactoring for nested struct definitions and a lot of addtions to the tc spec. Do you mind if I post it and we review to see if there is anything from your patchset that is missing from mine?

Thanks,
Donald

> Patch 1 corrects a typo and the docstring for SpecSubMessageFormat
> Patch 2 adds the multi-attr attribute to the entry object for taprio
> Patch 3 updates the _add_attr method to support sub-message encoding
> 
> It is now possible to add a taprio qdisc using ynl:
> # /tools/net/ynl/cli.py --spec Documentation/netlink/specs/tc.yaml --do newqdisc --create --json '{"family":1, "ifindex":4, "handle":65536, "parent":4294967295, "info":0, "kind":"taprio", "stab":{"base":"000000000000001f00000000000000000000000000000000"}, "options":{"priomap":"03010101010101010101010101010101010001000100020000000000000000000000000000000000000000000000000000000100020003000000000000000000000000000000000000000000000000000000", "sched-clockid":11, "sched-entry-list":[{"entry":{"index":0, "cmd":0, "gate-mask":1, "interval":300000}}, {"entry":{"index":1, "cmd":0, "gate-mask":2, "interval":300000}}, {"entry":{"index":2, "cmd":0, "gate-mask":4, "interval":400000}}], "sched-base-time":1528743495910289987, "flags": 1}}'
> 
> Alessandro Marcolini (3):
>  tools: ynl: correct typo and docstring
>  doc: netlink: specs: tc: add multi-attr to tc-taprio-sched-entry
>  tools: ynl: add encoding support for 'sub-message' to ynl
> 
> Documentation/netlink/specs/tc.yaml |  3 +-
> tools/net/ynl/lib/nlspec.py         |  7 ++--
> tools/net/ynl/lib/ynl.py            | 54 +++++++++++++++++++++++++----
> 3 files changed, 53 insertions(+), 11 deletions(-)
> 
> --
> 2.43.0
> 

Re: [PATCH net-next 0/3] tools: ynl: Add sub-message and multi-attr encoding support

[Alessandro Marcolini]

On 1/22/24 22:43, Donald Hunter wrote:
> I have a longer patchset that covers this plus some refactoring for nested struct definitions and a lot of addtions to the tc spec. Do you mind if I post it and we review to see if there is anything from your patchset that is missing from mine?
>
> Thanks,
> Donald

Hi Donald,

Yes for sure, post it and we will review it together.

Re: [PATCH net-next 1/3] tools: ynl: correct typo and docstring

[Donald Hunter]

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> Correct typo in SpecAttr docstring. Changed SpecSubMessageFormat
> docstring.

These docstring updates lgtm.

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

> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
> ---
>  tools/net/ynl/lib/nlspec.py | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
> index 44f13e383e8a..f8feae363970 100644
> --- a/tools/net/ynl/lib/nlspec.py
> +++ b/tools/net/ynl/lib/nlspec.py
> @@ -144,7 +144,7 @@ class SpecEnumSet(SpecElement):
>  
>  
>  class SpecAttr(SpecElement):
> -    """ Single Netlink atttribute type
> +    """ Single Netlink attribute type
>  
>      Represents a single attribute type within an attr space.
>  
> @@ -306,10 +306,9 @@ class SpecSubMessage(SpecElement):
>  
>  
>  class SpecSubMessageFormat(SpecElement):
> -    """ Netlink sub-message definition
> +    """ Netlink sub-message format definition
>  
> -    Represents a set of sub-message formats for polymorphic nlattrs
> -    that contain type-specific sub messages.
> +    Represents a single format for a sub-message.
>  
>      Attributes:
>          value         attribute value to match against type selector

Re: [PATCH net-next 2/3] doc: netlink: specs: tc: add multi-attr to tc-taprio-sched-entry

[Donald Hunter]

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> Add multi-attr attribute to tc-taprio-sched-entry to specify multiple
> entries.
> Also remove the TODO that will be fixed by the next commit.
>
> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
> ---
>  Documentation/netlink/specs/tc.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
> index 4346fa402fc9..5e520d3125b6 100644
> --- a/Documentation/netlink/specs/tc.yaml
> +++ b/Documentation/netlink/specs/tc.yaml
> @@ -1573,6 +1573,7 @@ attribute-sets:
>          name: entry
>          type: nest
>          nested-attributes: tc-taprio-sched-entry
> +        multi-attr: true

Good catch for the mulit-attr. I don't have this in my tc patch.

>    -
>      name: tc-taprio-sched-entry
>      attributes:
> @@ -1667,7 +1668,7 @@ attribute-sets:
>          type: binary
>        -
>          name: app
> -        type: binary # TODO sub-message needs 2+ level deep lookup
> +        type: binary
>          sub-message: tca-stats-app-msg
>          selector: kind
>        -

I have this in my tc patch. It should be 'type: sub-message'.

https://lore.kernel.org/netdev/20240123160538.172-13-donald.hunter@gmail.com/T/#u

Re: [PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Donald Hunter]

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> Add encoding support for 'sub-message' attribute and for resolving
> sub-message selectors at different nesting level from the key
> attribute.

I think the relevant patches in my series are:

https://lore.kernel.org/netdev/20240123160538.172-3-donald.hunter@gmail.com/T/#u
https://lore.kernel.org/netdev/20240123160538.172-5-donald.hunter@gmail.com/T/#u

>
> Also, add encoding support for multi-attr attributes.

This would be better as a separate patch since it is unrelated to the
other changes.

> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
> ---
>  tools/net/ynl/lib/ynl.py | 54 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 1e10512b2117..f8c56944f7e7 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
>          self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
>                               mcast_id)
>  
> -    def _add_attr(self, space, name, value):
> +    def _add_attr(self, space, name, value, vals):
>          try:
>              attr = self.attr_sets[space][name]
>          except KeyError:
> @@ -458,8 +458,13 @@ class YnlFamily(SpecFamily):
>          if attr["type"] == 'nest':
>              nl_type |= Netlink.NLA_F_NESTED
>              attr_payload = b''
> -            for subname, subvalue in value.items():
> -                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
> +            # Check if it's a list of values (i.e. it contains multi-attr elements)
> +            for subname, subvalue in (
> +                ((k, v) for item in value for k, v in item.items())
> +                if isinstance(value, list)
> +                else value.items()
> +            ):
> +                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)

Should really check whether multi-attr is true in the spec before
processing the json input as a list of values.

>          elif attr["type"] == 'flag':
>              attr_payload = b''
>          elif attr["type"] == 'string':
> @@ -481,6 +486,12 @@ class YnlFamily(SpecFamily):
>              attr_payload = format.pack(int(value))
>          elif attr['type'] in "bitfield32":
>              attr_payload = struct.pack("II", int(value["value"]), int(value["selector"]))
> +        elif attr['type'] == "sub-message":
> +            spec = self._resolve_selector(attr, vals)
> +            attr_spec = spec["attribute-set"]
> +            attr_payload = b''
> +            for subname, subvalue in value.items():
> +                attr_payload += self._add_attr(attr_spec, subname, subvalue, vals)
>          else:
>              raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
>  
> @@ -555,9 +566,40 @@ class YnlFamily(SpecFamily):
>          sub_msg_spec = self.sub_msgs[sub_msg]
>  
>          selector = attr_spec.selector
> -        if selector not in vals:
> +
> +        def _find_attr_path(attr, vals, path=None):
> +            if path is None:
> +                path = []
> +            if isinstance(vals, dict):
> +                if attr in vals:
> +                    return path
> +                for k, v in vals.items():
> +                    result = _find_attr_path(attr, v, path + [k])
> +                    if result is not None:
> +                        return result
> +            elif isinstance(vals, list):
> +                for idx, v in enumerate(vals):
> +                    result = _find_attr_path(attr, v, path + [idx])
> +                    if result is not None:
> +                        return result
> +            return None
> +
> +        def _find_selector_val(sel, vals, path):
> +            while path != []:
> +                v = vals.copy()
> +                for step in path:
> +                    v = v[step]
> +                if sel in v:
> +                    return v[sel]
> +                path.pop()
> +            return vals[sel] if sel in vals else None
> +
> +        attr_path = _find_attr_path(attr_spec.name, vals)
> +        value = _find_selector_val(selector, vals, attr_path)
> +
> +        if value is None:
>              raise Exception(f"There is no value for {selector} to resolve '{attr_spec.name}'")
> -        value = vals[selector]
> +
>          if value not in sub_msg_spec.formats:
>              raise Exception(f"No message format for '{value}' in sub-message spec '{sub_msg}'")
>  
> @@ -772,7 +814,7 @@ class YnlFamily(SpecFamily):
>                      format = NlAttr.get_format(m.type, m.byte_order)
>                      msg += format.pack(value)
>          for name, value in vals.items():
> -            msg += self._add_attr(op.attr_set.name, name, value)
> +            msg += self._add_attr(op.attr_set.name, name, value, vals)
>          msg = _genl_msg_finalize(msg)
>  
>          self.sock.send(msg, 0)

Re: [PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Alessandro Marcolini]

On 1/23/24 17:44, Donald Hunter wrote:
> Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:
>
>> Add encoding support for 'sub-message' attribute and for resolving
>> sub-message selectors at different nesting level from the key
>> attribute.
> I think the relevant patches in my series are:
>
> https://lore.kernel.org/netdev/20240123160538.172-3-donald.hunter@gmail.com/T/#u
> https://lore.kernel.org/netdev/20240123160538.172-5-donald.hunter@gmail.com/T/#u
I really like your idea of using ChainMap, I think it's better than mine and more concise.
>> Also, add encoding support for multi-attr attributes.
> This would be better as a separate patch since it is unrelated to the
> other changes.
You mean as a separate patch in this patchset or as an entirely new patch?
>> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
>> ---
>>  tools/net/ynl/lib/ynl.py | 54 +++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 48 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
>> index 1e10512b2117..f8c56944f7e7 100644
>> --- a/tools/net/ynl/lib/ynl.py
>> +++ b/tools/net/ynl/lib/ynl.py
>> @@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
>>          self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
>>                               mcast_id)
>>  
>> -    def _add_attr(self, space, name, value):
>> +    def _add_attr(self, space, name, value, vals):
>>          try:
>>              attr = self.attr_sets[space][name]
>>          except KeyError:
>> @@ -458,8 +458,13 @@ class YnlFamily(SpecFamily):
>>          if attr["type"] == 'nest':
>>              nl_type |= Netlink.NLA_F_NESTED
>>              attr_payload = b''
>> -            for subname, subvalue in value.items():
>> -                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
>> +            # Check if it's a list of values (i.e. it contains multi-attr elements)
>> +            for subname, subvalue in (
>> +                ((k, v) for item in value for k, v in item.items())
>> +                if isinstance(value, list)
>> +                else value.items()
>> +            ):
>> +                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
> Should really check whether multi-attr is true in the spec before
> processing the json input as a list of values.
Yes, you're right. Maybe I could resend this on top of your changes, what do you think?

Re: [PATCH net-next 3/3] tools: ynl: add encoding support for 'sub-message' to ynl

[Donald Hunter]

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> On 1/23/24 17:44, Donald Hunter wrote:
>> Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:
>>
>>> Add encoding support for 'sub-message' attribute and for resolving
>>> sub-message selectors at different nesting level from the key
>>> attribute.
>> I think the relevant patches in my series are:
>>
>> https://lore.kernel.org/netdev/20240123160538.172-3-donald.hunter@gmail.com/T/#u
>> https://lore.kernel.org/netdev/20240123160538.172-5-donald.hunter@gmail.com/T/#u
> I really like your idea of using ChainMap, I think it's better than mine and more concise.
>>> Also, add encoding support for multi-attr attributes.
>> This would be better as a separate patch since it is unrelated to the
>> other changes.
> You mean as a separate patch in this patchset or as an entirely new patch?

I was thinking as a separate patch in this patchset, yes.

>>> Signed-off-by: Alessandro Marcolini <alessandromarcolini99@gmail.com>
>>> ---
>>>  tools/net/ynl/lib/ynl.py | 54 +++++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 48 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
>>> index 1e10512b2117..f8c56944f7e7 100644
>>> --- a/tools/net/ynl/lib/ynl.py
>>> +++ b/tools/net/ynl/lib/ynl.py
>>> @@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
>>>          self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
>>>                               mcast_id)
>>>  
>>> -    def _add_attr(self, space, name, value):
>>> +    def _add_attr(self, space, name, value, vals):
>>>          try:
>>>              attr = self.attr_sets[space][name]
>>>          except KeyError:
>>> @@ -458,8 +458,13 @@ class YnlFamily(SpecFamily):
>>>          if attr["type"] == 'nest':
>>>              nl_type |= Netlink.NLA_F_NESTED
>>>              attr_payload = b''
>>> -            for subname, subvalue in value.items():
>>> -                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
>>> +            # Check if it's a list of values (i.e. it contains multi-attr elements)
>>> +            for subname, subvalue in (
>>> +                ((k, v) for item in value for k, v in item.items())
>>> +                if isinstance(value, list)
>>> +                else value.items()
>>> +            ):
>>> +                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue, vals)
>> Should really check whether multi-attr is true in the spec before
>> processing the json input as a list of values.
> Yes, you're right. Maybe I could resend this on top of your changes, what do you think?

Yes, that would be great. Thanks!