From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Hershberger Date: Wed, 19 Jul 2017 13:13:44 -0500 Subject: [U-Boot] [PATCH] net: Mark the ip_udp_hdr struct as packed In-Reply-To: <20170719070121.66z7fopxh5juffdi@flea> References: <20170712143428.11953-1-maxime.ripard@free-electrons.com> <20170719070121.66z7fopxh5juffdi@flea> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Jul 19, 2017 at 2:01 AM, Maxime Ripard 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 >> 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 >> > --- >> > 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