Messages in this thread | | | Date | Mon, 25 Jul 2022 11:53:24 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next 0/4] netlink: add 'bitmap' attribute type and API |
| |
On Mon, 25 Jul 2022 15:02:55 +0200 Alexander Lobakin wrote: > > Actually once a field crosses the biggest natural int size I was > > thinking that the user would go to a bitmap. > > > > So at the netlink level the field is bigint (LE, don't care about BE). > > Kernel side API is: > > > > nla_get_b8, nla_get_b16, nla_get_b32, nla_get_b64, > > nla_get_bitmap > > nla_put_b8, nla_put_b16 etc. > > > > u16 my_flags_are_so_toight; > > > > my_flags_are_so_toight = nla_get_b16(attr[BLAA_BLA_BLA_FLAGS]); > > > > The point is - the representation can be more compact than u64 and will > > therefore encourage anyone who doesn't have a strong reason to use > > fixed size fields to switch to the bigint. > > Ahh looks like I got it! So you mean that at Netlink level we should > exchange with bigint/u64arrs, but there should be an option to > get/set only 16/32/64 bits from them to simplify (or keep simple) > users?
Not exactly. I'd prefer if the netlink level was in u32 increments. u64 requires padding (so the nla_put..() calls will have more args). Netlink requires platform alignment and rounds up to 4B, so u32 is much more convenient than u64. Similarly - it doesn't make sense to represent sizes smaller than 4B because of the rounding up, so nla_put_b8() can be a define to nla_put_b32(). Ethool's choice of u32 is not without merit.
> Like, if we have `u16 uuid`, to not do > > unsigned long uuid_bitmap; > > nla_get_bitmap(attr[FLAGS], &uuid_bitmap, BITS_PER_TYPE(u16)); > uuid = (u16)uuid_bitmap; > > but instead > > uuid = nla_get_b16(attr[FLAGS]); > > ?
Yes.
> > about being flexible when it comes to size, I guess, more than > > bitmap in particular. > > Probably, but you also said above that for anything bigger than > longs you'd go for bitmaps, didn't you? So I guess that series > goes in the right direction, just needs a couple more inlines > to be able to get/put u{32, 64; 8, 16 -- not sure about these two > after reading your follow-up mail} as easy as nla_{get,put}<size>() > and probably dropping Endianness stuff? Hope I got it right ._.
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.
| |