Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/2] bonding:delete two unused variables
From: Weiping Pan(潘卫平) @ 2011-04-11  8:17 UTC (permalink / raw)
  To: fubar, andy; +Cc: netdev, linux-kernel, Weiping Pan 

I found that variable alb_timer and rlb_interval_counter in struct
alb_bond_info are not used by other codes any more, so delete them.

Weiping Pan(潘卫平) (2):
  bonding:delete unused alb_timer
  bonding:delete unused rlb_interval_counter

 drivers/net/bonding/bond_alb.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

-- 
1.7.4

^ permalink raw reply

* [PATCH 2/2] bonding:delete unused rlb_interval_counter
From: Weiping Pan(潘卫平) @ 2011-04-11  8:17 UTC (permalink / raw)
  To: fubar, andy; +Cc: netdev, linux-kernel, Weiping Pan 
In-Reply-To: <cover.1302509480.git.panweiping3@gmail.com>

Now, alb_bond_info uses rx_ntt,rlb_update_delay_counter and
rlb_update_retry_counter to decide when to call rlb_update_rx_clients().

Signed-off-by: Weiping Pan(潘卫平) <panweiping3@gmail.com>
---
 drivers/net/bonding/bond_alb.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index f2a1d0a..01ed1fb 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -139,7 +139,6 @@ struct alb_bond_info {
 	struct slave		*next_rx_slave;/* next slave to be assigned
 						* to a new rx client for
 						*/
-	u32			rlb_interval_counter;
 	u8			primary_is_promisc;	   /* boolean */
 	u32			rlb_promisc_timeout_counter;/* counts primary
 							     * promiscuity time
-- 
1.7.4

^ permalink raw reply related

* oops during unregister_netdevice interface enslaved to bond
From: Frank Blaschka @ 2011-04-11  9:06 UTC (permalink / raw)
  To: netdev, linux-s390, opurdila, davem, fubar

Hi,

with 2.6.39-rc1/2 I realized and oops in one of our bonding tests.
The test is:
1) enslave netdevice to a bond
2) close the netdevcie
3) hot unplug the netdevice

    <1>[27649.970474] Unable to handle kernel pointer dereference at virtual kernel address           (null)
    <4>[27649.970477] Oops: 0004 [#1] SMP
    <4>[27649.970479] Modules linked in: bonding sunrpc qeth_l2 qeth_l3 binfmt_misc dm_multipath scsi_dh dm_mod ipv6 lcs qeth c
cwgroup [last unloaded: scsi_wait_scan]
    <4>[27649.970488] CPU: 0 Tainted: G        W   2.6.39-rc2.48.x.20110407-s390xgit #1
    <4>[27649.970490] Process kworker/u:1 (pid: 25, task: 000000007ec4c838, ksp: 000000007ec535a8)
    <4>[27649.970493] Krnl PSW : 0704100180000000 000000000055444e (klist_put+0x46/0xd4)
    <4>[27649.970498]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:1 PM:0 EA:3
    <4>[27649.970501] Krnl GPRS: 0000000000000410 07000000ffffffff 0000000000000000 0000000000000001
    <4>[27649.970504]            00000000003e57c6 0000000000000001 000000007bac3d30 000000007bad5005
    <4>[27649.970507]            000000007a2bb000 0000000000000000 0000000000000001 0000000000000000
    <4>[27649.970509]            000000007d3f2c28 00000000005c1230 000000007ec53a98 000000007ec53a58
    <4>[27649.970518] Krnl Code: 0000000000554440: 5710d000             x       %r1,0(%r13)
    <4>[27649.970521]            0000000000554444: e3b090200004 lg      %r11,32(%r9)
    <4>[27649.970524]            000000000055444a: a7280000             lhi     %r2,0
    <4>[27649.970528]           >000000000055444e: ba219000             cs      %r2,%r1,0(%r9)
    <4>[27649.970531]            0000000000554452: 1222         ltr     %r2,%r2
    <4>[27649.970534]            0000000000554454: a774003c             brc     7,5544cc
    <4>[27649.970537]            0000000000554458: b90200aa             ltgr    %r10,%r10
    <4>[27649.970540]            000000000055445c: a784000e             brc     8,554478
    <4>[27649.970542] Call Trace:
    <4>[27649.970543] ([<000000000058a848>] bin_vm_ops+0x28/0xe8)
    <4>[27649.970548]  [<00000000003e57de>] device_del+0x7e/0x1d0
    <4>[27649.970551]  [<00000000004af858>] rollback_registered_many+0x1ac/0x268
    <4>[27649.970554]  [<00000000004af9f2>] rollback_registered+0x52/0x74
    <4>[27649.970556]  [<00000000004afa9e>] unregister_netdevice_queue+0x8a/0xe0
    <4>[27649.970559]  [<00000000004afc40>] unregister_netdev+0x34/0x40
    <4>[27649.970562]  [<000003c001a74cfc>] qeth_l2_remove_device+0xf8/0x120 [qeth_l2]
    <4>[27649.970566]  [<000003c003d87040>] qeth_core_remove_device+0x94/0x180 [qeth]
    <4>[27649.970572]  [<000003c00124c83e>] ccwgroup_remove+0x66/0x74 [ccwgroup]
    <4>[27649.970575]  [<00000000003e8d24>] __device_release_driver+0x7c/0xec
    <4>[27649.970578]  [<00000000003e8dcc>] device_release_driver+0x38/0x48
    <4>[27649.970581]  [<00000000003e87ee>] bus_remove_device+0xca/0xf4
    <4>[27649.970584]  [<00000000003e58b0>] device_del+0x150/0x1d0
    <4>[27649.970587]  [<00000000003e5956>] device_unregister+0x26/0x38
    <4>[27649.970589]  [<000003c00124c7bc>] ccwgroup_ungroup_callback+0x5c/0x78 [ccwgroup]
    <4>[27649.970592]  [<00000000002a3ca0>] sysfs_schedule_callback_work+0x38/0xa8
    <4>[27649.970595]  [<000000000015d1c6>] process_one_work+0x176/0x428
    <4>[27649.970598]  [<0000000000160ec2>] worker_thread+0x17a/0x398
    <4>[27649.970601]  [<0000000000166e2a>] kthread+0xa6/0xb0
    <4>[27649.970603]  [<00000000005614de>] kernel_thread_starter+0x6/0xc
    <4>[27649.970606]  [<00000000005614d8>] kernel_thread_starter+0x0/0xc
    <4>[27649.970609] Last Breaking-Event-Address:
    <4>[27649.970610]  [<0000000000554538>] klist_del+0x4/0xc
    <4>[27649.970613]
    <0>[27649.970614] Kernel panic - not syncing: Fatal exception: panic_on_oops
    <4>[27649.970617] CPU: 0 Tainted: G      D W   2.6.39-rc2.48.x.20110407-s390xgit #1
    <4>[27649.970619] Process kworker/u:1 (pid: 25, task: 000000007ec4c838, ksp: 000000007ec535a8)
    <4>[27649.970622] 000000007ec53700 000000007ec53680 0000000000000002 0000000000000000
    <4>[27649.970625]        000000007ec53720 000000007ec53698 000000007ec53698 000000000055ddae
    <4>[27649.970629]        0000000000000001 0000000000000000 000000007bad5005 0000000000100ebe
    <4>[27649.970632]        000000000000000d 000000000000000c 000000007ec536e8 0000000000000000
    <4>[27649.970636]        0000000000000000 0000000000100a00 000000007ec53680 000000007ec536c0
    <4>[27649.970640] Call Trace:
    <4>[27649.970641] ([<0000000000882408>] die_lock+0x0/0x4)

I bisect the problem down to 2.6.38 development. Commit introduced the problem is:

commit 443457242beb6716b43db4d62fe148eab5515505
Author: Octavian Purdila <opurdila@ixiacom.com>
Date:   Mon Dec 13 12:44:07 2010 +0000

    net: factorize sync-rcu call in unregister_netdevice_many

    Add dev_close_many and dev_deactivate_many to factorize another
    sync-rcu operation on the netdevice unregister path.

    $ modprobe dummy numdummies=10000
    $ ip link set dev dummy* up
    $ time rmmod dummy

    Without the patch           With the patch

    real    0m 24.63s           real    0m 5.15s
    user    0m 0.00s            user    0m 0.00s
    sys     0m 6.05s            sys     0m 5.14s

I don't know if this commit is bad or if it exposes a problem in the bonding code.
Without bonding I'm not able to reproduce the problem. Can anybody help?

Thanks,

Frank

^ permalink raw reply

* [PATCH 2/3] stmmac: fix open funct when exit on error
From: Giuseppe CAVALLARO @ 2011-04-11  9:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro, Shiraz Hashim
In-Reply-To: <1302513406-3758-1-git-send-email-peppe.cavallaro@st.com>

This patch reviews the open function and fixes some
errors when exit with an error state.
It also moves the request_irq after core is initialized
when interrupts are properly masked.

Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac_main.c |   48 ++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 62fa51e..442a7ca 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -780,21 +780,6 @@ static int stmmac_open(struct net_device *dev)
 
 	stmmac_verify_args();
 
-	ret = stmmac_init_phy(dev);
-	if (unlikely(ret)) {
-		pr_err("%s: Cannot attach to PHY (error: %d)\n", __func__, ret);
-		return ret;
-	}
-
-	/* Request the IRQ lines */
-	ret = request_irq(dev->irq, stmmac_interrupt,
-			  IRQF_SHARED, dev->name, dev);
-	if (unlikely(ret < 0)) {
-		pr_err("%s: ERROR: allocating the IRQ %d (error: %d)\n",
-		       __func__, dev->irq, ret);
-		return ret;
-	}
-
 #ifdef CONFIG_STMMAC_TIMER
 	priv->tm = kzalloc(sizeof(struct stmmac_timer *), GFP_KERNEL);
 	if (unlikely(priv->tm == NULL)) {
@@ -813,6 +798,11 @@ static int stmmac_open(struct net_device *dev)
 	} else
 		priv->tm->enable = 1;
 #endif
+	ret = stmmac_init_phy(dev);
+	if (unlikely(ret)) {
+		pr_err("%s: Cannot attach to PHY (error: %d)\n", __func__, ret);
+		goto open_error;
+	}
 
 	/* Create and initialize the TX/RX descriptors chains. */
 	priv->dma_tx_size = STMMAC_ALIGN(dma_txsize);
@@ -821,12 +811,11 @@ static int stmmac_open(struct net_device *dev)
 	init_dma_desc_rings(dev);
 
 	/* DMA initialization and SW reset */
-	if (unlikely(priv->hw->dma->init(priv->ioaddr, priv->plat->pbl,
-					 priv->dma_tx_phy,
-					 priv->dma_rx_phy) < 0)) {
-
+	ret = priv->hw->dma->init(priv->ioaddr, priv->plat->pbl,
+				  priv->dma_tx_phy, priv->dma_rx_phy);
+	if (ret < 0) {
 		pr_err("%s: DMA initialization failed\n", __func__);
-		return -1;
+		goto open_error;
 	}
 
 	/* Copy the MAC addr into the HW  */
@@ -848,6 +837,15 @@ static int stmmac_open(struct net_device *dev)
 	writel(0xffffffff, priv->ioaddr + MMC_HIGH_INTR_MASK);
 	writel(0xffffffff, priv->ioaddr + MMC_LOW_INTR_MASK);
 
+	/* Request the IRQ lines */
+	ret = request_irq(dev->irq, stmmac_interrupt,
+			 IRQF_SHARED, dev->name, dev);
+	if (unlikely(ret < 0)) {
+		pr_err("%s: ERROR: allocating the IRQ %d (error: %d)\n",
+		       __func__, dev->irq, ret);
+		goto open_error;
+	}
+
 	/* Enable the MAC Rx/Tx */
 	stmmac_enable_mac(priv->ioaddr);
 
@@ -878,7 +876,17 @@ static int stmmac_open(struct net_device *dev)
 	napi_enable(&priv->napi);
 	skb_queue_head_init(&priv->rx_recycle);
 	netif_start_queue(dev);
+
 	return 0;
+
+open_error:
+#ifdef CONFIG_STMMAC_TIMER
+	kfree(priv->tm);
+#endif
+	if (priv->phydev)
+		phy_disconnect(priv->phydev);
+
+	return ret;
 }
 
 /**
-- 
1.7.4.2


^ permalink raw reply related

* [PATCH 3/3] stmmac: fix Transmit Underflow error
From: Giuseppe CAVALLARO @ 2011-04-11  9:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro
In-Reply-To: <1302513406-3758-1-git-send-email-peppe.cavallaro@st.com>

On some old MAC chips without COE sometime the
Transmit Underflow error is issued.

The driver aborted all the transmission process
and initialized it from scratch.
This breaks the network activity as raised by Nachiketa
on a SPEAr board.

The patch is to fix this rare underflow event.
The driver will only clear the interrupt and the Tx
DMA will go out the Suspend state as soon as the
descriptor is fetched again.
The driver will continue to bump-up the DMA FIFO threshold
that, indeed, helped somebody to prevent this kind of error
in the past as well.

Reported-by: Nachiketa Prachanda <nprachanda@ncomputing.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac_main.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 442a7ca..ba9daec 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -749,7 +749,6 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
 			priv->hw->dma->dma_mode(priv->ioaddr, tc, SF_DMA_MODE);
 			priv->xstats.threshold = tc;
 		}
-		stmmac_tx_err(priv);
 	} else if (unlikely(status == tx_hard_error))
 		stmmac_tx_err(priv);
 }
-- 
1.7.4.2


^ permalink raw reply related

* [PATCH 1/3] stmmac: fixed dma lib build when turn-on the debug option
From: Giuseppe CAVALLARO @ 2011-04-11  9:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro

This patch fixes a compilation error when build the
dwmac_lib with the DEBUG option enabled.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/dwmac_lib.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/stmmac/dwmac_lib.c b/drivers/net/stmmac/dwmac_lib.c
index d65fab1..e250935 100644
--- a/drivers/net/stmmac/dwmac_lib.c
+++ b/drivers/net/stmmac/dwmac_lib.c
@@ -26,9 +26,9 @@
 
 #undef DWMAC_DMA_DEBUG
 #ifdef DWMAC_DMA_DEBUG
-#define DBG(fmt, args...)  printk(fmt, ## args)
+#define DWMAC_LIB_DBG(fmt, args...)  printk(fmt, ## args)
 #else
-#define DBG(fmt, args...)  do { } while (0)
+#define DWMAC_LIB_DBG(fmt, args...)  do { } while (0)
 #endif
 
 /* CSR1 enables the transmit DMA to check for new descriptor */
@@ -152,7 +152,7 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
 	/* read the status register (CSR5) */
 	u32 intr_status = readl(ioaddr + DMA_STATUS);
 
-	DBG(INFO, "%s: [CSR5: 0x%08x]\n", __func__, intr_status);
+	DWMAC_LIB_DBG(KERN_INFO "%s: [CSR5: 0x%08x]\n", __func__, intr_status);
 #ifdef DWMAC_DMA_DEBUG
 	/* It displays the DMA process states (CSR5 register) */
 	show_tx_process_state(intr_status);
@@ -160,43 +160,43 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
 #endif
 	/* ABNORMAL interrupts */
 	if (unlikely(intr_status & DMA_STATUS_AIS)) {
-		DBG(INFO, "CSR5[15] DMA ABNORMAL IRQ: ");
+		DWMAC_LIB_DBG(KERN_INFO "CSR5[15] DMA ABNORMAL IRQ: ");
 		if (unlikely(intr_status & DMA_STATUS_UNF)) {
-			DBG(INFO, "transmit underflow\n");
+			DWMAC_LIB_DBG(KERN_INFO "transmit underflow\n");
 			ret = tx_hard_error_bump_tc;
 			x->tx_undeflow_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_TJT)) {
-			DBG(INFO, "transmit jabber\n");
+			DWMAC_LIB_DBG(KERN_INFO "transmit jabber\n");
 			x->tx_jabber_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_OVF)) {
-			DBG(INFO, "recv overflow\n");
+			DWMAC_LIB_DBG(KERN_INFO "recv overflow\n");
 			x->rx_overflow_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_RU)) {
-			DBG(INFO, "receive buffer unavailable\n");
+			DWMAC_LIB_DBG(KERN_INFO "receive buffer unavailable\n");
 			x->rx_buf_unav_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_RPS)) {
-			DBG(INFO, "receive process stopped\n");
+			DWMAC_LIB_DBG(KERN_INFO "receive process stopped\n");
 			x->rx_process_stopped_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_RWT)) {
-			DBG(INFO, "receive watchdog\n");
+			DWMAC_LIB_DBG(KERN_INFO "receive watchdog\n");
 			x->rx_watchdog_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_ETI)) {
-			DBG(INFO, "transmit early interrupt\n");
+			DWMAC_LIB_DBG(KERN_INFO "transmit early interrupt\n");
 			x->tx_early_irq++;
 		}
 		if (unlikely(intr_status & DMA_STATUS_TPS)) {
-			DBG(INFO, "transmit process stopped\n");
+			DWMAC_LIB_DBG(KERN_INFO "transmit process stopped\n");
 			x->tx_process_stopped_irq++;
 			ret = tx_hard_error;
 		}
 		if (unlikely(intr_status & DMA_STATUS_FBI)) {
-			DBG(INFO, "fatal bus error\n");
+			DWMAC_LIB_DBG(KERN_INFO "fatal bus error\n");
 			x->fatal_bus_error_irq++;
 			ret = tx_hard_error;
 		}
