Netdev List
 help / color / mirror / Atom feed
* [PATCH 26/27] e1000e: increment the driver version
From: Jeff Kirsher @ 2010-12-11  6:20 UTC (permalink / raw)
  To: davem, davem; +Cc: Bruce Allan, netdev, gospo, bphilips, Jeff Kirsher
In-Reply-To: <1292048440-22125-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/e1000e/netdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 6e1f3a3..5530d0b 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -54,7 +54,7 @@
 
 #define DRV_EXTRAVERSION "-k2"
 
-#define DRV_VERSION "1.2.7" DRV_EXTRAVERSION
+#define DRV_VERSION "1.2.20" DRV_EXTRAVERSION
 char e1000e_driver_name[] = "e1000e";
 const char e1000e_driver_version[] = DRV_VERSION;
 
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 27/27] igb: Add new function to read part number from EEPROM in string format
From: Jeff Kirsher @ 2010-12-11  6:20 UTC (permalink / raw)
  To: davem, davem; +Cc: Carolyn Wyborny, netdev, gospo, bphilips, Jeff Kirsher
In-Reply-To: <1292048440-22125-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

New adapters will have part numbers stored in string format rather than
simple hex format. This function will read part number formats in either
hex or string.

Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/igb/e1000_defines.h |    7 +++
 drivers/net/igb/e1000_nvm.c     |   93 ++++++++++++++++++++++++++++++++++++---
 drivers/net/igb/e1000_nvm.h     |    2 +
 drivers/net/igb/igb_main.c      |   11 +++--
 4 files changed, 102 insertions(+), 11 deletions(-)

diff --git a/drivers/net/igb/e1000_defines.h b/drivers/net/igb/e1000_defines.h
index 6222279..6319ed9 100644
--- a/drivers/net/igb/e1000_defines.h
+++ b/drivers/net/igb/e1000_defines.h
@@ -419,6 +419,9 @@
 #define E1000_ERR_SWFW_SYNC 13
 #define E1000_NOT_IMPLEMENTED 14
 #define E1000_ERR_MBX      15
+#define E1000_ERR_INVALID_ARGUMENT  16
+#define E1000_ERR_NO_SPACE          17
+#define E1000_ERR_NVM_PBA_SECTION   18
 
 /* Loop limit on how long we wait for auto-negotiation to complete */
 #define COPPER_LINK_UP_LIMIT              10
@@ -580,11 +583,15 @@
 
 /* Mask bits for fields in Word 0x1a of the NVM */
 
+/* length of string needed to store part num */
+#define E1000_PBANUM_LENGTH         11
+
 /* For checksumming, the sum of all words in the NVM should equal 0xBABA. */
 #define NVM_SUM                    0xBABA
 
 #define NVM_PBA_OFFSET_0           8
 #define NVM_PBA_OFFSET_1           9
+#define NVM_PBA_PTR_GUARD          0xFAFA
 #define NVM_WORD_SIZE_BASE_SHIFT   6
 
 /* NVM Commands - Microwire */
diff --git a/drivers/net/igb/e1000_nvm.c b/drivers/net/igb/e1000_nvm.c
index d83b77fa..6b5cc2c 100644
--- a/drivers/net/igb/e1000_nvm.c
+++ b/drivers/net/igb/e1000_nvm.c
@@ -445,31 +445,112 @@ out:
 }
 
 /**
- *  igb_read_part_num - Read device part number
+ *  igb_read_part_string - Read device part number
  *  @hw: pointer to the HW structure
  *  @part_num: pointer to device part number
+ *  @part_num_size: size of part number buffer
  *
  *  Reads the product board assembly (PBA) number from the EEPROM and stores
  *  the value in part_num.
  **/
