Netdev List
 help / color / mirror / Atom feed
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Andrej Ota @ 2010-12-03 14:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <1291381791.2897.250.camel@edumazet-laptop>

>> Patch that works for me is below. Now I only hope I haven't
>> (re)introduced a memory leak...

> Problem comes from commit 55c95e738da85 (fix return value of
> __pppoe_xmit() method)
> 
> I am not sure patch is OK


Me neither. That's why I wrote "works for me". All I dare say is that it
works better than current code and is probably no worse than it was before
above mentioned commit. Apart from that, there is no point in having return
value for __pppoe_xmit if return value isn't needed.

Easiest way of triggering this BUG is by terminating PPPoE on the server
side, which then hits "if (!dev) { goto abort; }". This in turn calls
"kfree_skb(skb); return 0;" which returns to pppoe_rcv_core which then
goto-s to "abort_put" which again calls "kfree_skb(skb)". Voila the bug.

I don't know how to trigger "if (skb_cow_head(skb, ..." to see if I have
just caused another BUG. However, if I read file comments at the top, I see
a comment from 19/07/01 stating that I have to delete original skb if code
succeeds and never delete it on failure. About the skb copy mentioned in
the same comment, I don't know. 2001 was many commits ago.

Andrej Ota.

^ permalink raw reply

* Re: ctnetlink loop
From: Holger Eitzenberger @ 2010-12-03 13:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: netdev, LKML
In-Reply-To: <20101203133903.GG13225@mail.eitzenberger.org>

[changed netfilter-devel address]

I is even stuck for O_NONBLOCK sockets, which may be due to nfnl_mutex
being held in the receive path.

 /holger


^ permalink raw reply

* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Horman @ 2010-12-03 13:39 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Eric Dumazet, Simon Kirby, netdev
In-Reply-To: <1291382839.8025.40.camel@localhost>

On Fri, Dec 03, 2010 at 01:27:19PM +0000, Ben Hutchings wrote:
> On Fri, 2010-12-03 at 21:36 +0900, Simon Horman wrote:
> > On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
> [...]
> > > I believe Simon Horman has some patches for GRO and LVS.
> > > 
> > > Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> > > 
> > > For TSO, I am not sure why and where it could matter...
> > 
> > There is a patch to teach LVS how to cope with GRO in nf-next-2.6
> > and I expect it to be included in 2.6.38. The patch is "ipvs: allow
> > transmit of GRO aggregated skbs" and perhaps it should be considered
> > for 2.6.37 and stable. In general the work around is to disable GRO.
> >
> > The patch does not resolve the incompatibility of LVS with LRO.
> > The work around there is to disable LRO. I'm not entirely sure
> > how to teach LVS to disable LRO automatically, or if its desirable.
> [...]
> 
> I think it is desirable, and we already do it for bridging and
> forwarding - we call dev_disable_lro() whenever a device is added to a
> bridge or has forwarding enabled.  For IPVS it looks like you would have
> to resolve the server IP address to a device first.  Though if the
> administrator assigns the server address to a different interface later
> then the failure mode will be very hard to understand.

Thanks, I'll try and make it so.


^ permalink raw reply

* ctnetlink loop
From: Holger Eitzenberger @ 2010-12-03 13:39 UTC (permalink / raw)
  To: netfilter-devel; +Cc: netdev, LKML

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

Hi,

I see a problem with how ctnetlink GET requests are being
processed in the kernel (2.6.32.24) under high load.

Initially I saw this problem on a large performance testing
system when getting HTTP proxy performance numbers, but lately
there have been two reports on large customers boxes (both
many-core with 10G NICs).

The sympton is Netlink looping around nfnetlink_rcv_msg(),
which is just because netlink_unicast() came back with -EAGAIN
when trying to write the newly created Netlink skb to the SK
receive buffer in ctnetlink_get_conntrack().  In this case a
(possibly) infinit loop is entered.  Mostly infinit in fact in
case the userland party trying to receive those messages may
be stuck in the sendmsg() call, being unable to read anything
if being single threaded.

I tried to reproduce several times, a few times the loop
disappeared and the box proceeded normally after some time.
I have no explanation for this.

The attached patch tries to solve it by simple not trying
again to netlink_unicast() the reply skb and just fail with
-ENOBUFS.  The reasoning is that at the point a Netlink overrun
is observed it seems counter intuitive to insist on sending
one more Netlink message.

I checked for possible side effects to other Netlink requests,
please check.

The patch applies to net-next-2.6.

Feedback appreciated.

 /holger


[-- Attachment #2: nfnl-fix.diff --]
[-- Type: text/x-diff, Size: 1696 bytes --]

nfnetlink: avoid unbound loop on busy Netlink socket

I see a problem with how ctnetlink GET requests are being
processed in the kernel (2.6.32.24) under high load.

The sympton is Netlink looping around nfnetlink_rcv_msg(), which
is just because netlink_unicast() came back with EAGAIN when
trying to write the newly created Netlink skb to the SK receive
buffer in ctnetlink_get_conntrack().  In this case a (possibly)
infinit loop is entered.  Mostly infinit I think in case the
userland party trying to receive those messages may be stuck in
the sendmsg() call, being unable to read anything if being single
threaded.

I tried to reproduce several times, a few times the loop
disappeared and the box proceeded normally after some minutes.
I have no explanation for this.

The attached patch tries to solve it by simple not trying again
to netlink_unicast() the reply skb and just fail with -ENOBUFS.
The reasoning is that at the point a Netlink overrun is detected
it seems counter intuitive to insist on sending one more Netlink
message.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>

Index: net-next-2.6/net/netfilter/nfnetlink.c
===================================================================
--- net-next-2.6.orig/net/netfilter/nfnetlink.c	2010-12-03 14:33:32.000000000 +0100
+++ net-next-2.6/net/netfilter/nfnetlink.c	2010-12-03 14:34:21.000000000 +0100
@@ -138,7 +138,6 @@
 		return 0;
 
 	type = nlh->nlmsg_type;
-replay:
 	ss = nfnetlink_get_subsys(type);
 	if (!ss) {
 #ifdef CONFIG_MODULES
@@ -169,7 +168,7 @@
 
 		err = nc->call(net->nfnl, skb, nlh, (const struct nlattr **)cda);
 		if (err == -EAGAIN)
-			goto replay;
+			err = -ENOBUFS;
 		return err;
 	}
 }

^ permalink raw reply

* [net-next-2.6 PATCH] ixgbe: add WOL support for SFP+ subdevice
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher

From: Don Skidmore <donald.c.skidmore@intel.com>

