Re: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding issue.

On 12/05/2017 12:06 AM, Claudiu Manoil wrote:

quoted

-----Original Message-----
From: Zumeng Chen [mailto:zumeng.chen@gmail.com]
Sent: Monday, December 04, 2017 5:22 AM
To: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net
Subject: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding
issue.

According to LS1021A RM, the value of PAL can be set so that the start of the
IP header in the receive data buffer is aligned to a 32-bit boundary. Normally,
setting PAL = 2 provides minimal padding to ensure such alignment of the IP
header.

However every incoming packet's 8-byte time stamp will be inserted into the
packet data buffer as padding alignment bytes when hardware time stamping is
enabled.

So we set the padding 8+2 here to avoid the flooded alignment faults:

root@128:~# cat /proc/cpu/alignment
User:           0
System:         17539 (inet_gro_receive+0x114/0x2c0)
Skipped:        0
Half:           0
Word:           0
DWord:          0
Multi:          17539
User faults:    2 (fixup)

[...]

quoted

drivers/net/ethernet/freescale/gianfar.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c

b/drivers/net/ethernet/freescale/gianfar.c
index e616b71..e47945f 100644

--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c

@@ -1413,9 +1413,11 @@ static int gfar_probe(struct platform_device *ofdev)

	gfar_init_addr_hash_table(priv);

-	/* Insert receive time stamps into padding alignment bytes */
+	/* Insert receive time stamps into padding alignment bytes, and
+	 * plus 2 bytes padding to ensure the cpu alignment.
+	 */
	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
-		priv->padding = 8;
+		priv->padding = 8 + DEFAULT_PADDING;

	if (dev->features & NETIF_F_IP_CSUM ||
	    priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
--
2.5.0

Why handle only the rx timestamp path (HAS_TIMER) when the issue,

Sorry, missed this one. Because the mis-alignment is only been
seen in gfar_clean_rx_ring from padding issue so far.

  as presented
by you, should be applicable to the default path as well?

The code change according to the patch description should be more likely:

+       priv->padding = DEFAULT_PADDING;
         /* Insert receive time stamps into padding alignment bytes */
         if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
-               priv->padding = 8;
+               priv->padding += 8;

I just did a sanity testing on your change, it's OK.

root@1021:~# cat /proc/cpu/alignment
User:        0
System:        0 (  (null))
Skipped:    0
Half:        0
Word:        0
DWord:        0
Multi:        0
User faults:    2 (fixup)

Do you have any performance numbers for this change?

Thanks,
Claudiu