netdev - RE: [patch net-next 1/7] skbuff: Add the offload_mr_fwd

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]

Date:   Fri, 29 Sep 2017 11:36:53 +0000
From:   Yuval Mintz <yuvalm@...lanox.com>
To:     Davide Caratti <dcaratti@...hat.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        Yotam Gigi <yotamg@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>, mlxsw <mlxsw@...lanox.com>,
        "nikolay@...ulusnetworks.com" <nikolay@...ulusnetworks.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "dsa@...ulusnetworks.com" <dsa@...ulusnetworks.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "willemb@...gle.com" <willemb@...gle.com>,
        "johannes.berg@...el.com" <johannes.berg@...el.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "fw@...len.de" <fw@...len.de>,
        "gfree.wind@....163.com" <gfree.wind@....163.com>
Subject: RE: [patch net-next 1/7] skbuff: Add the offload_mr_fwd_mark field

> hello Jiri and Yotam,
> 
> On Thu, 2017-09-28 at 19:34 +0200, Jiri Pirko wrote:
> > From: Yotam Gigi <yotamg@...lanox.com>
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 19e64bf..ada8214 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -772,6 +772,7 @@ struct sk_buff {
> >  	__u8			remcsum_offload:1;
> >  #ifdef CONFIG_NET_SWITCHDEV
> >  	__u8			offload_fwd_mark:1;
> > +	__u8			offload_mr_fwd_mark:1;
> 
> I had a look at the pahole output:
> 
> $ make allyesconfig
> $ make net/core/skbuff.o
> $ pahole net/core/skbuff.o | grep -C7 tc_from_ingress
> 
> 
>         __u8                       ipvs_property:1;      /*   147: 7  1 */
>         __u8                       inner_protocol_type:1; /*   147: 6  1 */
>         __u8                       remcsum_offload:1;    /*   147: 5  1 */
>         __u8                       offload_fwd_mark:1;   /*   147: 4  1 */
>         __u8                       tc_skip_classify:1;   /*   147: 3  1 */
>         __u8                       tc_at_ingress:1;      /*   147: 2  1 */
>         __u8                       tc_redirected:1;      /*   147: 1  1 */
>         __u8                       tc_from_ingress:1;    /*   147: 0  1 */
>         __u16                      tc_index;             /*   148     2 */
> 
>         /* XXX 2 bytes hole, try to pack */
> 
>         union {
>                 __wsum             csum;                 /*           4 */
>                 struct {
> 
> apparently there are no more spare bits to use at that offset: therefore,
> adding 'offload_mr_fwd_mark' before 'tc_skip_classify' will make
> 'tc_from_ingress' slip at offset 148, and tc_index at offset 150.
> I think you can use that 2-bytes hole below tc_index, and also move the
> offload_fwd_mark bit there, as we use both when
> CONFIG_NET_SWITCHDEV is
> enabled. This way we will also gain one spare bit, without changing the
> struct size or worsening the cacheline alignments.
> 
> what do you think?

Your pahole output still shows a 2B hole until the following union
which is 4B-aligned.
While it's true tc_index moves to offset 150, the union will not move 
[I.e., stay at offset 152] so the layout doesn't really change [greatly]
nor the size of the struct. And we have the benefit of all the bits
remaining consecutive.