-s32 igb_read_part_num(struct e1000_hw *hw, u32 *part_num)
+s32 igb_read_part_string(struct e1000_hw *hw, u8 *part_num, u32 part_num_size)
 {
-	s32  ret_val;
+	s32 ret_val;
 	u16 nvm_data;
+	u16 pointer;
+	u16 offset;
+	u16 length;
+
+	if (part_num == NULL) {
+		hw_dbg("PBA string buffer was null\n");
+		ret_val = E1000_ERR_INVALID_ARGUMENT;
+		goto out;
+	}
 
 	ret_val = hw->nvm.ops.read(hw, NVM_PBA_OFFSET_0, 1, &nvm_data);
 	if (ret_val) {
 		hw_dbg("NVM Read Error\n");
 		goto out;
 	}
-	*part_num = (u32)(nvm_data << 16);
 
-	ret_val = hw->nvm.ops.read(hw, NVM_PBA_OFFSET_1, 1, &nvm_data);
+	ret_val = hw->nvm.ops.read(hw, NVM_PBA_OFFSET_1, 1, &pointer);
+	if (ret_val) {
+		hw_dbg("NVM Read Error\n");
+		goto out;
+	}
+
+	/*
+	 * if nvm_data is not ptr guard the PBA must be in legacy format which
+	 * means pointer is actually our second data word for the PBA number
+	 * and we can decode it into an ascii string
+	 */
+	if (nvm_data != NVM_PBA_PTR_GUARD) {
+		hw_dbg("NVM PBA number is not stored as string\n");
+
+		/* we will need 11 characters to store the PBA */
+		if (part_num_size < 11) {
+			hw_dbg("PBA string buffer too small\n");
+			return E1000_ERR_NO_SPACE;
+		}
+
+		/* extract hex string from data and pointer */
+		part_num[0] = (nvm_data >> 12) & 0xF;
+		part_num[1] = (nvm_data >> 8) & 0xF;
+		part_num[2] = (nvm_data >> 4) & 0xF;
+		part_num[3] = nvm_data & 0xF;
+		part_num[4] = (pointer >> 12) & 0xF;
+		part_num[5] = (pointer >> 8) & 0xF;
+		part_num[6] = '-';
+		part_num[7] = 0;
+		part_num[8] = (pointer >> 4) & 0xF;
+		part_num[9] = pointer & 0xF;
+
+		/* put a null character on the end of our string */
+		part_num[10] = '\0';
+
+		/* switch all the data but the '-' to hex char */
+		for (offset = 0; offset < 10; offset++) {
+			if (part_num[offset] < 0xA)
+				part_num[offset] += '0';
+			else if (part_num[offset] < 0x10)
+				part_num[offset] += 'A' - 0xA;
+		}
+
+		goto out;
+	}
+
+	ret_val = hw->nvm.ops.read(hw, pointer, 1, &length);
 	if (ret_val) {
 		hw_dbg("NVM Read Error\n");
 		goto out;
 	}
-	*part_num |= nvm_data;
+
+	if (length == 0xFFFF || length == 0) {
+		hw_dbg("NVM PBA number section invalid length\n");
+		ret_val = E1000_ERR_NVM_PBA_SECTION;
+		goto out;
+	}
+	/* check if part_num buffer is big enough */
+	if (part_num_size < (((u32)length * 2) - 1)) {
+		hw_dbg("PBA string buffer too small\n");
+		ret_val = E1000_ERR_NO_SPACE;
+		goto out;
+	}
+
+	/* trim pba length from start of string */
+	pointer++;
+	length--;
+
+	for (offset = 0; offset < length; offset++) {
+		ret_val = hw->nvm.ops.read(hw, pointer + offset, 1, &nvm_data);
+		if (ret_val) {
+			hw_dbg("NVM Read Error\n");
+			goto out;
+		}
+		part_num[offset * 2] = (u8)(nvm_data >> 8);
+		part_num[(offset * 2) + 1] = (u8)(nvm_data & 0xFF);
+	}
+	part_num[offset * 2] = '\0';
 
 out:
 	return ret_val;
diff --git a/drivers/net/igb/e1000_nvm.h b/drivers/net/igb/e1000_nvm.h
index 1041c34..29c956a 100644
--- a/drivers/net/igb/e1000_nvm.h
+++ b/drivers/net/igb/e1000_nvm.h
@@ -32,6 +32,8 @@ s32  igb_acquire_nvm(struct e1000_hw *hw);
 void igb_release_nvm(struct e1000_hw *hw);
 s32  igb_read_mac_addr(struct e1000_hw *hw);
 s32  igb_read_part_num(struct e1000_hw *hw, u32 *part_num);
+s32  igb_read_part_string(struct e1000_hw *hw, u8 *part_num,
+                          u32 part_num_size);
 s32  igb_read_nvm_eerd(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
 s32  igb_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
 s32  igb_validate_nvm_checksum(struct e1000_hw *hw);
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 67ea262..041f8e6 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1729,12 +1729,13 @@ static int __devinit igb_probe(struct pci_dev *pdev,
 	struct igb_adapter *adapter;
 	struct e1000_hw *hw;
 	u16 eeprom_data = 0;
+	s32 ret_val;
 	static int global_quad_port_a; /* global quad port a indication */
 	const struct e1000_info *ei = igb_info_tbl[ent->driver_data];
 	unsigned long mmio_start, mmio_len;
 	int err, pci_using_dac;
 	u16 eeprom_apme_mask = IGB_EEPROM_APME;
-	u32 part_num;
+	u8 part_str[E1000_PBANUM_LENGTH];
 
 	/* Catch broken hardware that put the wrong VF device ID in
 	 * the PCIe SR-IOV capability.
@@ -2000,10 +2001,10 @@ static int __devinit igb_probe(struct pci_dev *pdev,
 		   "unknown"),
 		 netdev->dev_addr);
 
-	igb_read_part_num(hw, &part_num);
-	dev_info(&pdev->dev, "%s: PBA No: %06x-%03x\n", netdev->name,
-		(part_num >> 8), (part_num & 0xff));
-
+	ret_val = igb_read_part_string(hw, part_str, E1000_PBANUM_LENGTH);
+	if (ret_val)
+		strcpy(part_str, "Unknown");
+	dev_info(&pdev->dev, "%s: PBA No: %s\n", netdev->name, part_str);
 	dev_info(&pdev->dev,
 		"Using %s interrupts. %d rx queue(s), %d tx queue(s)\n",
 		adapter->msix_entries ? "MSI-X" :
-- 
1.7.3.2


^ permalink raw reply related

* Re: [PATCH 04/27] Documentation/networking/igbvf.txt: Update documentation
From: Ben Hutchings @ 2010-12-11  6:34 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, bphilips
In-Reply-To: <1292048241-22026-5-git-send-email-jeffrey.t.kirsher@intel.com>

On Fri, 2010-12-10 at 22:16 -0800, Jeff Kirsher wrote:
> Update Intel Wired LAN igbvf documentation.
> 
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  Documentation/networking/igbvf.txt |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/networking/igbvf.txt b/Documentation/networking/igbvf.txt
> index 0560281..694817b 100644
> --- a/Documentation/networking/igbvf.txt
> +++ b/Documentation/networking/igbvf.txt
> @@ -58,7 +58,9 @@ Additional Configurations
>    Ethtool
>    -------
>    The driver utilizes the ethtool interface for driver configuration and
> -  diagnostics, as well as displaying statistical information.
> +  diagnostics, as well as displaying statistical information.  Ethtool
> +  version 3.0 or later is required for this functionality, although we
> +  strongly recommend downloading the latest version at:
>  
>    http://sourceforge.net/projects/gkernel.

Do I have to point out every instance of this URL individually?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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] rfc: ethtool: early-orphan control
From: Eric Dumazet @ 2010-12-11  8:03 UTC (permalink / raw)
  To: Simon Horman; +Cc: netdev, Ben Hutchings
In-Reply-To: <20101211042434.GB32453@verge.net.au>

Le samedi 11 décembre 2010 à 13:24 +0900, Simon Horman a écrit :
> On Sat, Dec 11, 2010 at 01:13:35PM +0900, Simon Horman wrote:
> > Early orphaning is an optimisation which avoids unnecessary cache misses by
> > orphaning an skb just before it is handed to a device for transmit thus
> > avoiding the case where the orphaning occurs on a different CPU.
> > 
> > In the case of bonded devices this has the unfortunate side-effect of
> > breaking down flow control allowing a socket to send UDP packets as fast as
> > the CPU will allow. This is particularly undesirable in virtualised
> > network environments.
> > 
> > This patch introduces ethtool control of early orphaning.
> > It remains on by default by it now may be disabled on a per-interface basis.
> > 
> > I have implemented this as a generic flag.
> > As it seems to be the first generic flag that requires
> > no driver awareness I also supplied a default flag handler.
> > I am unsure if any aspect of this approach is acceptable.
> > 
> > I believe Eric has it in mind that some of the calls
> > to skb_orphan() in drivers can be removed with the addition
> > of this feature. I need to discuss that with him further.
> > 
> > A patch for the ethtool user-space utility accompanies this patch.
> 
> The following results were measured using kvm using virto without vhost net.
> The virtio device is bridged to a bond device which has one gigabit slave.
> 

As you know, vhost net does the orphaning, as well as some NIC drivers,
so one UDP flood would have same problem.

I wonder if this problem could not be solved in other ways.


We might do early orphaning only for sockets with SOCK_USE_WRITE_QUEUE
flag asserted. (tcp sets it)

Then, we could also say : Why tcp use sock_wfree() at all...


Hmm...



^ permalink raw reply

* Re: [net-next 00/27][pull-request] Intel Wired LAN Driver Updates
From: David Miller @ 2010-12-11 19:44 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1292048241-22026-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 10 Dec 2010 22:16:54 -0800

> Here are a batch of fixes and cleanups intended for 2.6.38.
> 
> v2- updated igb.txt and e1000e (patch 27) based on feedback from
>     Ben Hutchings and Joe Perches.
> 
> The following changes since commit:
> 
> commit a5d62a149bb8f5359aff7ed7dce339752fbabfd9
> Author: David S. Miller <davem@davemloft.net>
> Date:   Fri Dec 10 16:49:24 2010 -0800
>   isdn: Fix printed out copy_from_user() return value after previous change
> 
> are available in the git repository at:
> 
>   master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6.git master

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [PATCH 04/27] Documentation/networking/igbvf.txt: Update documentation
From: David Miller @ 2010-12-11 19:44 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: bhutchings, netdev, gospo, bphilips
In-Reply-To: <AANLkTikUqZsaSg-q+GJvRrUb4C8rFZSbBta0tyeh==Ay@mail.gmail.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 11 Dec 2010 02:45:55 -0800

> I have additional changes for all the docs which I planned on
> submitting next week.  I will update all the URL as well in that
> update.

This makes sense to me, thanks Jeff.


^ permalink raw reply

* Re: [PATCH 1/4] s2io: rx_ring_sz bounds checking
From: David Miller @ 2010-12-11 19:47 UTC (permalink / raw)
  To: jon.mason; +Cc: netdev, sivakumar.subramani, sreenivasa.honnur, ram.vepa
In-Reply-To: <1292031604-16628-1-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:01 -0600

> modparm rx_ring_sz can be set to be greater than the maximum allowable
> number of blocks.  This results in an array overrun when probing the
> driver, and causes memory corruption.
> 
> Also, the MAX_RX_DESC_1 multiply the max number of rings by max number
> of blocker per ring by 127, but the driver does the same calculation
> with 127+1.  This results in the possibility of the value being set
> being larger than the maximum allowable value.
> 
> Finally, clean-up the s2io_ethtool_gringparam code to be more
> intuitive.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/4] s2io: make strings at tables const
From: David Miller @ 2010-12-11 19:47 UTC (permalink / raw)
  To: jon.mason
  Cc: netdev, sivakumar.subramani, sreenivasa.honnur, ram.vepa,
	shemminger
In-Reply-To: <1292031604-16628-2-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:02 -0600

> Put immutable data in read/only section.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

^ permalink raw reply

* Re: [PATCH 3/4] s2io: Update Driver Version
From: David Miller @ 2010-12-11 19:47 UTC (permalink / raw)
  To: jon.mason; +Cc: netdev, sivakumar.subramani, sreenivasa.honnur, ram.vepa
In-Reply-To: <1292031604-16628-3-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:03 -0600

> Update Driver Version
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

^ permalink raw reply

* Re: [PATCH 4/4] Using static const generally increases object text and decreases data size. It also generally decreases overall object size.
From: David Miller @ 2010-12-11 19:48 UTC (permalink / raw)
  To: jon.mason; +Cc: netdev, sivakumar.subramani, sreenivasa.honnur, ram.vepa, joe
In-Reply-To: <1292031604-16628-4-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:04 -0600

>    text	   data	    bss	    dec	    hex	filename
>  109387	    389	  24432	 134208	  20c40	drivers/net/s2io.o.old
>  109358	    389	  24432	 134179	  20c23	drivers/net/s2io.o.new
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> Acked-by: Jon Mason <jon.mason@exar.com>

Applied, but I had to add an "s2io: " prefix to the subject line.

Thanks.

^ permalink raw reply

* [PATCH] net: au1000_eth: remove unused global variable.
From: Manuel Lauss @ 2010-12-11 19:53 UTC (permalink / raw)
  To: netdev; +Cc: Florian Fainelli, Manuel Lauss

The driver global au_macs[] is unused in the entire kernel tree,
so remove it.

Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
 drivers/net/au1000_eth.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index 53eff9b..b9debcf 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -106,8 +106,6 @@ MODULE_VERSION(DRV_VERSION);
  * complete immediately.
  */
 
-struct au1000_private *au_macs[NUM_ETH_INTERFACES];
-
 /*
  * board-specific configurations
  *
-- 
1.7.3.3


^ permalink raw reply related

* Re: [PATCH] net: au1000_eth: remove unused global variable.
From: David Miller @ 2010-12-11 20:01 UTC (permalink / raw)
  To: manuel.lauss; +Cc: netdev, florian
In-Reply-To: <1292097222-11319-1-git-send-email-manuel.lauss@googlemail.com>

From: Manuel Lauss <manuel.lauss@googlemail.com>
Date: Sat, 11 Dec 2010 20:53:42 +0100

> The driver global au_macs[] is unused in the entire kernel tree,
> so remove it.
> 
> Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] pppoe.c: Fix kernel panic caused by __pppoe_xmit
From: Jarek Poplawski @ 2010-12-11 20:08 UTC (permalink / raw)
  To: Andrej Ota
  Cc: Paweł Staszewski, Andrew Morton, netdev, Paul Mackerras,
	bugzilla-daemon, bugme-daemon, pstaszewski, Eric Dumazet,
	David Miller, Gorik Van Steenberge, Daniel Kenzelmann,
	Denys Fedoryshchenko
In-Reply-To: <4D037236.4080903@ota.si>

On Sat, Dec 11, 2010 at 01:44:38PM +0100, Andrej Ota wrote:
> __pppoe_xmit function return value was invalid resulting in
> additional call to kfree_skb on already freed skb. This resulted in
> memory corruption and consequent kernel panic after PPPoE peer
> terminated the link.
> 
> This fixes commit 55c95e738da85373965cb03b4f975d0fd559865b.
> 
> Signed-off-by: Jarek Poplawski [jarkao2@gmail.com]
> Signed-off-by: Andrej Ota [andrej@ota.si]
> Reported-by: Pawel Staszewski [pstaszewski@artcom.pl]

Thanks Andrej! I've only updated emails a bit.
Jarek P.

Reported-by: Gorik Van Steenberge <gvs@zemos.net>
Reported-by: Daniel Kenzelmann <kernel.bugzilla@kenzelmann.dyndns.info>
Reported-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>
Reported-by: Pawel Staszewski <pstaszewski@artcom.pl>
Diagnosed-by: Andrej Ota <andrej@ota.si>
Diagnosed-by: Eric Dumazet <eric.dumazet@gmail.com>
Tested-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>
Tested-by: Pawel Staszewski <pstaszewski@artcom.pl>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Signed-off-by: Andrej Ota <andrej@ota.si>

^ permalink raw reply

* [net-2.6 PATCH 1/1] qlge: Fix deadlock when cancelling worker.
From: Ron Mercer @ 2010-12-11 21:06 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer, jarkao2, mingo, Linux-Driver
In-Reply-To: <4CF95995.1070506@gmail.com>

Removing usage of rtnl_lock() to protect firmware interface registers.
These registers are accessed in some worker threads and can create a
deadlock if rtnl_lock is taken by upper layers while the worker is still
pending.
We remove rtnl_lock and use a driver mutex just while mailboxes are
accessed.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |    1 +
 drivers/net/qlge/qlge_mpi.c  |   12 ++++--------
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 2282139..9787dff 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2083,6 +2083,7 @@ struct ql_adapter {
 	u32 mailbox_in;
 	u32 mailbox_out;
 	struct mbox_params idc_mbc;
+	struct mutex	mpi_mutex;
 
 	int tx_ring_size;
 	int rx_ring_size;
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 528eaef..2555b1d 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4629,6 +4629,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev,
 	INIT_DELAYED_WORK(&qdev->mpi_idc_work, ql_mpi_idc_work);
 	INIT_DELAYED_WORK(&qdev->mpi_core_to_log, ql_mpi_core_to_log);
 	init_completion(&qdev->ide_completion);
+	mutex_init(&qdev->mpi_mutex);
 
 	if (!cards_found) {
 		dev_info(&pdev->dev, "%s\n", DRV_STRING);
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 0e7c7c7..a2e919b 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -534,6 +534,7 @@ static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
 	int status;
 	unsigned long count;
 
+	mutex_lock(&qdev->mpi_mutex);
 
 	/* Begin polled mode for MPI */
 	ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16));
@@ -603,6 +604,7 @@ done:
 end:
 	/* End polled mode for MPI */
 	ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16) | INTR_MASK_PI);
