Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/4] sky2: version 1.21
From: Stephen Hemminger @ 2008-01-11  0:14 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev

Set of sky2 patches:
  1) bug fix for 4G mem
  2) low priority fix for WOL default
 --- cut line for 2.6.24 ---
  3) new hardware support
  4) verson 1.21

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>


^ permalink raw reply

* [PATCH 2/4] sky2: remove check for PCI wakeup setting from BIOS
From: Stephen Hemminger @ 2008-01-11  0:14 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20080111001411.158113585@linux-foundation.org>

[-- Attachment #1: sky2-remove-pci-check.patch --]
[-- Type: text/plain, Size: 2641 bytes --]

The driver checks status of PCI power management to mark 
default setting of Wake On Lan. On some systems this works, but often
it reports a that WOL is disabled when it isn't.

This patch gets rid of that check and just reports the wake on
lan status based on the hardware capablity.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

---
Please apply for 2.6.24.

--- a/drivers/net/sky2.c	2008-01-10 15:49:12.000000000 -0800
+++ b/drivers/net/sky2.c	2008-01-10 15:49:20.000000000 -0800
@@ -3946,7 +3946,7 @@ static __exit void sky2_debug_cleanup(vo
 /* Initialize network device */
 static __devinit struct net_device *sky2_init_netdev(struct sky2_hw *hw,
 						     unsigned port,
-						     int highmem, int wol)
+						     int highmem)
 {
 	struct sky2_port *sky2;
 	struct net_device *dev = alloc_etherdev(sizeof(*sky2));
@@ -3986,7 +3986,7 @@ static __devinit struct net_device *sky2
 	sky2->speed = -1;
 	sky2->advertising = sky2_supported_modes(hw);
 	sky2->rx_csum = (hw->chip_id != CHIP_ID_YUKON_XL);
-	sky2->wol = wol;
+	sky2->wol = sky2_wol_supported(hw) & WAKE_MAGIC;
 
 	spin_lock_init(&sky2->phy_lock);
 	sky2->tx_pending = TX_DEF_PENDING;
@@ -4083,24 +4083,12 @@ static int __devinit sky2_test_msi(struc
 	return err;
 }
 
-static int __devinit pci_wake_enabled(struct pci_dev *dev)
-{
-	int pm  = pci_find_capability(dev, PCI_CAP_ID_PM);
-	u16 value;
-
-	if (!pm)
-		return 0;
-	if (pci_read_config_word(dev, pm + PCI_PM_CTRL, &value))
-		return 0;
-	return value & PCI_PM_CTRL_PME_ENABLE;
-}
-
 static int __devinit sky2_probe(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
 	struct net_device *dev;
 	struct sky2_hw *hw;
-	int err, using_dac = 0, wol_default;
+	int err, using_dac = 0;
 
 	err = pci_enable_device(pdev);
 	if (err) {
@@ -4133,8 +4121,6 @@ static int __devinit sky2_probe(struct p
 		}
 	}
 
-	wol_default = pci_wake_enabled(pdev) ? WAKE_MAGIC : 0;
-
 	err = -ENOMEM;
 	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
 	if (!hw) {
@@ -4178,7 +4164,7 @@ static int __devinit sky2_probe(struct p
 
 	sky2_reset(hw);
 
-	dev = sky2_init_netdev(hw, 0, using_dac, wol_default);
+	dev = sky2_init_netdev(hw, 0, using_dac);
 	if (!dev) {
 		err = -ENOMEM;
 		goto err_out_free_pci;
@@ -4215,7 +4201,7 @@ static int __devinit sky2_probe(struct p
 	if (hw->ports > 1) {
 		struct net_device *dev1;
 
-		dev1 = sky2_init_netdev(hw, 1, using_dac, wol_default);
+		dev1 = sky2_init_netdev(hw, 1, using_dac);
 		if (!dev1)
 			dev_warn(&pdev->dev, "allocation for second device failed\n");
 		else if ((err = register_netdev(dev1))) {

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>


^ permalink raw reply

* [PATCH 3/4] sky2: support for Yukon Supreme
From: Stephen Hemminger @ 2008-01-11  0:14 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20080111001411.158113585@linux-foundation.org>

[-- Attachment #1: sky2-supreme.patch --]
[-- Type: text/plain, Size: 4815 bytes --]

Add support from sk98lin vendor driver 10.50.1.3 for 88E8055 and
88E8075 chips.  I don't have this hardware to test, so this changes
are untested.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

---
Please add to staging tree for 2.6.25.

--- a/drivers/net/sky2.c	2008-01-10 15:49:20.000000000 -0800
+++ b/drivers/net/sky2.c	2008-01-10 15:49:27.000000000 -0800
@@ -134,6 +134,8 @@ static const struct pci_device_id sky2_i
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, 0x436A) }, /* 88E8058 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, 0x436B) }, /* 88E8071 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, 0x436C) }, /* 88E8072 */
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, 0x436D) }, /* 88E8055 */
+	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL, 0x4370) }, /* 88E8075 */
 	{ 0 }
 };
 
@@ -547,6 +549,7 @@ static void sky2_phy_init(struct sky2_hw
 
 	case CHIP_ID_YUKON_EC_U:
 	case CHIP_ID_YUKON_EX:
+	case CHIP_ID_YUKON_SUPR:
 		pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
 
 		/* select page 3 to access LED control register */
@@ -713,23 +716,33 @@ static void sky2_set_tx_stfwd(struct sky
 {
 	struct net_device *dev = hw->dev[port];
 
-	if (dev->mtu <= ETH_DATA_LEN)
-		sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T),
-			     TX_JUMBO_DIS | TX_STFW_ENA);
+	if ( (hw->chip_id == CHIP_ID_YUKON_EX &&
+	      hw->chip_rev != CHIP_REV_YU_EX_A0) ||
+	     hw->chip_id == CHIP_ID_YUKON_FE_P ||
+	     hw->chip_id == CHIP_ID_YUKON_SUPR) {
+		/* Yukon-Extreme B0 and further Extreme devices */
+		/* enable Store & Forward mode for TX */
+
+		if (dev->mtu <= ETH_DATA_LEN)
+			sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T),
+				     TX_JUMBO_DIS | TX_STFW_ENA);
 
-	else if (hw->chip_id != CHIP_ID_YUKON_EC_U)
-		sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T),
-			     TX_STFW_ENA | TX_JUMBO_ENA);
-	else {
-		/* set Tx GMAC FIFO Almost Empty Threshold */
-		sky2_write32(hw, SK_REG(port, TX_GMF_AE_THR),
-			     (ECU_JUMBO_WM << 16) | ECU_AE_THR);
+		else
+			sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T),
+				     TX_JUMBO_ENA| TX_STFW_ENA);
+	} else {
+		if (dev->mtu <= ETH_DATA_LEN)
+			sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T), TX_STFW_ENA);
+		else {
+			/* set Tx GMAC FIFO Almost Empty Threshold */
+			sky2_write32(hw, SK_REG(port, TX_GMF_AE_THR),
+				     (ECU_JUMBO_WM << 16) | ECU_AE_THR);
 
-		sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T),
-			     TX_JUMBO_ENA | TX_STFW_DIS);
+			sky2_write32(hw, SK_REG(port, TX_GMF_CTRL_T), TX_STFW_DIS);
 
