| 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: <e34bcab1-303e-a4bd-862c-125f254e93d3@marvell.com>
Date: Mon, 27 Apr 2020 18:38:21 +0300
From: Igor Russkikh <irusskikh@...vell.com>
To: David Miller <davem@...emloft.net>
CC: <kuba@...nel.org>, <netdev@...r.kernel.org>,
Mark Starovoytov <mstarovoitov@...vell.com>,
Dmitry Bogdanov <dbogdanov@...vell.com>
Subject: Re: [EXT] Re: [PATCH net-next 08/17] net: atlantic: A2
driver-firmware interface
>>>
>>>> On Fri, 24 Apr 2020 10:27:20 +0300 Igor Russkikh wrote:
>>>>> +/* Start of HW byte packed interface declaration */
>>>>> +#pragma pack(push, 1)
>>>>
>>>> Does any structure here actually require packing?
>>>
>>> Yes, please use the packed attribute as an absolute _last_ resort.
>>
>> These are HW bit-mapped layout API, without packing compiler may screw
> up
>> alignments in some of these structures.
>
> The compiler will not do that if you used fixed sized types properly.
>
> Please remove __packed unless you can prove it matters.
Just double checked the layout without packed pragma, below is what pahole
diff gives just on one structure.
Compiler does obviously insert cache optimization holes without pragmas.
And since these structures are HW mapped API - this all will not work without
pack(1).
$ diff -u3 packed.txt packednon.txt
@@ -15073,51 +15074,48 @@
u32 rsvd:2; /* 176:24 4 */
u32 echo_max_len:16; /* 176: 8 4 */
- /* Bitfield combined with next fields */
+ /* XXX 8 bits hole, try to pack */
- u32 ipv4[8]; /* 179 32 */
- /* --- cacheline 3 boundary (192 bytes) was 19 bytes ago --- */
- u32 reserved[8]; /* 211 32 */
- } ipv4_offload; /* 176 67 */
+ u32 ipv4[8]; /* 180 32 */
+ /* --- cacheline 3 boundary (192 bytes) was 20 bytes ago --- */
+ u32 reserved[8]; /* 212 32 */
+ } ipv4_offload; /* 176 68 */
struct {
- u32 ns_responder:1; /* 243:31 4 */
- u32 echo_responder:1; /* 243:30 4 */
- u32 mld_client:1; /* 243:29 4 */
- u32 echo_truncate:1; /* 243:28 4 */
- u32 address_guard:1; /* 243:27 4 */
- u32 rsvd:3; /* 243:24 4 */
- u32 echo_max_len:16; /* 243: 8 4 */
-
- /* Bitfield combined with next fields */
-
- u32 ipv6[16][4]; /* 246 256 */
- } ipv6_offload; /* 243 259 */
- /* --- cacheline 7 boundary (448 bytes) was 54 bytes ago --- */
+ u32 ns_responder:1; /* 244:31 4 */
+ u32 echo_responder:1; /* 244:30 4 */
+ u32 mld_client:1; /* 244:29 4 */
+ u32 echo_truncate:1; /* 244:28 4 */
+ u32 address_guard:1; /* 244:27 4 */
+ u32 rsvd:3; /* 244:24 4 */
+ u32 echo_max_len:16; /* 244: 8 4 */
+
+ /* XXX 8 bits hole, try to pack */
+
+ u32 ipv6[16][4]; /* 248 256 */
+ } ipv6_offload; /* 244 260 */
+ /* --- cacheline 7 boundary (448 bytes) was 56 bytes ago --- */
struct {
- u16 ports[16]; /* 502 32 */
- } tcp_port_offload; /* 502 32 */
- /* --- cacheline 8 boundary (512 bytes) was 22 bytes ago --- */
+ u16 ports[16]; /* 504 32 */
+ } tcp_port_offload; /* 504 32 */
+ /* --- cacheline 8 boundary (512 bytes) was 24 bytes ago --- */
struct {
- u16 ports[16]; /* 534 32 */
- } udp_port_offload; /* 534 32 */
- struct ka4_offloads_s ka4_offload; /* 566 712 */
- /* --- cacheline 19 boundary (1216 bytes) was 62 bytes ago --- */
- struct ka6_offloads_s ka6_offload; /* 1278 1096 */
- /* --- cacheline 37 boundary (2368 bytes) was 6 bytes ago --- */
+ u16 ports[16]; /* 536 32 */
+ } udp_port_offload; /* 536 32 */
+ struct ka4_offloads_s ka4_offload; /* 568 712 */
+ /* --- cacheline 20 boundary (1280 bytes) --- */
+ struct ka6_offloads_s ka6_offload; /* 1280 1096 */
+ /* --- cacheline 37 boundary (2368 bytes) was 8 bytes ago --- */
struct {
- u32 rr_count; /* 2374 4 */
- u32 rr_buf_len; /* 2378 4 */
- u32 idx_offset; /* 2382 4 */
- u32 rr__offset; /* 2386 4 */
- } mdns; /* 2374 16 */
-
- /* Bitfield combined with next fields */
+ u32 rr_count; /* 2376 4 */
+ u32 rr_buf_len; /* 2380 4 */
+ u32 idx_offset; /* 2384 4 */
+ u32 rr__offset; /* 2388 4 */
+ } mdns; /* 2376 16 */
+ u32 reserve_fw_gap:16; /* 2392:16 4 */
- u32 reserve_fw_gap:16; /* 2388: 0 4 */
-
- /* size: 2392, cachelines: 38, members: 9 */
+ /* size: 2396, cachelines: 38, members: 9 */
/* bit_padding: 16 bits */
- /* last cacheline: 24 bytes */
+ /* last cacheline: 28 bytes */
};
Regards,
Igor
Powered by blists - more mailing lists