Thanks Eric,

On 09/23/2016 09:54 AM, Eric Dumazet wrote:

On Fri, Sep 23, 2016 at 9:43 AM, Eric Nelson [off-list ref] wrote:

quoted

Hello all,

We're seeing alignment issues from the ethernet stack on an i.MX6UL board:

<snip>

quoted

- id = ntohl(*(__be32 *)&iph->id);
- flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (id & ~IP_DF));
- id >>= 16;
+ id = ntohs(*(__be16 *)&iph->id);
+ frag = ntohs(*(__be16 *)&iph->frag_off);
+ flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (frag &
~IP_DF));

for (p = *head; p; p = p->next) {
struct iphdr *iph2;

This solves nothing, because a few lines after you'll have yet another
unaligned access :

Oddly, it does prevent the vast majority (90%+) of the alignment errors.

I believe this is because the compiler is generating an ldm instruction
when the ntohl() call is used, but I'm stumped about why these aren't
generating faults:

((__force u32)iph->saddr ^ (__force u32)iph2->saddr) |
((__force u32)iph->daddr ^ (__force u32)iph2->daddr)) {

So you might have one less problematic access, out of hundreds of them
all over the places.

Really the problem is that whole stack depends on the assumption that
IP headers are aligned on arches that care
(ie where NET_IP_ALIGN == 2)

If your build does have NET_IP_ALIGN = 2 and you get a fault here, it
might be because of a buggy driver.

NET_IP_ALIGN is set to 2.

The other known case is some GRE encapsulations that break the
assumption, and this is discussed somewhere else.

I don't think that's the case.

# CONFIG_IPV6_GRE is not set

Hmm... Instrumenting the kernel, it seems that iphdr **is** aligned on
a 4-byte boundary.

Does the ldm instruction require 8-byte alignment?

There's definitely a compiler-version dependency involved here,
since using gcc 4.9 also reduced the number of faults dramatically.