@@ -215,7 +215,7 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
 	/* Clear the interrupt by writing a logic 1 to the CSR5[15-0] */
 	writel((intr_status & 0x1ffff), ioaddr + DMA_STATUS);
 
-	DBG(INFO, "\n\n");
+	DWMAC_LIB_DBG(KERN_INFO "\n\n");
 	return ret;
 }
 
-- 
1.7.4.2


^ permalink raw reply related

* [PATCH 2/4] net: fix tranmitted/tranmitting typo
From: Weiping Pan(潘卫平) @ 2011-04-11 10:15 UTC (permalink / raw)
  Cc: Weiping Pan , Jay Vosburgh, Andy Gospodarek, Lucas De Marchi,
	open list:BONDING DRIVER, open list

replace tranmitted with transmitted.
replace tranmitting with transmitting.

Signed-off-by: Weiping Pan(潘卫平) <panweiping3@gmail.com>
---
 drivers/net/bonding/bond_alb.h      |    2 +-
 drivers/net/tokenring/3c359.c       |    4 ++--
 drivers/net/tokenring/lanstreamer.c |    2 +-
 drivers/net/tokenring/olympic.c     |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index 86861f0..9af19fa 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -75,7 +75,7 @@ struct tlb_client_info {
 				 * gave this entry index.
 				 */
 	u32 tx_bytes;		/* Each Client accumulates the BytesTx that
-				 * were tranmitted to it, and after each
+				 * were transmitted to it, and after each
 				 * CallBack the LoadHistory is divided
 				 * by the balance interval
 				 */
diff --git a/drivers/net/tokenring/3c359.c b/drivers/net/tokenring/3c359.c
index 8a3b191..ff32bef 100644
--- a/drivers/net/tokenring/3c359.c
+++ b/drivers/net/tokenring/3c359.c
@@ -1251,7 +1251,7 @@ static netdev_tx_t xl_xmit(struct sk_buff *skb, struct net_device *dev)
 /* 
  * The NIC has told us that a packet has been downloaded onto the card, we must
  * find out which packet it has done, clear the skb and information for the packet
- * then advance around the ring for all tranmitted packets
+ * then advance around the ring for all transmitted packets
  */
 
 static void xl_dn_comp(struct net_device *dev) 
@@ -1568,7 +1568,7 @@ static void xl_arb_cmd(struct net_device *dev)
 			if (lan_status_diff & LSC_SOFT_ERR)
 					printk(KERN_WARNING "%s: Adapter transmitted Soft Error Report Mac Frame\n",dev->name);
 			if (lan_status_diff & LSC_TRAN_BCN) 
-					printk(KERN_INFO "%s: We are tranmitting the beacon, aaah\n",dev->name);
+					printk(KERN_INFO "%s: We are transmitting the beacon, aaah\n",dev->name);
 			if (lan_status_diff & LSC_SS) 
 					printk(KERN_INFO "%s: Single Station on the ring\n", dev->name);
 			if (lan_status_diff & LSC_RING_REC)
diff --git a/drivers/net/tokenring/lanstreamer.c b/drivers/net/tokenring/lanstreamer.c
index 5bd1407..9354ca9 100644
--- a/drivers/net/tokenring/lanstreamer.c
+++ b/drivers/net/tokenring/lanstreamer.c
@@ -1675,7 +1675,7 @@ drop_frame:
 			if (lan_status_diff & LSC_SOFT_ERR)
 				printk(KERN_WARNING "%s: Adapter transmitted Soft Error Report Mac Frame\n", dev->name);
 			if (lan_status_diff & LSC_TRAN_BCN)
-				printk(KERN_INFO "%s: We are tranmitting the beacon, aaah\n", dev->name);
+				printk(KERN_INFO "%s: We are transmitting the beacon, aaah\n", dev->name);
 			if (lan_status_diff & LSC_SS)
 				printk(KERN_INFO "%s: Single Station on the ring\n", dev->name);
 			if (lan_status_diff & LSC_RING_REC)
diff --git a/drivers/net/tokenring/olympic.c b/drivers/net/tokenring/olympic.c
index 3d2fbe6..2684003 100644
--- a/drivers/net/tokenring/olympic.c
+++ b/drivers/net/tokenring/olympic.c
@@ -1500,7 +1500,7 @@ drop_frame:
 			if (lan_status_diff & LSC_SOFT_ERR)
 					printk(KERN_WARNING "%s: Adapter transmitted Soft Error Report Mac Frame\n",dev->name);
 			if (lan_status_diff & LSC_TRAN_BCN) 
-					printk(KERN_INFO "%s: We are tranmitting the beacon, aaah\n",dev->name);
+					printk(KERN_INFO "%s: We are transmitting the beacon, aaah\n",dev->name);
 			if (lan_status_diff & LSC_SS) 
 					printk(KERN_INFO "%s: Single Station on the ring\n", dev->name);
 			if (lan_status_diff & LSC_RING_REC)
-- 
1.7.4

^ permalink raw reply related

* [PATCH 4/4] bonding:fix two typos
From: Weiping Pan(潘卫平) @ 2011-04-11 10:16 UTC (permalink / raw)
  Cc: Weiping Pan , Jay Vosburgh, Andy Gospodarek,
	open list:BONDING DRIVER, open list

replace relpy with reply.
replace premanent with permanent.

Signed-off-by: Weiping Pan(潘卫平) <panweiping3@gmail.com>
---
 drivers/net/bonding/bond_alb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 9bc5de3..6e176d6 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -701,7 +701,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
 		 */
 		rlb_choose_channel(skb, bond);
 
-		/* The ARP relpy packets must be delayed so that
+		/* The ARP reply packets must be delayed so that
 		 * they can cancel out the influence of the ARP request.
 		 */
 		bond->alb_info.rlb_update_delay_counter = RLB_UPDATE_DELAY;
@@ -1042,7 +1042,7 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla
  *
  * If the permanent hw address of @slave is @bond's hw address, we need to
  * find a different hw address to give @slave, that isn't in use by any other
- * slave in the bond. This address must be, of course, one of the premanent
+ * slave in the bond. This address must be, of course, one of the permanent
  * addresses of the other slaves.
  *
  * We go over the slave list, and for each slave there we compare its
-- 
1.7.4

^ permalink raw reply related

* NCM driver improvments
From: Indrek Peri @ 2011-04-11 11:42 UTC (permalink / raw)
  To: alexey.orishko, netdev@vger.kernel.org

Hi all

I have some ideas what could improve NCM driver. This is just for
open discussion. 

* Timer

Current NCM driver introduces timer for sending collected packets.
Driver keeps track of actual point of sending with member tx_timer_pending
which is a counter. This counter is decreased in timeout function and
timer is restarted if counter is bigger than 0. So, currently timeouts
are static.

One idea would be to reduce of amount of timeouts in system to replace
tx_timer_pending counter with varing timer divider. For example, divider
can be from 100 to 1000 (in scales).

 ctx->tx_timer.expires = jiffies + ((HZ + 999) / tmr_divider);

Scale can be calculated form packets and their data sizes collected.
For example, if there is only one TCP download session then TX collects
many small ACKs into buffer. In this case, for example, tmr_divider is 
100 and when timer expires packets will be send out. This avoids system
timeouts.

Another idea is to try to remove timer at all if traffic is low. Every
packet will be send over in one NCM frame. This reduces RTT time for
packet.


* Locks in timeout
        
        spin_lock(&ctx->mtx);
        if (ctx->tx_timer_pending != 0) {
                ctx->tx_timer_pending--;
                restart = 1;
        } else {
                restart = 0;
        }
        spin_unlock(&ctx->mtx);

        if (restart) {
                spin_lock(&ctx->mtx);
                cdc_ncm_tx_timeout_start(ctx);
                spin_unlock(&ctx->mtx);
        } else if (ctx->netdev != NULL) {
                usbnet_start_xmit(NULL, ctx->netdev);
        }

Timer divider would reduce locking because cdc_ncm_fill_tx_frame
function keeps the lock until to the end of function.
Timeout would mean then call to send.

* reset_resume

In cdc_ncm_driver struct is function:

        .reset_resume = usbnet_resume,

I think that this is good to add.

* usbnet_tx_timeout

I have seen slow path warnings in kernel and call to usbnet_tx_timeout 
with NCM driver. There is FIXME in usbnet and may be there is a need for 
reset function inside NCM because NCM driver collects data? 

BR, Indrek Peri

^ permalink raw reply

* [PATCH NET-2.6 1/1] netxen: limit skb frags for non tso packet
From: Amit Kumar Salecha @ 2011-04-11 12:10 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, anirban.chakraborty, stable

Machines are getting deadlock in four node cluster environment.
All nodes are accessing (find /gfs2 -depth -print|cpio -ocv > /dev/null)
200 GB storage on a GFS2 filesystem.
This result in memory fragmentation and driver receives 18 frags for
1448 byte packets.
For non tso packet, fw drops the tx request, if it has >14 frags.

Fixing it by pulling extra frags.

Cc: stable@kernel.org
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h      |    4 ++--
 drivers/net/netxen/netxen_nic_main.c |   17 +++++++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index d7299f1..679dc85 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -174,7 +174,7 @@
 
 #define	MAX_NUM_CARDS		4
 
-#define MAX_BUFFERS_PER_CMD	32
+#define NETXEN_MAX_FRAGS_PER_TX	14
 #define MAX_TSO_HEADER_DESC	2
 #define MGMT_CMD_DESC_RESV	4
 #define TX_STOP_THRESH		((MAX_SKB_FRAGS >> 2) + MAX_TSO_HEADER_DESC \
@@ -558,7 +558,7 @@ struct netxen_recv_crb {
  */
 struct netxen_cmd_buffer {
 	struct sk_buff *skb;
-	struct netxen_skb_frag frag_array[MAX_BUFFERS_PER_CMD + 1];
+	struct netxen_skb_frag frag_array[MAX_SKB_FRAGS + 1];
 	u32 frag_count;
 };
 
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 83348dc..e8a4b66 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -1844,6 +1844,8 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	struct cmd_desc_type0 *hwdesc, *first_desc;
 	struct pci_dev *pdev;
 	int i, k;
+	int delta = 0;
+	struct skb_frag_struct *frag;
 
 	u32 producer;
 	int frag_count, no_of_desc;
@@ -1851,6 +1853,21 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 
 	frag_count = skb_shinfo(skb)->nr_frags + 1;
 
+	/* 14 frags supported for normal packet and
+	 * 32 frags supported for TSO packet
+	 */
+	if (!skb_is_gso(skb) && frag_count > NETXEN_MAX_FRAGS_PER_TX) {
+
+		for (i = 0; i < (frag_count - NETXEN_MAX_FRAGS_PER_TX); i++) {
+			frag = &skb_shinfo(skb)->frags[i];
+			delta += frag->size;
+		}
+
+		if (!__pskb_pull_tail(skb, delta))
+			goto drop_packet;
+
+		frag_count = 1 + skb_shinfo(skb)->nr_frags;
+	}
 	/* 4 fragments per cmd des */
 	no_of_desc = (frag_count + 3) >> 2;
 
-- 
1.7.3.2


^ permalink raw reply related

* Re: [PATCH RESEND] uts: Set default hostname to "localhost", rather than "(none)"
From: Serge E. Hallyn @ 2011-04-11 12:39 UTC (permalink / raw)
  To: Josh Triplett
  Cc: David Miller, netdev, Andrew Morton, Linus Torvalds, linux-kernel
In-Reply-To: <20110411050155.GA2507@feather>

Quoting Josh Triplett (josh@joshtriplett.org):
> The "hostname" tool falls back to setting the hostname to "localhost" if
> /etc/hostname does not exist.  Distribution init scripts have the same
> fallback.  However, if userspace never calls sethostname, such as when
> booting with init=/bin/sh, or otherwise booting a minimal system without
> the usual init scripts, the default hostname of "(none)" remains,
> unhelpfully appearing in various places such as prompts
> ("root@(none):~#") and logs.  Furthrmore, "(none)" doesn't typically
> resolve to anything useful, while "localhost" does.
> 
> Change the default hostname to "localhost".  This removes the need for
> the standard fallback, provides a useful default for systems that never
> call sethostname, and makes minimal systems that much more useful with
> less configuration.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Seems good to me.  I'd have no idea if there were valid reasons to object
to such a thing so my ack means nothing, but

Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>

thanks,
-serge

> ---
> 
> Looked at "(none)" one too many times, and figured I ought to do
> something about it.
> 
> Resending, and adding CCs for networking and UTS.
> 
>  include/linux/uts.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/uts.h b/include/linux/uts.h
> index 73eb1ed..610bec2 100644
> --- a/include/linux/uts.h
> +++ b/include/linux/uts.h
> @@ -9,7 +9,7 @@
>  #endif
>  
>  #ifndef UTS_NODENAME
> -#define UTS_NODENAME "(none)"	/* set by sethostname() */
> +#define UTS_NODENAME "localhost"	/* set by sethostname() */
>  #endif
>  
>  #ifndef UTS_DOMAINNAME
> -- 
> 1.7.4.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/

^ permalink raw reply

* Re: [PATCH v2] net: r8169: convert to hw_features
From: Michał Mirosław @ 2011-04-11 13:30 UTC (permalink / raw)
  To: François Romieu; +Cc: netdev, David Dillow, Hayes Wang
In-Reply-To: <20110410154508.GA17091@electric-eye.fr.zoreil.com>

On Sun, Apr 10, 2011 at 05:45:08PM +0200, François Romieu wrote:
> 
> On Fri, Apr 08, 2011 at 06:35:56PM +0200, Michał Mirosław wrote:
> > Simple conversion with a bit of needed cleanup.
> The description should include :
> - Rx VLAN tag stripping is now enabled by default

Actually, it was enabled by default before. NETIF_F_HW_VLAN_TX_RX was
#defined to be NETIF_F_HW_VLAN_TX+NETIF_F_HW_VLAN_RX.

> As far as I understand it, it is fine to have dev->hw_features
> strictly smaller than dev->features, right ?

Yes. hw_features signifies what can be toggled by user, but does not
imply state of features not present there.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH v2] net: bnx2x: convert to hw_features
From: Vladislav Zolotarov @ 2011-04-11 14:10 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <20110410153506.86FA613909@rere.qmqm.pl>

On Sun, 2011-04-10 at 08:35 -0700, Michał Mirosław wrote:
> Since ndo_fix_features callback is postponing features change when
> bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> has to be called again when this condition changes.

Unfortunately, NACK again. See below, pls.

> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> [changes from v1: comment in ndo_fix_features callback]
> 
>  drivers/net/bnx2x/bnx2x_cmn.c     |   52 ++++++++++++++++++--
>  drivers/net/bnx2x/bnx2x_cmn.h     |    3 +
>  drivers/net/bnx2x/bnx2x_ethtool.c |   95 -------------------------------------
>  drivers/net/bnx2x/bnx2x_main.c    |   29 +++++++-----
>  4 files changed, 67 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
> index e83ac6d..9691b67 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/bnx2x/bnx2x_cmn.c
> @@ -2443,11 +2443,21 @@ alloc_err:
>  
>  }
>  
> +static int bnx2x_reload_if_running(struct net_device *dev)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +
> +	if (unlikely(!netif_running(dev)))
> +		return 0;
> +
> +	bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> +	return bnx2x_nic_load(bp, LOAD_NORMAL);
> +}
> +
>  /* called with rtnl_lock */
>  int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
> -	int rc = 0;
>  
>  	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
>  		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> @@ -2464,12 +2474,44 @@ int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
>  	 */
>  	dev->mtu = new_mtu;
>  
> -	if (netif_running(dev)) {
> -		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> -		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> +	return bnx2x_reload_if_running(dev);
> +}
> +
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +
> +	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> +		netdev_err(dev, "Handling parity error recovery. Try again later\n");
> +
> +		/* Don't allow bnx2x_set_features() to be called now. */
> +		return dev->features;
> +	}
> +
> +	/* TPA requires Rx CSUM offloading */
> +	if (!(features & NETIF_F_RXCSUM) || bp->disable_tpa)
> +		features &= ~NETIF_F_LRO;

