diff mbox series

[v3,net-next,3/3] tools: ynl: add support for encoding multi-attr

Message ID 9399f6f7bda6c845194419952dfbcf0d42142652.1706882196.git.alessandromarcolini99@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add support for encoding multi-attr to ynl | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 1 this patch: 0
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alessandro Marcolini Feb. 2, 2024, 2 p.m. UTC
Multi-attr elements could not be encoded because of missing logic in the
ynl code. Enable encoding of these attributes by checking if the
attribute is a multi-attr 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 | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Donald Hunter Feb. 2, 2024, 2:34 p.m. UTC | #1
Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> Multi-attr elements could not be encoded because of missing logic in the
> ynl code. Enable encoding of these attributes by checking if the
> attribute is a multi-attr 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>

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

> ---
>  tools/net/ynl/lib/ynl.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 0f4193cc2e3b..d5779d023b10 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -444,6 +444,11 @@ class YnlFamily(SpecFamily):
>          except KeyError:
>              raise Exception(f"Space '{space}' has no attribute '{name}'")
>          nl_type = attr.value
> +        if attr.is_multi and isinstance(value, list):
> +            attr_payload = b''
> +            for subvalue in value:
> +                attr_payload += self._add_attr(space, name, subvalue, search_attrs)
> +            return attr_payload
>          if attr["type"] == 'nest':
>              nl_type |= Netlink.NLA_F_NESTED
>              attr_payload = b''
Jakub Kicinski Feb. 3, 2024, 2:17 a.m. UTC | #2
On Fri,  2 Feb 2024 15:00:05 +0100 Alessandro Marcolini wrote:
>          nl_type = attr.value
> +        if attr.is_multi and isinstance(value, list):
> +            attr_payload = b''
> +            for subvalue in value:
> +                attr_payload += self._add_attr(space, name, subvalue, search_attrs)
> +            return attr_payload
>          if attr["type"] == 'nest':

nit: would you mind adding an empty line before and after the new
block? It's logically separate from the other code, sort of an
alternative to the "actual" handling, as well as finding the attr set.

Also agreed on adding an example to the cover letter (either one or
both).

With that feel free to add:

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

on all 3 patches in v4.
Alessandro Marcolini Feb. 3, 2024, noon UTC | #3
On 2/3/24 03:17, Jakub Kicinski wrote:
> On Fri,  2 Feb 2024 15:00:05 +0100 Alessandro Marcolini wrote:
>>          nl_type = attr.value
>> +        if attr.is_multi and isinstance(value, list):
>> +            attr_payload = b''
>> +            for subvalue in value:
>> +                attr_payload += self._add_attr(space, name, subvalue, search_attrs)
>> +            return attr_payload
>>          if attr["type"] == 'nest':
> nit: would you mind adding an empty line before and after the new
> block? It's logically separate from the other code, sort of an
> alternative to the "actual" handling, as well as finding the attr set.

This makes sense.

> Also agreed on adding an example to the cover letter (either one or
> both).
>
> With that feel free to add:
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
> on all 3 patches in v4.
Perfect, I'll post the v4 soon with both the examples. Thanks!
diff mbox series

Patch

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 0f4193cc2e3b..d5779d023b10 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -444,6 +444,11 @@  class YnlFamily(SpecFamily):
         except KeyError:
             raise Exception(f"Space '{space}' has no attribute '{name}'")
         nl_type = attr.value
+        if attr.is_multi and isinstance(value, list):
+            attr_payload = b''
+            for subvalue in value:
+                attr_payload += self._add_attr(space, name, subvalue, search_attrs)
+            return attr_payload
         if attr["type"] == 'nest':
             nl_type |= Netlink.NLA_F_NESTED
             attr_payload = b''