+	mutex_unlock(&qdev->mpi_mutex);
 	return status;
 }
 
@@ -1099,9 +1101,7 @@ int ql_wait_fifo_empty(struct ql_adapter *qdev)
 static int ql_set_port_cfg(struct ql_adapter *qdev)
 {
 	int status;
-	rtnl_lock();
 	status = ql_mb_set_port_cfg(qdev);
-	rtnl_unlock();
 	if (status)
 		return status;
 	status = ql_idc_wait(qdev);
@@ -1122,9 +1122,7 @@ void ql_mpi_port_cfg_work(struct work_struct *work)
 	    container_of(work, struct ql_adapter, mpi_port_cfg_work.work);
 	int status;
 
-	rtnl_lock();
 	status = ql_mb_get_port_cfg(qdev);
-	rtnl_unlock();
 	if (status) {
 		netif_err(qdev, drv, qdev->ndev,
 			  "Bug: Failed to get port config data.\n");
@@ -1167,7 +1165,6 @@ void ql_mpi_idc_work(struct work_struct *work)
 	u32 aen;
 	int timeout;
 
-	rtnl_lock();
 	aen = mbcp->mbox_out[1] >> 16;
 	timeout = (mbcp->mbox_out[1] >> 8) & 0xf;
 
@@ -1231,7 +1228,6 @@ void ql_mpi_idc_work(struct work_struct *work)
 		}
 		break;
 	}
-	rtnl_unlock();
 }
 
 void ql_mpi_work(struct work_struct *work)
@@ -1242,7 +1238,7 @@ void ql_mpi_work(struct work_struct *work)
 	struct mbox_params *mbcp = &mbc;
 	int err = 0;
 
-	rtnl_lock();
+	mutex_lock(&qdev->mpi_mutex);
 	/* Begin polled mode for MPI */
 	ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16));
 
