On 10/05/2012 08:51 AM, Mikael Pettersson wrote: > Rob Herring writes: > > On 10/05/2012 03:24 AM, Russell King - ARM Linux wrote: > > > On Fri, Oct 05, 2012 at 09:20:56AM +0100, Mans Rullgard wrote: > > >> On 5 October 2012 08:12, Russell King - ARM Linux > > >> <li...@arm.linux.org.uk> wrote: > > >>> On Fri, Oct 05, 2012 at 03:25:16AM +0100, Mans Rullgard wrote: > > >>>> On 5 October 2012 02:56, Rob Herring <robherri...@gmail.com> wrote: > > >>>>> This struct is the IP header, so a struct ptr is just set to the > > >>>>> beginning of the received data. Since ethernet headers are 14 bytes, > > >>>>> often the IP header is not aligned unless the NIC can place the > frame at > > >>>>> a 2 byte offset (which is something I need to investigate). So this > > >>>>> function cannot make any assumptions about the alignment. Does the > ABI > > >>>>> define structs have some minimum alignment? Does the struct need to > be > > >>>>> declared as packed or something? > > >>>> > > >>>> The ABI defines the alignment of structs as the maximum alignment of > its > > >>>> members. Since this struct contains 32-bit members, the alignment > for the > > >>>> whole struct becomes 32 bits as well. Declaring it as packed tells > gcc it > > >>>> might be unaligned (in addition to removing any holes within). > > >>> > > >>> This has come up before in the past. > > >>> > > >>> The Linux network folk will _not_ allow - in any shape or form - for > > >>> this struct to be marked packed (it's the struct which needs to be > > >>> marked packed) because by doing so, it causes GCC to issue byte loads/ > > >>> stores on architectures where there isn't a problem, and that decreases > > >>> the performance of the Linux IP stack unnecessarily. > > >> > > >> Which architectures? I have never seen anything like that. > > > > > > Does it matter? I'm just relaying the argument against adding __packed > > > which was used before we were forced (by the networking folk) to > implement > > > the alignment fault handler. > > > > It doesn't really matter what will be accepted or not as adding __packed > > to struct iphdr doesn't fix the problem anyway. gcc still emits a ldm. > > The only way I've found to eliminate the alignment fault is adding a > > barrier between the 2 loads. That seems like a compiler issue to me if > > there is not a better fix. > > If you suspect a GCC bug, please prepare a standalone user-space test case > and submit it to GCC's bugzilla (I can do the latter if you absolutely do not > want to). It wouldn't be the first alignment-related GCC bug... >
Here's a testcase. Compiled on ubuntu precise with "arm-linux-gnueabihf-gcc -O2 -marm -march=armv7-a test.c". typedef unsigned short u16; typedef unsigned short __sum16; typedef unsigned int __u32; typedef unsigned char __u8; typedef __u32 __be32; typedef u16 __be16; struct iphdr { __u8 ihl:4, version:4; __u8 tos; __be16 tot_len; __be16 id; __be16 frag_off; __u8 ttl; __u8 protocol; __sum16 check; __be32 saddr; __be32 daddr; /*The options start here. */ }; #define ntohl(x) __swab32((__u32)(__be32)(x)) #define IP_DF 0x4000 /* Flag: "Don't Fragment" */ static inline __attribute__((const)) __u32 __swab32(__u32 x) { __asm__ ("rev %0, %1" : "=r" (x) : "r" (x)); return x; } int main(void * buffer, unsigned int *p_id) { unsigned int id; int flush = 1; const struct iphdr *iph = buffer; __u32 len = *p_id; id = ntohl(*(__be32 *)&iph->id); flush = (u16)((ntohl(*(__be32 *)iph) ^ len) | (id ^ IP_DF)); id >>= 16; *p_id = id; return flush; } _______________________________________________ linaro-toolchain mailing list linaro-toolchain@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-toolchain