This patch will add wake on LAN support to the dev/sub_dev 10FB 11A9.  This
will also include ixgbe ethtool support for this device.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_ethtool.c |   11 +++++++++++
 drivers/net/ixgbe/ixgbe_main.c    |   15 ++++++++++-----
 drivers/net/ixgbe/ixgbe_type.h    |    1 +
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index f9b5839..ef3f910 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -1879,7 +1879,16 @@ static int ixgbe_wol_exclusion(struct ixgbe_adapter *adapter,
 	struct ixgbe_hw *hw = &adapter->hw;
 	int retval = 1;
 
+	/* WOL not supported except for the following */
 	switch(hw->device_id) {
+	case IXGBE_DEV_ID_82599_SFP:
+		/* Only this subdevice supports WOL */
+		if (hw->subsystem_device_id != IXGBE_SUBDEV_ID_82599_SFP) {
+			wol->supported = 0;
+			break;
+		}
+		retval = 0;
+		break;
 	case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
 		/* All except this subdevice support WOL */
 		if (hw->subsystem_device_id ==
@@ -1887,6 +1896,8 @@ static int ixgbe_wol_exclusion(struct ixgbe_adapter *adapter,
 			wol->supported = 0;
 			break;
 		}
+		retval = 0;
+		break;
 	case IXGBE_DEV_ID_82599_KX4:
 		retval = 0;
 		break;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index fb81418..c876a69 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -7226,13 +7226,18 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		goto err_sw_init;
 
 	switch (pdev->device) {
+	case IXGBE_DEV_ID_82599_SFP:
+		/* Only this subdevice supports WOL */
+		if (pdev->subsystem_device == IXGBE_SUBDEV_ID_82599_SFP)
+			adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
+			                IXGBE_WUFC_MC | IXGBE_WUFC_BC);
+		break;
 	case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
 		/* All except this subdevice support WOL */
-		if (pdev->subsystem_device ==
-		    IXGBE_SUBDEV_ID_82599_KX4_KR_MEZZ) {
-			adapter->wol = 0;
-			break;
-		}
+		if (pdev->subsystem_device != IXGBE_SUBDEV_ID_82599_KX4_KR_MEZZ)
+			adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
+			                IXGBE_WUFC_MC | IXGBE_WUFC_BC);
+		break;
 	case IXGBE_DEV_ID_82599_KX4:
 		adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
 				IXGBE_WUFC_MC | IXGBE_WUFC_BC);
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 20e1f03..bbcfafc 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -54,6 +54,7 @@
 #define IXGBE_DEV_ID_82599_T3_LOM        0x151C
 #define IXGBE_DEV_ID_82599_CX4           0x10F9
 #define IXGBE_DEV_ID_82599_SFP           0x10FB
+#define IXGBE_SUBDEV_ID_82599_SFP        0x11A9
 #define IXGBE_DEV_ID_82599_SFP_EM        0x1507
 #define IXGBE_DEV_ID_82599_XAUI_LOM      0x10FC
 #define IXGBE_DEV_ID_82599_COMBO_BACKPLANE 0x10F8


^ permalink raw reply related

* Re: Bonding, GRO and tcp_reordering
From: Simon Horman @ 2010-12-03 13:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Ben Hutchings, netdev
In-Reply-To: <20101201043445.GC3485@verge.net.au>

On Wed, Dec 01, 2010 at 01:34:45PM +0900, Simon Horman wrote:
> On Tue, Nov 30, 2010 at 05:04:33PM +0100, Eric Dumazet wrote:
> > Le mardi 30 novembre 2010 à 15:42 +0000, Ben Hutchings a écrit :
> > > On Tue, 2010-11-30 at 22:55 +0900, Simon Horman wrote:

To clarify my statement in a previous email that GSO had no effect: I
re-ran the tests and I still haven't observed any affect of GSO on my
results. However, I did notice that in order for GRO on the server to have
effect I also need TSO enabled on the client.  I thought that I had
previously checked that but I was mistaken.

Enabling TSO on the client while leaving GSO disabled on the server
resulted in increased CPU utilisation on the client, from ~15% to ~20%.

> > > > The only other parameter that seemed to have significant effect was to
> > > > increase the mtu.  In the case of MTU=9000, GRO seemed to have a negative
> > > > impact on throughput, though a significant positive effect on CPU
> > > > utilisation.
> > > [...]
> > > 
> > > Increasing MTU also increases the interval between packets on a TCP flow
> > > using maximum segment size so that it is more likely to exceed the
> > > difference in delay.
> > > 
> > 
> > GRO really is operational _if_ we receive in same NAPI run several
> > packets for the same flow.
> > 
> > As soon as we exit NAPI mode, GRO packets are flushed.
> > 
> > Big MTU --> bigger delays between packets, so big chance that GRO cannot
> > trigger at all, since NAPI runs for one packet only.
> > 
> > One possibility with big MTU is to tweak "ethtool -c eth0" params
> > rx-usecs: 20
> > rx-frames: 5
> > rx-usecs-irq: 0
> > rx-frames-irq: 5
> > so that "rx-usecs" is bigger than the delay between two MTU full sized
> > packets.
> > 
> > Gigabit speed means 1 nano second per bit, and MTU=9000 means 72 us
> > delay between packets.
> > 
> > So try :
> > 
> > ethtool -C eth0 rx-usecs 100
> > 
> > to get chance that several packets are delivered at once by NIC.
> > 
> > Unfortunately, this also add some latency, so it helps bulk transferts,
> > and slowdown interactive traffic 
> 
> Thanks Eric,
> 
> I was tweaking those values recently for some latency tuning
> but I didn't think of them in relation to last night's tests.
> 
> In terms of my measurements, its just benchmarking at this stage.
> So a trade-off between throughput and latency is acceptable, so long
> as I remember to measure what it is.

Thanks, rx-usecs was set to 3 and changing it to 15 on the server
did seem increase throughput with 1500 byte packets. Although
CPU utilisation increased too, disproportionally so on the client.

MTU=1500, client,server:tcp_reordering=3, client:GSO=off,
	client:TSO=on, server:GRO=off, server:rx-usecs=3(default)
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    10.00      1591.34   16.35    5.80     1.683   2.390

MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
	client:TSO=on, server:GRO=off server:rx-usecs=15
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    10.00      1774.38   23.75    7.58     2.193   2.801

I also saw an improvement with GRO enabled on the server and TSO enabled on
the client.  Although in this case I found rx-usecs=45 to be the best
value.

MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
	client:TSO=on, server:GRO=on server:rx-usecs=3(default)
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    10.00      2553.27   13.31    3.35     0.854   0.860

MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
	client:TSO=on, server:GRO=on server:rx-usecs=45
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    10.00      2727.53   29.45    9.48     1.769   2.278

I did not observe any improvement in throughput when increasing rx-usecs
from 3 when using mtu=9000 although there was a slight increase in CPU
utilisation (maybe, there is quite a lot of noise in the results).


^ permalink raw reply

* [net-next-2.6 PATCH] ixgbe: fix link behavior for SFP+ when driver is brought down
From: Jeff Kirsher @ 2010-12-03 13:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher

From: Don Skidmore <donald.c.skidmore@intel.com>

We have had several requests to have ifconfig down command disable
the SFP+ laser and thus make link go down.  Likewise on ifconfig up
the laser would be enabled and link would come up.  This patch enables
that behavior.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_82599.c |   12 +++++++++---
 drivers/net/ixgbe/ixgbe_main.c  |   26 ++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 26b8ceb..743bdec 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -65,9 +65,9 @@ static s32 ixgbe_verify_fw_version_82599(struct ixgbe_hw *hw);
 static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
-	if (hw->phy.multispeed_fiber) {
-		/* Set up dual speed SFP+ support */
-		mac->ops.setup_link = &ixgbe_setup_mac_link_multispeed_fiber;
+
+	/* enable the laser control functions for SFP+ fiber */
+	if (mac->ops.get_media_type(hw) == ixgbe_media_type_fiber) {
 		mac->ops.disable_tx_laser =
 		                       &ixgbe_disable_tx_laser_multispeed_fiber;
 		mac->ops.enable_tx_laser =
@@ -77,6 +77,12 @@ static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
 		mac->ops.disable_tx_laser = NULL;
 		mac->ops.enable_tx_laser = NULL;
 		mac->ops.flap_tx_laser = NULL;
+	}
+
+	if (hw->phy.multispeed_fiber) {
+		/* Set up dual speed SFP+ support */
+		mac->ops.setup_link = &ixgbe_setup_mac_link_multispeed_fiber;
+	} else {
 		if ((mac->ops.get_media_type(hw) ==
 		     ixgbe_media_type_backplane) &&
 		    (hw->phy.smart_speed == ixgbe_smart_speed_auto ||
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index bd64a4d..fb81418 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -3793,8 +3793,11 @@ static int ixgbe_up_complete(struct ixgbe_adapter *adapter)
 	else
 		ixgbe_configure_msi_and_legacy(adapter);
 
-	/* enable the optics */
-	if (hw->phy.multispeed_fiber && hw->mac.ops.enable_tx_laser)
+	/* enable the optics for both mult-speed fiber and 82599 SFP+ fiber */
+	if (hw->mac.ops.enable_tx_laser &&
+	    ((hw->phy.multispeed_fiber) ||
+	     ((hw->phy.type == ixgbe_media_type_fiber) &&
+	      (hw->mac.type == ixgbe_mac_82599EB))))
 		hw->mac.ops.enable_tx_laser(hw);
 
 	clear_bit(__IXGBE_DOWN, &adapter->state);
@@ -4106,15 +4109,19 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 		break;
 	}
 
-	/* power down the optics */
-	if (hw->phy.multispeed_fiber && hw->mac.ops.disable_tx_laser)
-		hw->mac.ops.disable_tx_laser(hw);
-
 	/* clear n-tuple filters that are cached */
 	ethtool_ntuple_flush(netdev);
 
 	if (!pci_channel_offline(adapter->pdev))
 		ixgbe_reset(adapter);
+
+	/* power down the optics for multispeed fiber and 82599 SFP+ fiber */
+	if (hw->mac.ops.disable_tx_laser &&
+	    ((hw->phy.multispeed_fiber) ||
+	     ((hw->phy.type == ixgbe_media_type_fiber) &&
+	      (hw->mac.type == ixgbe_mac_82599EB))))
+		hw->mac.ops.disable_tx_laser(hw);
+
 	ixgbe_clean_all_tx_rings(adapter);
 	ixgbe_clean_all_rx_rings(adapter);
 
@@ -7200,8 +7207,11 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		goto err_eeprom;
 	}
 
-	/* power down the optics */
-	if (hw->phy.multispeed_fiber && hw->mac.ops.disable_tx_laser)
+	/* power down the optics for multispeed fiber and 82599 SFP+ fiber */
+	if (hw->mac.ops.disable_tx_laser &&
+	    ((hw->phy.multispeed_fiber) ||
+	     ((hw->phy.type == ixgbe_media_type_fiber) &&
+	      (hw->mac.type == ixgbe_mac_82599EB))))
 		hw->mac.ops.disable_tx_laser(hw);
 
 	init_timer(&adapter->watchdog_timer);


^ permalink raw reply related

* [net-next-2.6 PATCH] ixgbe: add support for new format of PBA numbers
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher

From: Don Skidmore <donald.c.skidmore@intel.com>

The new PBA format is stored as a string.  This patch allows the
driver to support both the old and new format.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_common.c |   94 +++++++++++++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_common.h |    3 +
 drivers/net/ixgbe/ixgbe_main.c   |   18 ++++---
 drivers/net/ixgbe/ixgbe_type.h   |    6 ++
 4 files changed, 105 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_common.c b/drivers/net/ixgbe/ixgbe_common.c
index 5605257..cc11e42 100644
--- a/drivers/net/ixgbe/ixgbe_common.c
+++ b/drivers/net/ixgbe/ixgbe_common.c
@@ -196,30 +196,110 @@ s32 ixgbe_clear_hw_cntrs_generic(struct ixgbe_hw *hw)
 }
 
 /**
- *  ixgbe_read_pba_num_generic - Reads part number from EEPROM
+ *  ixgbe_read_pba_string_generic - Reads part number string from EEPROM
  *  @hw: pointer to hardware structure
- *  @pba_num: stores the part number from the EEPROM
+ *  @pba_num: stores the part number string from the EEPROM
+ *  @pba_num_size: part number string buffer length
  *
- *  Reads the part number from the EEPROM.
+ *  Reads the part number string from the EEPROM.
  **/
-s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num)
+s32 ixgbe_read_pba_string_generic(struct ixgbe_hw *hw, u8 *pba_num,
+                                  u32 pba_num_size)
 {
 	s32 ret_val;
 	u16 data;
+	u16 pba_ptr;
+	u16 offset;
+	u16 length;
+
+	if (pba_num == NULL) {
+		hw_dbg(hw, "PBA string buffer was null\n");
+		return IXGBE_ERR_INVALID_ARGUMENT;
+	}
 
 	ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM0_PTR, &data);
 	if (ret_val) {
 		hw_dbg(hw, "NVM Read Error\n");
 		return ret_val;
 	}
-	*pba_num = (u32)(data << 16);
 
-	ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM1_PTR, &data);
+	ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM1_PTR, &pba_ptr);
 	if (ret_val) {
 		hw_dbg(hw, "NVM Read Error\n");
 		return ret_val;
 	}
-	*pba_num |= data;
+
+	/*
+	 * if data is not ptr guard the PBA must be in legacy format which
+	 * means pba_ptr is actually our second data word for the PBA number
+	 * and we can decode it into an ascii string
+	 */
+	if (data != IXGBE_PBANUM_PTR_GUARD) {
+		hw_dbg(hw, "NVM PBA number is not stored as string\n");
+
+		/* we will need 11 characters to store the PBA */
+		if (pba_num_size < 11) {
+			hw_dbg(hw, "PBA string buffer too small\n");
+			return IXGBE_ERR_NO_SPACE;
+		}
+
+		/* extract hex string from data and pba_ptr */
+		pba_num[0] = (data >> 12) & 0xF;
+		pba_num[1] = (data >> 8) & 0xF;
+		pba_num[2] = (data >> 4) & 0xF;
+		pba_num[3] = data & 0xF;
+		pba_num[4] = (pba_ptr >> 12) & 0xF;
+		pba_num[5] = (pba_ptr >> 8) & 0xF;
+		pba_num[6] = '-';
+		pba_num[7] = 0;
+		pba_num[8] = (pba_ptr >> 4) & 0xF;
+		pba_num[9] = pba_ptr & 0xF;
+
+		/* put a null character on the end of our string */
+		pba_num[10] = '\0';
+
+		/* switch all the data but the '-' to hex char */
+		for (offset = 0; offset < 10; offset++) {
+			if (pba_num[offset] < 0xA)
+				pba_num[offset] += '0';
+			else if (pba_num[offset] < 0x10)
+				pba_num[offset] += 'A' - 0xA;
+		}
+
+		return 0;
+	}
+
+	ret_val = hw->eeprom.ops.read(hw, pba_ptr, &length);
+	if (ret_val) {
+		hw_dbg(hw, "NVM Read Error\n");
+		return ret_val;
+	}
+
+	if (length == 0xFFFF || length == 0) {
+		hw_dbg(hw, "NVM PBA number section invalid length\n");
+		return IXGBE_ERR_PBA_SECTION;
+	}
+
+	/* check if pba_num buffer is big enough */
+	if (pba_num_size  < (((u32)length * 2) - 1)) {
+		hw_dbg(hw, "PBA string buffer too small\n");
+		return IXGBE_ERR_NO_SPACE;
+	}
+
+	/* trim pba length from start of string */
+	pba_ptr++;
+	length--;
+
+	for (offset = 0; offset < length; offset++) {
+		ret_val = hw->eeprom.ops.read(hw, pba_ptr + offset, &data);
+		if (ret_val) {
+			hw_dbg(hw, "NVM Read Error\n");
+			return ret_val;
+		}
+		pba_num[offset * 2] = (u8)(data >> 8);
+		pba_num[(offset * 2) + 1] = (u8)(data & 0xFF);
+	}
+	pba_num[offset * 2] = '\0';
 
 	return 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_common.h b/drivers/net/ixgbe/ixgbe_common.h
