| 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
| ||
|
Message-ID: <ZFy0QjGpUCOfaSxA@corigine.com>
Date: Thu, 11 May 2023 11:24:18 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Pablo Neira Ayuso <pablo@...filter.org>,
Jozsef Kadlecsik <kadlec@...filter.org>,
Florian Westphal <fw@...len.de>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
netfilter-devel@...r.kernel.org, coreteam@...filter.org,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next] netfilter: Reorder fields in 'struct
nf_conntrack_expect'
On Mon, May 08, 2023 at 06:53:14PM +0200, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce holes.
> On x86_64, this shrinks the size of 'struct nf_conntrack_expect' from 264
> to 256 bytes.
>
> This structure deserve a dedicated cache, so reducing its size looks nice.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Thanks, this much nicer to me.
I slightly concerned that there may be implications for
fields that were on the same cacheline now being on different cachelines.
But only very slightly.
Reviewed-by: Simon Horman <simon.horman@...igine.com>
> ---
> Using pahole
>
> Before:
> ======
> struct nf_conntrack_expect {
> struct hlist_node lnode; /* 0 16 */
> struct hlist_node hnode; /* 16 16 */
> struct nf_conntrack_tuple tuple; /* 32 40 */
> /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> struct nf_conntrack_tuple_mask mask; /* 72 20 */
>
> /* XXX 4 bytes hole, try to pack */
>
> void (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /* 96 8 */
> struct nf_conntrack_helper * helper; /* 104 8 */
> struct nf_conn * master; /* 112 8 */
> struct timer_list timeout; /* 120 88 */
> /* --- cacheline 3 boundary (192 bytes) was 16 bytes ago --- */
> refcount_t use; /* 208 4 */
> unsigned int flags; /* 212 4 */
> unsigned int class; /* 216 4 */
> union nf_inet_addr saved_addr; /* 220 16 */
> union nf_conntrack_man_proto saved_proto; /* 236 2 */
>
> /* XXX 2 bytes hole, try to pack */
>
> enum ip_conntrack_dir dir; /* 240 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct callback_head rcu __attribute__((__aligned__(8))); /* 248 16 */
>
> /* size: 264, cachelines: 5, members: 15 */
> /* sum members: 254, holes: 3, sum holes: 10 */
> /* forced alignments: 1, forced holes: 1, sum forced holes: 4 */
> /* last cacheline: 8 bytes */
> } __attribute__((__aligned__(8)));
>
>
> After:
> =====
> struct nf_conntrack_expect {
> struct hlist_node lnode; /* 0 16 */
> struct hlist_node hnode; /* 16 16 */
> struct nf_conntrack_tuple tuple; /* 32 40 */
> /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> struct nf_conntrack_tuple_mask mask; /* 72 20 */
> refcount_t use; /* 92 4 */
> unsigned int flags; /* 96 4 */
> unsigned int class; /* 100 4 */
> void (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /* 104 8 */
> struct nf_conntrack_helper * helper; /* 112 8 */
> struct nf_conn * master; /* 120 8 */
> /* --- cacheline 2 boundary (128 bytes) --- */
> struct timer_list timeout; /* 128 88 */
> /* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
> union nf_inet_addr saved_addr; /* 216 16 */
> union nf_conntrack_man_proto saved_proto; /* 232 2 */
>
> /* XXX 2 bytes hole, try to pack */
>
> enum ip_conntrack_dir dir; /* 236 4 */
> struct callback_head rcu __attribute__((__aligned__(8))); /* 240 16 */
>
> /* size: 256, cachelines: 4, members: 15 */
> /* sum members: 254, holes: 1, sum holes: 2 */
> /* forced alignments: 1 */
> } __attribute__((__aligned__(8)));
> ---
> include/net/netfilter/nf_conntrack_expect.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
> index 0855b60fba17..cf0d81be5a96 100644
> --- a/include/net/netfilter/nf_conntrack_expect.h
> +++ b/include/net/netfilter/nf_conntrack_expect.h
> @@ -26,6 +26,15 @@ struct nf_conntrack_expect {
> struct nf_conntrack_tuple tuple;
> struct nf_conntrack_tuple_mask mask;
>
> + /* Usage count. */
> + refcount_t use;
> +
> + /* Flags */
> + unsigned int flags;
> +
> + /* Expectation class */
> + unsigned int class;
> +
> /* Function to call after setup and insertion */
> void (*expectfn)(struct nf_conn *new,
> struct nf_conntrack_expect *this);
> @@ -39,15 +48,6 @@ struct nf_conntrack_expect {
> /* Timer function; deletes the expectation. */
> struct timer_list timeout;
>
> - /* Usage count. */
> - refcount_t use;
> -
> - /* Flags */
> - unsigned int flags;
> -
> - /* Expectation class */
> - unsigned int class;
> -
> #if IS_ENABLED(CONFIG_NF_NAT)
> union nf_inet_addr saved_addr;
> /* This is the original per-proto part, used to map the
> --
> 2.34.1
>
>
Powered by blists - more mailing lists