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

[Joe Hershberger]

On Wed, Jul 19, 2017 at 2:01 AM, Maxime Ripard
<maxime.ripard at free-electrons.com> wrote:
> Hi Joe,
>
> On Tue, Jul 18, 2017 at 01:10:59PM -0500, Joe Hershberger wrote:
>> 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?
>
> You can have the discussion that led to this patch in "Data abort with
> gcc 7.1", started a week ago.
>
>> It seems that the structure itself is naturally packed (each element
>> is aligned to its access size).
>
> That's true.
>
>> 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?
>
> That's exactly what happens, the pointer to the ip_up_hdr is not
> aligned on 32 bits, and triggers an alignment error.
>
> However, I'm not sure how feasible it would be to align the IP packets
> on 32-bits, since the Ethernet header is only 14 bytes, right?  We
> could use a bounce buffer for each packet, but that's not really
> optimized either.

Yeah, good point.

> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com