index 341ca51..e1f980a 100644
--- a/drivers/net/ixgbe/ixgbe_common.h
+++ b/drivers/net/ixgbe/ixgbe_common.h
@@ -35,7 +35,8 @@ s32 ixgbe_init_ops_generic(struct ixgbe_hw *hw);
 s32 ixgbe_init_hw_generic(struct ixgbe_hw *hw);
 s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw);
 s32 ixgbe_clear_hw_cntrs_generic(struct ixgbe_hw *hw);
-s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num);
+s32 ixgbe_read_pba_string_generic(struct ixgbe_hw *hw, u8 *pba_num,
+                                  u32 pba_num_size);
 s32 ixgbe_get_mac_addr_generic(struct ixgbe_hw *hw, u8 *mac_addr);
 s32 ixgbe_get_bus_info_generic(struct ixgbe_hw *hw);
 void ixgbe_set_lan_id_multi_port_pcie(struct ixgbe_hw *hw);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index c3338b5..c5998ca 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6955,11 +6955,12 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 	const struct ixgbe_info *ii = ixgbe_info_tbl[ent->driver_data];
 	static int cards_found;
 	int i, err, pci_using_dac;
+	u8 part_str[IXGBE_PBANUM_LENGTH];
 	unsigned int indices = num_possible_cpus();
 #ifdef IXGBE_FCOE
 	u16 device_caps;
 #endif
-	u32 part_num, eec;
+	u32 eec;
 
 	/* Catch broken hardware that put the wrong VF device ID in
 	 * the PCIe SR-IOV capability.
@@ -7265,16 +7266,17 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		    hw->bus.width == ixgbe_bus_width_pcie_x1 ? "Width x1" :
 		    "Unknown"),
 		   netdev->dev_addr);
-	ixgbe_read_pba_num_generic(hw, &part_num);
+
+	err = ixgbe_read_pba_string_generic(hw, part_str, IXGBE_PBANUM_LENGTH);
+	if (err)
+		strcpy(part_str, "Unknown");
 	if (ixgbe_is_sfp(hw) && hw->phy.sfp_type != ixgbe_sfp_type_not_present)
-		e_dev_info("MAC: %d, PHY: %d, SFP+: %d, "
-			   "PBA No: %06x-%03x\n",
+		e_dev_info("MAC: %d, PHY: %d, SFP+: %d, PBA No: %s\n",
 			   hw->mac.type, hw->phy.type, hw->phy.sfp_type,
-			   (part_num >> 8), (part_num & 0xff));
+		           part_str);
 	else
-		e_dev_info("MAC: %d, PHY: %d, PBA No: %06x-%03x\n",
-			   hw->mac.type, hw->phy.type,
-			   (part_num >> 8), (part_num & 0xff));
+		e_dev_info("MAC: %d, PHY: %d, PBA No: %s\n",
+			   hw->mac.type, hw->phy.type, part_str);
 
 	if (hw->bus.width <= ixgbe_bus_width_pcie_x4) {
 		e_dev_warn("PCI-Express bandwidth available for this card is "
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 44bcabb..0f80893 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -1510,7 +1510,11 @@
 #define IXGBE_EEPROM_WORD_SIZE_SHIFT  6
 #define IXGBE_EEPROM_OPCODE_BITS      8
 
+/* Part Number String Length */
+#define IXGBE_PBANUM_LENGTH 11
+
 /* Checksum and EEPROM pointers */
+#define IXGBE_PBANUM_PTR_GUARD  0xFAFA
 #define IXGBE_EEPROM_CHECKSUM   0x3F
 #define IXGBE_EEPROM_SUM        0xBABA
 #define IXGBE_PCIE_ANALOG_PTR   0x03
@@ -2640,6 +2644,8 @@ struct ixgbe_info {
 #define IXGBE_ERR_OVERTEMP                      -26
 #define IXGBE_ERR_RAR_INDEX                     -27
 #define IXGBE_ERR_SFP_SETUP_NOT_COMPLETE        -30
+#define IXGBE_ERR_PBA_SECTION                   -31
+#define IXGBE_ERR_INVALID_ARGUMENT              -32
 #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
 
 #endif /* _IXGBE_TYPE_H_ */


^ permalink raw reply related

* [net-next-2.6 PATCH] ixgbe: add support for 82599 FCoE SKU
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher

From: Don Skidmore <donald.c.skidmore@intel.com>

Add both NIC and backplane support for FCoE enabled devices IDs.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_82599.c |    2 ++
 drivers/net/ixgbe/ixgbe_main.c  |    4 ++++
 drivers/net/ixgbe/ixgbe_type.h  |    2 ++
 3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 743bdec..6827ddd 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -341,11 +341,13 @@ static enum ixgbe_media_type ixgbe_get_media_type_82599(struct ixgbe_hw *hw)
 	case IXGBE_DEV_ID_82599_KX4_MEZZ:
 	case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
 	case IXGBE_DEV_ID_82599_KR:
+	case IXGBE_DEV_ID_82599_BACKPLANE_FCOE:
 	case IXGBE_DEV_ID_82599_XAUI_LOM:
 		/* Default device ID is mezzanine card KX/KX4 */
 		media_type = ixgbe_media_type_backplane;
 		break;
 	case IXGBE_DEV_ID_82599_SFP:
+	case IXGBE_DEV_ID_82599_SFP_FCOE:
 	case IXGBE_DEV_ID_82599_SFP_EM:
 		media_type = ixgbe_media_type_fiber;
 		break;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index c876a69..c3338b5 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -109,6 +109,10 @@ static DEFINE_PCI_DEVICE_TABLE(ixgbe_pci_tbl) = {
 	 board_82599 },
 	{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_CX4),
 	 board_82599 },
+	{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_BACKPLANE_FCOE),
+	 board_82599 },
+	{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_SFP_FCOE),
+	 board_82599 },
 	{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_T3_LOM),
 	 board_82599 },
 	{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_COMBO_BACKPLANE),
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index bbcfafc..44bcabb 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -54,6 +54,8 @@
 #define IXGBE_DEV_ID_82599_T3_LOM        0x151C
 #define IXGBE_DEV_ID_82599_CX4           0x10F9
 #define IXGBE_DEV_ID_82599_SFP           0x10FB
+#define IXGBE_DEV_ID_82599_BACKPLANE_FCOE       0x152a
+#define IXGBE_DEV_ID_82599_SFP_FCOE      0x1529
 #define IXGBE_SUBDEV_ID_82599_SFP        0x11A9
 #define IXGBE_DEV_ID_82599_SFP_EM        0x1507
 #define IXGBE_DEV_ID_82599_XAUI_LOM      0x10FC


^ permalink raw reply related

* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Ben Hutchings @ 2010-12-03 13:27 UTC (permalink / raw)
  To: Simon Horman; +Cc: Eric Dumazet, Simon Kirby, netdev
In-Reply-To: <20101203123617.GA6993@verge.net.au>

On Fri, 2010-12-03 at 21:36 +0900, Simon Horman wrote:
> On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
[...]
> > I believe Simon Horman has some patches for GRO and LVS.
> > 
> > Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> > 
> > For TSO, I am not sure why and where it could matter...
> 
> There is a patch to teach LVS how to cope with GRO in nf-next-2.6
> and I expect it to be included in 2.6.38. The patch is "ipvs: allow
> transmit of GRO aggregated skbs" and perhaps it should be considered
> for 2.6.37 and stable. In general the work around is to disable GRO.
>
> The patch does not resolve the incompatibility of LVS with LRO.
> The work around there is to disable LRO. I'm not entirely sure
> how to teach LVS to disable LRO automatically, or if its desirable.
[...]

I think it is desirable, and we already do it for bridging and
forwarding - we call dev_disable_lro() whenever a device is added to a
bridge or has forwarding enabled.  For IPVS it looks like you would have
to resolve the server IP address to a device first.  Though if the
administrator assigns the server address to a different interface later
then the failure mode will be very hard to understand.

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: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Eric Dumazet @ 2010-12-03 13:09 UTC (permalink / raw)
  To: Toshio; +Cc: linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <0fe401cb92e7$85ba2260$912e6720$@si>

Le vendredi 03 décembre 2010 à 13:42 +0100, Toshio a écrit :
> I have also hit on Bug 20292 (https://bugzilla.kernel.org/show_bug.cgi?id=20292) in final 2.6.36. After investigating changes made between 2.6.35.4, which worked, and 2.6.36 which started oopsing, I think the problem was in double freeing of skb caused by change of return value for __pppoe_xmit in case of errors.
> 
> As it turned out this might be the cause of random BUG reports throught the kernel, whenever something touched skb. Most common BUG with my use case happened at skb_dequeue:
> 

CC netdev and Rami Rosen

> 00000060 <skb_dequeue>:
>       60:       53                      push   %ebx
>       61:       89 c2                   mov    %eax,%edx
>       63:       9c                      pushf  
>       64:       59                      pop    %ecx
>       65:       fa                      cli    
>       66:       8b 00                   mov    (%eax),%eax
>       68:       39 c2                   cmp    %eax,%edx
>       6a:       74 24                   je     90 <skb_dequeue+0x30>
>       6c:       85 c0                   test   %eax,%eax
>       6e:       74 1a                   je     8a <skb_dequeue+0x2a>
>       70:       ff 4a 08                decl   0x8(%edx)
>       73:       8b 18                   mov    (%eax),%ebx
>       75:       c7 00 00 00 00 00       movl   $0x0,(%eax)
>       7b:       8b 50 04                mov    0x4(%eax),%edx
>       7e:       c7 40 04 00 00 00 00    movl   $0x0,0x4(%eax)
>       85:       89 53 04                mov    %edx,0x4(%ebx)
>       88:*      89 1a                   mov    %ebx,(%edx)
>       8a:       51                      push   %ecx
>       8b:       9d                      popf   
>       8c:       5b                      pop    %ebx
>       8d:       c3                      ret    
>       8e:       66 90                   xchg   %ax,%ax
>       90:       b8 00 00 00 00          mov    $0x0,%eax
>       95:       eb f3                   jmp    8a <skb_dequeue+0x2a>
>       97:       89 f6                   mov    %esi,%esi
>       99:       8d bc 27 00 00 00 00    lea    0x0(%edi,%eiz,1),%edi
> 
> 
> This location corresponds to line "next = next->next" from inlined __skb_dequeue(). The only reason BUG could happen here is something overwriting or otherwise corrupting skb list.
> 
> Patch that works for me is below. Now I only hope I haven't (re)introduced a memory leak...
> 
> I am not subscribed to LKML, so please reply-to-all if you need to contact me.
> 
> -----------------------------------------------------------------------------
> --- linux-2.6.36/drivers/net/pppoe.c    2010-10-20 22:30:22.000000000 +0200
> +++ linux-2.6.36.toshio/drivers/net/pppoe.c     2010-12-03 13:11:56.000000000 +0100
> @@ -924,8 +924,10 @@
>         /* Copy the data if there is no space for the header or if it's
>          * read-only.
>          */
> -       if (skb_cow_head(skb, sizeof(*ph) + dev->hard_header_len))
> +       if (skb_cow_head(skb, sizeof(*ph) + dev->hard_header_len)) {
> +               kfree_skb(skb);
>                 goto abort;
> +       }
>  
>         __skb_push(skb, sizeof(*ph));
>         skb_reset_network_header(skb);
> @@ -947,7 +949,6 @@
>         return 1;
>  
>  abort:
> -       kfree_skb(skb);
>         return 0;
>  }
> 