Shouldn't it be (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM) and not
NETIF_F_RXCSUM?

> +
> +	return features;
> +}

In addition this function should ensure NETIF_F_IP_CSUM and
NETIF_F_IPV6_CSUM are changed together.

> +
> +int bnx2x_set_features(struct net_device *dev, u32 features)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +	u32 flags = bp->flags;
> +
> +	if (features & NETIF_F_LRO)
> +		flags |= TPA_ENABLE_FLAG;
> +	else
> +		flags &= ~TPA_ENABLE_FLAG;
> +
> +	if (flags ^ bp->flags) {
> +		bp->flags = flags;
> +
> +		return bnx2x_reload_if_running(dev);
>  	}
>  
> -	return rc;
> +	return 0;
>  }

Since there is no set_rx_csum() anymore the above function has to handle
bp->rx_csum namely correlate it with (NETIF_F_IP_CSUM |
NETIF_F_IPV6_CSUM) bits in the 'features'. 

>  
>  void bnx2x_tx_timeout(struct net_device *dev)
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
> index 775fef0..1cdab69 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.h
> +++ b/drivers/net/bnx2x/bnx2x_cmn.h
> @@ -431,6 +431,9 @@ void bnx2x_free_mem_bp(struct bnx2x *bp);
>   */
>  int bnx2x_change_mtu(struct net_device *dev, int new_mtu);
>  
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features);
> +int bnx2x_set_features(struct net_device *dev, u32 features);
> +
>  /**
>   * tx timeout netdev callback
>   *
> diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
> index 1479994..ad7d91e 100644
> --- a/drivers/net/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/bnx2x/bnx2x_ethtool.c
> @@ -1299,91 +1299,6 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
>  	return 0;
>  }
>  
> -static int bnx2x_set_flags(struct net_device *dev, u32 data)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -	int changed = 0;
> -	int rc = 0;
> -
> -	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> -		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> -		return -EAGAIN;
> -	}
> -
> -	if (!(data & ETH_FLAG_RXVLAN))
> -		return -EINVAL;
> -
> -	if ((data & ETH_FLAG_LRO) && bp->rx_csum && bp->disable_tpa)
> -		return -EINVAL;
> -
> -	rc = ethtool_op_set_flags(dev, data, ETH_FLAG_LRO | ETH_FLAG_RXVLAN |
> -					ETH_FLAG_TXVLAN | ETH_FLAG_RXHASH);
> -	if (rc)
> -		return rc;
> -
> -	/* TPA requires Rx CSUM offloading */
> -	if ((data & ETH_FLAG_LRO) && bp->rx_csum) {
> -		if (!(bp->flags & TPA_ENABLE_FLAG)) {
> -			bp->flags |= TPA_ENABLE_FLAG;
> -			changed = 1;
> -		}
> -	} else if (bp->flags & TPA_ENABLE_FLAG) {
> -		dev->features &= ~NETIF_F_LRO;
> -		bp->flags &= ~TPA_ENABLE_FLAG;
> -		changed = 1;
> -	}
> -
> -	if (changed && netif_running(dev)) {
> -		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> -		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> -	}
> -
> -	return rc;
> -}
> -
> -static u32 bnx2x_get_rx_csum(struct net_device *dev)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -
> -	return bp->rx_csum;
> -}
> -
> -static int bnx2x_set_rx_csum(struct net_device *dev, u32 data)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -	int rc = 0;
> -
> -	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> -		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> -		return -EAGAIN;
> -	}
> -
> -	bp->rx_csum = data;
> -
> -	/* Disable TPA, when Rx CSUM is disabled. Otherwise all
> -	   TPA'ed packets will be discarded due to wrong TCP CSUM */
> -	if (!data) {
> -		u32 flags = ethtool_op_get_flags(dev);
> -
> -		rc = bnx2x_set_flags(dev, (flags & ~ETH_FLAG_LRO));
> -	}
> -
> -	return rc;
> -}
> -
> -static int bnx2x_set_tso(struct net_device *dev, u32 data)
> -{
> -	if (data) {
> -		dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -		dev->features |= NETIF_F_TSO6;
> -	} else {
> -		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
> -		dev->features &= ~NETIF_F_TSO6;
> -	}
> -
> -	return 0;
> -}
> -
>  static const struct {
>  	char string[ETH_GSTRING_LEN];
>  } bnx2x_tests_str_arr[BNX2X_NUM_TESTS] = {
> @@ -2207,16 +2122,6 @@ static const struct ethtool_ops bnx2x_ethtool_ops = {
>  	.set_ringparam		= bnx2x_set_ringparam,
>  	.get_pauseparam		= bnx2x_get_pauseparam,
>  	.set_pauseparam		= bnx2x_set_pauseparam,
> -	.get_rx_csum		= bnx2x_get_rx_csum,
> -	.set_rx_csum		= bnx2x_set_rx_csum,
> -	.get_tx_csum		= ethtool_op_get_tx_csum,
> -	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
> -	.set_flags		= bnx2x_set_flags,
> -	.get_flags		= ethtool_op_get_flags,
> -	.get_sg			= ethtool_op_get_sg,
> -	.set_sg			= ethtool_op_set_sg,
> -	.get_tso		= ethtool_op_get_tso,
> -	.set_tso		= bnx2x_set_tso,
>  	.self_test		= bnx2x_self_test,
>  	.get_sset_count		= bnx2x_get_sset_count,
>  	.get_strings		= bnx2x_get_strings,
> diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
> index f3cf889..ffa0611 100644
> --- a/drivers/net/bnx2x/bnx2x_main.c
> +++ b/drivers/net/bnx2x/bnx2x_main.c
> @@ -7661,6 +7661,7 @@ exit_leader_reset:
>  	bp->is_leader = 0;
>  	bnx2x_release_hw_lock(bp, HW_LOCK_RESOURCE_RESERVED_08);
>  	smp_wmb();
> +	netdev_update_features(bp->dev);
>  	return rc;
>  }

Before I continue I'd like to clarify one thing: there is no sense to
call for netdev_update_features() if bnx2x_nic_load(), called right
before it, has failed as long as the following bnx2x_nic_load() that
will be called from the netdev_update_features() flow will also fail
(for the same reasons as the previous one). If bnx2x_nic_load() fails
for the certain NIC we actually shut this NIC down. So, the following
remarks will be based on the above statement.

netdev_update_features(bp->dev) should be called after a successful
"leader"'s call for a bnx2x_nic_load() and not as above. See the patch
below:

diff --git a/drivers/net/bnx2x/bnx2x_main.c
b/drivers/net/bnx2x/bnx2x_main.c
index f3cf889..71e7818 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -7728,6 +7728,8 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
 						return;
 					}
 
