Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1302262 > unrolled thread

[PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding

Started byJarod Wilson <jarod@redhat.com>
First post2016-01-06 00:20 +0100
Last post2016-01-06 21:10 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding Jarod Wilson <jarod@redhat.com> - 2016-01-06 00:20 +0100
    Re: [PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing  and padding David Miller <davem@davemloft.net> - 2016-01-06 07:30 +0100
      [PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer sizing and padding Jarod Wilson <jarod@redhat.com> - 2016-01-06 15:40 +0100
        Re: [PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer  sizing and padding David Miller <davem@davemloft.net> - 2016-01-06 21:10 +0100

#1302262 — [PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding

FromJarod Wilson <jarod@redhat.com>
Date2016-01-06 00:20 +0100
Subject[PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding
Message-ID<qNJ3s-1f2-11@gated-at.bofh.it>
This is based on the work done by Przemek Rudy in bug 70761 at
bugzilla.kernel.org, but with some work done to disentagle and clarify
things a bit.

Similar to Przemek's work and other drivers, we're adding a padding of 16
here, but we're also disentangling mtu size calculations from max buffer
size calculations a bit, and adding ETH_HLEN to the value written into
ALX_MTU. Hopefully, with a bit more consistency and clarity, things behave
better here. Sadly, I can only test in my alx-driven E2200, which worked
just fine before this patch.

In comment #58 of bug 70761, Eugene A. Shatokhin reports that this patch
does help considerably for a ROSA Linux user of his with an AR8162 network
adapter when patched into a 4.1.x-based kernel, with several days of
normal operation where wired network previously wasn't usable without
setting MTU to 9000 as a work-around. Other users report less substatial
improvements, but no regressions thus far.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70761
CC: "Eugene A. Shatokhin" <eugene.shatokhin@rosalab.ru>
CC: Przemek Rudy <prudy1@o2.pl>
CC: Jay Cliburn <jcliburn@gmail.com>
CC: Chris Snook <chris.snook@gmail.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/ethernet/atheros/alx/hw.c   | 10 +++++-----
 drivers/net/ethernet/atheros/alx/hw.h   |  9 ++++++---
 drivers/net/ethernet/atheros/alx/main.c |  7 +++----
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 7712f06..841bc0d 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -958,13 +958,13 @@ void alx_configure_basic(struct alx_hw *hw)
 	alx_write_mem32(hw, ALX_TINT_TPD_THRSHLD, hw->ith_tpd);
 	alx_write_mem32(hw, ALX_TINT_TIMER, hw->imt);
 
-	raw_mtu = hw->mtu + ETH_HLEN;
-	alx_write_mem32(hw, ALX_MTU, raw_mtu + 8);
-	if (raw_mtu > ALX_MTU_JUMBO_TH)
+	raw_mtu = ALX_RAW_MTU(hw->mtu);
+	alx_write_mem32(hw, ALX_MTU, raw_mtu);
+	if (raw_mtu > (ALX_MTU_JUMBO_TH + ETH_FCS_LEN + VLAN_HLEN))
 		hw->rx_ctrl &= ~ALX_MAC_CTRL_FAST_PAUSE;
 
-	if ((raw_mtu + 8) < ALX_TXQ1_JUMBO_TSO_TH)
-		val = (raw_mtu + 8 + 7) >> 3;
+	if ((raw_mtu) < ALX_TXQ1_JUMBO_TSO_TH)
+		val = (raw_mtu + 7) >> 3;
 	else
 		val = ALX_TXQ1_JUMBO_TSO_TH >> 3;
 	alx_write_mem32(hw, ALX_TXQ1, val | ALX_TXQ1_ERRLGPKT_DROP_EN);
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 1554880..f289c05 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -37,6 +37,7 @@
 #include <linux/types.h>
 #include <linux/mdio.h>
 #include <linux/pci.h>
+#include <linux/if_vlan.h>
 #include "reg.h"
 
 /* Transmit Packet Descriptor, contains 4 32-bit words.
@@ -343,12 +344,14 @@ struct alx_rrd {
 					 ALX_RSS_HASH_TYPE_IPV4_TCP | \
 					 ALX_RSS_HASH_TYPE_IPV6 | \
 					 ALX_RSS_HASH_TYPE_IPV6_TCP)
-#define ALX_DEF_RXBUF_SIZE	1536
+#define ALX_FRAME_PAD		16
+#define ALX_RAW_MTU(_mtu)	(_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
+#define ALX_MAX_FRAME_LEN(_mtu)	(ALIGN((ALX_RAW_MTU(_mtu) + ALX_FRAME_PAD), 8))
+#define ALX_DEF_RXBUF_SIZE	ALX_MAX_FRAME_LEN(1500)
 #define ALX_MAX_JUMBO_PKT_SIZE	(9*1024)
 #define ALX_MAX_TSO_PKT_SIZE	(7*1024)
 #define ALX_MAX_FRAME_SIZE	ALX_MAX_JUMBO_PKT_SIZE
-#define ALX_MIN_FRAME_SIZE	68
-#define ALX_RAW_MTU(_mtu)	(_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
+#define ALX_MIN_FRAME_SIZE	(ETH_ZLEN + ETH_FCS_LEN + VLAN_HLEN)
 
 #define ALX_MAX_RX_QUEUES	8
 #define ALX_MAX_TX_QUEUES	4
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index d3763bc..55b118e 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -704,7 +704,7 @@ static int alx_init_sw(struct alx_priv *alx)
 
 	hw->smb_timer = 400;
 	hw->mtu = alx->dev->mtu;
-	alx->rxbuf_size = ALIGN(ALX_RAW_MTU(hw->mtu), 8);
+	alx->rxbuf_size = ALX_MAX_FRAME_LEN(hw->mtu);
 	alx->tx_ringsz = 256;
 	alx->rx_ringsz = 512;
 	hw->imt = 200;
@@ -805,7 +805,7 @@ static void alx_reinit(struct alx_priv *alx)
 static int alx_change_mtu(struct net_device *netdev, int mtu)
 {
 	struct alx_priv *alx = netdev_priv(netdev);
-	int max_frame = mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+	int max_frame = ALX_MAX_FRAME_LEN(mtu);
 
 	if ((max_frame < ALX_MIN_FRAME_SIZE) ||
 	    (max_frame > ALX_MAX_FRAME_SIZE))
@@ -816,8 +816,7 @@ static int alx_change_mtu(struct net_device *netdev, int mtu)
 
 	netdev->mtu = mtu;
 	alx->hw.mtu = mtu;
-	alx->rxbuf_size = mtu > ALX_DEF_RXBUF_SIZE ?
-			   ALIGN(max_frame, 8) : ALX_DEF_RXBUF_SIZE;
+	alx->rxbuf_size = max(max_frame, ALX_DEF_RXBUF_SIZE);
 	netdev_update_features(netdev);
 	if (netif_running(netdev))
 		alx_reinit(alx);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1302412 — Re: [PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding

FromDavid Miller <davem@davemloft.net>
Date2016-01-06 07:30 +0100
SubjectRe: [PATCH net-next] ethernet/atheros/alx: sanitize buffer sizing and padding
Message-ID<qNPLz-5Qn-5@gated-at.bofh.it>
In reply to#1302262
From: Jarod Wilson <jarod@redhat.com>
Date: Tue,  5 Jan 2016 18:12:34 -0500

> -	if ((raw_mtu + 8) < ALX_TXQ1_JUMBO_TSO_TH)
> -		val = (raw_mtu + 8 + 7) >> 3;
> +	if ((raw_mtu) < ALX_TXQ1_JUMBO_TSO_TH)
> +		val = (raw_mtu + 7) >> 3;

The parenthesis around 'raw_mtu' is superfluous, please
remove it.

Otherwise this looks great, please resubmit with that fixed
up.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302821 — [PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer sizing and padding

FromJarod Wilson <jarod@redhat.com>
Date2016-01-06 15:40 +0100
Subject[PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer sizing and padding
Message-ID<qNXpM-2lM-19@gated-at.bofh.it>
In reply to#1302412
This is based on the work done by Przemek Rudy in bug 70761 at
bugzilla.kernel.org, but with some work done to disentagle and clarify
things a bit.

Similar to Przemek's work and other drivers, we're adding a padding of 16
here, but we're also disentangling mtu size calculations from max buffer
size calculations a bit, and adding ETH_HLEN to the value written into
ALX_MTU. Hopefully, with a bit more consistency and clarity, things behave
better here. Sadly, I can only test in my alx-driven E2200, which worked
just fine before this patch.

In comment #58 of bug 70761, Eugene A. Shatokhin reports that this patch
does help considerably for a ROSA Linux user of his with an AR8162 network
adapter when patched into a 4.1.x-based kernel, with several days of
normal operation where wired network previously wasn't usable without
setting MTU to 9000 as a work-around.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70761
CC: "Eugene A. Shatokhin" <eugene.shatokhin@rosalab.ru>
CC: Przemek Rudy <prudy1@o2.pl>
CC: Jay Cliburn <jcliburn@gmail.com>
CC: Chris Snook <chris.snook@gmail.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
v2: remove superfluous parens around raw_mtu, pointed out by davem

 drivers/net/ethernet/atheros/alx/hw.c   | 10 +++++-----
 drivers/net/ethernet/atheros/alx/hw.h   |  9 ++++++---
 drivers/net/ethernet/atheros/alx/main.c |  7 +++----
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 7712f06..1fe35e4 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -958,13 +958,13 @@ void alx_configure_basic(struct alx_hw *hw)
 	alx_write_mem32(hw, ALX_TINT_TPD_THRSHLD, hw->ith_tpd);
 	alx_write_mem32(hw, ALX_TINT_TIMER, hw->imt);
 
-	raw_mtu = hw->mtu + ETH_HLEN;
-	alx_write_mem32(hw, ALX_MTU, raw_mtu + 8);
-	if (raw_mtu > ALX_MTU_JUMBO_TH)
+	raw_mtu = ALX_RAW_MTU(hw->mtu);
+	alx_write_mem32(hw, ALX_MTU, raw_mtu);
+	if (raw_mtu > (ALX_MTU_JUMBO_TH + ETH_FCS_LEN + VLAN_HLEN))
 		hw->rx_ctrl &= ~ALX_MAC_CTRL_FAST_PAUSE;
 
-	if ((raw_mtu + 8) < ALX_TXQ1_JUMBO_TSO_TH)
-		val = (raw_mtu + 8 + 7) >> 3;
+	if (raw_mtu < ALX_TXQ1_JUMBO_TSO_TH)
+		val = (raw_mtu + 7) >> 3;
 	else
 		val = ALX_TXQ1_JUMBO_TSO_TH >> 3;
 	alx_write_mem32(hw, ALX_TXQ1, val | ALX_TXQ1_ERRLGPKT_DROP_EN);
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 1554880..f289c05 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -37,6 +37,7 @@
 #include <linux/types.h>
 #include <linux/mdio.h>
 #include <linux/pci.h>
+#include <linux/if_vlan.h>
 #include "reg.h"
 
 /* Transmit Packet Descriptor, contains 4 32-bit words.
@@ -343,12 +344,14 @@ struct alx_rrd {
 					 ALX_RSS_HASH_TYPE_IPV4_TCP | \
 					 ALX_RSS_HASH_TYPE_IPV6 | \
 					 ALX_RSS_HASH_TYPE_IPV6_TCP)
-#define ALX_DEF_RXBUF_SIZE	1536
+#define ALX_FRAME_PAD		16
+#define ALX_RAW_MTU(_mtu)	(_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
+#define ALX_MAX_FRAME_LEN(_mtu)	(ALIGN((ALX_RAW_MTU(_mtu) + ALX_FRAME_PAD), 8))
+#define ALX_DEF_RXBUF_SIZE	ALX_MAX_FRAME_LEN(1500)
 #define ALX_MAX_JUMBO_PKT_SIZE	(9*1024)
 #define ALX_MAX_TSO_PKT_SIZE	(7*1024)
 #define ALX_MAX_FRAME_SIZE	ALX_MAX_JUMBO_PKT_SIZE
-#define ALX_MIN_FRAME_SIZE	68
-#define ALX_RAW_MTU(_mtu)	(_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
+#define ALX_MIN_FRAME_SIZE	(ETH_ZLEN + ETH_FCS_LEN + VLAN_HLEN)
 
 #define ALX_MAX_RX_QUEUES	8
 #define ALX_MAX_TX_QUEUES	4
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index d3763bc..55b118e 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -704,7 +704,7 @@ static int alx_init_sw(struct alx_priv *alx)
 
 	hw->smb_timer = 400;
 	hw->mtu = alx->dev->mtu;
-	alx->rxbuf_size = ALIGN(ALX_RAW_MTU(hw->mtu), 8);
+	alx->rxbuf_size = ALX_MAX_FRAME_LEN(hw->mtu);
 	alx->tx_ringsz = 256;
 	alx->rx_ringsz = 512;
 	hw->imt = 200;
@@ -805,7 +805,7 @@ static void alx_reinit(struct alx_priv *alx)
 static int alx_change_mtu(struct net_device *netdev, int mtu)
 {
 	struct alx_priv *alx = netdev_priv(netdev);
-	int max_frame = mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
+	int max_frame = ALX_MAX_FRAME_LEN(mtu);
 
 	if ((max_frame < ALX_MIN_FRAME_SIZE) ||
 	    (max_frame > ALX_MAX_FRAME_SIZE))
@@ -816,8 +816,7 @@ static int alx_change_mtu(struct net_device *netdev, int mtu)
 
 	netdev->mtu = mtu;
 	alx->hw.mtu = mtu;
-	alx->rxbuf_size = mtu > ALX_DEF_RXBUF_SIZE ?
-			   ALIGN(max_frame, 8) : ALX_DEF_RXBUF_SIZE;
+	alx->rxbuf_size = max(max_frame, ALX_DEF_RXBUF_SIZE);
 	netdev_update_features(netdev);
 	if (netif_running(netdev))
 		alx_reinit(alx);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303036 — Re: [PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer sizing and padding

FromDavid Miller <davem@davemloft.net>
Date2016-01-06 21:10 +0100
SubjectRe: [PATCH v2 net-next] ethernet/atheros/alx: sanitize buffer sizing and padding
Message-ID<qO2z7-5S9-11@gated-at.bofh.it>
In reply to#1302821
From: Jarod Wilson <jarod@redhat.com>
Date: Wed,  6 Jan 2016 09:36:37 -0500

> This is based on the work done by Przemek Rudy in bug 70761 at
> bugzilla.kernel.org, but with some work done to disentagle and clarify
> things a bit.
> 
> Similar to Przemek's work and other drivers, we're adding a padding of 16
> here, but we're also disentangling mtu size calculations from max buffer
> size calculations a bit, and adding ETH_HLEN to the value written into
> ALX_MTU. Hopefully, with a bit more consistency and clarity, things behave
> better here. Sadly, I can only test in my alx-driven E2200, which worked
> just fine before this patch.
> 
> In comment #58 of bug 70761, Eugene A. Shatokhin reports that this patch
> does help considerably for a ROSA Linux user of his with an AR8162 network
> adapter when patched into a 4.1.x-based kernel, with several days of
> normal operation where wired network previously wasn't usable without
> setting MTU to 9000 as a work-around.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70761
> CC: "Eugene A. Shatokhin" <eugene.shatokhin@rosalab.ru>
> CC: Przemek Rudy <prudy1@o2.pl>
> CC: Jay Cliburn <jcliburn@gmail.com>
> CC: Chris Snook <chris.snook@gmail.com>
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
> v2: remove superfluous parens around raw_mtu, pointed out by davem

Looks great, applied, thanks Jarod.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web