-		/* Can't do offload because of lack of store/forward */
-		dev->features &= ~(NETIF_F_TSO | NETIF_F_SG | NETIF_F_ALL_CSUM);
+			/* Can't do offload because of lack of store/forward */
+			dev->features &= ~(NETIF_F_TSO | NETIF_F_SG | NETIF_F_ALL_CSUM);
+		}
 	}
 }
 
@@ -2660,6 +2673,7 @@ static u32 sky2_mhz(const struct sky2_hw
 	case CHIP_ID_YUKON_EC:
 	case CHIP_ID_YUKON_EC_U:
 	case CHIP_ID_YUKON_EX:
+	case CHIP_ID_YUKON_SUPR:
 		return 125;
 
 	case CHIP_ID_YUKON_FE:
@@ -2743,6 +2757,15 @@ static int __devinit sky2_init(struct sk
 			| SKY2_HW_AUTO_TX_SUM
 			| SKY2_HW_ADV_POWER_CTL;
 		break;
+
+	case CHIP_ID_YUKON_SUPR:
+		hw->flags = SKY2_HW_GIGABIT
+			| SKY2_HW_NEWER_PHY
+			| SKY2_HW_NEW_LE
+			| SKY2_HW_AUTO_TX_SUM
+			| SKY2_HW_ADV_POWER_CTL;
+		break;
+
 	default:
 		dev_err(&hw->pdev->dev, "unsupported chip type 0x%x\n",
 			hw->chip_id);
@@ -2813,7 +2836,8 @@ static void sky2_reset(struct sky2_hw *h
 		sky2_write8(hw, SK_REG(i, GMAC_LINK_CTRL), GMLC_RST_SET);
 		sky2_write8(hw, SK_REG(i, GMAC_LINK_CTRL), GMLC_RST_CLR);
 
-		if (hw->chip_id == CHIP_ID_YUKON_EX)
+		if (hw->chip_id == CHIP_ID_YUKON_EX ||
+		    hw->chip_id == CHIP_ID_YUKON_SUPR)
 			sky2_write16(hw, SK_REG(i, GMAC_CTRL),
 				     GMC_BYP_MACSECRX_ON | GMC_BYP_MACSECTX_ON
 				     | GMC_BYP_RETR_ON);
--- a/drivers/net/sky2.h	2008-01-10 15:49:12.000000000 -0800
+++ b/drivers/net/sky2.h	2008-01-10 15:49:27.000000000 -0800
@@ -425,12 +425,13 @@ enum {
 
 /*	B2_CHIP_ID		 8 bit 	Chip Identification Number */
 enum {
-	CHIP_ID_YUKON_XL   = 0xb3, /* Chip ID for YUKON-2 XL */
-	CHIP_ID_YUKON_EC_U = 0xb4, /* Chip ID for YUKON-2 EC Ultra */
-	CHIP_ID_YUKON_EX   = 0xb5, /* Chip ID for YUKON-2 Extreme */
-	CHIP_ID_YUKON_EC   = 0xb6, /* Chip ID for YUKON-2 EC */
- 	CHIP_ID_YUKON_FE   = 0xb7, /* Chip ID for YUKON-2 FE */
- 	CHIP_ID_YUKON_FE_P = 0xb8, /* Chip ID for YUKON-2 FE+ */
+	CHIP_ID_YUKON_XL   = 0xb3, /* YUKON-2 XL */
+	CHIP_ID_YUKON_EC_U = 0xb4, /* YUKON-2 EC Ultra */
+	CHIP_ID_YUKON_EX   = 0xb5, /* YUKON-2 Extreme */
+	CHIP_ID_YUKON_EC   = 0xb6, /* YUKON-2 EC */
+ 	CHIP_ID_YUKON_FE   = 0xb7, /* YUKON-2 FE */
+ 	CHIP_ID_YUKON_FE_P = 0xb8, /* YUKON-2 FE+ */
+	CHIP_ID_YUKON_SUPR = 0xb9, /* YUKON-2 Supreme */
 };
 enum yukon_ec_rev {
 	CHIP_REV_YU_EC_A1    = 0,  /* Chip Rev. for Yukon-EC A1/A0 */

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>


^ permalink raw reply

* Re: [ipw3945-devel] [PATCH 4/5] iwlwifi: iwl3945 eliminate sleepable task queue from context
From: Joonwoo Park @ 2008-01-11  2:22 UTC (permalink / raw)
  To: Zhu Yi
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, lkml,
	ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1200016915.3530.161.camel-F7Q/YMNgnyhhZnBKgDE8z0EOCMrvLtNR@public.gmane.org>

2008/1/11, Zhu Yi <yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
>
> The version doesn't work on a .24-rc kernel. Can you compile it
> with .23?
>

Thank you for your help, I built it.

Thanks,
Joonwoo

^ permalink raw reply

* Re: [ipw3945-devel] [PATCH 4/5] iwlwifi: iwl3945 eliminate sleepable task queue from context
From: Zhu Yi @ 2008-01-11  2:01 UTC (permalink / raw)
  To: Joonwoo Park
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, lkml,
	ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <b25c3fa70801101738j512a6189i3e4541bf0348b91f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


On Fri, 2008-01-11 at 10:38 +0900, Joonwoo Park wrote:
> I should have checked code base, it's my fault.
> it's seems very similar, but I could not made build unfortunately.
> Can you introduce to me how to build it please?
> 
> my build error is here:
> jason@ehus ~/SRC/DRIVERS/iwlwifi $
> KSRC=/home/jason/SRC/LINUX/linux-2.6 make

The version doesn't work on a .24-rc kernel. Can you compile it
with .23?

Thanks,
-yi

^ permalink raw reply

* Re: [PATCH 0/4] Pull request for 'ipg-fixes' branch
From: linux @ 2008-01-11  1:58 UTC (permalink / raw)
  To: davem, romieu; +Cc: akpm, jeff, linux, netdev
In-Reply-To: <20080110233508.GA13315@electric-eye.fr.zoreil.com>

Thank you very much, this appears to work.

> The driver is still a POMS but it seems better now.

I notice that the vendor-supplied driver doesn't have these bugs.
Now, it does have a bug in that it doesn't have an "is this
interrupt for me?" test at all (and always returns "I handled it"),
but the bypass and its locking screwups are a later addition.

The same with the sp->rx_current bugs.  The original loop which used
rx_current as the loop iteration variable wasn't great style, precisely
because it hides the interaction that someone's "optimization" broke,
but I don't want to blame the vendor for things they didn't do.

Would you be interested in some cleanup patches?  In particular, I think I
can get rid of tx->lock entirely, or at least take it off the fast path.
All it's protecting is the write to sp->tx_current, and a few judicious
memory barriers can deal with that.


(Oh, another BUG: the sp->ResetCurrentTFD logic in hard_start_xmit is
just plain broken.  It writes the new data to entry 0, then increments
sp->tx_current just like usual.  THAT isn't in the vendor driver that
I see, either.)

^ permalink raw reply

* Re: [PATCH net-2.6.25 0/6][NETNS]: Make ipv6_devconf (all and default) live in net namespaces
From: David Miller @ 2008-01-11  1:54 UTC (permalink / raw)
  To: xemul; +Cc: netdev, devel, dlezcano, benjamin.thery
In-Reply-To: <478623C0.7030008@openvz.org>

From: Pavel Emelyanov <xemul@openvz.org>
Date: Thu, 10 Jan 2008 16:55:12 +0300

> The ipv6_devconf_(all) and ipv6_devconf_dflt are currently
> global, but should be per-namespace.
> 
> This set moves them on the struct net. Or, more precisely,
> on the struct netns_ipv6, which is already added.
> 
> Unfortunately, many code in the ipv6 cannot yet provide a 
> correct struct net to get the ipv6_devconf from (e.g. routing 
> code), so this part of job is to be done after the appropriate 
> parts are virtualized.
> 
> However, after this set user can play with the ipv6_devconf 
> inside a namespace not affecting the others.
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

All 6 patches applied, thanks Pavel.

^ permalink raw reply

* Re: my NAPI patches
From: Jay Cliburn @ 2008-01-11  1:44 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20080108.150848.116653308.davem@davemloft.net>

On Tue, 08 Jan 2008 15:08:48 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> 
> Can people do me a favor and do more constructive things like test
> that my patches really do fix the "device down during packet flood"
> bug 

Hi Dave,

Your patches seem to work for the atl1 NAPI implementation.  I can drop
the interface just fine while hammering the NIC with packets from iperf.

Jay

^ permalink raw reply

* Re: [ipw3945-devel] [PATCH 4/5] iwlwifi: iwl3945 eliminate sleepable task queue from context
From: Joonwoo Park @ 2008-01-11  1:38 UTC (permalink / raw)
  To: Zhu Yi
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, lkml,
	ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1200013607.3530.128.camel-F7Q/YMNgnyhhZnBKgDE8z0EOCMrvLtNR@public.gmane.org>

2008/1/11, Zhu Yi <yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> Hi Joonwoo,
>
> We already did something similiar in our code base. Could you please
> take a look at this patch?
>
> http://intellinuxwireless.org/repos/?p=iwlwifi.git;a=commitdiff;h=57aa02255e9d7be5e2494683fc2793bd1d0707e2
>
> Thanks,
> -yi

Ooops :-)
I should have checked code base, it's my fault.
it's seems very similar, but I could not made build unfortunately.
Can you introduce to me how to build it please?

my build error is here:
jason@ehus ~/SRC/DRIVERS/iwlwifi $ KSRC=/home/jason/SRC/LINUX/linux-2.6 make
make -C /home/jason/SRC/LINUX/linux-2.6 O=
M=/home/jason/SRC/DRIVERS/iwlwifi/compatible/
EXTRA_CFLAGS="-DCONFIG_IWL3945_DEBUG=y -DCONFIG_IWL4965_DEBUG=y
-DCONFIG_IWL3945_SPECTRUM_MEASUREMENT=y
-DCONFIG_IWL4965_SPECTRUM_MEASUREMENT=y -DCONFIG_IWL4965_SENSITIVITY=y
-DCONFIG_IWL3945_QOS=y -DCONFIG_IWL4965_QOS=y" modules
make[1]: Entering directory `/home/jason/SRC/LINUX/linux-2.6'
  CC [M]  /home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.o
In file included from
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:51:
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl-3945.h:443: error:
expected specifier-qualifier-list before 'ieee80211_key_alg'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c: In function
'iwl3945_add_station':
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:539: error:
implicit declaration of function 'MAC_ARG'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:539:
warning: too few arguments for format
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c: In function
'iwl3945_commit_rxon':
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:1161:
warning: too few arguments for format
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c: In function
'iwl3945_update_sta_key_info':
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:1400: error:
'struct iwl3945_hw_key' has no member named 'alg'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:1401: error:
'struct iwl3945_hw_key' has no member named 'keylen'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:1402: error:
'struct iwl3945_hw_key' has no member named 'key'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:1402: error:
'struct iwl3945_hw_key' has no member named 'key'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c: In function
'iwl3945_build_tx_cmd_hwcrypto':
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2594: error:
'struct iwl3945_hw_key' has no member named 'alg'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2597: error:
'struct iwl3945_hw_key' has no member named 'keylen'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2597: error:
'struct iwl3945_hw_key' has no member named 'key'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2597: error:
'struct iwl3945_hw_key' has no member named 'keylen'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2597: error:
'struct iwl3945_hw_key' has no member named 'key'
/home/jason/SRC/DRIVERS/iwlwifi/compatible/iwl3945-base.c:2597: error:
'struct iwl3945_hw_key' has no member named 'keylen'


Thanks,
Joonwoo

^ permalink raw reply

* Re: e1000 performance issue in 4 simultaneous links
From: David Miller @ 2008-01-11  1:28 UTC (permalink / raw)
  To: jesse.brandeburg; +Cc: leitao, netdev
In-Reply-To: <36D9DB17C6DE9E40B059440DB8D95F5204275B04@orsmsx418.amr.corp.intel.com>

From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Date: Thu, 10 Jan 2008 12:52:15 -0800

> I hope this explanation makes sense, but what it comes down to is that
> combining hardware round robin balancing with NAPI is a BAD IDEA.

Absolutely agreed on all counts.

No IRQ balancing should be done at all for networking device
interrupts, with zero exceptions.  It destroys performance.

^ permalink raw reply

* Re: [PATCH 2/5] iwlwifi: iwl3945 synchronize interruptand tasklet for down iwlwifi
From: Joonwoo Park @ 2008-01-11  1:24 UTC (permalink / raw)
  To: Chatre, Reinette; +Cc: netdev, Zhu, Yi, linux-wireless, lkml, ipw3945-devel
In-Reply-To: <D936D925018D154694D8A362EEB08920035CCD70@orsmsx416.amr.corp.intel.com>

Hello Reinette,

2008/1/11, Chatre, Reinette <reinette.chatre@intel.com>:
>
> Could synchronize_irq() be moved into iwl_disable_interrupts() ? I am

At this time, iwl_disable_interrupts() can be called with irq
disabled, so for do that I think additional modification would be
needed.

> also wondering if we cannot call tasklet_kill() before
> iwl_disable_interrupts() ... thus preventing it from being scheduled
> when we are going down.

Thanks for your catch, it seems tasklet can re-enable interrupts.
I'll handle and make an another patch for them at this weekend :)

Thanks,
Joonwoo

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

^ permalink raw reply

* Re: questions on NAPI processing latency and dropped network packets
From: David Miller @ 2008-01-11  1:20 UTC (permalink / raw)
  To: cfriesen; +Cc: netdev, linux-kernel
In-Reply-To: <478654C3.60806@nortel.com>

From: "Chris Friesen" <cfriesen@nortel.com>
Date: Thu, 10 Jan 2008 11:24:19 -0600

> I've got an issue that's popped up with a deployed system running 
> 2.6.10.
 ...
> So...anyone have any ideas/suggestions?

You have to be kidding, coming here for help with a nearly
4 year old kernel.

The networking stack as well as the e1000 driver has been
practically rewritten several times over since then.

^ permalink raw reply

* handing cloned frames to netif_rx()?
From: Johannes Berg @ 2008-01-11  1:17 UTC (permalink / raw)
  To: netdev; +Cc: Ron Rindjunsky, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

In 802.11n, there is a case where multiple data frames are received
aggregated into a single frame (A-MSDU).

Currently, we copy each of these frames out into their own skb, but
because of the alignment with that etc. I started to think that we could
simply pass up a clone of the original skb with start/length adjusted
properly so that it windows only the contained packet.

The buffer would be shared but the data within the original window
(starting with the 802.3 header) could even be written to, it won't be
needed again by mac80211 once it's handed off to netif_rx(). The skb
will obviously have lots of head- and tailroom but that space would be
part of other packets.

Is it ok to do this? Will something freak out if we pass a cloned skb to
netif_rx()?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
From: Jay Vosburgh @ 2008-01-11  1:06 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
	David Miller
In-Reply-To: <20080110210507.GA17344@gondor.apana.org.au>

Herbert Xu <herbert@gondor.apana.org.au> wrote:

>On Thu, Jan 10, 2008 at 04:03:53PM -0500, Andy Gospodarek wrote:
>>
>> > >Sure, but where do you call that function while holding the bond lock?
>> > 
>> > 	If I recall correctly, the problem was that tg3, et al, did
>> > things that might sleep, and bonding was calling from a timer context,
>> > which couldn't sleep.  It wasn't about the lock.
>> 
>> Exactly, I was just about to post the same.
>
>In other words, changing read_lock on bond->lock to read_lock_bh doesn't
>affect this one bit.

	For the case of the bond_set_multicast_list function, changing
the existing write_lock to a read_lock_bh doesn't affect any calls to
dev_set_mac_address (since it isn't called from there) and, apparently,
also resolves the lockdep warning.  I'm still trying to get lockdep to
generate the warning for me locally; I'm not sure which magic thing I'm
missing.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [ipw3945-devel] [PATCH 4/5] iwlwifi: iwl3945 eliminate sleepable task queue from context
From: Zhu Yi @ 2008-01-11  1:06 UTC (permalink / raw)
  To: Joonwoo Park
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, lkml,
	ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <11998765783953-git-send-email-joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Joonwoo,

We already did something similiar in our code base. Could you please
take a look at this patch?

http://intellinuxwireless.org/repos/?p=iwlwifi.git;a=commitdiff;h=57aa02255e9d7be5e2494683fc2793bd1d0707e2

Thanks,
-yi
On Wed, 2008-01-09 at 20:02 +0900, Joonwoo Park wrote:
> Eleminiate task queuing of iwl_pci_probe, register hw to ieee80211 immediately
> 
> Signed-off-by: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   66 +++++++++++++++++---------
>  1 files changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index f95f226..7e8d8b3 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -6171,27 +6171,9 @@ static void iwl_alive_start(struct iwl_priv *priv)
>  	if (iwl_is_rfkill(priv))
>  		return;
>  
> -	if (!priv->mac80211_registered) {
> -		/* Unlock so any user space entry points can call back into
> -		 * the driver without a deadlock... */
> -		mutex_unlock(&priv->mutex);
> -		iwl_rate_control_register(priv->hw);
> -		rc = ieee80211_register_hw(priv->hw);
> -		priv->hw->conf.beacon_int = 100;
> -		mutex_lock(&priv->mutex);
> -
> -		if (rc) {
> -			iwl_rate_control_unregister(priv->hw);
> -			IWL_ERROR("Failed to register network "
> -				  "device (error %d)\n", rc);
> -			return;
> -		}
> -
> -		priv->mac80211_registered = 1;
> +	iwl_reset_channel_flag(priv);
>  
> -		iwl_reset_channel_flag(priv);
> -	} else
> -		ieee80211_start_queues(priv->hw);
> +	ieee80211_start_queues(priv->hw);
>  
>  	priv->active_rate = priv->rates_mask;
>  	priv->active_rate_basic = priv->rates_mask & IWL_BASIC_RATES_MASK;
> @@ -6369,7 +6351,8 @@ static int __iwl_up(struct iwl_priv *priv)
>  
>  	/* clear (again), then enable host interrupts */
>  	iwl_write32(priv, CSR_INT, 0xFFFFFFFF);
> -	iwl_enable_interrupts(priv);
> +	if (priv->mac80211_registered)
> +		iwl_enable_interrupts(priv);
>  
>  	/* really make sure rfkill handshake bits are cleared */
>  	iwl_write32(priv, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
> @@ -6887,10 +6870,21 @@ static void iwl_bg_scan_completed(struct work_struct *work)
>  
>  static int iwl_mac_start(struct ieee80211_hw *hw)
>  {
> +	int ret;
>  	struct iwl_priv *priv = hw->priv;
>  
>  	IWL_DEBUG_MAC80211("enter\n");
>  
> +	ret = wait_event_interruptible_timeout(priv->wait_command_queue,
> +			iwl_is_ready(priv), HOST_COMPLETE_TIMEOUT);
> +
> +	if (ret == -ERESTARTSYS)
> +		return ret;
> +	else if (ret == 0 && !iwl_is_ready(priv)) {
> +		IWL_ERROR("IWL ready timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
>  	/* we should be verifying the device is ready to be opened */
>  	mutex_lock(&priv->mutex);
>  
> @@ -8299,6 +8293,19 @@ static void iwl_cancel_deferred_work(struct iwl_priv *priv)
>  	cancel_work_sync(&priv->beacon_update);
>  }
>  
> +static int iwl_register_hw(struct iwl_priv *priv)
> +{
> +	int err;
> +	IWL_DEBUG_INFO("register_hw\n");
> +	iwl_rate_control_register(priv->hw);
> +	err = ieee80211_register_hw(priv->hw);
> +	if (!err) {
> +		priv->hw->conf.beacon_int = 100;
> +		priv->mac80211_registered = 1;
> +	}
> +	return err;
> +}
> +
>  static struct attribute *iwl_sysfs_entries[] = {
>  	&dev_attr_antenna.attr,
>  	&dev_attr_channels.attr,
> @@ -8546,11 +8553,24 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		goto out_pci_alloc;
>  	}
>  
> +	err = __iwl_up(priv);
> +	if (err) {
> +		IWL_ERROR("Could not make up interface : %d\n", err);
> +		mutex_unlock(&priv->mutex);
> +		goto out_pci_alloc;
> +	}
> +
>  	mutex_unlock(&priv->mutex);
>  
> -	IWL_DEBUG_INFO("Queing UP work.\n");
> +	err = iwl_register_hw(priv);
> +	if (err) {
> +		iwl_rate_control_unregister(priv->hw);
> +		IWL_ERROR("Failed to register network "
> +			  "device (error %d)\n", err);
> +		goto out_pci_alloc;
> +	}
>  
> -	queue_work(priv->workqueue, &priv->up);
> +	iwl_enable_interrupts(priv);
>  
>  	return 0;
>  

^ permalink raw reply

* Re: RFC: igb: Intel 82575 gigabit ethernet driver (take #3)
From: Kok, Auke @ 2008-01-11  0:22 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: NetDev, Arjan van de Ven, Jesse Brandeburg, Ronciak, John,
	Andrew Morton
In-Reply-To: <4786B1E7.5030700@pobox.com>

Jeff Garzik wrote:
> Kok, Auke wrote:
>> All,
>>
>> here is the third version of the igb (82575) ethernet controller
>> driver. This
>> driver was previously posted 2007-07-13 and 2007-12-11. Many comments
>> received
>> were addressed:
>>
>> - removed indirection wrappers in the same way as e1000e and ixgbe.
>> - cleaned up largely against sparse, checkpatch
>> - removed module parameters and moved functionality to ethtool ioctls
>> - new NAPI API rewrites
>> - by default the driver runs in multiqueue mode with 2 to 40 RX queues
>> enabled.
>>
>> and specifically in this version:
>>
>> - register macro's were condensed for readability
>> - fixed namespace collisions by renaming functions to igb_*
>>
>> Since the driver is still too large (allthough the patch shrunk from
>> 558k to 416k
>> to 407k, almost 38% of its size) to post to this list I am attaching
>> the bzipped
>> patch here. You can get the same driver alternatively from here:
>>
>> http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch
>>
>> [407k]
>> http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch.bz2
>>
>> [74k]
>>
>> or through git:
>>     git://lost.foo-projects.org/~ahkok/git/linux-2.6 #igb
>>
>>
>> There are several concerns still open for this driver:
>> - hardware code is still a large API. we're expecting more hardware to be
>> supported by this driver in the future. The API has already been
>> scrubbed but we
>> anticipate that the remaining hooks will be used in the future.
>> - The register defines are still named "E1000_" as they are mostly
>> identical to
>> the e1000 chipsets (igb register space is a superset of most recent
>> e1000 register
>> sets).
> 
> I think we can throw it into netdev#upstream if you're ready...

yes, of course :)

both the patch file and the git tree should work for you.


Cheers,

Auke

^ permalink raw reply

* Re: RFC: igb: Intel 82575 gigabit ethernet driver (take #3)
From: Jeff Garzik @ 2008-01-11  0:01 UTC (permalink / raw)
  To: Kok, Auke
  Cc: NetDev, Arjan van de Ven, Jesse Brandeburg, Ronciak, John,
	Andrew Morton
In-Reply-To: <4786AB0C.6010202@intel.com>

Kok, Auke wrote:
> All,
> 
> here is the third version of the igb (82575) ethernet controller driver. This
> driver was previously posted 2007-07-13 and 2007-12-11. Many comments received
> were addressed:
> 
> - removed indirection wrappers in the same way as e1000e and ixgbe.
> - cleaned up largely against sparse, checkpatch
> - removed module parameters and moved functionality to ethtool ioctls
> - new NAPI API rewrites
> - by default the driver runs in multiqueue mode with 2 to 40 RX queues enabled.
> 
> and specifically in this version:
> 
> - register macro's were condensed for readability
> - fixed namespace collisions by renaming functions to igb_*
> 
> Since the driver is still too large (allthough the patch shrunk from 558k to 416k
> to 407k, almost 38% of its size) to post to this list I am attaching the bzipped
> patch here. You can get the same driver alternatively from here:
> 
> http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch
> [407k]
> http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch.bz2
> [74k]
> 
> or through git:
>     git://lost.foo-projects.org/~ahkok/git/linux-2.6 #igb
> 
> 
> There are several concerns still open for this driver:
> - hardware code is still a large API. we're expecting more hardware to be
> supported by this driver in the future. The API has already been scrubbed but we
> anticipate that the remaining hooks will be used in the future.
> - The register defines are still named "E1000_" as they are mostly identical to
> the e1000 chipsets (igb register space is a superset of most recent e1000 register
> sets).

I think we can throw it into netdev#upstream if you're ready...

	Jeff





^ permalink raw reply

* Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache
From: Herbert Xu @ 2008-01-11  0:00 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Eric Dumazet, Paul E. McKenney, davem, dipankar, netdev
In-Reply-To: <20080110231042.GA3199@ami.dom.local>

On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote:
>
> It seems this optimization could've a side effect: if during such a
> loop updates are done, and r is seen !NULL during while() check, but
> NULL after rcu_dereference(), the listing/counting could stop too
> soon. So, IMHO, probably the first version of this patch is more
> reliable. (Or alternatively additional check is needed before return.)

No, while the value of r->u.dst.rt_next can change between two readings,
the value of r cannot.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 3/4] ipg: fix queue stop condition in the xmit handler
From: Francois Romieu @ 2008-01-10 23:37 UTC (permalink / raw)
  To: David Miller; +Cc: linux, jeff, Andrew Morton, netdev
In-Reply-To: <20080110233508.GA13315@electric-eye.fr.zoreil.com>

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ipg.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 9752902..b234b29 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -1994,7 +1994,7 @@ static int ipg_nic_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	ipg_w32(IPG_DC_TX_DMA_POLL_NOW, DMA_CTRL);
 
 	if (sp->tx_current == (sp->tx_dirty + IPG_TFDLIST_LENGTH))
-		netif_wake_queue(dev);
+		netif_stop_queue(dev);
 
 	spin_unlock_irqrestore(&sp->lock, flags);
 
-- 
1.5.3.3


^ permalink raw reply related

* RFC: igb: Intel 82575 gigabit ethernet driver (take #3)
From: Kok, Auke @ 2008-01-10 23:32 UTC (permalink / raw)
  To: NetDev, Jeff Garzik
  Cc: Arjan van de Ven, Jesse Brandeburg, Ronciak, John, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 4517 bytes --]


All,

here is the third version of the igb (82575) ethernet controller driver. This
driver was previously posted 2007-07-13 and 2007-12-11. Many comments received
were addressed:

- removed indirection wrappers in the same way as e1000e and ixgbe.
- cleaned up largely against sparse, checkpatch
- removed module parameters and moved functionality to ethtool ioctls
- new NAPI API rewrites
- by default the driver runs in multiqueue mode with 2 to 40 RX queues enabled.

and specifically in this version:

- register macro's were condensed for readability
- fixed namespace collisions by renaming functions to igb_*

Since the driver is still too large (allthough the patch shrunk from 558k to 416k
to 407k, almost 38% of its size) to post to this list I am attaching the bzipped
patch here. You can get the same driver alternatively from here:

http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch
[407k]
http://foo-projects.org/~sofar/0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch.bz2
[74k]

or through git:
    git://lost.foo-projects.org/~ahkok/git/linux-2.6 #igb


There are several concerns still open for this driver:
- hardware code is still a large API. we're expecting more hardware to be
supported by this driver in the future. The API has already been scrubbed but we
anticipate that the remaining hooks will be used in the future.
- The register defines are still named "E1000_" as they are mostly identical to
the e1000 chipsets (igb register space is a superset of most recent e1000 register
sets).


Please review,


Cheers,

Auke

---

>From 4ec9e52f44de0c1c41265c5f326b573643f24da7 Mon Sep 17 00:00:00 2001
From: Auke Kok <auke-jan.h.kok@intel.com>
Date: Thu, 10 Jan 2008 14:55:46 -0800
Subject: [PATCH] igb: PCI-Express 82575 Gigabit Ethernet driver

We are pleased to announce a new Gigabit Ethernet product and its
driver to the linux community. This product is the Intel(R) 82575
Gigabit Ethernet adapter family. Physical adapters will be available
to the public soon. These adapters come in 2- and 4-port versions
(copper PHY) currently. Other variants will be available later.

The 82575 chipset supports significantly different features that
warrant a new driver. The descriptor format is (just like the
ixgbe driver) different. The device can use multiple MSI-X vectors
and multiple queues for both send and receive. This allows us to
optimize some of the driver code specifically as well compared to
the e1000-supported devices.

This version of the igb driver no lnger uses fake netdevices and
incorporates napi_struct members for each ring to do the multi-
queue polling. multi-queue is enabled by default and the driver
supports NAPI mode only.

All the namespace collisions should be gone in this version too. The
register macro's have been condensed to improve readability.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---
 drivers/net/Kconfig             |   22 +
 drivers/net/Makefile            |    1 +
 drivers/net/igb/Makefile        |   37 +
 drivers/net/igb/e1000_82575.c   | 1269 ++++++++++++
 drivers/net/igb/e1000_82575.h   |  150 ++
 drivers/net/igb/e1000_defines.h |  772 ++++++++
 drivers/net/igb/e1000_hw.h      |  599 ++++++
 drivers/net/igb/e1000_mac.c     | 1505 ++++++++++++++
 drivers/net/igb/e1000_mac.h     |   98 +
 drivers/net/igb/e1000_nvm.c     |  605 ++++++
 drivers/net/igb/e1000_nvm.h     |   40 +
 drivers/net/igb/e1000_phy.c     | 1807 +++++++++++++++++
 drivers/net/igb/e1000_phy.h     |   98 +
 drivers/net/igb/e1000_regs.h    |  270 +++
 drivers/net/igb/igb.h           |  300 +++
 drivers/net/igb/igb_ethtool.c   | 1927 ++++++++++++++++++
 drivers/net/igb/igb_main.c      | 4138 +++++++++++++++++++++++++++++++++++++++
 17 files changed, 13638 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/igb/Makefile
 create mode 100644 drivers/net/igb/e1000_82575.c
 create mode 100644 drivers/net/igb/e1000_82575.h
 create mode 100644 drivers/net/igb/e1000_defines.h
 create mode 100644 drivers/net/igb/e1000_hw.h
 create mode 100644 drivers/net/igb/e1000_mac.c
 create mode 100644 drivers/net/igb/e1000_mac.h
 create mode 100644 drivers/net/igb/e1000_nvm.c
 create mode 100644 drivers/net/igb/e1000_nvm.h
 create mode 100644 drivers/net/igb/e1000_phy.c
 create mode 100644 drivers/net/igb/e1000_phy.h
 create mode 100644 drivers/net/igb/e1000_regs.h
 create mode 100644 drivers/net/igb/igb.h
 create mode 100644 drivers/net/igb/igb_ethtool.c
 create mode 100644 drivers/net/igb/igb_main.c



[-- Attachment #2: 0001-igb-PCI-Express-82575-Gigabit-Ethernet-driver.patch.bz2 --]
[-- Type: application/x-bzip2, Size: 75975 bytes --]

^ permalink raw reply

* [PATCH 4/4] ipg: fix Tx completion irq request
From: Francois Romieu @ 2008-01-10 23:38 UTC (permalink / raw)
  To: David Miller; +Cc: linux, jeff, Andrew Morton, netdev
In-Reply-To: <20080110233508.GA13315@electric-eye.fr.zoreil.com>

The current logic will only request an ack for the first pending
packet. No irq is triggered as soon as the CPU submits a few
packets a bit quickly.  Let's request an irq for every packet
instead.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ipg.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index b234b29..50f0c17 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -1934,10 +1934,7 @@ static int ipg_nic_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	if (sp->tenmbpsmode)
 		txfd->tfc |= cpu_to_le64(IPG_TFC_TXINDICATE);
-	else if (!((sp->tx_current - sp->tx_dirty + 1) >
-	    IPG_FRAMESBETWEENTXDMACOMPLETES)) {
-		txfd->tfc |= cpu_to_le64(IPG_TFC_TXDMAINDICATE);
-	}
+	txfd->tfc |= cpu_to_le64(IPG_TFC_TXDMAINDICATE);
 	/* Based on compilation option, determine if FCS is to be
 	 * appended to transmit frame by IPG.
 	 */
-- 
1.5.3.3


^ permalink raw reply related

* Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache
From: Paul E. McKenney @ 2008-01-10 23:51 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Eric Dumazet, Herbert Xu, davem, dipankar, netdev
In-Reply-To: <20080110231042.GA3199@ami.dom.local>

On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote:
> Eric Dumazet wrote, On 01/09/2008 11:37 AM:
> ...
> > [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache
> ...
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index d337706..28484f3 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -283,12 +283,12 @@ static struct rtable *rt_cache_get_first(struct seq_file *seq)
> >  			break;
> >  		rcu_read_unlock_bh();
> >  	}
> > -	return r;
> > +	return rcu_dereference(r);
> >  }
> >  
> >  static struct rtable *rt_cache_get_next(struct seq_file *seq, struct rtable *r)
> >  {
> > -	struct rt_cache_iter_state *st = rcu_dereference(seq->private);
> > +	struct rt_cache_iter_state *st = seq->private;
> >  
> >  	r = r->u.dst.rt_next;
> >  	while (!r) {
> > @@ -298,7 +298,7 @@ static struct rtable *rt_cache_get_next(struct seq_file *seq, struct rtable *r)
> >  		rcu_read_lock_bh();
> >  		r = rt_hash_table[st->bucket].chain;
> >  	}
> > -	return r;
> > +	return rcu_dereference(r);
> >  }
> 
> It seems this optimization could've a side effect: if during such a
> loop updates are done, and r is seen !NULL during while() check, but
> NULL after rcu_dereference(), the listing/counting could stop too
> soon. So, IMHO, probably the first version of this patch is more
> reliable. (Or alternatively additional check is needed before return.)

Looks to me like "r" is a local variable (argument list), so there
should not be any possibility of it being changed by some other
task, right?

							Thanx, Paul

^ permalink raw reply

* [PATCH 2/4] ipg: plug Tx completion leak
From: Francois Romieu @ 2008-01-10 23:37 UTC (permalink / raw)
  To: David Miller; +Cc: linux, jeff, Andrew Morton, netdev
In-Reply-To: <20080110233508.GA13315@electric-eye.fr.zoreil.com>

The Tx skb release could not free more than one skb per call.
Add it to the fact that the xmit handler does not check for
a queue full condition and you have a recipe to leak quickly.

Let's release every pending Tx descriptor which has been given
back to the host CPU by the network controller. The xmit handler
suggests that it is done through the IPG_TFC_TFDDONE bit.

Remove the former "curr" computing: it does not produce anything
usable in its current form.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ipg.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index cd1650e..9752902 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -857,21 +857,14 @@ static void init_tfdlist(struct net_device *dev)
 static void ipg_nic_txfree(struct net_device *dev)
 {
 	struct ipg_nic_private *sp = netdev_priv(dev);
-	void __iomem *ioaddr = sp->ioaddr;
-	unsigned int curr;
-	u64 txd_map;
-	unsigned int released, pending;
-
-	txd_map = (u64)sp->txd_map;
-	curr = ipg_r32(TFD_LIST_PTR_0) -
-		do_div(txd_map, sizeof(struct ipg_tx)) - 1;
+	unsigned int released, pending, dirty;
 
 	IPG_DEBUG_MSG("_nic_txfree\n");
 
 	pending = sp->tx_current - sp->tx_dirty;
+	dirty = sp->tx_dirty % IPG_TFDLIST_LENGTH;
 
 	for (released = 0; released < pending; released++) {
-		unsigned int dirty = sp->tx_dirty % IPG_TFDLIST_LENGTH;
 		struct sk_buff *skb = sp->TxBuff[dirty];
 		struct ipg_tx *txfd = sp->txd + dirty;
 
@@ -882,11 +875,8 @@ static void ipg_nic_txfree(struct net_device *dev)
 		 * If the TFDDone bit is set, free the associated
 		 * buffer.
 		 */
-		if (dirty == curr)
-			break;
-
-		/* Setup TFDDONE for compatible issue. */
-		txfd->tfc |= cpu_to_le64(IPG_TFC_TFDDONE);
+		if (!(txfd->tfc & cpu_to_le64(IPG_TFC_TFDDONE)))
+                        break;
 
 		/* Free the transmit buffer. */
 		if (skb) {
@@ -898,6 +888,7 @@ static void ipg_nic_txfree(struct net_device *dev)
 
 			sp->TxBuff[dirty] = NULL;
 		}
+		dirty = (dirty + 1) % IPG_TFDLIST_LENGTH;
 	}
 
 	sp->tx_dirty += released;
-- 
1.5.3.3


^ permalink raw reply related

* [PATCH 0/4] Pull request for 'ipg-fixes' branch
From: Francois Romieu @ 2008-01-10 23:35 UTC (permalink / raw)
  To: David Miller; +Cc: linux, jeff, Andrew Morton, netdev

[-- Attachment #1: Type: text/plain, Size: 5177 bytes --]

Please pull from branch 'ipg-fixes' in repository

git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git ipg-fixes

to get the changes below.

I have tested the driver with a PIV HT based motherboard. The network
controller is connected to a fast ethernet switch (yeah, I'm cheap).
A second host is performing two loop of scp in both direction for a
400Mb file. The files are sha1sumed after each scp. I have added a
'ping -q -f -l16' from the computer under test after some time.

After 35 copies from the computer under test and 28 copies to it
(the ping eats a bit):
             total       used       free     shared    buffers     cached
Mem:       1019040    1003860      15180          0      20556     936792
-/+ buffers/cache:      46512     972528
Swap:      2031608          0    2031608

Before:
             total       used       free     shared    buffers     cached
Mem:       1019040     572036     447004          0      14988     525924
-/+ buffers/cache:      31124     987916
Swap:      2031608          0    2031608

/proc/slabinfo before and after the test are attached.

The driver is still a POMS but it seems better now.

I will not be available to work further on this issue before sunday.
I'd appreciate being Cced though.

Distance from 'net-2.6/master' (27d1cba21fcc50c37eef5042c6be9fa7135e88fc)
-------------------------------------------------------------------------

286c83ce6e8263a5c4c55a57b4c1040800de0171
d42f3afc953f9c99ffe84667a3ecf0d3b69f3d64
358bf4b8e8cbde5d6411b219e93a61728c892685
a58cceed4464ba8ae94294184c15f43e92a5de89

Diffstat
--------

 drivers/net/ipg.c |   36 ++++++++++++------------------------
 1 files changed, 12 insertions(+), 24 deletions(-)

Shortlog
--------

Francois Romieu (4):
      ipg: balance locking in irq handler
      ipg: plug Tx completion leak
      ipg: fix queue stop condition in the xmit handler
      ipg: fix Tx completion irq request

Patch
-----

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index dbd23bb..50f0c17 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -857,21 +857,14 @@ static void init_tfdlist(struct net_device *dev)
 static void ipg_nic_txfree(struct net_device *dev)
 {
 	struct ipg_nic_private *sp = netdev_priv(dev);
-	void __iomem *ioaddr = sp->ioaddr;
-	unsigned int curr;
-	u64 txd_map;
-	unsigned int released, pending;
-
-	txd_map = (u64)sp->txd_map;
-	curr = ipg_r32(TFD_LIST_PTR_0) -
-		do_div(txd_map, sizeof(struct ipg_tx)) - 1;
+	unsigned int released, pending, dirty;
 
 	IPG_DEBUG_MSG("_nic_txfree\n");
 
 	pending = sp->tx_current - sp->tx_dirty;
+	dirty = sp->tx_dirty % IPG_TFDLIST_LENGTH;
 
 	for (released = 0; released < pending; released++) {
-		unsigned int dirty = sp->tx_dirty % IPG_TFDLIST_LENGTH;
 		struct sk_buff *skb = sp->TxBuff[dirty];
 		struct ipg_tx *txfd = sp->txd + dirty;
 
@@ -882,11 +875,8 @@ static void ipg_nic_txfree(struct net_device *dev)
 		 * If the TFDDone bit is set, free the associated
 		 * buffer.
 		 */
-		if (dirty == curr)
-			break;
-
-		/* Setup TFDDONE for compatible issue. */
-		txfd->tfc |= cpu_to_le64(IPG_TFC_TFDDONE);
+		if (!(txfd->tfc & cpu_to_le64(IPG_TFC_TFDDONE)))
+                        break;
 
 		/* Free the transmit buffer. */
 		if (skb) {
@@ -898,6 +888,7 @@ static void ipg_nic_txfree(struct net_device *dev)
 
 			sp->TxBuff[dirty] = NULL;
 		}
+		dirty = (dirty + 1) % IPG_TFDLIST_LENGTH;
 	}
 
 	sp->tx_dirty += released;
@@ -1630,6 +1621,8 @@ static irqreturn_t ipg_interrupt_handler(int irq, void *dev_inst)
 #ifdef JUMBO_FRAME
 	ipg_nic_rxrestore(dev);
 #endif
+	spin_lock(&sp->lock);
+
 	/* Get interrupt source information, and acknowledge
 	 * some (i.e. TxDMAComplete, RxDMAComplete, RxEarly,
 	 * IntRequested, MacControlFrame, LinkEvent) interrupts
@@ -1647,9 +1640,7 @@ static irqreturn_t ipg_interrupt_handler(int irq, void *dev_inst)
 	handled = 1;
 
 	if (unlikely(!netif_running(dev)))
-		goto out;
-
-	spin_lock(&sp->lock);
+		goto out_unlock;
 
 	/* If RFDListEnd interrupt, restore all used RFDs. */
 	if (status & IPG_IS_RFD_LIST_END) {
@@ -1733,9 +1724,9 @@ out_enable:
 	ipg_w16(IPG_IE_TX_DMA_COMPLETE | IPG_IE_RX_DMA_COMPLETE |
 		IPG_IE_HOST_ERROR | IPG_IE_INT_REQUESTED | IPG_IE_TX_COMPLETE |
 		IPG_IE_LINK_EVENT | IPG_IE_UPDATE_STATS, INT_ENABLE);
-
+out_unlock:
 	spin_unlock(&sp->lock);
-out:
+
 	return IRQ_RETVAL(handled);
 }
 
@@ -1943,10 +1934,7 @@ static int ipg_nic_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	if (sp->tenmbpsmode)
 		txfd->tfc |= cpu_to_le64(IPG_TFC_TXINDICATE);
-	else if (!((sp->tx_current - sp->tx_dirty + 1) >
-	    IPG_FRAMESBETWEENTXDMACOMPLETES)) {
-		txfd->tfc |= cpu_to_le64(IPG_TFC_TXDMAINDICATE);
-	}
+	txfd->tfc |= cpu_to_le64(IPG_TFC_TXDMAINDICATE);
 	/* Based on compilation option, determine if FCS is to be
 	 * appended to transmit frame by IPG.
 	 */
@@ -2003,7 +1991,7 @@ static int ipg_nic_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	ipg_w32(IPG_DC_TX_DMA_POLL_NOW, DMA_CTRL);
 
 	if (sp->tx_current == (sp->tx_dirty + IPG_TFDLIST_LENGTH))
-		netif_wake_queue(dev);
+		netif_stop_queue(dev);
 
 	spin_unlock_irqrestore(&sp->lock, flags);
 
-- 
Ueimor

[-- Attachment #2: slab.tgz --]
[-- Type: application/x-gzip, Size: 9304 bytes --]

^ permalink raw reply related

* [PATCH 1/4] ipg: balance locking in irq handler
From: Francois Romieu @ 2008-01-10 23:35 UTC (permalink / raw)
  To: David Miller; +Cc: linux, jeff, Andrew Morton, netdev
In-Reply-To: <20080110233508.GA13315@electric-eye.fr.zoreil.com>

Spotted-by: <linux@horizon.com>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ipg.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index dbd23bb..cd1650e 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -1630,6 +1630,8 @@ static irqreturn_t ipg_interrupt_handler(int irq, void *dev_inst)
 #ifdef JUMBO_FRAME
 	ipg_nic_rxrestore(dev);
 #endif
+	spin_lock(&sp->lock);
+
 	/* Get interrupt source information, and acknowledge
 	 * some (i.e. TxDMAComplete, RxDMAComplete, RxEarly,
 	 * IntRequested, MacControlFrame, LinkEvent) interrupts
@@ -1647,9 +1649,7 @@ static irqreturn_t ipg_interrupt_handler(int irq, void *dev_inst)
 	handled = 1;
 
 	if (unlikely(!netif_running(dev)))
-		goto out;
-
-	spin_lock(&sp->lock);
+		goto out_unlock;
 
 	/* If RFDListEnd interrupt, restore all used RFDs. */
 	if (status & IPG_IS_RFD_LIST_END) {
@@ -1733,9 +1733,9 @@ out_enable:
 	ipg_w16(IPG_IE_TX_DMA_COMPLETE | IPG_IE_RX_DMA_COMPLETE |
 		IPG_IE_HOST_ERROR | IPG_IE_INT_REQUESTED | IPG_IE_TX_COMPLETE |
 		IPG_IE_LINK_EVENT | IPG_IE_UPDATE_STATS, INT_ENABLE);
-
+out_unlock:
 	spin_unlock(&sp->lock);
-out:
+
 	return IRQ_RETVAL(handled);
 }
 
-- 
1.5.3.3


^ permalink raw reply related


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