Thread (21 messages) 21 messages, 4 authors, 2023-08-18

Re: [PATCH net-next v2 05/10] tools/net/ynl: Refactor decode_fixed_header into NlMsg

From: Jakub Kicinski <kuba@kernel.org>
Date: 2023-08-18 01:38:34
Also in: linux-doc

On Thu, 17 Aug 2023 16:14:35 +0100 Donald Hunter wrote:
quoted
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?
Why do we need to fix up the .raw of NlMsg underlying the GenlMsg
in the first place? GenlMsg by itself didn't need to do that until now.

Another option to consider which would make things more symmetric
between raw and genetlink would be to add a wrapper class for old
families, too. ClassicMsg? CnlMsg? That way we could retain the
separation of NlMsg is just a raw message which could be a NLM_DONE or
some other control thing, and higher level class being used to pull
fixed headers and separate out attrs. Just a thought, not sure it helps.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help