From: Donald Hunter <donald.hunter@gmail.com> Date: 2024-03-01 17:14:40
Extack decoding was using a hard-coded msg header size of 20 but
netlink-raw has a header size of 16.
Use a protocol specific msghdr_size() when decoding the attr offssets.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
tools/net/ynl/lib/ynl.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -348,6 +348,9 @@ class NetlinkProtocol:raiseException(f'Multicast group "{mcast_name}" not present in the spec')returnmcast_groups[mcast_name].value+defmsghdr_size(self):+return16+classGenlProtocol(NetlinkProtocol):def__init__(self,family_name):
@@ -373,6 +376,8 @@ class GenlProtocol(NetlinkProtocol):raiseException(f'Multicast group "{mcast_name}" not present in the family')returnself.genl_family['mcast'][mcast_name]+defmsghdr_size(self):+returnsuper().msghdr_size()+4classSpaceAttrs:
@@ -693,7 +698,7 @@ class YnlFamily(SpecFamily):returnmsg=self.nlproto.decode(self,NlMsg(request,0,op.attr_set))-offset=20+self._struct_size(op.fixed_header)+offset=self.nlproto.msghdr_size()+self._struct_size(op.fixed_header)path=self._decode_extack_path(msg.raw_attrs,op.attr_set,offset,extack['bad-attr-offs'])ifpath:
From: Donald Hunter <donald.hunter@gmail.com> Date: 2024-03-01 17:14:42
ynl does not handle NlError exceptions so they get reported like program
failures. Handle the NlError exceptions and report the netlink errors
more cleanly.
Example now:
Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
error: -2 extack: {'bad-attr': '.op'}
Example before:
Traceback (most recent call last):
File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 81, in <module>
main()
File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 69, in main
reply = ynl.dump(args.dump, attrs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 906, in dump
return self._op(method, vals, [], dump=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 872, in _op
raise NlError(nl_msg)
lib.ynl.NlError: Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
error: -2 extack: {'bad-attr': '.op'}
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
tools/net/ynl/cli.py | 18 +++++++++++-------
tools/net/ynl/lib/__init__.py | 4 ++--
2 files changed, 13 insertions(+), 9 deletions(-)
From: Donald Hunter <donald.hunter@gmail.com> Date: 2024-03-01 17:14:43
The nlctrl family uses 2 levels of array nesting for policy attributes.
Add a 'nest-depth' property to genetlink-legacy and extend ynl to use
it.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
Documentation/netlink/genetlink-legacy.yaml | 3 +++
tools/net/ynl/lib/nlspec.py | 2 ++
tools/net/ynl/lib/ynl.py | 9 ++++++---
3 files changed, 11 insertions(+), 3 deletions(-)
@@ -261,6 +261,9 @@ properties:struct:description:Name of the struct type used for the attribute.type:string+nest-depth:+description:Depth of nesting for an array-nest, defaults to 1.+type:integer# End genetlink-legacy# Make sure name-prefix does not appear in subsets (subsets inherit naming)
From: Jakub Kicinski <kuba@kernel.org> Date: 2024-03-03 04:05:37
On Fri, 1 Mar 2024 17:14:30 +0000 Donald Hunter wrote:
The nlctrl family uses 2 levels of array nesting for policy attributes.
Add a 'nest-depth' property to genetlink-legacy and extend ynl to use
it.
Hm, I'm 90% sure we don't need this... because nlctrl is basically what
the legacy level was written for, initially. The spec itself wasn't
sent, because the C codegen for it was quite painful. And the Python
CLI was an afterthought.
Could you describe what nesting you're trying to cover here?
Isn't it a type-value?
BTW we'll also need to deal with the C codegen situation somehow.
Try making it work, if it's not a simple matter of fixing up the
names to match the header - we can grep nlctrl out in the Makefile.
From: Donald Hunter <donald.hunter@gmail.com> Date: 2024-03-03 10:50:22
On Sun, 3 Mar 2024 at 04:05, Jakub Kicinski [off-list ref] wrote:
On Fri, 1 Mar 2024 17:14:30 +0000 Donald Hunter wrote:
quoted
The nlctrl family uses 2 levels of array nesting for policy attributes.
Add a 'nest-depth' property to genetlink-legacy and extend ynl to use
it.
Hm, I'm 90% sure we don't need this... because nlctrl is basically what
the legacy level was written for, initially. The spec itself wasn't
sent, because the C codegen for it was quite painful. And the Python
CLI was an afterthought.
Could you describe what nesting you're trying to cover here?
Isn't it a type-value?
BTW we'll also need to deal with the C codegen situation somehow.
Try making it work, if it's not a simple matter of fixing up the
names to match the header - we can grep nlctrl out in the Makefile.
Yeah, I forgot to check codegen but saw the failures on patchwork. I
have fixed the names but still have a couple more things to fix.
BTW, this patchset was a step towards experimenting with removing the
hard-coded msg decoding in the Python library. Not so much for
genetlink families, more for the extack decoding so that I could add
policy attr decoding. Thinking about it some more, that might be
better done with a "core" spec that contains just extack-attrs and
policy-attrs because they don't belong to any single family - they're
kinda infrastructure for all families.
From: Jakub Kicinski <kuba@kernel.org> Date: 2024-03-04 15:22:33
On Sun, 3 Mar 2024 10:50:09 +0000 Donald Hunter wrote:
On Sun, 3 Mar 2024 at 04:05, Jakub Kicinski [off-list ref] wrote:
quoted
On Fri, 1 Mar 2024 17:14:30 +0000 Donald Hunter wrote:
quoted
The nlctrl family uses 2 levels of array nesting for policy attributes.
Add a 'nest-depth' property to genetlink-legacy and extend ynl to use
it.
Hm, I'm 90% sure we don't need this... because nlctrl is basically what
the legacy level was written for, initially. The spec itself wasn't
sent, because the C codegen for it was quite painful. And the Python
CLI was an afterthought.
Could you describe what nesting you're trying to cover here?
Isn't it a type-value?
BTW we'll also need to deal with the C codegen situation somehow.
Try making it work, if it's not a simple matter of fixing up the
names to match the header - we can grep nlctrl out in the Makefile.
Yeah, I forgot to check codegen but saw the failures on patchwork. I
have fixed the names but still have a couple more things to fix.
BTW, this patchset was a step towards experimenting with removing the
hard-coded msg decoding in the Python library. Not so much for
genetlink families, more for the extack decoding so that I could add
policy attr decoding. Thinking about it some more, that might be
better done with a "core" spec that contains just extack-attrs and
policy-attrs because they don't belong to any single family - they're
kinda infrastructure for all families.
YAML specs describe information on how to parse data YNL doesn't have
to understand, just format correctly. The base level of netlink
processing, applicable to all families, is a different story.
I think hand-coding that is more than okay. The goal is not to express
everything in YAML but to avoid duplicated work per family, if that
makes sense.
Apologies, I totally missed this. Is the intended usage something like this:
-
name: policy
type: nest-type-value
type-value: [ policy-id, attr-id ]
nested-attributes: policy-attrs
YAML specs describe information on how to parse data YNL doesn't have
to understand, just format correctly. The base level of netlink
processing, applicable to all families, is a different story.
I think hand-coding that is more than okay. The goal is not to express
everything in YAML but to avoid duplicated work per family, if that
makes sense.
Okay, I can go ahead and hard-code the policy attr decoding for extack messages.