+					netdev_update_features(bp->dev);
+
					return;
 				}
 
>  
> @@ -7759,6 +7760,7 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
>  					bp->recovery_state =
>  						BNX2X_RECOVERY_DONE;
>  					smp_wmb();
> +					netdev_update_features(bp->dev);
>  					return;
>  				}
>  			}

Same here: netdev_update_features() should be called only if the
previous bnx2x_nic_load() call has succeeded.

> @@ -8954,6 +8956,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>  static int bnx2x_open(struct net_device *dev)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
> +	int rc;
>  
>  	netif_carrier_off(dev);
>  
> @@ -8993,7 +8996,9 @@ static int bnx2x_open(struct net_device *dev)
>  
>  	bp->recovery_state = BNX2X_RECOVERY_DONE;
>  
> -	return bnx2x_nic_load(bp, LOAD_OPEN);
> +	rc = bnx2x_nic_load(bp, LOAD_OPEN);
> +	netdev_update_features(bp->dev);
> +	return rc;
>  }

U shouldn't call for netdev_update_features(bp->dev) if bnx2x_nic_load()
has failed. It would also be nice if netdev_update_features() would
propagate the exit status of ndo_set_features() when ndo_set_features()
fails in the __netdev_update_features(). See the patch for the bnx2x
below:

