Re: [PATCH v2 net-next 6/6] netlink: add universal 'bigint' attribute type

From: Andy Shevchenko
Date: Tue Oct 18 2022 - 10:13:45 EST


On Tue, Oct 18, 2022 at 04:00:27PM +0200, Alexander Lobakin wrote:
> Add a new type of Netlink attribute -- big integer.
>
> Basically bigints are just arrays of u32s, but can carry anything,
> with 1 bit precision. Using variable-length arrays of a fixed type
> gives the following:
>
> * versatility: one type can carry scalars from u8 to u64, bitmaps,
> binary data etc.;
> * scalability: the same Netlink attribute can be changed to a wider
> (or shorter) data type with no compatibility issues, same for
> growing bitmaps;
> * optimization: 4-byte units don't require wasting slots for empty
> padding attributes (they always have natural alignment in Netlink
> messages).
>
> The only downside is that get/put functions sometimes are not just
> direct assignment inlines due to the internal representation using
> bitmaps (longs) and the bitmap API.
>
> Basic consumer functions/macros are:
> * nla_put_bigint() and nla_get_bigint() -- to easily put a bigint to
> an skb or get it from a received message (only pointer to an
> unsigned long array and the number of bits in it are needed);
> * nla_put_bigint_{u,be,le,net}{8,16,32,64}() -- alternatives to the
> already existing family to send/receive scalars using the new type
> (instead of distinct attr types);
> * nla_total_size_bigint*() -- to provide estimate size in bytes to
> Netlink needed to store a bigint/type;
> * NLA_POLICY_BIGINT*() -- to declare a Netlink policy for a bigint
> attribute.
>
> There are also *_bitmap() aliases for the *_bigint() helpers which
> have no differences and designed to distinguish bigints from bitmaps
> in the call sites (for readability).
>
> Netlink policy for a bigint can have an optional bitmap mask of bits
> supported by the code -- for example, to filter out obsolete bits
> removed some time ago or limit value to n bits (e.g. 53 instead of
> 64). Without it, Netlink will just make sure no bits past the passed
> number are set. Both variants can be requested from the userspace
> and the kernel will put a mask into a new policy attribute
> (%NL_POLICY_TYPE_ATTR_BIGINT_MASK).
>
> Note on including <linux/bitmap.h> into <net/netlink.h>: seems to
> introduce no visible compilation time regressions, make includecheck
> doesn't see anything illegit as well. Hiding everything inside
> lib/nlattr.c would require making a couple dozens optimizable
> inlines external, doesn't sound optimal.

...

> #ifndef __NET_NETLINK_H
> #define __NET_NETLINK_H
>
> -#include <linux/types.h>
> +#include <linux/bitmap.h>

types.h is not guaranteed to be included by bitmap.h. So, if you want to
clean up the headers in this header, do it in a separate change.

Also I would suggest to check what Ingo did in his 2000+ patch series
to see if there is anything interesting towards this header.

> #include <linux/netlink.h>
> #include <linux/jiffies.h>
> #include <linux/in6.h>

--
With Best Regards,
Andy Shevchenko