@@ -1259,7 +1255,7 @@ void ql_mpi_work(struct work_struct *work)
 
 	/* End polled mode for MPI */
 	ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16) | INTR_MASK_PI);
-	rtnl_unlock();
+	mutex_unlock(&qdev->mpi_mutex);
 	ql_enable_completion_interrupt(qdev, 0);
 }
 
-- 
1.6.0.2


^ permalink raw reply related

* Re: [PATCH] rfc: ethtool: early-orphan control
From: Simon Horman @ 2010-12-11 22:40 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Ben Hutchings
In-Reply-To: <1292087480.2746.54.camel@edumazet-laptop>

On Sat, Dec 11, 2010 at 06:11:20PM +0100, Eric Dumazet wrote:
> Le samedi 11 décembre 2010 à 09:03 +0100, Eric Dumazet a écrit :
> > Le samedi 11 décembre 2010 à 13:24 +0900, Simon Horman a écrit :
> > > On Sat, Dec 11, 2010 at 01:13:35PM +0900, Simon Horman wrote:
> > > > Early orphaning is an optimisation which avoids unnecessary cache misses by
> > > > orphaning an skb just before it is handed to a device for transmit thus
> > > > avoiding the case where the orphaning occurs on a different CPU.
> > > > 
> > > > In the case of bonded devices this has the unfortunate side-effect of
> > > > breaking down flow control allowing a socket to send UDP packets as fast as
> > > > the CPU will allow. This is particularly undesirable in virtualised
> > > > network environments.
> > > > 
> > > > This patch introduces ethtool control of early orphaning.
> > > > It remains on by default by it now may be disabled on a per-interface basis.
> > > > 
> > > > I have implemented this as a generic flag.
> > > > As it seems to be the first generic flag that requires
> > > > no driver awareness I also supplied a default flag handler.
> > > > I am unsure if any aspect of this approach is acceptable.
> > > > 
> > > > I believe Eric has it in mind that some of the calls
> > > > to skb_orphan() in drivers can be removed with the addition
> > > > of this feature. I need to discuss that with him further.
> > > > 
> > > > A patch for the ethtool user-space utility accompanies this patch.
> > > 
> > > The following results were measured using kvm using virto without vhost net.
> > > The virtio device is bridged to a bond device which has one gigabit slave.
> > > 
> > 
> > As you know, vhost net does the orphaning, as well as some NIC drivers,
> > so one UDP flood would have same problem.
> > 
> > I wonder if this problem could not be solved in other ways.
> > 
> > 
> > We might do early orphaning only for sockets with SOCK_USE_WRITE_QUEUE
> > flag asserted. (tcp sets it)
> > 
> > Then, we could also say : Why tcp use sock_wfree() at all...
> > 
> 
> I removed skb_orphan_try() and did a quick test, with bonding or not,
> same results on a Gigabit interface.
> 
> $ netperf -C -c -4 -t UDP_STREAM -H 55.225.18.57 -- -m 1000
> UDP UNIDIRECTIONAL SEND TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 55.225.18.57 (55.225.18.57) port 0 AF_INET
> Socket  Message  Elapsed      Messages                   CPU      Service
> Size    Size     Time         Okay Errors   Throughput   Util     Demand
> bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB
> 
> 10000000    1000   10.00     6611385      0     5289.0     13.18    9.278 
> 1000000           10.00     1163454             930.7     4.58     6.456 
> 
> 
> As soon as 'socket size' is big enough, UDP flow control is ineffective,
> and no error is reported to user. sendto() says all frames were properly sent.
> 

Yes, I've done that test too (as you suggested previously). But my thought
was that in a virtualised environment the administrator of the host can set
the socket size to be small enough and the guest can't change it.

However, I now realise that the same effect can be produced
in the guest's network stack by increasing wmem_default there.
So I'm not sure that this change is useful after all. And I've
got a worse flow control problem than I previously realised.


^ permalink raw reply

* Re: [PATCH] pppoe.c: Fix kernel panic caused by __pppoe_xmit
From: Andrej Ota @ 2010-12-11 23:23 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <20101211200823.GA1917@del.dom.local>

> Thanks Andrej! I've only updated emails a bit.

Thank you for your help and support in submitting this patch.

Andrej Ota.

^ permalink raw reply

* RE: [RFC][net-next-2.6 PATCH 0/2] rtnetlink: New IFLA_PORT_PROTO_* attr
From: Christian Benvenuti (benve) @ 2010-12-12  0:53 UTC (permalink / raw)
  To: Stefan Berger
  Cc: netdev, chrisw, arnd, David Miller, Roopa Prabhu (roprabhu),
	David Wang (dwang2)
In-Reply-To: <loom.20101209T141255-700@post.gmane.org>

Hi,

>> >> The following series add the new IFLA_PORT_PROTO_* nested protocol 
>> >> attributes to rtnetlink and it updates the enic driver to support 
>> >> them.
>> >> 
>> >> 01/2 - Add new protocol nested IFLA_PORT_PROTO_* attrs
>> >> 02/2 - Update enic driver to support new IFLA_PORT_PROTO_* attrs
>> >> 
>> >> Signed-off-by: Christian Benvenuti <benve <at> cisco.com>
>> >> Signed-off-by: Roopa Prabhu <roprabhu <at> cisco.com>
>> >> Signed-off-by: David Wang <dwang2 <at> cisco.com>
>> >> 
>> > [...]
>> > 
>> >> When the protocol nested attributes IFLA_PORT_PROTO_* will be 
>> >> populated with new sub-attributes (like the CLUSTER_UUID we would 
>> >> like to add), the user space clients will have to adapt to the new 
>> >> attribute scheme if they want to be able to see/receive the new 
>> >> attributes (like CLUSTER_UUID).
>> 
>> >I don't have a problem with these changes. Just on the libvirt level 
>> >it's going to be a lot more messy. We'll need another level of 
>> >#ifdef's
>> 
>> >for when these new attributes became available. In case they are 
>> >there we should not just create the netlink messages with the new 
>> >attributes but first independently probe for 802.1Qbg and 802.1Qbh 
>> >for whether lldpad or the kernel respectively saw the same level of 
>> >if_link.h include and/or support the new attributes and fall back to 
>> >using the old ones in case the probing failed. That way we can 
>> >support multi-boot
>> 
>> >installations with kernels before and after these changes or an 
>> >lldpad that doesn't support the new attributes and still give the 
>> >user the experience that the starting of the VM 'works' as before 
>> >(the new
>> kernel was installed).
>> 
>> We will definitely take care of the libvirt changes necessary to 
>> support this.
>> I can see two points in your comment above. Please correct me if I 
>> did not understand them correctly:
>> 
>> 1) libvirt compilation on multi-boot installations
>> 
>> First let me verify if I understand what you mean by multi-boot
>> installations:
>> 
>>   systems where there are multiple kernel versions installed and
>>   therefore where the running system may potentially be using
>>   different if_link.h versions, with or without the new attributes
>>   ("new" = replacements for the deprecated ones)
>> 
>> Is this correct? 
>
>Yes. The problem case would be a user (re-)compiles libvirt with 
>support for the new attributes on the new kernel and then boots into 
>the old one. The user would expect it to still work as before the
>re-compilation.

(*)

With our proposal it would work because, regardless of the kernel
version in use, libvirt would always send both old and new
attributes (and the kernel, if/when asked, would do the same).
For example, supposing the user wanted to configure a port
profile, libvirt would send (among the other attributes) the
following ones:

...
[IFLA_PORT_PROFILE]  <-------------- OLD ONE

[IFLA_PORT_PROTO_8021QBH]
    [IFLA_PORT_8021QBH_PROFILE] <--- NEW ONE ...

Note that the “NEW” attribute above does not add any new
functionality, and it does not replace the old/legacy one.

Those recipients that support the new attributes will look
for IFLA_PORT_8021QBH_PROFILE and, if not present, will
check for IFLA_PORT_PROFILE (if expected to work with older
kernels/apps).