@@ -8993,7 +8995,14 @@ static int bnx2x_open(struct net_device *dev)
 
        bp->recovery_state = BNX2X_RECOVERY_DONE;
 
-       return bnx2x_nic_load(bp, LOAD_OPEN);
+       rc = bnx2x_nic_load(bp, LOAD_OPEN);
+       if (!rc)
+               netdev_update_features(bp->dev);
+
+       if (bp->state == BNX2X_STATE_OPEN)
+               return 0;
+       else
+               return -EBUSY;
 }
 
 /* called with rtnl_lock */
>  
>  /* called with rtnl_lock */
> @@ -9304,6 +9309,8 @@ static const struct net_device_ops bnx2x_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  	.ndo_do_ioctl		= bnx2x_ioctl,
>  	.ndo_change_mtu		= bnx2x_change_mtu,
> +	.ndo_fix_features	= bnx2x_fix_features,
> +	.ndo_set_features	= bnx2x_set_features,
>  	.ndo_tx_timeout		= bnx2x_tx_timeout,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller	= poll_bnx2x,
> @@ -9430,20 +9437,18 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
>  
>  	dev->netdev_ops = &bnx2x_netdev_ops;
>  	bnx2x_set_ethtool_ops(dev);
> -	dev->features |= NETIF_F_SG;
> -	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> +
>  	if (bp->flags & USING_DAC_FLAG)
>  		dev->features |= NETIF_F_HIGHDMA;
> -	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -	dev->features |= NETIF_F_TSO6;
> -	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
>  
> -	dev->vlan_features |= NETIF_F_SG;
> -	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> -	if (bp->flags & USING_DAC_FLAG)
> -		dev->vlan_features |= NETIF_F_HIGHDMA;
> -	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -	dev->vlan_features |= NETIF_F_TSO6;
> +	dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
> +		NETIF_F_HW_VLAN_TX;

