[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD4GDZza0TWkzmwW_qP8GFXzr7DOGVfg-xsiXOVUyRqe47Rung@mail.gmail.com>
Date: Fri, 23 Aug 2024 22:22:37 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"pabeni@...hat.com" <pabeni@...hat.com>, "jiri@...nulli.us" <jiri@...nulli.us>,
"Keller, Jacob E" <jacob.e.keller@...el.com>, "liuhangbin@...il.com" <liuhangbin@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net] tools/net/ynl: fix cli.py --subscribe feature
On Fri, 23 Aug 2024 at 20:31, Kubalewski, Arkadiusz
<arkadiusz.kubalewski@...el.com> wrote:
>
> >The problem is that we are trying to look up the op before calling
> >nlproto.decode(...) but it wants to know the op to check if it has a
> >fixed header.
> >
> >I think the fix would be to change NetlinkProtocol.decode() to perform
> >the op lookup, if necessary, after it has called self._decode() to
> >unpack the GenlMsg.
> >
> >How about changing NetlinkProtocol.decode() to be:
> >
> >def decode(self, ynl, nl_msg, op, ops_by_value):
> > msg = self._decode(nl_msg)
> > if op is None:
> > op = ops_by_value[msg.cmd()]
> > ...
> >
> >The main loop can call it like this:
> >
> >nlproto.decode(self, nl_msg, op, self.rsp_by_value)
> >
> >and check_ntf() can call it like this:
> >
> >nlproto.decode(self, nl_msg, None, self.rsp_by_value)
> >
>
> Yes, again, this seems much better, I will prepare new patch and send
> the non-RFC version soon.
>
> Thanks for your help!
> Arkadiusz
I tried the changes locally to check it would work and ended up with
the following patch. Can you verify that it fixes the issue you had?
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index d42c1d605969..311542a8aa24 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -386,8 +386,10 @@ class NetlinkProtocol:
def _decode(self, nl_msg):
return nl_msg
- def decode(self, ynl, nl_msg, op):
+ def decode(self, ynl, nl_msg, op, ops_by_value):
msg = self._decode(nl_msg)
+ if op is None:
+ op = ops_by_value[msg.cmd()]
fixed_header_size = ynl._struct_size(op.fixed_header)
msg.raw_attrs = NlAttrs(msg.raw, fixed_header_size)
return msg
@@ -921,8 +923,7 @@ class YnlFamily(SpecFamily):
print("Netlink done while checking for ntf!?")
continue
- op = self.rsp_by_value[nl_msg.cmd()]
- decoded = self.nlproto.decode(self, nl_msg, op)
+ decoded = self.nlproto.decode(self, nl_msg, None,
self.rsp_by_value)
if decoded.cmd() not in self.async_msg_ids:
print("Unexpected msg id done while checking for
ntf", decoded)
continue
@@ -980,7 +981,7 @@ class YnlFamily(SpecFamily):
if nl_msg.extack:
self._decode_extack(req_msg, op, nl_msg.extack)
else:
- op = self.rsp_by_value[nl_msg.cmd()]
+ op = None
req_flags = []
if nl_msg.error:
@@ -1004,7 +1005,7 @@ class YnlFamily(SpecFamily):
done = len(reqs_by_seq) == 0
break
- decoded = self.nlproto.decode(self, nl_msg, op)
+ decoded = self.nlproto.decode(self, nl_msg, op,
self.rsp_by_value)
# Check if this is a reply to our request
if nl_msg.nl_seq not in reqs_by_seq or decoded.cmd()
!= op.rsp_value:
Powered by blists - more mailing lists