[U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
Heinrich Schuchardt
xypron.glpk at gmx.de
Sat Oct 12 13:29:03 UTC 2019
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
> 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
>
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich
More information about the U-Boot
mailing list