* [PATCH net-next 0/2] tools: ynl-gen: fix parse multi-attr enum attribute
@ 2023-07-13 9:05 Arkadiusz Kubalewski
2023-07-13 9:05 ` [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..) Arkadiusz Kubalewski
2023-07-13 9:05 ` [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
0 siblings, 2 replies; 7+ messages in thread
From: Arkadiusz Kubalewski @ 2023-07-13 9:05 UTC (permalink / raw)
To: netdev, linux-kernel, kuba, davem, pabeni, edumazet, chuck.lever
Cc: Arkadiusz Kubalewski
Fix the issues with parsing enums in ynl.py script.
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 | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..)
2023-07-13 9:05 [PATCH net-next 0/2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
@ 2023-07-13 9:05 ` Arkadiusz Kubalewski
2023-07-13 16:02 ` Jakub Kicinski
2023-07-13 9:05 ` [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
1 sibling, 1 reply; 7+ messages in thread
From: Arkadiusz Kubalewski @ 2023-07-13 9:05 UTC (permalink / raw)
To: netdev, linux-kernel, kuba, davem, pabeni, edumazet, chuck.lever
Cc: Arkadiusz Kubalewski
Remove wrong index adjustment, which is a 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 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 1b3a36fbb1c3..3908438d3716 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -420,16 +420,14 @@ 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']:
value = set()
while raw:
if raw & 1:
- value.add(enum.entries_by_val[i].name)
+ value.add(enum.entries_by_val[raw & 1].name)
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.37.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute
2023-07-13 9:05 [PATCH net-next 0/2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
2023-07-13 9:05 ` [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..) Arkadiusz Kubalewski
@ 2023-07-13 9:05 ` Arkadiusz Kubalewski
2023-07-13 16:08 ` Jakub Kicinski
1 sibling, 1 reply; 7+ messages in thread
From: Arkadiusz Kubalewski @ 2023-07-13 9:05 UTC (permalink / raw)
To: netdev, linux-kernel, kuba, davem, pabeni, edumazet, chuck.lever
Cc: 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 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):
--
2.37.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..)
2023-07-13 9:05 ` [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..) Arkadiusz Kubalewski
@ 2023-07-13 16:02 ` Jakub Kicinski
2023-07-18 16:30 ` Kubalewski, Arkadiusz
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2023-07-13 16:02 UTC (permalink / raw)
To: Arkadiusz Kubalewski
Cc: netdev, linux-kernel, davem, pabeni, edumazet, chuck.lever
On Thu, 13 Jul 2023 11:05:49 +0200 Arkadiusz Kubalewski wrote:
> - i = attr_spec.get('value-start', 0)
> if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
> value = set()
> while raw:
> if raw & 1:
> - value.add(enum.entries_by_val[i].name)
> + value.add(enum.entries_by_val[raw & 1].name)
> raw >>= 1
> - i += 1
This doesn't make sense, as I suggested you need to keep i for this
loop. Move it to the inside of the if 'enum-as-fla... and init to 0.
i is tracking which bit number we are at as we consume / shift out
bits from raw.
Have you ever used ChatGPT? No shame, just curious.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute
2023-07-13 9:05 ` [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
@ 2023-07-13 16:08 ` Jakub Kicinski
2023-07-18 16:56 ` Kubalewski, Arkadiusz
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2023-07-13 16:08 UTC (permalink / raw)
To: Arkadiusz Kubalewski, Donald Hunter; +Cc: netdev, davem, pabeni, edumazet
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").
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..)
2023-07-13 16:02 ` Jakub Kicinski
@ 2023-07-18 16:30 ` Kubalewski, Arkadiusz
0 siblings, 0 replies; 7+ messages in thread
From: Kubalewski, Arkadiusz @ 2023-07-18 16:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, linux-kernel, davem, pabeni, edumazet, chuck.lever
>From: Jakub Kicinski <kuba@kernel.org>
>Sent: Thursday, July 13, 2023 6:03 PM
>
>On Thu, 13 Jul 2023 11:05:49 +0200 Arkadiusz Kubalewski wrote:
>> - i = attr_spec.get('value-start', 0)
>> if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
>> value = set()
>> while raw:
>> if raw & 1:
>> - value.add(enum.entries_by_val[i].name)
>> + value.add(enum.entries_by_val[raw & 1].name)
>> raw >>= 1
>> - i += 1
>
>This doesn't make sense, as I suggested you need to keep i for this
>loop. Move it to the inside of the if 'enum-as-fla... and init to 0.
>
>i is tracking which bit number we are at as we consume / shift out
>bits from raw.
>
Yeah, you are right, I don't think I had clear mind when created those..
>Have you ever used ChatGPT? No shame, just curious.
I have used it, but I am not using it to prepare code, this was my fault,
chat would probably do better..
Anyway just sent v3 with this part fixed.
Thank you!
Arkadiusz
>--
>pw-bot: cr
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute
2023-07-13 16:08 ` Jakub Kicinski
@ 2023-07-18 16:56 ` Kubalewski, Arkadiusz
0 siblings, 0 replies; 7+ messages in thread
From: Kubalewski, Arkadiusz @ 2023-07-18 16:56 UTC (permalink / raw)
To: Jakub Kicinski, Donald Hunter; +Cc: netdev, davem, pabeni, edumazet
>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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-18 17:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-13 9:05 [PATCH net-next 0/2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
2023-07-13 9:05 ` [PATCH net-next 1/2 v2] tools: ynl-gen: fix enum index in _decode_enum(..) Arkadiusz Kubalewski
2023-07-13 16:02 ` Jakub Kicinski
2023-07-18 16:30 ` Kubalewski, Arkadiusz
2023-07-13 9:05 ` [PATCH net-next 2/2 v2] tools: ynl-gen: fix parse multi-attr enum attribute Arkadiusz Kubalewski
2023-07-13 16:08 ` Jakub Kicinski
2023-07-18 16:56 ` Kubalewski, Arkadiusz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).