hw_features are missing NETIF_F_GRO and NETIF_F_LRO flags that are
currently configured in bnx2x_init_bp(). 

> +
> +	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_RX;
> +
> +	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;

I'm not sure if it's safe to set NETIF_F_HIGHDMA unconditionally. I
think it's better to correlate it with the USING_DAC_FLAG which is set
according to what is returned by 
dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)).

thanks,
vlad

>  
>  #ifdef BCM_DCBNL
>  	dev->dcbnl_ops = &bnx2x_dcbnl_ops;






^ permalink raw reply related

* Re: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel
From: Alexander Duyck @ 2011-04-11 14:50 UTC (permalink / raw)
  To: Wei Gu; +Cc: Eric Dumazet, netdev, Kirsher, Jeffrey T
In-Reply-To: <D12839161ADD3A4B8DA63D1A134D084026E48BA730@ESGSCCMS0001.eapac.ericsson.se>

On 4/10/2011 12:02 AM, Wei Gu wrote:
> Hi , I haven't enable the packet header spilting, so I think no
> netdev_alloc_page() will be called in my case.
>
> BTW, I also did the same test from 2.6.33 to 2.6.35 kernel, looks the
> problem happens from 2.6.35, cause 2.6.32/33/34 do not see this
> problem at all, they all works pretty good.
>
> I modify the .configs base on the FC13/14, and also manully set the
> DMAR DEFAULT to off, chose the SLAB as the memory allocator (same as
> RHEL6 2.6.32). For more detail about the config, please check the
> attached file
>
> Thanks WeiGu

WeiGu,

In order to try and isolate this issue I was wondering if you could try 
our out-of-tree ixgbe driver from e1000.sf.net?  The idea would be to 
test it on 2.6.34 and 2.6.35 in order to determine if the issue is due 
to a change in the kernel or a change in the driver.  If the performance 
is the same on these two kernels with our out-of-tree driver then we 
know the issue is a change somewhere in the ixgbe driver in the kernel.

Thanks,

Alex


^ permalink raw reply

* Response Needed
From: Grace Yee @ 2011-04-10 16:52 UTC (permalink / raw)


Deal worth US45,275,000.00. Interested?



^ permalink raw reply

* Re: [PATCH] net: sky2: convert to hw_features
From: Stephen Hemminger @ 2011-04-11 14:58 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20110411005100.GA22103@rere.qmqm.pl>

On Mon, 11 Apr 2011 02:51:00 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Sun, Apr 10, 2011 at 11:53:02AM -0700, Stephen Hemminger wrote:
> > On Sun, 10 Apr 2011 15:13:21 +0200 (CEST)
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > Caveats:
> > >  - driver modifies vlan_features on HW VLAN TX changes
> > >  - broken RX checksum will be reenabled on features change
> > To be more precise. This is acceptable if and only if all cases
> > where features are disabled in response to MTU and chip versions
> > are exactly the same. We don't want to let some user stumble upon
> > cases where hardware features don't work in their configuration.
> 
> I was referring to the unlikely case detected by sky2_rx_checksum().
> Before this conversion, user could reenable the feature using ethtool.
> The change is that now, in this case, it's reenabled also when other
> features are changed (i.e. whenever netdev_update_features() gets called).
> 
> Best Regards,
> Michał Mirosław

Ok.

It does expose a pre-existing issue. If this logic trips (and I have
gotten reports of it happening), then the GRO will not get disabled.
Probably need to mask of GRO as well, since GRO depends on RXCSUM.

--- a/drivers/net/sky2.c	2011-04-11 07:56:50.569361209 -0700
+++ b/drivers/net/sky2.c	2011-04-11 07:57:34.502312648 -0700
@@ -2538,7 +2538,7 @@ static void sky2_rx_checksum(struct sky2
 		 * It will be reenabled on next ndo_set_features, but if it's
 		 * really broken, will get disabled again
 		 */
-		sky2->netdev->features &= ~NETIF_F_RXCSUM;
+		sky2->netdev->features &= ~(NETIF_F_RXCSUM | NETIF_F_GRO);
 		sky2_write32(sky2->hw, Q_ADDR(rxqaddr[sky2->port], Q_CSR),
 			     BMU_DIS_RX_CHKSUM);
 	}

^ permalink raw reply

