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

[Maxime Ripard]

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.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170719/c1115a4c/attachment.sig>