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 linux@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 robherring2@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; }