netdev - RE: [EXT] Re: [net PATCH] octeontx2-af: Fix marking couple of structure as _

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: 
 <SJ0PR18MB521616A149B7A776F356C4E6DB97A@SJ0PR18MB5216.namprd18.prod.outlook.com>
Date: Tue, 19 Dec 2023 14:22:53 +0000
From: Suman Ghosh <sumang@...vell.com>
To: Jacob Keller <jacob.e.keller@...el.com>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Subbaraya Sundeep Bhatta
	<sbhatta@...vell.com>,
        Jerin Jacob Kollanukkaran <jerinj@...vell.com>,
        Geethasowjanya Akula <gakula@...vell.com>,
        Hariprasad Kelam
	<hkelam@...vell.com>,
        Linu Cherian <lcherian@...vell.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] Re: [net PATCH] octeontx2-af: Fix marking couple of
 structure as __packed

>Not sure I follow why lack of __packed would have performance
>implications? I get that __packed is important to ensure layout is
>correct or to ensure the whole structure has the right size rather than
>unexpected gaps. I'd guess maybe because the structures size would
>include padding without __packed, leading to a lot of gaps when
>combining several structures together...
>
>I did test on my system with pahole, and even without __packed, I don't
>get any gaps in the npc_lt_def_cfg structure:
>
>
>> struct npc_lt_def_cfg {
>>         struct npc_lt_def          rx_ol2;               /*     0
>3 */
>>         struct npc_lt_def          rx_oip4;              /*     3
>3 */
>>         struct npc_lt_def          rx_iip4;              /*     6
>3 */
>>         struct npc_lt_def          rx_oip6;              /*     9
>3 */
>>         struct npc_lt_def          rx_iip6;              /*    12
>3 */
>>         struct npc_lt_def          rx_otcp;              /*    15
>3 */
>>         struct npc_lt_def          rx_itcp;              /*    18
>3 */
>>         struct npc_lt_def          rx_oudp;              /*    21
>3 */
>>         struct npc_lt_def          rx_iudp;              /*    24
>3 */
>>         struct npc_lt_def          rx_osctp;             /*    27
>3 */
>>         struct npc_lt_def          rx_isctp;             /*    30
>3 */
>>         struct npc_lt_def_ipsec    rx_ipsec[2];          /*    33
>10 */
>>         struct npc_lt_def          pck_ol2;              /*    43
>3 */
>>         struct npc_lt_def          pck_oip4;             /*    46
>3 */
>>         struct npc_lt_def          pck_oip6;             /*    49
>3 */
>>         struct npc_lt_def          pck_iip4;             /*    52
>3 */
>>         struct npc_lt_def_apad     rx_apad0;             /*    55
>4 */
>>         struct npc_lt_def_apad     rx_apad1;             /*    59
>4 */
>>         struct npc_lt_def_color    ovlan;                /*    63
>5 */
>>         /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */
>>         struct npc_lt_def_color    ivlan;                /*    68
>5 */
>>         struct npc_lt_def_color    rx_gen0_color;        /*    73
>5 */
>>         struct npc_lt_def_color    rx_gen1_color;        /*    78
>5 */
>>         struct npc_lt_def_et       rx_et[2];             /*    83
>10 */
>>
>>         /* size: 93, cachelines: 2, members: 23 */
>>         /* last cacheline: 29 bytes */ };
>
>
>However that may not be true across all compilers etc. Also all the
>other structures are __packed. Makes sense.
>
>Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
>
[Suman] I agree, "having performance impact" is a wrong statement. I will update the same in v2.