[U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute

[Heinrich Schuchardt]

On 10/3/19 7:09 PM, Tom Rini wrote:
> On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
>
>> struct ip_udp_hdr is naturally packed. There is no point in adding a
>> __packed attribute. With the attribute the network stack does not compile
>> using GCC 9.2.1:
>>
>> net/net.c: In function ‘net_process_received_packet’:
>> net/net.c:1288:23: error: taking address of packed member of ‘struct
>> ip_udp_hdr’ may result in an unaligned pointer value
>> [-Werror=address-of-packed-member]
>>   1288 |    sumptr = (ushort *)&(ip->udp_src);
>>        |                       ^~~~~~~~~~~~~~
>>
>> So let's remove the attribute.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   include/net.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/net.h b/include/net.h
>> index 75a16e4c8f..bd875b56f5 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -390,7 +390,7 @@ struct ip_udp_hdr {
>>   	u16		udp_dst;	/* UDP destination port		*/
>>   	u16		udp_len;	/* Length of UDP packet		*/
>>   	u16		udp_xsum;	/* Checksum			*/
>> -} __attribute__((packed));
>> +};
>>
>>   #define IP_UDP_HDR_SIZE		(sizeof(struct ip_udp_hdr))
>>   #define UDP_HDR_SIZE		(IP_UDP_HDR_SIZE - IP_HDR_SIZE)
>
> This came from:
> commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
> Author: Denis Pynkin <denis.pynkin at collabora.com>
> Date:   Fri Jul 21 19:28:42 2017 +0300
>
>      net: Use packed structures for networking
>
>      PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
>      by default for '-O2':
>
>      BOOTP broadcast 1
>      data abort
>      pc : [<8ff8bb30>]          lr : [<00004f1f>]
>      reloc pc : [<17832b30>]    lr : [<878abf1f>]
>      sp : 8f558bc0  ip : 00000000     fp : 8ffef5a4
>      r10: 8ffed248  r9 : 8f558ee0     r8 : 8ffef594
>      r7 : 0000000e  r6 : 8ffed700     r5 : 00000000  r4 : 8ffed74e
>      r3 : 00060101  r2 : 8ffed230     r1 : 8ffed706  r0 : 00000ddd
>      Flags: nzcv  IRQs off  FIQs off  Mode SVC_32
>      Resetting CPU ...
>
>      Core reason is usage of structures for network headers without packed
>      attribute.
>
>      Reviewed-by: Yauheni Kaliuta <yauheni.kaliuta at redhat.com>
>      Signed-off-by: Denis Pynkin <denis.pynkin at collabora.com>
>      Acked-by: Joe Hershberger <joe.hershberger at ni.com>
>
> ... so adding a few folks to the list and lets see if patchwork picks up
> this tag:
> Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
>

Thanks for providing this background.

-fstore-merging is enabled for O2 and Os. But it should not change the
program behavior.

The relevant code is in gcc/gimple-ssa-store-merging.c. It has been
completely overhauled since GCC 7.1.

I have seen no problems with DHCP, ping and tFTP on arm and arm64 with
GCC 9.2.1.

Best regards

Heinrich

Since GCC 7.1 some bugs have been fixed, e.g.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492