Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling

From: Johannes Berg
Date: Thu Jul 04 2019 - 08:03:16 EST

On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
> There is still the question if it it should be implemented as a nested
> attribute which could look like the current compact form without the
> "list" flag (if there is no mask, it's a list). Or an unstructured data
> block consisting of u32 bit length

You wouldn't really need the length, since the attribute has a length
already :-)

And then, if you just concatenate the value and mask, the existing
NLA_BITFIELD32 becomes a special case.

> and one or two bitmaps of
> corresponding length. I would prefer the nested attribute, netlink was
> designed to represent structured data, passing structures as binary goes
> against the design (just looked at VFINFO in rtnetlink few days ago,
> it's awful, IMHO).

Yeah, I dunno. On the one hand I completely agree, on the other hand
NLA_BITFIELD32 already goes the other way, and is there now...

> Either way, I would still prefer to have bitmaps represented as an array
> of 32-bit blocks in host byte order. This would be easy to handle in
> kernel both in places where we have u32 based bitmaps and unsigned long
> based ones. Other options seem less appealing:
> - u8 based: only complicates processing
> - u64 based: have to care about alignment
> - unsigned long based: alignment and also problems with 64-bit kernel
> vs. 32-bit userspace

Agree with this.