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

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 17 Aug 2023 16:14:35 +0100 Donald Hunter wrote:
> > 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.