[U-Boot] [PATCH] net: Mark the ip_udp_hdr struct as packed

[Jeroen Hofstee]

Hello Siarhei,


On 07/21/2017 08:46 PM, Siarhei Siamashka wrote:
> On Wed, 19 Jul 2017 20:26:54 +0200
> Jeroen Hofstee <jeroen at myspectrum.nl> wrote:
>
>> Hi,
>>
>>
>> On 07/18/2017 08:10 PM, Joe Hershberger wrote:
>>> Hi Maxime,
>>>
>>> On Wed, Jul 12, 2017 at 9:34 AM, Maxime Ripard
>>> <maxime.ripard at free-electrons.com> wrote:
>>>> The -mno-unaligned-access flag used on ARM to prevent GCC from generating
>>>> unaligned accesses (obviously) will only do so on packed structures.
>>>>
>>>> It seems like gcc 7.1 is a bit stricter than previous gcc versions on this,
>>>> and using it lead to data abort for unaligned accesses when generating
>>>> network traffic.
>>>>
>>>> Fix this by adding the packed attribute to the ip_udp_hdr structure in
>>>> order to let GCC do its job.
>>>>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard at free-electrons.com>
>>>> ---
>>>>    include/net.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net.h b/include/net.h
>>>> index 997db9210a8f..7b815afffafa 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));
>>> Do you have an example of why this is unaligned? It seems that the
>>> structure itself is naturally packed (each element is aligned to its
>>> access size). It seems the only time this would hit a dabort is if the
>>> head of the buffer is not 32-bit aligned. Maybe we should address the
>>> place where that is the case instead of forcing byte-wise accesses in
>>> general for this structure?
>> |Perhaps __attribute__((aligned(2))) can prevent byte wise accesses?
>> Regards, Jeroen |
> https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html
>
> It says that "The aligned attribute can only increase alignment".
>

yes, __attribute__((packed, aligned(2))); would be needed it seems.
To first force alignment of 1 (which packed does) and thereafter increase
it to 2 again.

That _might_ prevent the 32 bit stores, and use 16 bit ones instead where
possible / not forcing byte accesses everywhere. Completely untested 
though ;)

Regards,
Jeroen