[PATCH net-next v4 0/2] tools: ynl-gen: fix parse multi-attr enum attribute

[PATCH net-next v4 0/2] tools: ynl-gen: fix parse multi-attr enum attribute

[Arkadiusz Kubalewski]

Fix the issues with parsing enums in ynl.py script.

v4->v5:
- fix indent issue in _decode_binary()

v3->v4:
- decoding 'enum-as-flag' starts from 0 instead of enum's start-value
- move back usage of _decode_enum() back to _decode_binary()
- fix typo in commit message

v2->v3:
- Restore i iterator helper for decoding 'enum-as-flag' attributes in
_decode_enum(..) function.
- Fix _decode_enum(..) usage in case of binary & struct type attributes
by moving it to as_struct() funtion while calling _decode_enum(..) for
individual struct members.

v1->v2:
Initially this was one patch, but review shown there is need to fix also
leftover issues with indexing in _decode_enum(..) function
("tools: ynl-gen: fix enum index in _decode_enum(..)").

*** BLURB HERE ***

Arkadiusz Kubalewski (2):
  tools: ynl-gen: fix enum index in _decode_enum(..)
  tools: ynl-gen: fix parse multi-attr enum attribute

 tools/net/ynl/lib/ynl.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
2.38.1

[PATCH net-next v5 1/2] tools: ynl-gen: fix enum index in _decode_enum(..)

[Arkadiusz Kubalewski]

Remove wrong index adjustment, which is leftover from adding
support for sparse enums.
enum.entries_by_val() function shall not subtract the start-value, as
it is indexed with real enum value.

Fixes: c311aaa74ca1 ("tools: ynl: fix enum-as-flags in the generic CLI")
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 tools/net/ynl/lib/ynl.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 1b3a36fbb1c3..027b1c0aecb4 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -420,8 +420,8 @@ class YnlFamily(SpecFamily):
     def _decode_enum(self, rsp, attr_spec):
         raw = rsp[attr_spec['name']]
         enum = self.consts[attr_spec['enum']]
-        i = attr_spec.get('value-start', 0)
         if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
+            i = 0
             value = set()
             while raw:
                 if raw & 1:
@@ -429,7 +429,7 @@ class YnlFamily(SpecFamily):
                 raw >>= 1
                 i += 1
         else:
-            value = enum.entries_by_val[raw - i].name
+            value = enum.entries_by_val[raw].name
         rsp[attr_spec['name']] = value
 
     def _decode_binary(self, attr, attr_spec):
-- 
2.38.1

Re: [PATCH net-next v5 1/2] tools: ynl-gen: fix enum index in _decode_enum(..)

[Donald Hunter]

Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> writes:

> Remove wrong index adjustment, which is leftover from adding
> support for sparse enums.
> enum.entries_by_val() function shall not subtract the start-value, as
> it is indexed with real enum value.
>
> Fixes: c311aaa74ca1 ("tools: ynl: fix enum-as-flags in the generic CLI")
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---
>  tools/net/ynl/lib/ynl.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 1b3a36fbb1c3..027b1c0aecb4 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -420,8 +420,8 @@ class YnlFamily(SpecFamily):
>      def _decode_enum(self, rsp, attr_spec):
>          raw = rsp[attr_spec['name']]
>          enum = self.consts[attr_spec['enum']]
> -        i = attr_spec.get('value-start', 0)
>          if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
> +            i = 0
>              value = set()
>              while raw:
>                  if raw & 1:
> @@ -429,7 +429,7 @@ class YnlFamily(SpecFamily):
>                  raw >>= 1
>                  i += 1
>          else:
> -            value = enum.entries_by_val[raw - i].name
> +            value = enum.entries_by_val[raw].name
>          rsp[attr_spec['name']] = value
>  
>      def _decode_binary(self, attr, attr_spec):

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

[PATCH net-next v5 2/2] tools: ynl-gen: fix parse multi-attr enum attribute

[Arkadiusz Kubalewski]