* Re: [PATCH] net: sky2: convert to hw_features
From: Stephen Hemminger @ 2011-04-11 15:00 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20110411005100.GA22103@rere.qmqm.pl>

On Mon, 11 Apr 2011 02:51:00 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Sun, Apr 10, 2011 at 11:53:02AM -0700, Stephen Hemminger wrote:
> > On Sun, 10 Apr 2011 15:13:21 +0200 (CEST)
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > Caveats:
> > >  - driver modifies vlan_features on HW VLAN TX changes
> > >  - broken RX checksum will be reenabled on features change
> > To be more precise. This is acceptable if and only if all cases
> > where features are disabled in response to MTU and chip versions
> > are exactly the same. We don't want to let some user stumble upon
> > cases where hardware features don't work in their configuration.
> 
> I was referring to the unlikely case detected by sky2_rx_checksum().
> Before this conversion, user could reenable the feature using ethtool.
> The change is that now, in this case, it's reenabled also when other
> features are changed (i.e. whenever netdev_update_features() gets called).
> 
> Best Regards,
> Michał Mirosław

Ok.

It does expose a pre-existing issue. If this logic trips (and I have
gotten reports of it happening), then the GRO will not get disabled.
Probably need to mask of GRO as well, since GRO depends on RXCSUM.

--- a/drivers/net/sky2.c	2011-04-11 07:56:50.569361209 -0700
+++ b/drivers/net/sky2.c	2011-04-11 07:57:34.502312648 -0700
@@ -2538,7 +2538,7 @@ static void sky2_rx_checksum(struct sky2
 		 * It will be reenabled on next ndo_set_features, but if it's
 		 * really broken, will get disabled again
 		 */
-		sky2->netdev->features &= ~NETIF_F_RXCSUM;
+		sky2->netdev->features &= ~(NETIF_F_RXCSUM | NETIF_F_GRO);
 		sky2_write32(sky2->hw, Q_ADDR(rxqaddr[sky2->port], Q_CSR),
 			     BMU_DIS_RX_CHKSUM);
 	}

^ permalink raw reply

* RE: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel
From: Wei Gu @ 2011-04-11 15:00 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: Eric Dumazet, netdev, Kirsher, Jeffrey T
In-Reply-To: <4DA3151B.4030507@intel.com>

I will try the lastest driver 3.3.8 on  e1000.sf.net:)

But I also tested the ixgbe-3.2.10 on the kernel 2.6.35.1 and 2.6.35.2,The problem start happens from 2.6.35.2, no problem at all with 2.6.35.1 kernel.
I don't know if some patch between 2.6.35.1 and 2.35.2 had some impact to this Intel 10GE ixgbe driver.

Thanks
WeiGu

-----Original Message-----
From: Alexander Duyck [mailto:alexander.h.duyck@intel.com]
Sent: Monday, April 11, 2011 10:50 PM
To: Wei Gu
Cc: Eric Dumazet; netdev; Kirsher, Jeffrey T
Subject: Re: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel

On 4/10/2011 12:02 AM, Wei Gu wrote:
> Hi , I haven't enable the packet header spilting, so I think no
> netdev_alloc_page() will be called in my case.
>
> BTW, I also did the same test from 2.6.33 to 2.6.35 kernel, looks the
> problem happens from 2.6.35, cause 2.6.32/33/34 do not see this
> problem at all, they all works pretty good.
>
> I modify the .configs base on the FC13/14, and also manully set the
> DMAR DEFAULT to off, chose the SLAB as the memory allocator (same as
> RHEL6 2.6.32). For more detail about the config, please check the
> attached file
>
> Thanks WeiGu

WeiGu,

In order to try and isolate this issue I was wondering if you could try our out-of-tree ixgbe driver from e1000.sf.net?  The idea would be to test it on 2.6.34 and 2.6.35 in order to determine if the issue is due to a change in the kernel or a change in the driver.  If the performance is the same on these two kernels with our out-of-tree driver then we know the issue is a change somewhere in the ixgbe driver in the kernel.

Thanks,

Alex


^ permalink raw reply

* Re: [PATCH RESEND] uts: Set default hostname to "localhost", rather than "(none)"
From: Stephen Hemminger @ 2011-04-11 15:02 UTC (permalink / raw)
  To: Josh Triplett
  Cc: David Miller, netdev, Serge E. Hallyn, Andrew Morton,
	Linus Torvalds, linux-kernel
In-Reply-To: <20110411050155.GA2507@feather>

On Sun, 10 Apr 2011 22:01:59 -0700
Josh Triplett <josh@joshtriplett.org> wrote:

> The "hostname" tool falls back to setting the hostname to "localhost" if
> /etc/hostname does not exist.  Distribution init scripts have the same
> fallback.  However, if userspace never calls sethostname, such as when
> booting with init=/bin/sh, or otherwise booting a minimal system without
> the usual init scripts, the default hostname of "(none)" remains,
> unhelpfully appearing in various places such as prompts
> ("root@(none):~#") and logs.  Furthrmore, "(none)" doesn't typically
> resolve to anything useful, while "localhost" does.
> 
> Change the default hostname to "localhost".  This removes the need for
> the standard fallback, provides a useful default for systems that never
> call sethostname, and makes minimal systems that much more useful with
> less configuration.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> Looked at "(none)" one too many times, and figured I ought to do
> something about it.
> 
> Resending, and adding CCs for networking and UTS.
> 
>  include/linux/uts.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/uts.h b/include/linux/uts.h
> index 73eb1ed..610bec2 100644
> --- a/include/linux/uts.h
> +++ b/include/linux/uts.h
> @@ -9,7 +9,7 @@
>  #endif
>  
>  #ifndef UTS_NODENAME
> -#define UTS_NODENAME "(none)"	/* set by sethostname() */
> +#define UTS_NODENAME "localhost"	/* set by sethostname() */
>  #endif
>  
>  #ifndef UTS_DOMAINNAME

It makes sense but this behavior has existed so long in Linux
that some distro might actually be depending on it.

-- 

^ permalink raw reply

* Re: [PATCH RESEND] uts: Set default hostname to "localhost", rather than "(none)"
From: Linus Torvalds @ 2011-04-11 15:12 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Josh Triplett, David Miller, netdev, Serge E. Hallyn,
	Andrew Morton, linux-kernel
In-Reply-To: <20110411080254.403d3166@nehalam>

On Mon, Apr 11, 2011 at 8:02 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
>
> It makes sense but this behavior has existed so long in Linux
> that some distro might actually be depending on it.

Yes. Also, quite frankly, I do _not_ think that "localhost" is in any
way a more correct hostname.

I'd rather have an obviously invalid hostname for a machine that
hasn't been set up correctly than one that might work by random
chance.

                                    Linus

^ permalink raw reply

* RE: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel
From: Wei Gu @ 2011-04-11 15:14 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: Eric Dumazet, netdev, Kirsher, Jeffrey T
In-Reply-To: <4DA3151B.4030507@intel.com>

 I tried the ixgbe-3.3.8 (insmod ixgbe.ko RSS=8,8,8,8,8,8,8,8 FdirMode=0,0,0,0,0,0,0,0 Node=0,0,1,1,2,2,3,3)  from e1000.sf.net both on 2.6.35.1 and 2.6.35.2, same observation as 3.2.10 ixgbe driver, On 2.6.35.2 it have high rx errors:
Ethtool -S eth10 |grep error
     rx_errors: 0
     tx_errors: 0
     rx_over_errors: 0
     rx_crc_errors: 0
     rx_frame_errors: 0
     rx_fifo_errors: 0
     rx_missed_errors: 2263088
     tx_aborted_errors: 0
     tx_carrier_errors: 0
     tx_fifo_errors: 0
     tx_heartbeat_errors: 0
     rx_long_length_errors: 0
     rx_short_length_errors: 0
     rx_csum_offload_errors: 0
     fcoe_last_errors: 0


-----Original Message-----
From: Wei Gu
Sent: Monday, April 11, 2011 11:01 PM
To: 'Alexander Duyck'
Cc: Eric Dumazet; netdev; Kirsher, Jeffrey T
Subject: RE: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel

