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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y060io/942BpWpQw@smile.fi.intel.com>
Date:   Tue, 18 Oct 2022 17:13:30 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Yury Norov <yury.norov@...il.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net-next 6/6] netlink: add universal 'bigint'
 attribute type

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