When attribute is enum type and marked as multi-attr, the netlink
respond is not parsed, fails with stack trace:
Traceback (most recent call last):
  File "/net-next/tools/net/ynl/./test.py", line 520, in <module>
    main()
  File "/net-next/tools/net/ynl/./test.py", line 488, in main
    dplls=dplls_get(282574471561216)
  File "/net-next/tools/net/ynl/./test.py", line 48, in dplls_get
    reply=act(args)
  File "/net-next/tools/net/ynl/./test.py", line 41, in act
    reply = ynl.dump(args.dump, attrs)
  File "/net-next/tools/net/ynl/lib/ynl.py", line 598, in dump
    return self._op(method, vals, dump=True)
  File "/net-next/tools/net/ynl/lib/ynl.py", line 584, in _op
    rsp_msg = self._decode(gm.raw_attrs, op.attr_set.name)
  File "/net-next/tools/net/ynl/lib/ynl.py", line 451, in _decode
    self._decode_enum(rsp, attr_spec)
  File "/net-next/tools/net/ynl/lib/ynl.py", line 408, in _decode_enum
    value = enum.entries_by_val[raw].name
TypeError: unhashable type: 'list'
error: 1

Redesign _decode_enum(..) to take a enum int value and translate
it to either a bitmask or enum name as expected.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 tools/net/ynl/lib/ynl.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 027b1c0aecb4..3ca28d4bcb18 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -417,8 +417,7 @@ class YnlFamily(SpecFamily):
         pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
         return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
 
-    def _decode_enum(self, rsp, attr_spec):
-        raw = rsp[attr_spec['name']]
+    def _decode_enum(self, raw, attr_spec):
         enum = self.consts[attr_spec['enum']]
         if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
             i = 0
@@ -430,7 +429,7 @@ class YnlFamily(SpecFamily):
                 i += 1
         else:
             value = enum.entries_by_val[raw].name
-        rsp[attr_spec['name']] = value
+        return value
 
     def _decode_binary(self, attr, attr_spec):
         if attr_spec.struct_name:
@@ -438,7 +437,7 @@ class YnlFamily(SpecFamily):
             decoded = attr.as_struct(members)
             for m in members:
                 if m.enum:
-                    self._decode_enum(decoded, m)
+                    decoded[m.name] = self._decode_enum(decoded[m.name], m)
         elif attr_spec.sub_type:
             decoded = attr.as_c_array(attr_spec.sub_type)
         else:
@@ -466,6 +465,9 @@ class YnlFamily(SpecFamily):
             else:
                 raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')
 
+            if 'enum' in attr_spec:
+                decoded = self._decode_enum(decoded, attr_spec)
+
             if not attr_spec.is_multi:
                 rsp[attr_spec['name']] = decoded
             elif attr_spec.name in rsp:
@@ -473,8 +475,6 @@ class YnlFamily(SpecFamily):
             else:
                 rsp[attr_spec.name] = [decoded]
 
-            if 'enum' in attr_spec:
-                self._decode_enum(rsp, attr_spec)
         return rsp
 
     def _decode_extack_path(self, attrs, attr_set, offset, target):
-- 
2.38.1

Re: [PATCH net-next v5 2/2] tools: ynl-gen: fix parse multi-attr enum attribute

[Donald Hunter]

Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> writes:

