Re: [RFC PATCH net] tools/net/ynl: fix cli.py --subscribe feature
From: Donald Hunter <donald.hunter@gmail.com>
Date: 2024-08-23 10:40:34
Also in:
lkml
Possibly related (same subject, not in this thread)
- 2024-08-23 · [RFC PATCH net] tools/net/ynl: fix cli.py --subscribe feature · Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Arkadiusz Kubalewski [off-list ref] writes:
quoted hunk
Execution of command: ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml / --subscribe "monitor" --sleep 10 fails with: Traceback (most recent call last): File "/root/arek/linux-dpll/./tools/net/ynl/cli.py", line 114, in <module> main() File "/root/arek/linux-dpll/./tools/net/ynl/cli.py", line 109, in main ynl.check_ntf() File "/root/arek/linux-dpll/tools/net/ynl/lib/ynl.py", line 924, in check_ntf op = self.rsp_by_value[nl_msg.cmd()] KeyError: 19 The key value of 19 returned from nl_msg.cmd() is a received message header's nl_type, which is the id value of generic netlink family being addressed in the OS on subscribing. It is wrong to use it for decoding the notification. Expected notification message on dpll subsystem is DPLL_CMD_PIN_CHANGE_NTF=13, seems at that point only available as first byte of RAW message payload, use it to target correct op and allow further parsing. Fixes: "0a966d606c68" ("tools/net/ynl: Fix extack decoding for directional ops") Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> --- tools/net/ynl/lib/ynl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index d42c1d605969..192d6c150303 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py@@ -921,7 +921,7 @@ class YnlFamily(SpecFamily): print("Netlink done while checking for ntf!?") continue - op = self.rsp_by_value[nl_msg.cmd()] + op = self.rsp_by_value[nl_msg.raw[0]]
I don't think that is the right fix. It would break notifications for
raw netlink messages. The point of NlMsg.cmd() is to abstract away where
the op id comes from. GenlMsg.cmd() returns the value unpacked from
raw[0].
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)
decoded = self.nlproto.decode(self, nl_msg, op)
if decoded.cmd() not in self.async_msg_ids:
print("Unexpected msg id done while checking for ntf", decoded)