Those recipients that only understand the old attributes will
only check for IFLA_PORT_PROFILE and will ignore completely
the new nested attr IFLA_PORT_PROTO_8021QBH (as per default
Netlink behavior).

The two attributes (old and new) carry the same exact piece
of information.
They only change the attribute scheme in order to make it
easier to maintain the attribute list:

   it classifies attributes by protocol.

Once this change is in, we will propose the addition of new
(8021Qbh) attributes (ie, CLUSTER_UUID), but the patches
currently pending in netdev do not introduce them yet.
Any new attribute added after the changes proposed with these
patches will be available only using the new scheme.
 
(Note that we are deprecating only two attributes in the
IFLA_PORT_* enum)

>> So what you are saying is that libvirt should use #ifdef to 
>> distinguish between
>> 
>>    if_link.h/pre_new_attributes
>> and 
>>    if_link.h/new_attributes? 
>
> Yes. It looks like you'll have to determine the availability in the 
> configure script and introduce a new #define there.

(*2)

I suppose that based on my comment (*) above, this should not be
necessary because any kernel supported as of today will continue to
work exactly the same way.
 
>> I guess this would be something similar to what 
>> doPortProfileOp8021Qbg (in src/util/macvtap.c) does by checking for 
>> IFLA_VF_PORT_MAX in order to distinguish between 
>> 8021Qbh/pre_port_profile and 8021Qbh/post_port_profile.
>> Is it correct? 
>
> Yes. If you provided the code for 802.1Qbh that'd be great. 

Based on my comment (*2) above we should not need to add any new #ifdef.

>> Is this #ifdef based approach the right way to keep track of the
>> if_link.h/IFLA_PORT_* changes? 
>
>If there's a better way of doing this, let me know. The problem is 
>people compiling libvirt may have all kinds of different levels of 
>systems, without even macvtap, then with macvtap but without the first 
>set of attributes for port profiles etc. - The progress in the kernel 
>is reflected in the code with these #ifdefs... not nice, but I
> doubt there's a better way of doing this.

