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  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:   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