diff mbox series

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

Message ID 20230713090550.132858-3-arkadiusz.kubalewski@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series tools: ynl-gen: fix parse multi-attr enum attribute | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
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/cc_maintainers warning 1 maintainers not CCed: donald.hunter@gmail.com
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, 42 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arkadiusz Kubalewski July 13, 2023, 9:05 a.m. UTC
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(-)

Comments

Jakub Kicinski July 13, 2023, 4:08 p.m. UTC | #1
On Thu, 13 Jul 2023 11:05:50 +0200 Arkadiusz Kubalewski wrote:
> @@ -436,7 +435,7 @@ class YnlFamily(SpecFamily):
>              decoded = attr.as_struct(members)
>              for m in members:
>                  if m.enum:
> -                    self._decode_enum(decoded, m)
> +                    decoded[m] = self._decode_enum(decoded[m], m)

Yeah, not sure this is right.

Adding Donald, dropping Chuck and LMKL (please use this CC list for v3).

Given the code before the change we can assume that m is a complex type
describing the member so decoded[m] is likely going to explode.

I think you should move the enum decoding into as_struct(), transforming
the code into similar fashion as you did for responses (changing the
@decoded value before "value[m.name] = decoded").
Arkadiusz Kubalewski July 18, 2023, 4:56 p.m. UTC | #2
>From: Jakub Kicinski <kuba@kernel.org>
>Sent: Thursday, July 13, 2023 6:09 PM
>
>On Thu, 13 Jul 2023 11:05:50 +0200 Arkadiusz Kubalewski wrote:
>> @@ -436,7 +435,7 @@ class YnlFamily(SpecFamily):
>>              decoded = attr.as_struct(members)
>>              for m in members:
>>                  if m.enum:
>> -                    self._decode_enum(decoded, m)
>> +                    decoded[m] = self._decode_enum(decoded[m], m)
>
>Yeah, not sure this is right.
>
>Adding Donald, dropping Chuck and LMKL (please use this CC list for v3).
>
>Given the code before the change we can assume that m is a complex type
>describing the member so decoded[m] is likely going to explode.
>
>I think you should move the enum decoding into as_struct(), transforming
>the code into similar fashion as you did for responses (changing the
>@decoded value before "value[m.name] = decoded").

Tried to improve in v3, please take a look, although not sure if passing
attr_spec from _decode_binary() further to as_struct() and _decode_enum()
is sufficient for us..

Thank you!
Arkadiusz
diff mbox series

Patch

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 3908438d3716..06d88f083f95 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']:
             value = set()
@@ -428,7 +427,7 @@  class YnlFamily(SpecFamily):
                 raw >>= 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:
@@ -436,7 +435,7 @@  class YnlFamily(SpecFamily):
             decoded = attr.as_struct(members)
             for m in members:
                 if m.enum:
-                    self._decode_enum(decoded, m)
+                    decoded[m] = self._decode_enum(decoded[m], m)
         elif attr_spec.sub_type:
             decoded = attr.as_c_array(attr_spec.sub_type)
         else:
@@ -464,6 +463,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(int.from_bytes(attr.raw, "big"), attr_spec)
+
             if not attr_spec.is_multi:
                 rsp[attr_spec['name']] = decoded
             elif attr_spec.name in rsp:
@@ -471,8 +473,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):