Problem comes from commit 55c95e738da85 (fix return value of
__pppoe_xmit() method)

I am not sure patch is OK

^ permalink raw reply

* Re: PCI: make pci_restore_state return void
From: Mauro Carvalho Chehab @ 2010-12-03 13:08 UTC (permalink / raw)
  To: Jon Mason
  Cc: Jesse Barnes, linux-pci, Jonathan Corbet, linux-media,
	Andrew Gallatin, Brice Goglin, netdev,
	Solarflare linux maintainers, Steve Hodgson, Ben Hutchings,
	Stephen Hemminger, Ivo van Doorn, Gertjan van Wingerde,
	linux-wireless, Brian King, Anil Ravindranath, linux-scsi,
	Jaya Kumar, boyod.yang
In-Reply-To: <1291160606-31494-1-git-send-email-jon.mason@exar.com>

Em 30-11-2010 21:43, Jon Mason escreveu:
> pci_restore_state only ever returns 0, thus there is no benefit in
> having it return any value.  Also, a large majority of the callers do
> not check the return code of pci_restore_state.  Make the
> pci_restore_state a void return and avoid the overhead.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>
> ---
>  drivers/media/video/cafe_ccic.c         |    4 +---

Seems ok to me.

Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>

>  drivers/net/myri10ge/myri10ge.c         |    4 +---
>  drivers/net/sfc/falcon.c                |   25 +++++--------------------
>  drivers/net/skge.c                      |    4 +---
>  drivers/net/sky2.c                      |    5 +----
>  drivers/net/wireless/rt2x00/rt2x00pci.c |    4 ++--
>  drivers/pci/pci-driver.c                |    3 ++-
>  drivers/pci/pci.c                       |    7 ++-----
>  drivers/scsi/ipr.c                      |    8 +-------
>  drivers/scsi/pmcraid.c                  |    7 +------
>  drivers/staging/sm7xx/smtcfb.c          |    2 +-
>  include/linux/pci.h                     |    8 +++-----
>  sound/pci/cs5535audio/cs5535audio_pm.c  |    7 +------
>  13 files changed, 22 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
> index 2934770..3e653f3 100644
> --- a/drivers/media/video/cafe_ccic.c
> +++ b/drivers/media/video/cafe_ccic.c
> @@ -2186,9 +2186,7 @@ static int cafe_pci_resume(struct pci_dev *pdev)
>  	struct cafe_camera *cam = to_cam(v4l2_dev);
>  	int ret = 0;
>  
> -	ret = pci_restore_state(pdev);
> -	if (ret)
> -		return ret;
> +	pci_restore_state(pdev);
>  	ret = pci_enable_device(pdev);
>  
>  	if (ret) {
> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
> index 8524cc4..d3c4a37 100644
> --- a/drivers/net/myri10ge/myri10ge.c
> +++ b/drivers/net/myri10ge/myri10ge.c
> @@ -3403,9 +3403,7 @@ static int myri10ge_resume(struct pci_dev *pdev)
>  		return -EIO;
>  	}
>  
> -	status = pci_restore_state(pdev);
> -	if (status)
> -		return status;
> +	pci_restore_state(pdev);
>  
>  	status = pci_enable_device(pdev);
>  	if (status) {
> diff --git a/drivers/net/sfc/falcon.c b/drivers/net/sfc/falcon.c
> index 267019b..1763b9a 100644
> --- a/drivers/net/sfc/falcon.c
> +++ b/drivers/net/sfc/falcon.c
> @@ -1066,22 +1066,9 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
>  
>  	/* Restore PCI configuration if needed */
>  	if (method == RESET_TYPE_WORLD) {
> -		if (efx_nic_is_dual_func(efx)) {
> -			rc = pci_restore_state(nic_data->pci_dev2);
> -			if (rc) {
> -				netif_err(efx, drv, efx->net_dev,
> -					  "failed to restore PCI config for "
> -					  "the secondary function\n");
> -				goto fail3;
> -			}
> -		}
> -		rc = pci_restore_state(efx->pci_dev);
> -		if (rc) {
> -			netif_err(efx, drv, efx->net_dev,
> -				  "failed to restore PCI config for the "
> -				  "primary function\n");
> -			goto fail4;
> -		}
> +		if (efx_nic_is_dual_func(efx))
> +			pci_restore_state(nic_data->pci_dev2);
> +		pci_restore_state(efx->pci_dev);
>  		netif_dbg(efx, drv, efx->net_dev,
>  			  "successfully restored PCI config\n");
>  	}
> @@ -1092,7 +1079,7 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
>  		rc = -ETIMEDOUT;
>  		netif_err(efx, hw, efx->net_dev,
>  			  "timed out waiting for hardware reset\n");
> -		goto fail5;
> +		goto fail3;
>  	}
>  	netif_dbg(efx, hw, efx->net_dev, "hardware reset complete\n");
>  
> @@ -1100,11 +1087,9 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
>  
>  	/* pci_save_state() and pci_restore_state() MUST be called in pairs */
>  fail2:
> -fail3:
>  	pci_restore_state(efx->pci_dev);
>  fail1:
> -fail4:
> -fail5:
> +fail3:
>  	return rc;
>  }
>  
> diff --git a/drivers/net/skge.c b/drivers/net/skge.c
> index 220e039..61553af 100644
> --- a/drivers/net/skge.c
> +++ b/drivers/net/skge.c
> @@ -4087,9 +4087,7 @@ static int skge_resume(struct pci_dev *pdev)
>  	if (err)
>  		goto out;
>  
> -	err = pci_restore_state(pdev);
> -	if (err)
> -		goto out;
> +	pci_restore_state(pdev);
>  
>  	err = skge_reset(hw);
>  	if (err)
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index d657708..be3aee7 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -4969,10 +4969,7 @@ static int sky2_resume(struct pci_dev *pdev)
>  	if (err)
>  		goto out;
>  
> -	err = pci_restore_state(pdev);
> -	if (err)
> -		goto out;
> -
> +	pci_restore_state(pdev);
>  	pci_enable_wake(pdev, PCI_D0, 0);
>  
>  	/* Re-enable all clocks */
> diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
> index 868ca19..5e3c46f 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
> @@ -356,12 +356,12 @@ int rt2x00pci_resume(struct pci_dev *pci_dev)
>  	struct rt2x00_dev *rt2x00dev = hw->priv;
>  
>  	if (pci_set_power_state(pci_dev, PCI_D0) ||
> -	    pci_enable_device(pci_dev) ||
> -	    pci_restore_state(pci_dev)) {
> +	    pci_enable_device(pci_dev)) {
>  		ERROR(rt2x00dev, "Failed to resume device.\n");
>  		return -EIO;
>  	}
>  
> +	pci_restore_state(pci_dev);
>  	return rt2x00lib_resume(rt2x00dev);
>  }
>  EXPORT_SYMBOL_GPL(rt2x00pci_resume);
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 8a6f797..80e551e 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -449,7 +449,8 @@ static int pci_restore_standard_config(struct pci_dev *pci_dev)
>  			return error;
>  	}
>  
> -	return pci_restore_state(pci_dev);
> +	pci_restore_state(pci_dev);
> +	return 0;
>  }
>  
>  static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e98c810..c711d1b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -937,14 +937,13 @@ pci_save_state(struct pci_dev *dev)
>   * pci_restore_state - Restore the saved state of a PCI device
>   * @dev: - PCI device that we're dealing with
>   */
> -int 
> -pci_restore_state(struct pci_dev *dev)
> +void pci_restore_state(struct pci_dev *dev)
>  {
>  	int i;
>  	u32 val;
>  
>  	if (!dev->state_saved)
> -		return 0;
> +		return;
>  
>  	/* PCI Express register must be restored first */
>  	pci_restore_pcie_state(dev);
> @@ -968,8 +967,6 @@ pci_restore_state(struct pci_dev *dev)
>  	pci_restore_iov_state(dev);
>  
>  	dev->state_saved = false;
> -
> -	return 0;
>  }
>  
>  static int do_pci_enable_device(struct pci_dev *dev, int bars)
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index fa60d7d..1d7dbe6 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -7485,16 +7485,10 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd)
>  {
>  	struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
>  	volatile u32 int_reg;
> -	int rc;
>  
>  	ENTER;
>  	ioa_cfg->pdev->state_saved = true;
> -	rc = pci_restore_state(ioa_cfg->pdev);
> -
> -	if (rc != PCIBIOS_SUCCESSFUL) {
> -		ipr_cmd->s.ioasa.hdr.ioasc = cpu_to_be32(IPR_IOASC_PCI_ACCESS_ERROR);
> -		return IPR_RC_JOB_CONTINUE;
> -	}
> +	pci_restore_state(ioa_cfg->pdev);
>  
>  	if (ipr_set_pcix_cmd_reg(ioa_cfg)) {
>  		ipr_cmd->s.ioasa.hdr.ioasc = cpu_to_be32(IPR_IOASC_PCI_ACCESS_ERROR);
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index cf89091..091baf2 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -2227,12 +2227,7 @@ static void pmcraid_ioa_reset(struct pmcraid_cmd *cmd)
>  		/* Once either bist or pci reset is done, restore PCI config
>  		 * space. If this fails, proceed with hard reset again
>  		 */
> -		if (pci_restore_state(pinstance->pdev)) {
> -			pmcraid_info("config-space error resetting again\n");
> -			pinstance->ioa_state = IOA_STATE_IN_RESET_ALERT;
> -			pmcraid_reset_alert(cmd);
> -			break;
> -		}
> +		pci_restore_state(pinstance->pdev);
>  
>  		/* fail all pending commands */
>  		pmcraid_fail_outstanding_cmds(pinstance);
> diff --git a/drivers/staging/sm7xx/smtcfb.c b/drivers/staging/sm7xx/smtcfb.c
> index 24f47d6..7162dee 100644
> --- a/drivers/staging/sm7xx/smtcfb.c
> +++ b/drivers/staging/sm7xx/smtcfb.c
> @@ -1071,7 +1071,7 @@ static int __maybe_unused smtcfb_resume(struct pci_dev *pdev)
>  	/* when resuming, restore pci data and fb cursor */
>  	if (pdev->dev.power.power_state.event != PM_EVENT_FREEZE) {
>  		retv = pci_set_power_state(pdev, PCI_D0);
> -		retv = pci_restore_state(pdev);
> +		pci_restore_state(pdev);
>  		if (pci_enable_device(pdev))
>  			return -1;
>  		pci_set_master(pdev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 7454408..63cbadc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -806,7 +806,7 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size);
>  
>  /* Power management related routines */
>  int pci_save_state(struct pci_dev *dev);
> -int pci_restore_state(struct pci_dev *dev);
> +void pci_restore_state(struct pci_dev *dev);
>  int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
>  int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
>  pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
> @@ -1168,10 +1168,8 @@ static inline int pci_save_state(struct pci_dev *dev)
>  	return 0;
>  }
>  
> -static inline int pci_restore_state(struct pci_dev *dev)
> -{
> -	return 0;
> -}
> +static inline void pci_restore_state(struct pci_dev *dev)
> +{ }
>  
>  static inline int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
>  {
> diff --git a/sound/pci/cs5535audio/cs5535audio_pm.c b/sound/pci/cs5535audio/cs5535audio_pm.c
> index a3301cc..185b000 100644
> --- a/sound/pci/cs5535audio/cs5535audio_pm.c
> +++ b/sound/pci/cs5535audio/cs5535audio_pm.c
> @@ -90,12 +90,7 @@ int snd_cs5535audio_resume(struct pci_dev *pci)
>  	int i;
>  
>  	pci_set_power_state(pci, PCI_D0);
> -	if (pci_restore_state(pci) < 0) {
> -		printk(KERN_ERR "cs5535audio: pci_restore_state failed, "
> -		       "disabling device\n");
> -		snd_card_disconnect(card);
> -		return -EIO;
> -	}
> +	pci_restore_state(pci);
>  	if (pci_enable_device(pci) < 0) {
>  		printk(KERN_ERR "cs5535audio: pci_enable_device failed, "
>  		       "disabling device\n");


^ permalink raw reply

* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Horman @ 2010-12-03 12:42 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Simon Kirby, netdev, lvs-devel, Julian Anastasov, Herbert Xu
In-Reply-To: <20101203123617.GA6993@verge.net.au>

[ CCed lvs-devel, Julian Anastasov and Herbert Xu ]

On Fri, Dec 03, 2010 at 09:36:19PM +0900, Simon Horman wrote:
> On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
> > Le vendredi 03 décembre 2010 à 02:34 -0800, Simon Kirby a écrit :
> > > Hello!
> > > 
> > > We upgraded some LVS (DR) servers from 2.6.35 to 2.6.36 on tg3 cards
> > > (partno(BCM95721) rev 4201) with VLAN tags in use, to think that
> > > everything looked great, but in fact...
> > > 
> > > LVS was receiving magically-merged TCP packets which it tried to forward
> > > on to the real server, only to get annoyed at itself for trying to
> > > forward a packet bigger than the device MTU:
> > > 
> > > IP A.47376 > B.529: . 175488:176936(1448) ack 1 win 92 <nop,nop,timestamp 52737308 29656081>
> > > IP A.47376 > B.529: . 176936:179832(2896) ack 1 win 92 <nop,nop,timestamp 52737325 29656098>
> > > IP B > A: ICMP B unreachable - need to frag (mtu 1500), length 556
> > > 
> > 
> > Hi Simon
> > 
> > This is a tcpdump on A ?
> > Could you take it also on B ?
> > 
> > tcpdump displays large buffers, but they should be split (of course)
> > when sent on wire.
> > 
> > > This caused packet loss for any merged frames, which caused abysmal
> > > performance for uploads via the LVS server.  Local performance to or
> > > from the box is still fine, because the stack doesn't care, only the
> > > forwarding part of LVS is running into the problem.
> > > 
> > > Furthermore, disabling _everything_ reported by ethtool -k doesn't seem
> > > to change the result, even if I down/up the interface after, and even if
> > > I try on every single interface including the VLANned ones.  This seems
> > > to be another bug.  Reverting to 2.6.35 makes it all work again.
> > > 
> > > Possibly related to commit 7fe876af921d1d2bc8353e0062c10ff35e902653
> > > 
> > > So how should this be fixed?  Should LVS be taught to fragment, or must
> > > we disable the merging in this case?  It seems like it would work well if
> > > the sending side could do the same offload in reverse, but I'm not sure
> > > if that would be possible.
> > > 
> > > Simon-
> > 
> > 
> > I believe Simon Horman has some patches for GRO and LVS.
> > 
> > Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> > 
> > For TSO, I am not sure why and where it could matter...
> 
> There is a patch to teach LVS how to cope with GRO in nf-next-2.6
> and I expect it to be included in 2.6.38. The patch is "ipvs: allow
> transmit of GRO aggregated skbs" and perhaps it should be considered
> for 2.6.37 and stable. In general the work around is to disable GRO.
> 
> The patch does not resolve the incompatibility of LVS with LRO.
> The work around there is to disable LRO. I'm not entirely sure
> how to teach LVS to disable LRO automatically, or if its desirable.
> 
> Simon, you mention that you disabled everything with ethtool, but the
> tcpdump above shows a 2896 byte packet, which seems that GRO (or LRO?) is
> active. So perhaps as you speculate that is a bug
> 
> I will prepare a backport of the "ipvs: allow transmit of GRO aggregated
> skbs" patch to v2.6.36 and post it shortly.  Testing to see if that
> resolves the problem that you are seeing would probably be a good start.

Here is the patch for v2.6.36.

From: Simon Horman <horms@verge.net.au>

ipvs: allow transmit of GRO aggregated skbs

Attempt at allowing LVS to transmit skbs of greater than MTU length that
have been aggregated by GRO and can thus be deaggregated by GSO.

Cc: Julian Anastasov <ja@ssi.bg>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Simon Horman <horms@verge.net.au>

---
 net/netfilter/ipvs/ip_vs_xmit.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 49df6be..577f502 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -247,7 +247,8 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !skb_is_gso(skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -311,7 +312,7 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !skb_is_gso(skb)) {
 		dst_release(&rt->dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -408,7 +409,8 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !skb_is_gso(skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL_PKT(0, pp, skb, 0, "ip_vs_nat_xmit(): frag needed for");
@@ -486,7 +488,7 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !skb_is_gso(skb)) {
 		dst_release(&rt->dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL_PKT(0, pp, skb, 0,
@@ -597,8 +599,8 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	df |= (old_iph->frag_off & htons(IP_DF));
 
-	if ((old_iph->frag_off & htons(IP_DF))
-	    && mtu < ntohs(old_iph->tot_len)) {
+	if ((old_iph->frag_off & htons(IP_DF) &&
+	    mtu < ntohs(old_iph->tot_len) && !skb_is_gso(skb))) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		ip_rt_put(rt);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -707,7 +709,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	if (skb_dst(skb))
 		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), mtu);
 
-	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr)) {
+	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
+	    !skb_is_gso(skb)) {
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		dst_release(&rt->dst);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -796,7 +799,8 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu) {
+	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu &&
+	    !skb_is_gso(skb)) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		ip_rt_put(rt);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -924,7 +928,8 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF)) &&
+	    !skb_is_gso(skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -999,7 +1004,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !skb_is_gso(skb)) {
 		dst_release(&rt->dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 2/2] The new jhash implementation
From: Jozsef Kadlecsik @ 2010-12-03 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: netdev, netfilter-devel, Linus Torvalds, Rusty Russell,
	Jozsef Kadlecsik
In-Reply-To: <1291379941-31565-2-git-send-email-kadlec@blackhole.kfki.hu>

The current jhash.h implements the lookup2() hash function by Bob Jenkins.
However, lookup2() is outdated as Bob wrote a new hash function called
lookup3(). The patch replaces the lookup2() implementation of the 'jhash*'
functions with that of lookup3().

You can read a longer comparison of the two and other hash functions at
http://burtleburtle.net/bob/hash/doobs.html.
---
 include/linux/jhash.h |  183 ++++++++++++++++++++++++++++---------------------
 1 files changed, 105 insertions(+), 78 deletions(-)

diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index ced1159..47cb09e 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -3,129 +3,156 @@
 
 /* jhash.h: Jenkins hash support.
  *
- * Copyright (C) 1996 Bob Jenkins (bob_jenkins@burtleburtle.net)
+ * Copyright (C) 2006. Bob Jenkins (bob_jenkins@burtleburtle.net)
  *
  * http://burtleburtle.net/bob/hash/
  *
  * These are the credits from Bob's sources:
  *
- * lookup2.c, by Bob Jenkins, December 1996, Public Domain.
- * hash(), hash2(), hash3, and mix() are externally useful functions.
- * Routines to test the hash are included if SELF_TEST is defined.
- * You can use this free for any purpose.  It has no warranty.
+ * lookup3.c, by Bob Jenkins, May 2006, Public Domain.
  *
- * Copyright (C) 2003 David S. Miller (davem@redhat.com)
+ * These are functions for producing 32-bit hashes for hash table lookup.
+ * hashword(), hashlittle(), hashlittle2(), hashbig(), mix(), and final()
+ * are externally useful functions.  Routines to test the hash are included
+ * if SELF_TEST is defined.  You can use this free for any purpose.  It's in
+ * the public domain.  It has no warranty.
+ *
+ * Copyright (C) 2009-2010 Jozsef Kadlecsik (kadlec@blackhole.kfki.hu)
  *
  * I've modified Bob's hash to be useful in the Linux kernel, and
- * any bugs present are surely my fault.  -DaveM
+ * any bugs present are my fault.
+ * Jozsef
  */
+#include <linux/bitops.h>
+#include <linux/unaligned/packed_struct.h>
+
+/* Best hash sizes are of power of two */
+#define jhash_size(n)   ((u32)1<<(n))
+/* Mask the hash value, i.e (value & jhash_mask(n)) instead of (value % n) */
+#define jhash_mask(n)   (jhash_size(n)-1)
+
+/* __jhash_mix -- mix 3 32-bit values reversibly. */
+#define __jhash_mix(a, b, c)			\
+{						\
+	a -= c;  a ^= rol32(c, 4);  c += b;	\
+	b -= a;  b ^= rol32(a, 6);  a += c;	\
+	c -= b;  c ^= rol32(b, 8);  b += a;	\
+	a -= c;  a ^= rol32(c, 16); c += b;	\
+	b -= a;  b ^= rol32(a, 19); a += c;	\
+	c -= b;  c ^= rol32(b, 4);  b += a;	\
+}
 
-/* NOTE: Arguments are modified. */
-#define __jhash_mix(a, b, c) \
-{ \
-  a -= b; a -= c; a ^= (c>>13); \
-  b -= c; b -= a; b ^= (a<<8); \
-  c -= a; c -= b; c ^= (b>>13); \
-  a -= b; a -= c; a ^= (c>>12);  \
-  b -= c; b -= a; b ^= (a<<16); \
-  c -= a; c -= b; c ^= (b>>5); \
-  a -= b; a -= c; a ^= (c>>3);  \
-  b -= c; b -= a; b ^= (a<<10); \
-  c -= a; c -= b; c ^= (b>>15); \
+/* __jhash_final - final mixing of 3 32-bit values (a,b,c) into c */
+#define __jhash_final(a, b, c)			\
+{						\
+	c ^= b; c -= rol32(b, 14);		\
+	a ^= c; a -= rol32(c, 11);		\
+	b ^= a; b -= rol32(a, 25);		\
+	c ^= b; c -= rol32(b, 16);		\
+	a ^= c; a -= rol32(c, 4);		\
+	b ^= a; b -= rol32(a, 14);		\
+	c ^= b; c -= rol32(b, 24);		\
 }
 
-/* The golden ration: an arbitrary value */
-#define JHASH_GOLDEN_RATIO	0x9e3779b9
+/* An arbitrary initial parameter */
+#define JHASH_INITVAL		0xdeadbeef
 
-/* The most generic version, hashes an arbitrary sequence
- * of bytes.  No alignment or length assumptions are made about
- * the input key.
+/* jhash - hash an arbitrary key
+ * @k: sequence of bytes as key
+ * @length: the length of the key
+ * @initval: the previous hash, or an arbitray value
+ *
+ * The generic version, hashes an arbitrary sequence of bytes.
+ * No alignment or length assumptions are made about the input key.
+ *
+ * Returns the hash value of the key. The result depends on endianness.
  */
 static inline u32 jhash(const void *key, u32 length, u32 initval)
 {
-	u32 a, b, c, len;
+	u32 a, b, c;
 	const u8 *k = key;
 
-	len = length;
-	a = b = JHASH_GOLDEN_RATIO;
-	c = initval;
-
-	while (len >= 12) {
-		a += (k[0] +((u32)k[1]<<8) +((u32)k[2]<<16) +((u32)k[3]<<24));
-		b += (k[4] +((u32)k[5]<<8) +((u32)k[6]<<16) +((u32)k[7]<<24));
-		c += (k[8] +((u32)k[9]<<8) +((u32)k[10]<<16)+((u32)k[11]<<24));
-
-		__jhash_mix(a,b,c);
+	/* Set up the internal state */
+	a = b = c = JHASH_INITVAL + length + initval;
 
+	/* All but the last block: affect some 32 bits of (a,b,c) */
+	while (length > 12) {
+		a += __get_unaligned_cpu32(k);
+		b += __get_unaligned_cpu32(k + 4);
+		c += __get_unaligned_cpu32(k + 8);
+		__jhash_mix(a, b, c);
+		length -= 12;
 		k += 12;
-		len -= 12;
 	}
-
-	c += length;
-	switch (len) {
-	case 11: c += ((u32)k[10]<<24);
-	case 10: c += ((u32)k[9]<<16);
-	case 9 : c += ((u32)k[8]<<8);
-	case 8 : b += ((u32)k[7]<<24);
-	case 7 : b += ((u32)k[6]<<16);
-	case 6 : b += ((u32)k[5]<<8);
-	case 5 : b += k[4];
-	case 4 : a += ((u32)k[3]<<24);
-	case 3 : a += ((u32)k[2]<<16);
-	case 2 : a += ((u32)k[1]<<8);
-	case 1 : a += k[0];
-	};
-
-	__jhash_mix(a,b,c);
+	/* Last block: affect all 32 bits of (c) */
+	/* All the case statements fall through */
+	switch (length) {
+	case 12: c += (u32)k[11]<<24;
+	case 11: c += (u32)k[10]<<16;
+	case 10: c += (u32)k[9]<<8;
+	case 9:  c += k[8];
+	case 8:  b += (u32)k[7]<<24;
+	case 7:  b += (u32)k[6]<<16;
+	case 6:  b += (u32)k[5]<<8;
+	case 5:  b += k[4];
+	case 4:  a += (u32)k[3]<<24;
+	case 3:  a += (u32)k[2]<<16;
+	case 2:  a += (u32)k[1]<<8;
+	case 1:  a += k[0];
+		 __jhash_final(a, b, c);
+	case 0: /* Nothing left to add */
+		break;
+	}
 
 	return c;
 }
 
-/* A special optimized version that handles 1 or more of u32s.
- * The length parameter here is the number of u32s in the key.
+/* jhash2 - hash an array of u32's
+ * @k: the key which must be an array of u32's
+ * @length: the number of u32's in the key
+ * @initval: the previous hash, or an arbitray value
+ *
+ * Returns the hash value of the key.
  */
 static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 {
-	u32 a, b, c, len;
+	u32 a, b, c;
 
-	a = b = JHASH_GOLDEN_RATIO;
-	c = initval;
-	len = length;
+	/* Set up the internal state */
+	a = b = c = JHASH_INITVAL + (length<<2) + initval;
 
-	while (len >= 3) {
+	/* Handle most of the key */
+	while (length > 3) {
 		a += k[0];
 		b += k[1];
 		c += k[2];
 		__jhash_mix(a, b, c);
-		k += 3; len -= 3;
+		length -= 3;
+		k += 3;
 	}
 
-	c += length * 4;
-
-	switch (len) {
-	case 2 : b += k[1];
-	case 1 : a += k[0];
-	};
-
-	__jhash_mix(a,b,c);
+	/* Handle the last 3 u32's: all the case statements fall through */
+	switch (length) {
+	case 3: c += k[2];
+	case 2: b += k[1];
+	case 1: a += k[0];
+		__jhash_final(a, b, c);
+	case 0:	/* Nothing left to add */
+		break;
+	}
 
 	return c;
 }
 
 
-/* A special ultra-optimized versions that knows they are hashing exactly
- * 3, 2 or 1 word(s).
- *
- * NOTE: In particular the "c += length; __jhash_mix(a,b,c);" normally
- *       done at the end is not done here.
- */
+/* jhash_3words - hash exactly 3, 2 or 1 word(s) */
 static inline u32 jhash_3words(u32 a, u32 b, u32 c, u32 initval)
 {
-	a += JHASH_GOLDEN_RATIO;
-	b += JHASH_GOLDEN_RATIO;
+	a += JHASH_INITVAL;
+	b += JHASH_INITVAL;
 	c += initval;
 
-	__jhash_mix(a, b, c);
+	__jhash_final(a, b, c);
 
 	return c;
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/2] Remove calls to jhash internals
From: Jozsef Kadlecsik @ 2010-12-03 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: netdev, netfilter-devel, Linus Torvalds, Rusty Russell,
	Jozsef Kadlecsik
In-Reply-To: <1291379941-31565-1-git-send-email-kadlec@blackhole.kfki.hu>

Because the jhash implementation changes, replace the calls to the jhash
internal macros with calls to the jhash functions.
---
 net/ipv6/inet6_connection_sock.c |   18 ++++++++----------
 net/ipv6/reassembly.c            |   36 ++++++++++++++++--------------------
 2 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index 8a16280..861d252 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -60,18 +60,16 @@ EXPORT_SYMBOL_GPL(inet6_csk_bind_conflict);
 static u32 inet6_synq_hash(const struct in6_addr *raddr, const __be16 rport,
 			   const u32 rnd, const u16 synq_hsize)
 {
-	u32 a = (__force u32)raddr->s6_addr32[0];
-	u32 b = (__force u32)raddr->s6_addr32[1];
-	u32 c = (__force u32)raddr->s6_addr32[2];
-
-	a += JHASH_GOLDEN_RATIO;
-	b += JHASH_GOLDEN_RATIO;
-	c += rnd;
-	__jhash_mix(a, b, c);
-
-	a += (__force u32)raddr->s6_addr32[3];
-	b += (__force u32)rport;
-	__jhash_mix(a, b, c);
+	u32 c;
+
+	c = jhash_3words((__force u32)raddr->s6_addr32[0],
+			 (__force u32)raddr->s6_addr32[1],
+			 (__force u32)raddr->s6_addr32[2],
+			 rnd);
+
+	c = jhash_2words((__force u32)raddr->s6_addr32[3],
+			 (__force u32)rport,
+			 c);
 
 	return c & (synq_hsize - 1);
 }
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index c7ba314..5e57490 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -104,26 +104,22 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
 unsigned int inet6_hash_frag(__be32 id, const struct in6_addr *saddr,
 			     const struct in6_addr *daddr, u32 rnd)
 {
-	u32 a, b, c;
-
-	a = (__force u32)saddr->s6_addr32[0];
-	b = (__force u32)saddr->s6_addr32[1];
-	c = (__force u32)saddr->s6_addr32[2];
-
-	a += JHASH_GOLDEN_RATIO;
-	b += JHASH_GOLDEN_RATIO;
-	c += rnd;
-	__jhash_mix(a, b, c);
-
-	a += (__force u32)saddr->s6_addr32[3];
-	b += (__force u32)daddr->s6_addr32[0];
-	c += (__force u32)daddr->s6_addr32[1];
-	__jhash_mix(a, b, c);
-
-	a += (__force u32)daddr->s6_addr32[2];
-	b += (__force u32)daddr->s6_addr32[3];
-	c += (__force u32)id;
-	__jhash_mix(a, b, c);
+	u32 c;
+
+	c = jhash_3words((__force u32)saddr->s6_addr32[0],
+			 (__force u32)saddr->s6_addr32[1],
+			 (__force u32)saddr->s6_addr32[2],
+			 rnd);
+
+	c = jhash_3words((__force u32)saddr->s6_addr32[3],
+			 (__force u32)daddr->s6_addr32[0],
+			 (__force u32)daddr->s6_addr32[1],
+			 c);
+
+	c =  jhash_3words((__force u32)daddr->s6_addr32[2],
+			  (__force u32)daddr->s6_addr32[3],
+			  (__force u32)id,
+			  c);
 
 	return c & (INETFRAGS_HASHSZ - 1);
 }
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 0/2] New jhash function
From: Jozsef Kadlecsik @ 2010-12-03 12:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: netdev, netfilter-devel, Linus Torvalds, Rusty Russell,
	Jozsef Kadlecsik
In-Reply-To: <201011271401.22773.rusty@rustcorp.com.au>

Hi,

The current jhash.h implements the lookup2() hash function by Bob Jenkins. 
However, lookup2() is outdated as Bob wrote a new hash function called 
lookup3(). There is a longer comparison of those two and other hash
functions at http://burtleburtle.net/bob/hash/doobs.html.

Please consider applying the following patches.

Speed

I wrote a small benchmark program to compare jhash2 and jhash3 (you can
download it from http://www.kfki.hu/~kadlec/sw/netfilter/jhash23.tgz).
On a machine with Core2 Duo and compiled with -O2, the ratio of the execution
time for the byte variants of the hash functions were (in parens the different
key sizes):

jhash2/jhash3 (4 bytes): 1.587518
jhash2/jhash3 (8 bytes): 1.282824
jhash2/jhash3 (12 bytes): 2.349628
jhash2/jhash3 (16 bytes): 1.466988
jhash2/jhash3 (32 bytes): 1.501063
jhash2/jhash3 (64 bytes): 1.587527
jhash2/jhash3 (128 bytes): 1.628037
jhash2/jhash3 (256 bytes): 1.648318

Similarly, for the word variants

jhash2/jhash3 (1 words): 1.300904
jhash2/jhash3 (2 words): 1.316108
jhash2/jhash3 (3 words): 2.249708
jhash2/jhash3 (4 words): 1.460192
jhash2/jhash3 (8 words): 1.501438
jhash2/jhash3 (16 words): 1.558039
jhash2/jhash3 (32 words): 1.520082
jhash2/jhash3 (64 words): 1.528347

Sizes

When compiling just the byte variants the sizes are

   text    data     bss     dec     hex filename
    661       0       0     661     295 jhashb2.o
    441       0       0     441     1b9 jhashb3.o

and for the word variants

   text    data     bss     dec     hex filename
    354       0       0     354     162 jhashw2.o
    248       0       0     248      f8 jhashw3.o

I compiled the kernel with "allyesconfig", in three variants: jhash2, jhash3 and
jhash3 un-inlined

   text    data     bss     dec     hex filename
69297477        11282083        35904032        116483592       6f16608 vmlinux.jhash2
69293829        11282083        35903728        116479640       6f15698 vmlinux.jhash3
69288578        11282083        35902336        116472997       6f13ca5 vmlinux.jhash3-uninlined

With jhash3 we can gain 3.6k and un-inlining shrinks the code with an additional
5.2k. In the patch I left jhash(3) inlined.

Uniformity

According to Bob Jenkins, lookup3() mixes better than lookup2(): it passes
the check that every input bit changes every output bit 50% of the time, while
lookup2() failed it. In order to verify it I added jhash3 to the cttest program,
which tests hash functions with (artifical, worst case) netfilter conntrack data
and calculates the statistics (chi-square, probability of longest bucket, etc).
You can find the program and the results here:
http://www.kfki.hu/~kadlec/sw/netfilter/ct3/ - to sum up, lookup3() produces
uniform key values and no weakness could be spotted.

Many thanks to Eric Dumazet for his thorough reviewing.

Dave applied the first patch to net-next-2.6.

Jozsef Kadlecsik (2):
  Remove calls to jhash internals
  The new jhash implementation

 include/linux/jhash.h            |  183 ++++++++++++++++++++++----------------
 net/ipv6/inet6_connection_sock.c |   18 ++--
 net/ipv6/reassembly.c            |   36 ++++----
 3 files changed, 129 insertions(+), 108 deletions(-)

^ permalink raw reply

* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Horman @ 2010-12-03 12:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Simon Kirby, netdev
In-Reply-To: <1291375743.2897.141.camel@edumazet-laptop>

On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
> Le vendredi 03 décembre 2010 à 02:34 -0800, Simon Kirby a écrit :
> > Hello!
> > 
> > We upgraded some LVS (DR) servers from 2.6.35 to 2.6.36 on tg3 cards
> > (partno(BCM95721) rev 4201) with VLAN tags in use, to think that
> > everything looked great, but in fact...
> > 
> > LVS was receiving magically-merged TCP packets which it tried to forward
> > on to the real server, only to get annoyed at itself for trying to
> > forward a packet bigger than the device MTU:
> > 
> > IP A.47376 > B.529: . 175488:176936(1448) ack 1 win 92 <nop,nop,timestamp 52737308 29656081>
> > IP A.47376 > B.529: . 176936:179832(2896) ack 1 win 92 <nop,nop,timestamp 52737325 29656098>
> > IP B > A: ICMP B unreachable - need to frag (mtu 1500), length 556
> > 
> 
> Hi Simon
> 
> This is a tcpdump on A ?
> Could you take it also on B ?
> 
> tcpdump displays large buffers, but they should be split (of course)
> when sent on wire.
> 
> > This caused packet loss for any merged frames, which caused abysmal
> > performance for uploads via the LVS server.  Local performance to or
> > from the box is still fine, because the stack doesn't care, only the
> > forwarding part of LVS is running into the problem.
> > 
> > Furthermore, disabling _everything_ reported by ethtool -k doesn't seem
> > to change the result, even if I down/up the interface after, and even if
> > I try on every single interface including the VLANned ones.  This seems
> > to be another bug.  Reverting to 2.6.35 makes it all work again.
> > 
> > Possibly related to commit 7fe876af921d1d2bc8353e0062c10ff35e902653
> > 
> > So how should this be fixed?  Should LVS be taught to fragment, or must
> > we disable the merging in this case?  It seems like it would work well if
> > the sending side could do the same offload in reverse, but I'm not sure
> > if that would be possible.
> > 
> > Simon-
> 
> 
> I believe Simon Horman has some patches for GRO and LVS.
> 
> Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> 
> For TSO, I am not sure why and where it could matter...

There is a patch to teach LVS how to cope with GRO in nf-next-2.6
and I expect it to be included in 2.6.38. The patch is "ipvs: allow
transmit of GRO aggregated skbs" and perhaps it should be considered
for 2.6.37 and stable. In general the work around is to disable GRO.

The patch does not resolve the incompatibility of LVS with LRO.
The work around there is to disable LRO. I'm not entirely sure
how to teach LVS to disable LRO automatically, or if its desirable.

Simon, you mention that you disabled everything with ethtool, but the
tcpdump above shows a 2896 byte packet, which seems that GRO (or LRO?) is
active. So perhaps as you speculate that is a bug

I will prepare a backport of the "ipvs: allow transmit of GRO aggregated
skbs" patch to v2.6.36 and post it shortly.  Testing to see if that
resolves the problem that you are seeing would probably be a good start.


^ permalink raw reply

* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Eric Dumazet @ 2010-12-03 11:29 UTC (permalink / raw)
  To: Simon Kirby; +Cc: netdev, Simon Horman
In-Reply-To: <20101203103447.GA29714@hostway.ca>

Le vendredi 03 décembre 2010 à 02:34 -0800, Simon Kirby a écrit :
> Hello!
> 
> We upgraded some LVS (DR) servers from 2.6.35 to 2.6.36 on tg3 cards
> (partno(BCM95721) rev 4201) with VLAN tags in use, to think that
> everything looked great, but in fact...
> 
> LVS was receiving magically-merged TCP packets which it tried to forward
> on to the real server, only to get annoyed at itself for trying to
> forward a packet bigger than the device MTU:
> 
> IP A.47376 > B.529: . 175488:176936(1448) ack 1 win 92 <nop,nop,timestamp 52737308 29656081>
> IP A.47376 > B.529: . 176936:179832(2896) ack 1 win 92 <nop,nop,timestamp 52737325 29656098>
> IP B > A: ICMP B unreachable - need to frag (mtu 1500), length 556
> 

Hi Simon

This is a tcpdump on A ?
Could you take it also on B ?

tcpdump displays large buffers, but they should be split (of course)
when sent on wire.

> This caused packet loss for any merged frames, which caused abysmal
> performance for uploads via the LVS server.  Local performance to or
> from the box is still fine, because the stack doesn't care, only the
> forwarding part of LVS is running into the problem.
> 
> Furthermore, disabling _everything_ reported by ethtool -k doesn't seem
> to change the result, even if I down/up the interface after, and even if
> I try on every single interface including the VLANned ones.  This seems
> to be another bug.  Reverting to 2.6.35 makes it all work again.
> 
> Possibly related to commit 7fe876af921d1d2bc8353e0062c10ff35e902653
> 
> So how should this be fixed?  Should LVS be taught to fragment, or must
> we disable the merging in this case?  It seems like it would work well if
> the sending side could do the same offload in reverse, but I'm not sure
> if that would be possible.
> 
> Simon-


I believe Simon Horman has some patches for GRO and LVS.

Please send the results of "ethtool -k eth0" on all your nics / vlans ?

For TSO, I am not sure why and where it could matter...




^ permalink raw reply

* Re: [RFC PATCH v1] iproute2: add IFLA_TC support to 'ip link'
From: jamal @ 2010-12-03 11:06 UTC (permalink / raw)
  To: John Fastabend
  Cc: shemminger@vyatta.com, netdev@vger.kernel.org,
	tgraf@infradead.org, eric.dumazet@gmail.com, davem@davemloft.net
In-Reply-To: <4CF7F8B4.4060807@intel.com>

On Thu, 2010-12-02 at 11:51 -0800, John Fastabend wrote:
> On 12/2/2010 2:40 AM, jamal wrote:


> I viewed the HW QOS as L2 link attributes more than a queuing discipline per se.
> Plus 'ip link' is already used to set things outside of ip. 
> For example 'txqueuelen' and 'vf x'.

the vf one maybe borderline-ok txquelen is probably inherited from
ifconfig (and not sure a single queue a scheduler qualifies)


> However thinking about this a bit more qdisc support seems cleaner. 
> For one we can configure QOS policies per class with Qdisc_class_ops. 
> And then also aggregate statistics with dump_stats. I would avoid the 
> "hardware-kinda-8021q-sched" name though to account for schedulers that 
> may not be 802.1Q compliant maybe 'mclass-sched' for multi-class scheduler. 

Typically the scheduler would be a very familiar one implemented
per-spec by many vendors and will have a name acceptable by all.
So pick an appropriate noun so the user expectation matches it.

> I'll look into this. Thanks for the suggestion!

> 
> On egress the skb priority is mapped to a class which is associated with a
> range of queues (qoffset:qoffset + qcount). 
> In the 802.1Q case this queue range is mapped to the 802.1Qp 
> traffic class in hardware. So the hardware traffic class is mapped 1-1 
> with the software class. Additionally in software the VLAN egress mapping
> is used to map the skb priority to the 802.1Q priority. Here I expect user
> policies to configure this to get a consistent mapping. On ingress the 
> skb priority is set using the 802.1Q ingress mapping. This case is 
> something a userspace policy could configure if egress/ingress mappings
> should be symmetric.
> 

Sounds sensible. 

> In the simpler case of hardware rate limiting (not 802.1Q) this is not
> really a concern at all. With this mechanism we can identify traffic 
> and push it to the correct queues that are grouped into a rate limited class.

Ok, so you can do rate control as well?

> If there are egress/ingress mappings then those will apply skb priority tags 
> on egress and the correct skb priority on ingress.

Curious how you would do this in a rate controlled environment. EX: on
egress, do you use whatever skb prio you get to map to a specific rate
queue in h/ware? Note: skb prio has a strict priority scheduling
semantics so a 1-1 mapping doesnt sound reasonable..

> Currently everything works reasonably well with this scheme and the mq qdisc.
>  The mq qdisc uses pfifo and the driver then pauses the queues as needed. 
> Using the enhanced transmission selection algorithm (ETS - 802.1Qaz pre-standard)
>  in hardware we see variations from expected bandwidth around +-5% with TCP/UDP. 
> Instrumenting HW rate limiters gives similar variations. I tested this is with 
> ixgbe and the 82599 device.
> 
> Bit long winded but hopefully that answers your question.

I am curious about the rate based scheme - and i hope you are looking at
a different qdisc for that?

cheers,
jamal


^ permalink raw reply

* TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Kirby @ 2010-12-03 10:34 UTC (permalink / raw)
  To: Eric Dumazet, netdev

Hello!

We upgraded some LVS (DR) servers from 2.6.35 to 2.6.36 on tg3 cards
(partno(BCM95721) rev 4201) with VLAN tags in use, to think that
everything looked great, but in fact...

LVS was receiving magically-merged TCP packets which it tried to forward
on to the real server, only to get annoyed at itself for trying to
forward a packet bigger than the device MTU:

IP A.47376 > B.529: . 175488:176936(1448) ack 1 win 92 <nop,nop,timestamp 52737308 29656081>
IP A.47376 > B.529: . 176936:179832(2896) ack 1 win 92 <nop,nop,timestamp 52737325 29656098>
IP B > A: ICMP B unreachable - need to frag (mtu 1500), length 556

This caused packet loss for any merged frames, which caused abysmal
performance for uploads via the LVS server.  Local performance to or
from the box is still fine, because the stack doesn't care, only the
forwarding part of LVS is running into the problem.

Furthermore, disabling _everything_ reported by ethtool -k doesn't seem
to change the result, even if I down/up the interface after, and even if
I try on every single interface including the VLANned ones.  This seems
to be another bug.  Reverting to 2.6.35 makes it all work again.

Possibly related to commit 7fe876af921d1d2bc8353e0062c10ff35e902653

So how should this be fixed?  Should LVS be taught to fragment, or must
we disable the merging in this case?  It seems like it would work well if
the sending side could do the same offload in reverse, but I'm not sure
if that would be possible.

Simon-

^ permalink raw reply

* Re: Question about __alloc_skb() speedup
From: Eric Dumazet @ 2010-12-03 10:50 UTC (permalink / raw)
  To: Junchang Wang; +Cc: netdev
In-Reply-To: <20101203101450.GA9573@Desktop-Junchang>

Le vendredi 03 décembre 2010 à 18:14 +0800, Junchang Wang a écrit :
> Hi Eric,
> 
> I'm reading your patch (ec7d2f2cf3a1 __alloc_skb() speedup),
> in which you prefetch skb and the shinfo part. I'm very
> curious why we don't prefetch skb->data. It seems that will
> help tx path a lot.
> 
> I added the following code
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 104f844..c60a808 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -222,6 +222,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>  
>  		child->fclone = SKB_FCLONE_UNAVAILABLE;
>  	}
> +	prefetchw(data);
> +
>  out:
>  	return skb;
>  nodata:
> 
> and the pktgen in my server (A Intel SR1625 server with two E5530 
> 4-core processors and a single ixgbe-based NIC) goes from 7.6Mpps to
> 8.4Mpps (64 byte), with 10% performance gain.
> 
> For rx path, I did experiments on both ixgbe and igb with pktgen+kute,
> and there is no change in system performance.
> 
> welcome any suggestions and corrections.
> 
> Thanks.

This is because __alloc_skb() is generic :

We dont know if the skb->data is going to be used right after or not at
all.

For example, NIC drivers call __alloc_skb() to refill their RX ring
buffer. There is no gain to prefetch data in this case since the data is
going to be written by the NIC hardware. The reverse would be needed
actually : ask to local cpu to evict data from its cache, so that device
can DMA it faster (less bus transactions)

By the way, adding prefetchw() right before the "return skb;" is
probably not very useful. You can certainly try to add the prefetchw()
in pktgen itself, since you know for sure you are going to write the
data.

I dont understand your 10% speedup because pktgen actually uses
__netdev_alloc_skb(), so it calls skb_reserve(skb, NET_SKB_PAD) : your
prefetchw is bringing a cache line that wont be used at all by pktgen.

I would say 10% sounds highly suspect to me...




^ permalink raw reply

* Re: TCP: big bursts due to undos resulting from reordering
From: Lennart Schulte @ 2010-12-03 10:47 UTC (permalink / raw)
  To: John Heffner; +Cc: netdev@vger.kernel.org, Ilpo Järvinen
In-Reply-To: <AANLkTi=ptmSAAjiwmFyYx3QmcSXz2aZ7vn_Vf8+cLKW3@mail.gmail.com>

Hi John,
Thanks for your answer!

On 02.12.2010 18:55, John Heffner wrote:
> The problem that occurred, prior to this patch, was that cwnd was not
> able to effectively grow under persistent reordering.

Could you explain that deeper? What was the exact situation?
Do you have an example of when the smaller max_burst keep the cwnd down?
At the moment I can't think of a case.

Thanks,
Lennart

> On Thu, Dec 2, 2010 at 10:36 AM, Lennart Schulte
> <lennart.schulte@nets.rwth-aachen.de> wrote:
>> Hi John, hi Ilpo,
>>
>> at the moment I look on many TCP plots with reordering. When reordering
>> occurs there are some spurious retransmissions which are later undone by
>> e.g. DSACKs. This undo results in a very big burst of packets when
>> tp->reordering is high, since the function tcp_max_burst() returns
>> tp->reordering.
>>
>> This behavior was introduced because of a bug when using SACK instead of
>> Reno. The thread concerning this fix can be found at [1].
>>
>> Before the patch, which results from this thread, Linux has done a burst
>> of 3 packets and then slow started to the undone ssthresh value, which
>> is a much better way of handling an undo then it is after the patch.
>>
>> Also I patched a kernel to use the old max_burst value of 3 again to see
>> if it has any effect. Then I set up some virtual nodes and emulated a
>> network with netem as it was done in the thread.
>> The settings are:
>> - RTT 40ms
>> - no congestion, application sending rate 20 Mbps
>> - forward path: reordering rate 20%, reordering delay 20ms
>> - timestamps on
>>
>> Until now I have not found any evidence that the problem occurs (perhaps
>> because I don't get the settings right, since in the thread there is no
>> information concerning the settings for reordering and also the ones of
>> the sysctls).
>>
>> My problem is to understand why the patch was necessary and under what
>> circumstances SACK has a lower throughput so that it may be possible for
>> me to find another way of fixing this without introducing and old bug.
>> Since I can't figure it out on my own I hope to get some insights this
>> way :)
>>
>> Thanks,
>> Lennart Schulte
>>
>> [1] http://marc.info/?t=120728958000004&r=2&w=2

^ permalink raw reply

* Question about __alloc_skb() speedup
From: Junchang Wang @ 2010-12-03 10:14 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev

Hi Eric,

I'm reading your patch (ec7d2f2cf3a1 __alloc_skb() speedup),
in which you prefetch skb and the shinfo part. I'm very
curious why we don't prefetch skb->data. It seems that will
help tx path a lot.

I added the following code

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 104f844..c60a808 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -222,6 +222,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 
 		child->fclone = SKB_FCLONE_UNAVAILABLE;
 	}
+	prefetchw(data);
+
 out:
 	return skb;
 nodata:

and the pktgen in my server (A Intel SR1625 server with two E5530 
4-core processors and a single ixgbe-based NIC) goes from 7.6Mpps to
8.4Mpps (64 byte), with 10% performance gain.

For rx path, I did experiments on both ixgbe and igb with pktgen+kute,
and there is no change in system performance.

welcome any suggestions and corrections.

Thanks.

--Junchang

^ permalink raw reply related

* Re: multi bpf filter will impact performance?
From: Junchang Wang @ 2010-12-03  9:40 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Rui, netdev, Fenghua Yu
In-Reply-To: <1291176215.2856.383.camel@edumazet-laptop>

On Wed, Dec 1, 2010 at 12:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> But its rather important for performance that each cpu store packets
> into its own packet socket or ring buffer, to avoid false sharing
> slowdowns.

Hi Eric,
But the current situation is that a single socket (AF_PACKET for example)
can only create a ring buffer, meaning that multiple cores will compete
for passing data to user-space programs. Right?

>
> With such a setup (split packets to four cpus, then make sure one cpu
> deliver packets to one particular PACKET socket/ring buffer), it should
> really be fast enough.
>

Is there any patch or configuration? I'm preparing such a patch. I wonder
whether there's conflict.


Thanks.

--Junchang

^ permalink raw reply

* Re: [PATCH 3/5] xfrm: Traffic Flow Confidentiality for IPv4 ESP
From: Herbert Xu @ 2010-12-03  8:39 UTC (permalink / raw)
  To: Martin Willi; +Cc: linux-crypto, netdev
In-Reply-To: <1291365175.1997.34.camel@martin>

On Fri, Dec 03, 2010 at 09:32:55AM +0100, Martin Willi wrote:
> 
> > What is the basis of this random length padding?
> 
> Let assume a peer does not support ESPv3 padding, but we have to pad a
> small packet with more than 255 bytes. We can't, the ESP padding length
> field is limited to 255.
> We could add 255 fixed bytes, but an eavesdropper could just subtract
> the 255 bytes from all packets smaller than the boundary, rendering our
> TFC efforts useless.
> By inserting a random length padding in the range possible, the
> eavesdropper knows that the packet has a length between "length" and
> "length - 255", but can't estimated its exact size. I'm aware that this
> is not optimal, but probably the best we can do(?).

I know why you want to do this, what I'm asking is do you have any
research behind this with regards to security (e.g., you're using an
insecure RNG to generate a value that is then used as the basis
for concealment)?

Has this scheme been discussed on a public forum somewhere?

> > I know that your last patch allows the padto to be set by PMTU.
> > But why would we ever want to use a padto that isn't clamped by
> > PMTU?
> 
> Probably never, valid point.
> 
> I'll add PMTU clamping to the next revision. We probably can drop the
> PMTU flag then and just use USHRT_MAX instead. 

Sounds good.

Thanks,
-- 
Email: Herbert Xu <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


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