netdev - Re: [RFC net-next 2/9] genetlink: reorg struct genl

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: <187cb655088747c42f1bf4b147b272eab8f968c0.camel@sipsolutions.net>
Date:   Thu, 01 Oct 2020 09:34:20 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc:     andrew@...n.ch, jiri@...nulli.us, mkubecek@...e.cz,
        dsahern@...nel.org, pablo@...filter.org
Subject: Re: [RFC net-next 2/9] genetlink: reorg struct genl_family

On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote:
> There are holes and oversized members in struct genl_family.
> 
> Before: /* size: 104, cachelines: 2, members: 16 */
> After:  /* size:  88, cachelines: 2, members: 16 */
> 
> The command field in struct genlmsghdr is a u8, so no point
> in the operation count being 32 bit. Also operation 0 is
> usually undefined, so we only need 255 entries.
> 
> netnsok and parallel_ops are only ever initialized to true.

The fundamentally are bools, so that makes sense :)

> We can grow the fields as needed, compiler should warn us
> if someone tries to assign larger constants.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  include/net/genetlink.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/genetlink.h b/include/net/genetlink.h
> index a3484fd736d6..0033c76ff094 100644
> --- a/include/net/genetlink.h
> +++ b/include/net/genetlink.h
> @@ -48,8 +48,11 @@ struct genl_family {
>  	char			name[GENL_NAMSIZ];
>  	unsigned int		version;
>  	unsigned int		maxattr;
> -	bool			netnsok;
> -	bool			parallel_ops;
> +	unsigned int		mcgrp_offset;	/* private */

In practice, we can probably also shrink that to u16, since it just
gives you the offset of the multicast groups this family has in the
whole table - and we'll hopefully never run more than 2**16 multicast
groups across all genetlink families :)

But it also doesn't matter much.

Reviewed-by: Johannes Berg <johannes@...solutions.net>

johannes