Re: [PATCH net-next 0/4] netlink: add 'bitmap' attribute type and API

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 26 Jul 2022 12:41:45 +0200 Alexander Lobakin wrote:
> > Modulo the fact that I do still want to pack to u32. Especially a
> > single u32 - perhaps once we cross 8B we can switch to requiring 8B
> > increments.
> > 
> > The nla_len is 16bit, which means that attrs nested inside other attrs
> > are quite tight for space (see the sad story of VF attrs in
> > RTM_GETLINK). If we don't represent u8/u16/u32 in a netlink-level
> > efficient manner we're back to people arguing for raw u32s rather than
> > using the new type.  
> 
> Ah, got it. I think you're right. The only thing that bothers me
> a bit is that we'll be converting arrays of u32s to unsigned longs
> each time, unlike u64s <--> longs which are 1:1 for 64 bits and LEs.

For LE the size of the word doesn't matter, right? Don't trust me on
endian questions, but I thought for LE you can just load the data as
ulongs as long as size is divisible by sizeof(ulong) and you're gonna
be good. It'd add some #ifdefs and ifs() to the bitmap code, but the
ethtool u32 conversions would probably also benefit?

> But I guess it's better anyway than waste Netlink attrs for
> paddings?

Yes, AFAIU it's pretty bad. Say you have a nest with objects with 
3 big ints inside. With u32 the size of a nest is 4 + 3*(4+4) = 28.
With u64 it's 4 + 4+8 + 2*(4+4+8) = 48 so 28 vs 48. Netlink header
being 4B almost always puts us out of alignment.

> So I'd like to summarize for a v2:
> 
> * represent as u32s, not u64s;
> * present it as "bigints", not bitmaps. It can carry bitmaps as
>   well, but also ->
> * add getters/setters for u8, 16, 32, 64s. Bitmaps are already here.
>   Probably u32 arrays as well? Just copy them directly. And maybe
>   u64 arrays, too (convert Netlink u32s to target u64s)?

I don't think we need the array helpers.

> * should I drop Endianness stuff? With it still in place, it would
>   be easier to use this new API to exchange with e.g. __be64.

If you have a use case, sure. AFAIU the explicit endian types are there
for carrying protocol fields. I don't think there are many protocols
that we deal with in netlink that have big int fields.

> * hope I didn't miss anything?

I think that's it, but I reserve the right to remember something after
you post the code :)

> Thanks a lot, some great ideas here from your side :)