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

[Andy Shevchenko]

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