On Wed, 16 Aug 2023 at 16:20, Jakub Kicinski [off-list ref] wrote:

On Tue, 15 Aug 2023 20:42:49 +0100 Donald Hunter wrote:

quoted

+    def __init__(self, nl_msg, ynl=None):
+        self.genl_cmd, self.genl_version, _ = struct.unpack_from("BBH", nl_msg.raw, 0)
+        nl_msg.raw = nl_msg.raw[4:]

It's a bit of a layering violation that we are futzing with the raw
member of NlMsg inside GenlMsg, no?

Should we add "fixed hdrs len" argument to NlMsg? Either directly or
pass ynl and let get the expected len from ynl? That way NlMsg can
split itself into hdr, userhdrs and attrs without GenlMsg "fixing it
up"?

I agree, it breaks the layering. The issue is that GenlMsg gets created at
some point after NlMsg, only when we know the nl_msg is suitable for
decoding. The fixed header bit is quite well encapsulated in NlMsg,
it's the genl header that needs pulled out and NlMsg shouldn't know
anything about it. How about I add a take_bytes(length) method or a
generic decode_subheader(format, length) method to NlMsg?