> When attribute is enum type and marked as multi-attr, the netlink
> respond is not parsed, fails with stack trace:
> Traceback (most recent call last):
>   File "/net-next/tools/net/ynl/./test.py", line 520, in <module>
>     main()
>   File "/net-next/tools/net/ynl/./test.py", line 488, in main
>     dplls=dplls_get(282574471561216)
>   File "/net-next/tools/net/ynl/./test.py", line 48, in dplls_get
>     reply=act(args)
>   File "/net-next/tools/net/ynl/./test.py", line 41, in act
>     reply = ynl.dump(args.dump, attrs)
>   File "/net-next/tools/net/ynl/lib/ynl.py", line 598, in dump
>     return self._op(method, vals, dump=True)
>   File "/net-next/tools/net/ynl/lib/ynl.py", line 584, in _op
>     rsp_msg = self._decode(gm.raw_attrs, op.attr_set.name)
>   File "/net-next/tools/net/ynl/lib/ynl.py", line 451, in _decode
>     self._decode_enum(rsp, attr_spec)
>   File "/net-next/tools/net/ynl/lib/ynl.py", line 408, in _decode_enum
>     value = enum.entries_by_val[raw].name
> TypeError: unhashable type: 'list'
> error: 1
>
> Redesign _decode_enum(..) to take a enum int value and translate
> it to either a bitmask or enum name as expected.
>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---
>  tools/net/ynl/lib/ynl.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 027b1c0aecb4..3ca28d4bcb18 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -417,8 +417,7 @@ class YnlFamily(SpecFamily):
>          pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
>          return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
>  
> -    def _decode_enum(self, rsp, attr_spec):
> -        raw = rsp[attr_spec['name']]
> +    def _decode_enum(self, raw, attr_spec):
>          enum = self.consts[attr_spec['enum']]
>          if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
>              i = 0
> @@ -430,7 +429,7 @@ class YnlFamily(SpecFamily):
>                  i += 1
>          else:
>              value = enum.entries_by_val[raw].name
> -        rsp[attr_spec['name']] = value
> +        return value
>  
>      def _decode_binary(self, attr, attr_spec):
>          if attr_spec.struct_name:
> @@ -438,7 +437,7 @@ class YnlFamily(SpecFamily):
>              decoded = attr.as_struct(members)
>              for m in members:
>                  if m.enum:
> -                    self._decode_enum(decoded, m)
> +                    decoded[m.name] = self._decode_enum(decoded[m.name], m)
>          elif attr_spec.sub_type:
>              decoded = attr.as_c_array(attr_spec.sub_type)
>          else:
> @@ -466,6 +465,9 @@ class YnlFamily(SpecFamily):
>              else:
>                  raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')
>  
> +            if 'enum' in attr_spec:
> +                decoded = self._decode_enum(decoded, attr_spec)
> +
>              if not attr_spec.is_multi:
>                  rsp[attr_spec['name']] = decoded
>              elif attr_spec.name in rsp:
> @@ -473,8 +475,6 @@ class YnlFamily(SpecFamily):
>              else:
>                  rsp[attr_spec.name] = [decoded]
>  
> -            if 'enum' in attr_spec:
> -                self._decode_enum(rsp, attr_spec)
>          return rsp
>  
>      def _decode_extack_path(self, attrs, attr_set, offset, target):

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

Re: [PATCH net-next v4 0/2] tools: ynl-gen: fix parse multi-attr enum attribute

[Jakub Kicinski]

On Tue, 25 Jul 2023 12:16:40 +0200 Arkadiusz Kubalewski wrote:
> Fix the issues with parsing enums in ynl.py script.

In case DaveM wants to take these in the morning already:

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

Re: [PATCH net-next v4 0/2] tools: ynl-gen: fix parse multi-attr enum attribute

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 25 Jul 2023 12:16:40 +0200 you wrote:
> Fix the issues with parsing enums in ynl.py script.
> 
> v4->v5:
> - fix indent issue in _decode_binary()
> 
> v3->v4:
> - decoding 'enum-as-flag' starts from 0 instead of enum's start-value
> - move back usage of _decode_enum() back to _decode_binary()
> - fix typo in commit message
> 
> [...]

Here is the summary with links:
  - [net-next,v5,1/2] tools: ynl-gen: fix enum index in _decode_enum(..)
    https://git.kernel.org/netdev/net/c/d7ddf5f4269f
  - [net-next,v5,2/2] tools: ynl-gen: fix parse multi-attr enum attribute
    https://git.kernel.org/netdev/net/c/df15c15e6c98

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

[PATCH net-next v4 0/2] tools: ynl-gen: fix parse multi-attr enum attribute

[Arkadiusz Kubalewski]

Fix the issues with parsing enums in ynl.py script.

v3->v4:
- decoding 'enum-as-flag' starts from 0 instead of enum's start-value
- move back usage of _decode_enum() back to _decode_binary()
- fix typo in commit message

v2->v3:
- Restore i iterator helper for decoding 'enum-as-flag' attributes in
_decode_enum(..) function.
- Fix _decode_enum(..) usage in case of binary & struct type attributes
by moving it to as_struct() funtion while calling _decode_enum(..) for
individual struct members.

v1->v2:
Initially this was one patch, but review shown there is need to fix also
leftover issues with indexing in _decode_enum(..) function
("tools: ynl-gen: fix enum index in _decode_enum(..)").

Arkadiusz Kubalewski (2):
  tools: ynl-gen: fix enum index in _decode_enum(..)
  tools: ynl-gen: fix parse multi-attr enum attribute

 tools/net/ynl/lib/ynl.py | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

-- 
2.38.1