netdev - RE: Alignment issues with freescale FEC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 24 Sep 2016 05:13:45 +0000
From:   Andy Duan <fugang.duan@....com>
To:     David Miller <davem@...emloft.net>,
        "eric@...int.com" <eric@...int.com>
CC:     "andrew@...n.ch" <andrew@...n.ch>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "otavio@...ystems.com.br" <otavio@...ystems.com.br>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "troy.kisky@...ndarydevices.com" <troy.kisky@...ndarydevices.com>,
        "rmk+kernel@....linux.org.uk" <rmk+kernel@....linux.org.uk>,
        "cjb.sw.nospam@...il.com" <cjb.sw.nospam@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: Alignment issues with freescale FEC driver

From: David Miller <davem@...emloft.net> Sent: Saturday, September 24, 2016 10:46 AM
> To: eric@...int.com
> Cc: andrew@...n.ch; edumazet@...gle.com; Andy Duan
> <fugang.duan@....com>; otavio@...ystems.com.br;
> netdev@...r.kernel.org; troy.kisky@...ndarydevices.com;
> rmk+kernel@....linux.org.uk; cjb.sw.nospam@...il.com; linux-arm-
> kernel@...ts.infradead.org
> Subject: Re: Alignment issues with freescale FEC driver
> 
> From: Eric Nelson <eric@...int.com>
> Date: Fri, 23 Sep 2016 11:35:17 -0700
> 
> > From the i.MX6DQ reference manual, bit 7 of ENET_RACC says this:
> >
> > "RX FIFO Shift-16
> >
> > When this field is set, the actual frame data starts at bit 16 of the
> > first word read from the RX FIFO aligning the Ethernet payload on a
> > 32-bit boundary."
> >
> > Same for the i.MX6UL.
> >
> > I'm not sure what it will take to use this, but it seems to be exactly
> > what we're looking for.
> 
> +1

RACC[SHIFT16] just instructs the MAC to write two additional bytes in front of each frame received into the RX FIFO to align
the Ethernet payload on a 32-bit boundary.
Eric's patch "net: fec: support RRACC_SHIFT16 to align IP header" works fine.

For the alignment issues, that is introduced by commit 1b7bde6d6 and c259c132a in net-next tree. Before these commits, no alignment issue.

How to fix the issue:
Solution1:  to enable HW RRACC_SHIFT16 feature (test pass):
	Eric's patch  "net: fec: support RRACC_SHIFT16 to align IP header".
Solution2: include the correct prefetch() header (test pass):
	--- a/drivers/net/ethernet/freescale/fec_main.c
	+++ b/drivers/net/ethernet/freescale/fec_main.c
	@@ -59,7 +59,7 @@
	 #include <linux/pinctrl/consumer.h>
 	#include <linux/pm_runtime.h>
 	#include <linux/busfreq-imx.h>
	-#include <linux/prefetch.h>
	+#include <asm/processor.h>
Solution3: use __netdev_alloc_skb_ip_align() instead of netdev_alloc_skb(). 
	     Or: still use the previous method before commit 1b7bde6d6:
		skb = netdev_alloc_skb(ndev, pkt_len - 4 + NET_IP_ALIGN);
		skb_reserve(skb, NET_IP_ALIGN);

Comparing these solutions:
	From sw effort and performance, I think these are the similar.  Enable RRACC_SHIFT16 doesn't take extra advantage.

Correct my if I am wrong. Thanks.

Regards,
Andy