I will try the lastest driver 3.3.8 on  e1000.sf.net:)

But I also tested the ixgbe-3.2.10 on the kernel 2.6.35.1 and 2.6.35.2,The problem start happens from 2.6.35.2, no problem at all with 2.6.35.1 kernel.
I don't know if some patch between 2.6.35.1 and 2.35.2 had some impact to this Intel 10GE ixgbe driver.

Thanks
WeiGu

-----Original Message-----
From: Alexander Duyck [mailto:alexander.h.duyck@intel.com]
Sent: Monday, April 11, 2011 10:50 PM
To: Wei Gu
Cc: Eric Dumazet; netdev; Kirsher, Jeffrey T
Subject: Re: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel

On 4/10/2011 12:02 AM, Wei Gu wrote:
> Hi , I haven't enable the packet header spilting, so I think no
> netdev_alloc_page() will be called in my case.
>
> BTW, I also did the same test from 2.6.33 to 2.6.35 kernel, looks the
> problem happens from 2.6.35, cause 2.6.32/33/34 do not see this
> problem at all, they all works pretty good.
>
> I modify the .configs base on the FC13/14, and also manully set the
> DMAR DEFAULT to off, chose the SLAB as the memory allocator (same as
> RHEL6 2.6.32). For more detail about the config, please check the
> attached file
>
> Thanks WeiGu

WeiGu,

In order to try and isolate this issue I was wondering if you could try our out-of-tree ixgbe driver from e1000.sf.net?  The idea would be to test it on 2.6.34 and 2.6.35 in order to determine if the issue is due to a change in the kernel or a change in the driver.  If the performance is the same on these two kernels with our out-of-tree driver then we know the issue is a change somewhere in the ixgbe driver in the kernel.

Thanks,

Alex


^ permalink raw reply

* RE: Low performance Intel 10GE NIC (3.2.10) on 2.6.38 Kernel
From: Eric Dumazet @ 2011-04-11 15:42 UTC (permalink / raw)
  To: Wei Gu; +Cc: Alexander Duyck, netdev, Kirsher, Jeffrey T
In-Reply-To: <D12839161ADD3A4B8DA63D1A134D084026E4909714@ESGSCCMS0001.eapac.ericsson.se>

Le lundi 11 avril 2011 à 23:14 +0800, Wei Gu a écrit :
> I tried the ixgbe-3.3.8 (insmod ixgbe.ko RSS=8,8,8,8,8,8,8,8 FdirMode=0,0,0,0,0,0,0,0 Node=0,0,1,1,2,2,3,3)  from e1000.sf.net both on 2.6.35.1 and 2.6.35.2, same observation as 3.2.10 ixgbe driver, On 2.6.35.2 it have high rx errors:
> Ethtool -S eth10 |grep error
>      rx_errors: 0
>      tx_errors: 0
>      rx_over_errors: 0
>      rx_crc_errors: 0
>      rx_frame_errors: 0
>      rx_fifo_errors: 0
>      rx_missed_errors: 2263088
>      tx_aborted_errors: 0
>      tx_carrier_errors: 0
>      tx_fifo_errors: 0
>      tx_heartbeat_errors: 0
>      rx_long_length_errors: 0
>      rx_short_length_errors: 0
>      rx_csum_offload_errors: 0
>      fcoe_last_errors: 0
> 

It would be nice you post perf record / perf report results

During your stress , do

perf record -a -g sleep 10
perf report

And post "top offenders"

Thanks



^ permalink raw reply

* Re: [PATCH net-next-2.6] ethtool: Compat handling for struct ethtool_rxnfc
From: Ben Hutchings @ 2011-04-11 17:10 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: David Miller, netdev
In-Reply-To: <1298917347.2569.5.camel@bwh-desktop>

On Mon, 2011-02-28 at 18:22 +0000, Ben Hutchings wrote:
> This structure was accidentally defined such that its layout can
> differ between 32-bit and 64-bit processes.  Add compat structure
> definitions and functions to copy from/to user-space with the
> necessary adjustments.

Damnit, we have to do the same thing for struct ethtool_rx_ntuple:

$ cc -m64 test.c
$ ./a.out
sizeof(struct ethtool_rx_ntuple) = 184
offsetof(ethtool_rx_ntuple, fs) = 8
offsetof(ethtool_rx_ntuple_flow_spec, vlan_tag) = 148
offsetof(ethtool_rx_ntuple_flow_spec, data) = 152
$ cc -m32 test.c
$ ./a.out
sizeof(struct ethtool_rx_ntuple) = 176
offsetof(ethtool_rx_ntuple, fs) = 4
offsetof(ethtool_rx_ntuple_flow_spec, vlan_tag) = 148
offsetof(ethtool_rx_ntuple_flow_spec, data) = 152

At least there's only one hole to deal with in this case.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH] net: sky2: convert to hw_features
From: Michał Mirosław @ 2011-04-11 17:58 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20110411075829.7b9bf1e5@nehalam>

On Mon, Apr 11, 2011 at 07:58:29AM -0700, Stephen Hemminger wrote:
> On Mon, 11 Apr 2011 02:51:00 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> 
> > On Sun, Apr 10, 2011 at 11:53:02AM -0700, Stephen Hemminger wrote:
> > > On Sun, 10 Apr 2011 15:13:21 +0200 (CEST)
> > > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > > Caveats:
> > > >  - driver modifies vlan_features on HW VLAN TX changes
> > > >  - broken RX checksum will be reenabled on features change
> > > To be more precise. This is acceptable if and only if all cases
> > > where features are disabled in response to MTU and chip versions
> > > are exactly the same. We don't want to let some user stumble upon
> > > cases where hardware features don't work in their configuration.
> > I was referring to the unlikely case detected by sky2_rx_checksum().
> > Before this conversion, user could reenable the feature using ethtool.
> > The change is that now, in this case, it's reenabled also when other
> > features are changed (i.e. whenever netdev_update_features() gets called).
> 
> Ok.
> 
> It does expose a pre-existing issue. If this logic trips (and I have
> gotten reports of it happening), then the GRO will not get disabled.
> Probably need to mask of GRO as well, since GRO depends on RXCSUM.

This is not a problem, since the code checks skb->ip_summed for protocols
that implement GRO. See tcp4_gro_receive() for example. This might be
a slight hit in performance, though.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] net: sky2: convert to hw_features
From: Stephen Hemminger @ 2011-04-11 18:09 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20110411175823.GA31338@rere.qmqm.pl>

On Mon, 11 Apr 2011 19:58:23 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Mon, Apr 11, 2011 at 07:58:29AM -0700, Stephen Hemminger wrote:
> > On Mon, 11 Apr 2011 02:51:00 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > 
> > > On Sun, Apr 10, 2011 at 11:53:02AM -0700, Stephen Hemminger wrote:
> > > > On Sun, 10 Apr 2011 15:13:21 +0200 (CEST)
> > > > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > > > Caveats:
> > > > >  - driver modifies vlan_features on HW VLAN TX changes
> > > > >  - broken RX checksum will be reenabled on features change
> > > > To be more precise. This is acceptable if and only if all cases
> > > > where features are disabled in response to MTU and chip versions
> > > > are exactly the same. We don't want to let some user stumble upon
> > > > cases where hardware features don't work in their configuration.
> > > I was referring to the unlikely case detected by sky2_rx_checksum().
> > > Before this conversion, user could reenable the feature using ethtool.
> > > The change is that now, in this case, it's reenabled also when other
> > > features are changed (i.e. whenever netdev_update_features() gets called).
> > 
> > Ok.
> > 
> > It does expose a pre-existing issue. If this logic trips (and I have
> > gotten reports of it happening), then the GRO will not get disabled.
> > Probably need to mask of GRO as well, since GRO depends on RXCSUM.
> 
> This is not a problem, since the code checks skb->ip_summed for protocols
> that implement GRO. See tcp4_gro_receive() for example. This might be
> a slight hit in performance, though.

The performance hit is expected, just thought that GRO depended on rx checksum.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox