All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Nelson <eric@nelint.com>
To: "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, edumazet@google.com,
	Fugang Duan <fugang.duan@nxp.com>,
	Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>,
	Simone <cjb.sw.nospam@gmail.com>
Subject: Alignment issues with freescale FEC driver
Date: Fri, 23 Sep 2016 09:43:14 -0700	[thread overview]
Message-ID: <02afb707-65de-5101-a79b-355929c4e00b@nelint.com> (raw)
In-Reply-To: Troy Kisky <troy.kisky@boundarydevices.com>

Hello all,

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

root@mx6ul:~# cat /proc/cpu/alignment
User: 0
System: 470010 (inet_gro_receive+0x104/0x278)

This seems to be related to the ip header alignment, and there
was much discussion in mailing list threads [1] and [2].

In particular, Russell referred to a patch here, but I haven't been
able to find it:
https://lists.linaro.org/pipermail/linaro-toolchain/2012-October/002844.html

Eric Dumazet also suggested a path toward fixing it, but I don't quite
understand the suggestion:
http://www.spinics.net/lists/netdev/msg213166.htm

The immediate problem is addressed by just reading the id and frag_offs
fields in the iphdr structure as shown in this patch:

commit 98810abc911b1286a7e4a2ebdfbad66f12fae19d
Author: Eric Nelson <eric@nelint.com>
Date: Fri Sep 23 08:26:03 2016 -0700

net: ipv4: af_inet: don't read multiple 16-bit iphdr fields as a 32-bit
value

Change-Id: Idc7122c22c13ca078be31907d30ab1c3148ba807
Signed-off-by: Eric Nelson <eric@nelint.com>

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 0cc98b1..c17ef6e 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1301,6 +1301,7 @@ static struct sk_buff **inet_gro_receive(struct
sk_buff **head,
unsigned int hlen;
unsigned int off;
unsigned int id;
+ unsigned int frag;
int flush = 1;
int proto;

@@ -1326,9 +1327,9 @@ static struct sk_buff **inet_gro_receive(struct
sk_buff **head,
if (unlikely(ip_fast_csum((u8 *)iph, 5)))
goto out_unlock;

- 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;


The reading of both fields in one "ntohl" seems obfuscated at best and
certainly worthy of a comment about the optimization but I understand
from other notes that the fundamental problem is that the IP header should
be aligned on a 4-byte boundary and that's not possible without a memcpy.

I'd like to hear suggestions about how we can address this.

Regards,


Eric

[1] - http://www.spinics.net/lists/netdev/msg213114.html
[2] -
https://lists.linaro.org/pipermail/linaro-toolchain/2012-October/002828.html

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 09:43:14 -0700	[thread overview]
Message-ID: <02afb707-65de-5101-a79b-355929c4e00b@nelint.com> (raw)
In-Reply-To: Troy Kisky <troy.kisky@boundarydevices.com>

Hello all,

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

root at mx6ul:~# cat /proc/cpu/alignment
User: 0
System: 470010 (inet_gro_receive+0x104/0x278)

This seems to be related to the ip header alignment, and there
was much discussion in mailing list threads [1] and [2].

In particular, Russell referred to a patch here, but I haven't been
able to find it:
https://lists.linaro.org/pipermail/linaro-toolchain/2012-October/002844.html

Eric Dumazet also suggested a path toward fixing it, but I don't quite
understand the suggestion:
http://www.spinics.net/lists/netdev/msg213166.htm

The immediate problem is addressed by just reading the id and frag_offs
fields in the iphdr structure as shown in this patch:

commit 98810abc911b1286a7e4a2ebdfbad66f12fae19d
Author: Eric Nelson <eric@nelint.com>
Date: Fri Sep 23 08:26:03 2016 -0700

net: ipv4: af_inet: don't read multiple 16-bit iphdr fields as a 32-bit
value

Change-Id: Idc7122c22c13ca078be31907d30ab1c3148ba807
Signed-off-by: Eric Nelson <eric@nelint.com>

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 0cc98b1..c17ef6e 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1301,6 +1301,7 @@ static struct sk_buff **inet_gro_receive(struct
sk_buff **head,
unsigned int hlen;
unsigned int off;
unsigned int id;
+ unsigned int frag;
int flush = 1;
int proto;

@@ -1326,9 +1327,9 @@ static struct sk_buff **inet_gro_receive(struct
sk_buff **head,
if (unlikely(ip_fast_csum((u8 *)iph, 5)))
goto out_unlock;

- 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;


The reading of both fields in one "ntohl" seems obfuscated at best and
certainly worthy of a comment about the optimization but I understand
from other notes that the fundamental problem is that the IP header should
be aligned on a 4-byte boundary and that's not possible without a memcpy.

I'd like to hear suggestions about how we can address this.

Regards,


Eric

[1] - http://www.spinics.net/lists/netdev/msg213114.html
[2] -
https://lists.linaro.org/pipermail/linaro-toolchain/2012-October/002828.html

             reply	other threads:[~2016-09-23 16:43 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-23 16:43 Eric Nelson [this message]
2016-09-23 16:43 ` Alignment issues with freescale FEC driver Eric Nelson
2016-09-23 16:54 ` Eric Dumazet
2016-09-23 16:54   ` Eric Dumazet
2016-09-23 17:19   ` Eric Nelson
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=02afb707-65de-5101-a79b-355929c4e00b@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: link
Be 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.