Well, “versioning” is not a default feature in (RT)Netlink, and other
proposals (in slightly different contexts) got rejected in the past.
(see for example: http://www.spinics.net/lists/netdev/msg127715.html)

Since we are changing the attribute scheme, this could be the right
moment to add versioning to the IFLA_PORT_* attributes group.
We can consider the current one as being version 1 (or 0) and from
now on increment the version by 1 each time we apply some changes to
the IFLA_PORT_* attributes.
The most likely changes that may take place would be the introduction
of new attributes (the BH/BG protocols are still evolving).
This is not the way Netlink is used normally (see Miller's comment
in the URL I provided above), but it would make libvirt code much
easier to update and we would not need to populate it with #ifdefs.

The presence of the version would not be an excuse to be allowed to
make bad changes to the attributes knowing that you can fix them
later with a newer version (I agree with Miller's comment), but
simply to help consumers (ie libvirt in this case) to determine
what level of if_link.h/IFLA_PORT_* they are dealing with, at
run/time (the idea is that of offering a GET for that version number).

Right now (RT)Netlink silently ignores the attributes it does not
understand (ie, type > maxtype), therefore there are no other
mechanisms for determining at run-time what version of the attribute
list the kernel/recipient understands.

>> We may need to change if_link.h again, for example to add new
>> IFLA_PORT_* attributes.
>> Does it mean that in libvirt we will have to add an #ifdef each time? 
>
>I think so, yes. And since they are enums (rather than #define's 
>themselves) you'll have to introduce the #define's name and probe for 
>it by trying to compiling something with it in the configure script.

If this is the only option, we can go for that. No problem. We will
submit the libvirt patches.

If however the versioning scheme was accepted, the libvirt code would
be cleaner and easier to maintain.
Before to discard this option I would like someone on netdev to
comment on that.
Dave, can you confirm us that versioning the IFLA_PORT_* attributes
(not the all if_link.h header) is not an option?

>>> 2) old vs new attributes handling
>>> 
>>> You proposed to probe the recipient of the message to see if it 
>>> supports the new attributes, and downgrade to the old attr scheme if 
>>> the
>> recipient does not understand the new ones.
>> 
>> Current Netlink implementation does not provide such (probing) 
>> facility, and the Kernel for example simply ignores any attr whose 
>> id/type is bigger than the defined __XXX_MAX for the domain the
>> attr belongs to.
>
>Ok, then that's a problem. But it's equally a problem if user's don't 
>see why things don't work anymore today with the new kernel whereas 
>things still worked yesterday with the old one.

As I said in (*) above, with the current changes nothing will break
because the new attributes deprecate the old ones, and support for
the old ones won’t change at all.
This therefore should not be any problem.

>Is there any RTM_GETLINK one could do to determine whether the new 
>attributes are being used?

No.
The versioning mentioned above would add support for that.

>> Because of that there would not be a clean way to probe for remote 
>> support of a given attribute.
>> Is this correct? Please let me know if I misunderstood your "probe"
>> comment. 
>
>You understood the probe comment correctly. 
> 
>> What we propose is this:
>> 
>> - Given that we deprecated only two attributes, we would send always
>>   NEW attr + OLD/DEPRECATED attr
>> 
>> - A recipient that only understands the OLD/DEPRECATED attributes
>>   will parse and process only the OLD/DEPRECATED attributes and ignore
>>   the NEW ones.
>>   Here by "NEW ones" I mean those that replaced the DEPRECATED ones
>>   (ie, I do not refer to those that do not exist today and will be
>>    introduced therefore using the new Netlink scheme directly)
>> 
>> - A recipient that supports the NEW ones can safely use the NEW ones
>>   only and ignore the OLD ones. 
>
>If mixing the old and new ones in one message is ok then probing won't 
>be necessary.

Exactly. That’s the idea.
 
>> Note that if/when we will add new attributes (for example 
>> CLUSTER_UUID) that do not exist today and that therefore do not 
>> represent replacements for deprecated attributes, we will add them 
>> using the new scheme and therefore all recipients/consumers must 
>> update to the new scheme if they want to be able to use any of them.
>> None of the applications using the old attribute-set can expect to be 
>> able to use the new attributes without an upgrade.
>> 
>> > I am assuming that it worked with 802.1Qbh before even though you 
>> > didn't have the CLUSTER_UUID support... Now that will probably add 
>> > quite a bit to the complexity of the code and the testing. I hope 
>> > you'll submit a patch like that to libvirt mailing list.
>> 
>> I am not sure I understand your point here. Can you please clarify? 
>
>802.1Qbh support has been in libvirt for quite a while. I assume it 
>worked without the CLUSTER_UUID attribute that you are introducing only 
>now. I have no 802.1Qbh hardware to verify this, though.

Yes, it worked because so far port profiles were global. Now we would
like to use the concept of CLUSTER_UUID (already in use by most VMM)
to scope port profiles. This way we can limit the scope of a port
profile to those hosts belonging to a given cluster.

The main purpose of the kernel patches we submitted was to cleanup
the attribute scheme used for BH/BG, and at the same time make it
easier to add new attributes if/when needed.
However, this should not come at the cost of making libvirt support
a nightmare (#ifdef, etc).
To me it looks like

(1) sending both legacy and new attributes
 + 
(2) using a versioning scheme for the IFLA_PORT_* attributes

make it easy/clean for libvirt to adapt to the underlying kernel
version (ie, if_link.h/IFLA_PORT_* version).

- Because of (1) above we are not breaking any current user of the
  IFLA_PORT_* attributes.

- (2) above is optional but it would make updating libvirt easier.

Again, if #ifdef is the only option, we will stick to it.

Do you have any comment on the optional attribute IFLA_PORT_DATA
I proposed in PATCH 0/2 section 8 (at the end of the post)?

Thanks
/Christian


^ permalink raw reply

* [PATCH] net: delete expired route in ip6_pmtu_deliver
From: Andrey Vagin @ 2010-12-12  1:20 UTC (permalink / raw)
  To: David S. Miller; +Cc: avagin, netdev, linux-kernel

The first big packets sent to a "low-MTU" client correctly
triggers the creation of a temporary route containing the reduced MTU.

But after the temporary route has expired, new ICMP6 "packet too big"
will be sent, rt6_pmtu_discovery will find the previous EXPIRED route
check that its mtu isn't bigger then in icmp packet and do nothing
before the temporary route will not deleted by gc.

I make the simple experiment:
while :; do
    time ( dd if=/dev/zero bs=10K count=1 | ssh hostname dd of=/dev/null ) || break;
done

The "time" reports real 0m0.197s if a temporary route isn't expired, but
it reports real 0m52.837s (!!!!) immediately after a temporare route has
expired.

Cc: stable@kernel.org
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 net/ipv6/route.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 96455ffb..7659d6f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1565,11 +1565,16 @@ static void rt6_do_pmtu_disc(struct in6_addr *daddr, struct in6_addr *saddr,
 {
 	struct rt6_info *rt, *nrt;
 	int allfrag = 0;
-
+again:
 	rt = rt6_lookup(net, daddr, saddr, ifindex, 0);
 	if (rt == NULL)
 		return;
 
+	if (rt6_check_expired(rt)) {
+		ip6_del_rt(rt);
+		goto again;
+	}
+
 	if (pmtu >= dst_mtu(&rt->dst))
 		goto out;
 
-- 
1.7.2.1


^ permalink raw reply related

* Re: [RFC][net-next-2.6 PATCH 0/2] rtnetlink: New IFLA_PORT_PROTO_* attr
From: Stefan Berger @ 2010-12-12  3:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <184D23435BECB444AB6B9D4630C8EC83BD7230@XMB-RCD-303.cisco.com>

"Christian Benvenuti (benve)" <benve@cisco.com> wrote on 12/11/2010 07:53:44 PM:

[posting via we page since my mail program puts html into the message]

> 
> (*)
> 

> 
> >> So what you are saying is that libvirt should use #ifdef to 
> >> distinguish between
> >> 
> >>    if_link.h/pre_new_attributes
> >> and 
> >>    if_link.h/new_attributes? 
> >
> > Yes. It looks like you'll have to determine the availability in the 
> > configure script and introduce a new #define there.
> 
> (*2)
> 
> I suppose that based on my comment (*) above, this should not be
> necessary because any kernel supported as of today will continue to
> work exactly the same way.
> 

What I meant is that the user-level code (i.e., libvirt) with your new
attributes will only compile on a system that has their names in the
if_link.h. So, users that compile libvirt with a kernel that doesn't have
these new attributes in if_link.h should still be able to compile the
macvtap.c there, but will of course only get support for the old attributes,
which at least seem to be in working condition  for 802.1Qbg. User who
compile libvirt with a new kernel should then get the support for the old
attributes and the new attributes compiled in. To cover these cases you'd
have to isolate the code dealing with the new attributes with for example

#ifdef NEW_8021QBGH_ATTRIBUTES 
[...] 

#endif 

where the #define NEW_8021QBGH_ATTRIBUTES would be set through 
libvirt's configure script that determines it by test-compiling a program 
using one of the new attributes and set the #define if one of the new 
attributes was found to be available.. Ideally your CLUSTER_UUID support
would not come too long after the first batch of new attributes, but would
be 'covered' with the NEW_8021QBGH_ATTRIBUTES. Along those lines, I hope you
won't have to introduce a VERY_NEW_8021QBGH_ATTRIBUTES #define to cover
anything coming even later.

 
> >> I guess this would be something similar to what 
> >> doPortProfileOp8021Qbg (in src/util/macvtap.c) does by checking for 
> >> IFLA_VF_PORT_MAX in order to distinguish between 
> >> 8021Qbh/pre_port_profile and 8021Qbh/post_port_profile.
> >> Is it correct? 
> >
> > Yes. If you provided the code for 802.1Qbh that'd be great. 
> 
> Based on my comment (*2) above we should not need to add any new #ifdef.
> 
> >> Is this #ifdef based approach the right way to keep track of the
> >> if_link.h/IFLA_PORT_* changes? 
> >
> >If there's a better way of doing this, let me know. The problem is 
> >people compiling libvirt may have all kinds of different levels of 
> >systems, without even macvtap, then with macvtap but without the first 
> >set of attributes for port profiles etc. - The progress in the kernel 
> >is reflected in the code with these #ifdefs... not nice, but I
> > doubt there's a better way of doing this.
> 
> Well, “versioning” is not a default feature in (RT)Netlink, and other
> proposals (in slightly different contexts) got rejected in the past.
> (see for example: http://www.spinics.net/lists/netdev/msg127715.html)
> 
> Since we are changing the attribute scheme, this could be the right
> moment to add versioning to the IFLA_PORT_* attributes group.
> We can consider the current one as being version 1 (or 0) and from
> now on increment the version by 1 each time we apply some changes to
> the IFLA_PORT_* attributes.
> The most likely changes that may take place would be the introduction
> of new attributes (the BH/BG protocols are still evolving).
> This is not the way Netlink is used normally (see Miller's comment
> in the URL I provided above), but it would make libvirt code much
> easier to update and we would not need to populate it with #ifdefs. 

The thing is that you may have users compiling an application, i.e.,
libvirt, on systems with different versions of the kernel, and with
that the if_link.h, and those users may want to be able to follow the
application source tree and not necessarily the linux tree. After
updating the application source tree they should still get working
802.1Qbg/h support. So requirements on the underlying kernel shouldn't
just 'move'.

I wouldn't like it if I updated my libvirt tree and all of a sudden the
802.1Qbg/h support wouldn't work anymore, due to requirements for new
attributes in libvirt's macvtap.c, after a re-compile of libvirt. From
a libvirt programmer perspective *all* the attributes would *ideally*
appear from one kernel version to the other. So if one kernel version
provides working support for 802.1Qbg/h I would want to be able to follow the
libvirt tree without having to upgrade the kernel. So that's why I am
proposing the #ifdef's around nearly each step of new attributes being added
in the if_link.h, assuming that at each step the technology was working
and new attributes only make it work 'better' than before.

> 
> The presence of the version would not be an excuse to be allowed to
> make bad changes to the attributes knowing that you can fix them
> later with a newer version (I agree with Miller's comment), but
> simply to help consumers (ie libvirt in this case) to determine
> what level of if_link.h/IFLA_PORT_* they are dealing with, at
> run/time (the idea is that of offering a GET for that version number). 

It's fine by me if we end up sending a message containing old and
new attributes. For dealing with the responses we will have to start
looking for the newest attributes first, then go backwards towards the
first ones. Knowing the version of the supported attributes we could
invoke specific code for the attributes that are expected to be supported.
So I think knowing the version would be advantageous in interpreting
the responses from the kernel.

Nevertheless I would go through the complication of #ifdef'ing
each new major step of new attributes in the libvirt code...

> 
> Right now (RT)Netlink silently ignores the attributes it does not
> understand (ie, type > maxtype), therefore there are no other
> mechanisms for determining at run-time what version of the attribute
> list the kernel/recipient understands. 
> 
> >> We may need to change if_link.h again, for example to add new
> >> IFLA_PORT_* attributes.
> >> Does it mean that in libvirt we will have to add an #ifdef each time? 
> >
> >I think so, yes. And since they are enums (rather than #define's 
> >themselves) you'll have to introduce the #define's name and probe for 
> >it by trying to compiling something with it in the configure script.
> 
> If this is the only option, we can go for that. No problem. We will
> submit the libvirt patches.
> 
> If however the versioning scheme was accepted, the libvirt code would
> be cleaner and easier to maintain. 

I agree. 

> Before to discard this option I would like someone on netdev to
> comment on that.
> Dave, can you confirm us that versioning the IFLA_PORT_* attributes
> (not the all if_link.h header) is not an option?
> >
> >802.1Qbh support has been in libvirt for quite a while. I assume it 
> >worked without the CLUSTER_UUID attribute that you are introducing only 
> >now. I have no 802.1Qbh hardware to verify this, though.
> 
> Yes, it worked because so far port profiles were global. Now we would
> like to use the concept of CLUSTER_UUID (already in use by most VMM)
> to scope port profiles. This way we can limit the scope of a port
> profile to those hosts belonging to a given cluster.
> 
> The main purpose of the kernel patches we submitted was to cleanup
> the attribute scheme used for BH/BG, and at the same time make it
> easier to add new attributes if/when needed.
> However, this should not come at the cost of making libvirt support
> a nightmare (#ifdef, etc). 

The complication comes through *following* an evolving standard rather
than having the *ideal* case where we go from darkness to light from one
kernel version to another -- or make support for the new technology
only public once it's done.

> To me it looks like
> 
> (1) sending both legacy and new attributes
>  + 
> (2) using a versioning scheme for the IFLA_PORT_* attributes
> 
> make it easy/clean for libvirt to adapt to the underlying kernel
> version (ie, if_link.h/IFLA_PORT_* version).
> 
> - Because of (1) above we are not breaking any current user of the
>   IFLA_PORT_* attributes.
> 
> - (2) above is optional but it would make updating libvirt easier.
> 
> Again, if #ifdef is the only option, we will stick to it. 

Ok.

> 
> Do you have any comment on the optional attribute IFLA_PORT_DATA
> I proposed in PATCH 0/2 section 8 (at the end of the post)? 

To quote from there:

"This attribute would allow the use of extra attributes that are not part of
the official protocol specs (802.1Qbg/bh for now) or simply allow device
drivers to start supporting pre-standard parameters that would not be included
in the Netlink scheme before they reach some stability."

Hm, does this sound like an 'experimental' attribute that can change over time
('some stability')? I hope not, because otherwise you may have mismatches
between what libvirt assumes it needs to pass versus what the evolving device
driver expects. If so, we would have to go as far as determining the version of
the device driver to match up libvirt code against it (?). 

  Regards 
     Stefan 

> 
> Thanks
> /Christian
> 



^ permalink raw reply

* Re: [PATCH net-next 1/4] bnx2x: LSO code was broken on BE platforms
From: Ben Hutchings @ 2010-12-12  4:14 UTC (permalink / raw)
  To: Vladislav Zolotarov; +Cc: Dave Miller, netdev list, Eilon Greenstein
In-Reply-To: <1291808589.30114.65.camel@lb-tlvb-vladz>

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

On Wed, 2010-12-08 at 13:43 +0200, Vladislav Zolotarov wrote:
> Make the LSO code work on BE platforms: parsing_data field of 
> a parsing BD (PBD) for 57712 was improperly composed which made FW read wrong 
> values for TCP header's length and offset and, as a result, the corresponding 
> PCI device was performing bad DMA reads triggering EEH.
[...]

This fix looks like it should go into the stable/longterm series.  Since
this driver has changed a lot since 2.6.32, you would need to provide a
backported patch for that.

Ben.

-- 
Ben Hutchings, Debian Developer and kernel team member


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

^ permalink raw reply

* Re: [PATCH v2] net/r8169: Remove the firmware of RTL8111D
From: Ben Hutchings @ 2010-12-12  4:25 UTC (permalink / raw)
  To: Hayes Wang; +Cc: romieu, netdev, linux-kernel
In-Reply-To: <1291619474-3244-1-git-send-email-hayeswang@realtek.com>

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

On Tue, 2010-12-07 at 15:19 +0000, Hayes Wang wrote:
> Remove the firmware of RTL8111D from the kernel.
> The binary file of firmware would be moved to linux-firmware repository.
> The firmwares are rtl_nic/rtl8168d-1.fw and rtl_nic/rtl8168d-2.fw.
> The driver would load the firmware through request_firmware. The driver
> would just go along if the firmware couldn't be found. However, it is
> suggested to be done with the suitable firmware.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/r8169.c |  796 ++++++++-------------------------------------------
>  1 files changed, 117 insertions(+), 679 deletions(-)
>  mode change 100644 => 100755 drivers/net/r8169.c
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> old mode 100644
> new mode 100755

Please do not make source files executable.

> index 7d33ef4..e0df607
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
[...]
> +static void
> +rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
> +{
> +	void __iomem *ioaddr = tp->mmio_addr;
> +	u32 *phytable = (u32 *)fw->data;

This line should use __le32 not u32 to make it clearer which byte order
is being used.

> +	u32 action;
> +	size_t len = fw->size / sizeof(*phytable);
> +	u32 regno, data;
> +
> +	while (len-- > 0 && *phytable != 0) {
> +		action = le32_to_cpu(*phytable);
> +		regno = (action & 0x0FFF0000) >> 16;
> +		data = action & 0x0000FFFF;
> +
> +		switch(action & 0xF0000000) {
> +		case PHY_WRITE:
> +			mdio_write(ioaddr, regno, data);
> +			phytable++;
> +			break;
> +		default:
> +			netif_err(tp, probe, tp->dev,
> +				  "Unknown action 0x%08x\n", action);
> +			return;
> +		}
> +	}
[...]

I think should validate the action types before starting.  Otherwise it
could begin loading the firmware and then abort (without even returning
an error code) which would be worse than not loading it at all.

Other than that, this is good, especially the addition of comments for
some of the register initialisation sequences.

Ben.

-- 
Ben Hutchings, Debian Developer and kernel team member


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

^ permalink raw reply

* [GIT PULL net-2.6] vhost-net: logging fixup
From: Michael S. Tsirkin @ 2010-12-12 10:09 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel, stable

Please merge the following fix for 2.6.37.
It is also applicable to -stable.
Thanks!

The following changes since commit a19faf0250e09b16cac169354126404bc8aa342b:

  net: fix skb_defer_rx_timestamp() (2010-12-10 16:20:56 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net

Michael S. Tsirkin (1):
      vhost: correctly set bits of dirty pages

 drivers/vhost/vhost.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

^ permalink raw reply

* Re: [net-next 2/8] bnx2x: add select queue callback
From: Eilon Greenstein @ 2010-12-12 10:25 UTC (permalink / raw)
  To: David Miller; +Cc: Dmitry Kravkov, netdev@vger.kernel.org, vladz
In-Reply-To: <20101210.141131.241939045.davem@davemloft.net>

On Fri, 2010-12-10 at 14:11 -0800, David Miller wrote:
> From: "Dmitry Kravkov" <dmitry@broadcom.com>
> Date: Thu, 9 Dec 2010 14:09:16 +0200
> 
> > +#endif
> > +	/* Select queue (if defined) adjust for fcoe */
> > +	fp_index = skb_tx_hash(dev, skb) - FCOE_CONTEXT_USE;
> > +
> > +	return (fp_index >= 0 ? fp_index : 0);
> 
> This doesn't make any sense, and it's one of the reasons I really
> hate the fact that drivers sometimes need to override the
> select_queue method.
> 
> Because 9 times out of 10 they get it wrong.
> 
> You're mapping queues 0 --> FCOE_CONTEXT_USE to zero.
> 
> But that's not what you actually want.
> 
> You're actually trying to make non-FCOE traffic hash only amongst the
> non-FCOE queues.
> 
> So make your code actually do that instead of screwing up the
> distribution of the hash result.
> 

Indeed, the current suggestion is giving queue 0 double weight compared
to all other queues just so the skb_tx_hash will be used as is to
benefit from future enhancement changes to it. We will re-spin this
patch and move the content of the skb_tx_hash to __skb_tx_hash that will
receive the netdev->skb_real_num_tx_queues as a parameter - this way, we
can call the __skb_tx_hash with a lower number.

Thanks,
Eilon





^ permalink raw reply

* [RFC PATCH net-next 0/6] Start moving ethernet drivers to new directory
From: Joe Perches @ 2010-12-12 12:03 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: Jeff Kirsher, Paul Gortmaker, linux-kernel

As discussed a few times, here's a start at moving various files
around in drivers/net to a new drivers/net/ethernet directory.

The general idea is to move one at a time compilation units
associated to the various subsystems in drivers/net, updating
the Kconfig and Makefile files. 

I think this is a form that doesn't break the build, is reviewable
and acceptable and makes the top level drivers/net directory much
cleaner.

The biggest downsides of this move style are:

o high number of trivial commits so:
o serialization: it's best done by one person because the likelihood
  of commit clash will be also be high.
o the order the entries in Kconfig are changed, so the order of entries
  in menuconfig/xconfig is also changed.

An alternative to the high commit count is to collect many of these patches
together into a single commit.

If this approach is acceptable, I'll do all the unmaintained ethernet
subsystems in drivers/net in this style, followed by adding separate
subdirectories for 3com and 8390, moving tulip, then onto the various
vendor directories as appropriate.

Moving bonding, ppp, and such to different directories will be done
separately.

David, the Sun GEM and Cassini files are currently selected under the
10/100 menu.  Shouldn't these be presented under the 1000Mb menu?
I didn't move them to that menu so all current configs should work,
but it'd be logically sensible to move the selection for these subsystems
to the 1000 menu though maybe that's not practical.

Comments?

Joe Perches (6):
  drivers/net: Add directory ethernet
  ethernet: Move ethoc.c
  ethernet: Move dnet.[ch]
  ethernet: Move Sun Happymeal sunhme.[ch]
  ethernet: Move Sun GEM sungem*.[ch]
  ethernet: Move Sun Cassini cassini.[ch]

 drivers/net/Kconfig                     |   56 +++---------------------------
 drivers/net/Makefile                    |    7 +---
 drivers/net/ethernet/Kconfig.10-100     |   53 +++++++++++++++++++++++++++++
 drivers/net/ethernet/Kconfig.1000       |    3 ++
 drivers/net/ethernet/Kconfig.10000      |    3 ++
 drivers/net/ethernet/Makefile           |    9 +++++
 drivers/net/{ => ethernet}/cassini.c    |    0
 drivers/net/{ => ethernet}/cassini.h    |    0
 drivers/net/{ => ethernet}/dnet.c       |    0
 drivers/net/{ => ethernet}/dnet.h       |    0
 drivers/net/{ => ethernet}/ethoc.c      |    0
 drivers/net/{ => ethernet}/sungem.c     |    0
 drivers/net/{ => ethernet}/sungem.h     |    0
 drivers/net/{ => ethernet}/sungem_phy.c |    0
 drivers/net/{ => ethernet}/sungem_phy.h |    0
 drivers/net/{ => ethernet}/sunhme.c     |    0
 drivers/net/{ => ethernet}/sunhme.h     |    0
 17 files changed, 76 insertions(+), 55 deletions(-)
 create mode 100644 drivers/net/ethernet/Kconfig.10-100
 create mode 100644 drivers/net/ethernet/Kconfig.1000
 create mode 100644 drivers/net/ethernet/Kconfig.10000
 create mode 100644 drivers/net/ethernet/Makefile
 rename drivers/net/{ => ethernet}/cassini.c (100%)
 rename drivers/net/{ => ethernet}/cassini.h (100%)
 rename drivers/net/{ => ethernet}/dnet.c (100%)
 rename drivers/net/{ => ethernet}/dnet.h (100%)
 rename drivers/net/{ => ethernet}/ethoc.c (100%)
 rename drivers/net/{ => ethernet}/sungem.c (100%)
 rename drivers/net/{ => ethernet}/sungem.h (100%)
 rename drivers/net/{ => ethernet}/sungem_phy.c (100%)
 rename drivers/net/{ => ethernet}/sungem_phy.h (100%)
 rename drivers/net/{ => ethernet}/sunhme.c (100%)
 rename drivers/net/{ => ethernet}/sunhme.h (100%)

-- 
1.7.3.3.398.g0b0cd.dirty


^ permalink raw reply

* [RFC PATCH net-next 1/6] drivers/net: Add directory ethernet
From: Joe Perches @ 2010-12-12 12:03 UTC (permalink / raw)
  To: David Miller, linux-kernel; +Cc: Jeff Kirsher, Paul Gortmaker, netdev
In-Reply-To: <cover.1292154142.git.joe@perches.com>

Add empty Kconfig files for various speeds and a no effect Makefile.
Modify drivers/net Kconfig and Makefile to include these new files.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/Kconfig                 |    6 ++++++
 drivers/net/Makefile                |    2 ++
 drivers/net/ethernet/Kconfig.10-100 |    3 +++
 drivers/net/ethernet/Kconfig.1000   |    3 +++
 drivers/net/ethernet/Kconfig.10000  |    3 +++
 drivers/net/ethernet/Makefile       |    9 +++++++++
 6 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/ethernet/Kconfig.10-100
 create mode 100644 drivers/net/ethernet/Kconfig.1000
 create mode 100644 drivers/net/ethernet/Kconfig.10000
 create mode 100644 drivers/net/ethernet/Makefile

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index a20693f..c1d1be8 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -222,6 +222,8 @@ menuconfig NET_ETHERNET
 
 if NET_ETHERNET
 
+source "drivers/net/ethernet/Kconfig.10-100"
+
 config MACB
 	tristate "Atmel MACB support"
 	depends on HAVE_NET_MACB
@@ -2041,6 +2043,8 @@ menuconfig NETDEV_1000
 
 if NETDEV_1000
 
+source "drivers/net/ethernet/Kconfig.1000"
+
 config ACENIC
 	tristate "Alteon AceNIC/3Com 3C985/NetGear GA620 Gigabit support"
 	depends on PCI
@@ -2569,6 +2573,8 @@ menuconfig NETDEV_10000
 
 if NETDEV_10000
 
+source "drivers/net/ethernet/Kconfig.10000"
+
 config MDIO
 	tristate
 
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 652fc6b..bfb5ade 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -2,6 +2,8 @@
 # Makefile for the Linux network (ethercard) device drivers.
 #
 
+obj-y += ethernet/
+
 obj-$(CONFIG_MII) += mii.o
 obj-$(CONFIG_MDIO) += mdio.o
 obj-$(CONFIG_PHYLIB) += phy/
diff --git a/drivers/net/ethernet/Kconfig.10-100 b/drivers/net/ethernet/Kconfig.10-100
new file mode 100644
index 0000000..8b1ee80
--- /dev/null
+++ b/drivers/net/ethernet/Kconfig.10-100
@@ -0,0 +1,3 @@
+#
+# 10 Mb and 100Mb ethernet device configuration
+#
diff --git a/drivers/net/ethernet/Kconfig.1000 b/drivers/net/ethernet/Kconfig.1000
new file mode 100644
index 0000000..f51b97c
--- /dev/null
+++ b/drivers/net/ethernet/Kconfig.1000
@@ -0,0 +1,3 @@
+#
+# 1 Gb ethernet device configuration
+#
diff --git a/drivers/net/ethernet/Kconfig.10000 b/drivers/net/ethernet/Kconfig.10000
new file mode 100644
index 0000000..a8e28fe
--- /dev/null
+++ b/drivers/net/ethernet/Kconfig.10000
@@ -0,0 +1,3 @@
+#
+# 10 Gb ethernet device configuration
+#
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
new file mode 100644
index 0000000..f2aebec
--- /dev/null
+++ b/drivers/net/ethernet/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for ethernet device drivers
+#
+
+# Dummy object just to avoid breaking build
+# These comments and the line below will be deleted on the
+# first addition to this file
+
+obj-$(CONFIG_ETHERNET_DUMMY_DRIVER) += /dev/null/dummy.o
-- 
1.7.3.3.398.g0b0cd.dirty


^ 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