From: Eric Nelson <eric@nelint.com> To: Eric Dumazet <edumazet@google.com> Cc: "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, rmk+kernel@arm.linux.org.uk, Fugang Duan <fugang.duan@nxp.com>, Troy Kisky <troy.kisky@boundarydevices.com>, Otavio Salvador <otavio@ossystems.com.br>, Simone <cjb.sw.nospam@gmail.com> Subject: Re: Alignment issues with freescale FEC driver Date: Fri, 23 Sep 2016 10:19:50 -0700 [thread overview] Message-ID: <5ee28ee0-cf0c-bdab-1271-f17755365c13@nelint.com> (raw) In-Reply-To: <CANn89iLb+QJu+MYh859oAAjcLCsKRpBbzVf43yOdq+HJVtZDpQ@mail.gmail.com> Thanks Eric, On 09/23/2016 09:54 AM, Eric Dumazet wrote: > On Fri, Sep 23, 2016 at 9:43 AM, Eric Nelson <eric@nelint.com> wrote: >> >> Hello all, >> >> We're seeing alignment issues from the ethernet stack on an i.MX6UL board: >> >> <snip> >> >> - 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.
WARNING: multiple messages have this Message-ID (diff)
From: eric@nelint.com (Eric Nelson) To: linux-arm-kernel@lists.infradead.org Subject: Alignment issues with freescale FEC driver Date: Fri, 23 Sep 2016 10:19:50 -0700 [thread overview] Message-ID: <5ee28ee0-cf0c-bdab-1271-f17755365c13@nelint.com> (raw) In-Reply-To: <CANn89iLb+QJu+MYh859oAAjcLCsKRpBbzVf43yOdq+HJVtZDpQ@mail.gmail.com> Thanks Eric, On 09/23/2016 09:54 AM, Eric Dumazet wrote: > On Fri, Sep 23, 2016 at 9:43 AM, Eric Nelson <eric@nelint.com> wrote: >> >> Hello all, >> >> We're seeing alignment issues from the ethernet stack on an i.MX6UL board: >> >> <snip> >> >> - 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.
next prev parent reply other threads:[~2016-09-23 17:19 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-09-23 16:43 Alignment issues with freescale FEC driver Eric Nelson 2016-09-23 16:43 ` Eric Nelson 2016-09-23 16:54 ` Eric Dumazet 2016-09-23 16:54 ` Eric Dumazet 2016-09-23 17:19 ` Eric Nelson [this message] 2016-09-23 17:19 ` Eric Nelson 2016-09-23 17:33 ` Eric Nelson 2016-09-23 17:33 ` Eric Nelson 2016-09-23 18:13 ` Andrew Lunn 2016-09-23 18:13 ` Andrew Lunn 2016-09-23 18:30 ` Russell King - ARM Linux 2016-09-23 18:30 ` Russell King - ARM Linux 2016-09-23 18:39 ` Eric Nelson 2016-09-23 18:39 ` Eric Nelson 2016-09-23 18:35 ` Eric Nelson 2016-09-23 18:35 ` Eric Nelson 2016-09-24 2:45 ` David Miller 2016-09-24 2:45 ` David Miller 2016-09-24 5:13 ` Andy Duan 2016-09-24 5:13 ` Andy Duan 2016-09-24 14:42 ` [PATCH 0/3] net: fec: updates to align IP header Eric Nelson 2016-09-24 14:42 ` [PATCH 1/3] net: fec: remove QUIRK_HAS_RACC from i.mx25 Eric Nelson 2016-09-24 14:42 ` [PATCH 2/3] net: fec: remove QUIRK_HAS_RACC from i.mx27 Eric Nelson 2016-09-24 14:42 ` [PATCH 3/3] net: fec: align IP header in hardware Eric Nelson 2016-09-26 9:26 ` David Laight 2016-09-26 18:39 ` Eric Nelson 2016-09-28 16:42 ` David Laight 2016-09-28 17:14 ` Eric Nelson 2016-09-28 17:25 ` Russell King - ARM Linux 2016-09-28 18:01 ` Eric Nelson 2016-09-29 11:07 ` David Laight 2016-09-30 13:27 ` Eric Nelson 2016-09-30 13:49 ` David Laight 2016-09-30 14:16 ` Eric Nelson 2016-10-01 19:52 ` Russell King - ARM Linux 2016-10-03 16:42 ` David Laight 2016-10-03 18:48 ` Eric Nelson 2016-10-08 2:44 ` Andy Duan 2016-09-24 15:09 ` [PATCH 0/3] net: fec: updates to align IP header Andy Duan 2016-09-24 15:29 ` Eric Nelson 2016-09-27 11:40 ` David Miller 2016-09-24 2:43 ` Alignment issues with freescale FEC driver David Miller 2016-09-24 2:43 ` David Miller 2016-09-24 12:27 ` Eric Nelson 2016-09-24 12:27 ` Eric Nelson 2016-09-23 17:37 ` Russell King - ARM Linux 2016-09-23 17:37 ` Russell King - ARM Linux 2016-09-23 18:26 ` Eric Nelson 2016-09-23 18:26 ` Eric Nelson 2016-09-23 18:37 ` Russell King - ARM Linux 2016-09-23 18:37 ` Russell King - ARM Linux 2016-09-23 18:49 ` Eric Nelson 2016-09-23 18:49 ` Eric Nelson 2016-09-23 20:22 ` Uwe Kleine-König 2016-09-23 20:22 ` Uwe Kleine-König
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5ee28ee0-cf0c-bdab-1271-f17755365c13@nelint.com \ --to=eric@nelint.com \ --cc=cjb.sw.nospam@gmail.com \ --cc=edumazet@google.com \ --cc=fugang.duan@nxp.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=netdev@vger.kernel.org \ --cc=otavio@ossystems.com.br \ --cc=rmk+kernel@arm.linux.org.uk \ --cc=troy.kisky@boundarydevices.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.