[net-next-2.6 PATCH 1/5] ixgbe: dcb, set DPF bit when PFC is enabled

[net-next-2.6 PATCH 1/5] ixgbe: dcb, set DPF bit when PFC is enabled

[Jeff Kirsher]

From: John Fastabend <john.r.fastabend@intel.com>

Set the DPF bit when PFC is enabled.  This will discard
PFC frames so they do not get passed up the stack.

The DPF bit is set for flow control, but not priority
flow control this brings pfc inline with fc.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_dcb_82599.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_dcb_82599.c b/drivers/net/ixgbe/ixgbe_dcb_82599.c
index 4f7a26a..25b02fb 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82599.c
+++ b/drivers/net/ixgbe/ixgbe_dcb_82599.c
@@ -346,7 +346,7 @@ s32 ixgbe_dcb_config_pfc_82599(struct ixgbe_hw *hw,
 	 */
 	reg = IXGBE_READ_REG(hw, IXGBE_MFLCN);
 	reg &= ~IXGBE_MFLCN_RFCE;
-	reg |= IXGBE_MFLCN_RPFCE;
+	reg |= IXGBE_MFLCN_RPFCE | IXGBE_MFLCN_DPF;
 	IXGBE_WRITE_REG(hw, IXGBE_MFLCN, reg);
 out:
 	return 0;

[net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change removes UDP from the supported protocols for RSS hashing.  The
reason for removing this protocol is because IP fragmentation was causing a
network flow to be broken into two streams, one for fragmented, and one for
non-fragmented and this in turn was causing out-of-order issues.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index b235aa1..813d2cb 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -2800,10 +2800,8 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
 		    /* Perform hash on these packet types */
 		mrqc |= IXGBE_MRQC_RSS_FIELD_IPV4
 		      | IXGBE_MRQC_RSS_FIELD_IPV4_TCP
-		      | IXGBE_MRQC_RSS_FIELD_IPV4_UDP
 		      | IXGBE_MRQC_RSS_FIELD_IPV6
-		      | IXGBE_MRQC_RSS_FIELD_IPV6_TCP
-		      | IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
+		      | IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
 	}
 	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
 

[net-next-2.6 PATCH 3/5] ixgbe: properly toggling netdev feature flags when disabling FCoE

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

When FCoE is disabled, there is a race condition that FCoE offload is
turned off but the FCoE protocol driver is still queuing I/O thinking
offload support still exists. This patch toggles off corresponding FCoE
netdev feature flags and notify the FCoE stack first, allowing FCoE
protocol stack driver to update its flags upon NETDEV_FEAT_CHANGE so no
I/O will be using offload.

Also, indicate FCoE offload flags in vlan_features in ixgbe_probe once
and do not toggle them in ixgbe_fcoe_enable/disable so when FCoE is
created on the VLAN interface, vlan_transfer_features() would properly
update the VLAN netdev features flag and notify the FCoE protocol driver
for NETDEV_FEAT_CHANGE.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_fcoe.c |   19 ++++++-------------
 drivers/net/ixgbe/ixgbe_main.c |    5 +++++
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
index f6ef4cd..1737d2b 100644
--- a/drivers/net/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ixgbe/ixgbe_fcoe.c
@@ -622,9 +622,6 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
 	netdev->features |= NETIF_F_FCOE_CRC;
 	netdev->features |= NETIF_F_FSO;
 	netdev->features |= NETIF_F_FCOE_MTU;
-	netdev->vlan_features |= NETIF_F_FCOE_CRC;
-	netdev->vlan_features |= NETIF_F_FSO;
-	netdev->vlan_features |= NETIF_F_FCOE_MTU;
 	netdev->fcoe_ddp_xid = IXGBE_FCOE_DDP_MAX - 1;
 
 	ixgbe_init_interrupt_scheme(adapter);
@@ -658,24 +655,20 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
 		goto out_disable;
 
 	e_info(drv, "Disabling FCoE offload features.\n");
+	netdev->features &= ~NETIF_F_FCOE_CRC;
+	netdev->features &= ~NETIF_F_FSO;
+	netdev->features &= ~NETIF_F_FCOE_MTU;
+	netdev->fcoe_ddp_xid = 0;
+	netdev_features_change(netdev);
+
 	if (netif_running(netdev))
 		netdev->netdev_ops->ndo_stop(netdev);
 
 	ixgbe_clear_interrupt_scheme(adapter);
-
 	adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
 	adapter->ring_feature[RING_F_FCOE].indices = 0;
-	netdev->features &= ~NETIF_F_FCOE_CRC;
-	netdev->features &= ~NETIF_F_FSO;
-	netdev->features &= ~NETIF_F_FCOE_MTU;
-	netdev->vlan_features &= ~NETIF_F_FCOE_CRC;
-	netdev->vlan_features &= ~NETIF_F_FSO;
-	netdev->vlan_features &= ~NETIF_F_FCOE_MTU;
-	netdev->fcoe_ddp_xid = 0;
-
 	ixgbe_cleanup_fcoe(adapter);
 	ixgbe_init_interrupt_scheme(adapter);
-	netdev_features_change(netdev);
 
 	if (netif_running(netdev))
 		netdev->netdev_ops->ndo_open(netdev);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 813d2cb..7d619d6 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6740,6 +6740,11 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 				adapter->flags &= ~IXGBE_FLAG_FCOE_CAPABLE;
 		}
 	}
+	if (adapter->flags & IXGBE_FLAG_FCOE_CAPABLE) {
+		netdev->vlan_features |= NETIF_F_FCOE_CRC;
+		netdev->vlan_features |= NETIF_F_FSO;
+		netdev->vlan_features |= NETIF_F_FCOE_MTU;
+	}
 #endif /* IXGBE_FCOE */
 	if (pci_using_dac)
 		netdev->features |= NETIF_F_HIGHDMA;

[net-next-2.6 PATCH 4/5] ixgbe: use GFP_ATOMIC when allocating FCoE DDP context from the dma pool

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

The FCoE protocol stack may hold a lock when this gets called.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_fcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
index 1737d2b..072327c 100644
--- a/drivers/net/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ixgbe/ixgbe_fcoe.c
@@ -190,7 +190,7 @@ int ixgbe_fcoe_ddp_get(struct net_device *netdev, u16 xid,
 	}
 
 	/* alloc the udl from our ddp pool */
-	ddp->udl = pci_pool_alloc(fcoe->pool, GFP_KERNEL, &ddp->udp);
+	ddp->udl = pci_pool_alloc(fcoe->pool, GFP_ATOMIC, &ddp->udp);
 	if (!ddp->udl) {
 		e_err(drv, "failed allocated ddp context\n");
 		goto out_noddp_unmap;

[net-next-2.6 PATCH 5/5] ixgbe: fix version string for ixgbe

[Jeff Kirsher]

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

Bump the version string to better reflect what is in the driver.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 7d619d6..9203759 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -52,7 +52,7 @@ char ixgbe_driver_name[] = "ixgbe";
 static const char ixgbe_driver_string[] =
                               "Intel(R) 10 Gigabit PCI Express Network Driver";
 
-#define DRV_VERSION "2.0.62-k2"
+#define DRV_VERSION "2.0.84-k2"
 const char ixgbe_driver_version[] = DRV_VERSION;
 static char ixgbe_copyright[] = "Copyright (c) 1999-2010 Intel Corporation.";
 

Re: [net-next-2.6 PATCH 1/5] ixgbe: dcb, set DPF bit when PFC is enabled

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 16:59:03 -0700

> From: John Fastabend <john.r.fastabend@intel.com>
> 
> Set the DPF bit when PFC is enabled.  This will discard
> PFC frames so they do not get passed up the stack.
> 
> The DPF bit is set for flow control, but not priority
> flow control this brings pfc inline with fc.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 16:59:27 -0700

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change removes UDP from the supported protocols for RSS hashing.  The
> reason for removing this protocol is because IP fragmentation was causing a
> network flow to be broken into two streams, one for fragmented, and one for
> non-fragmented and this in turn was causing out-of-order issues.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Acked-by: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-next-2.6 PATCH 3/5] ixgbe: properly toggling netdev feature flags when disabling FCoE

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 16:59:52 -0700

> From: Yi Zou <yi.zou@intel.com>
> 
> When FCoE is disabled, there is a race condition that FCoE offload is
> turned off but the FCoE protocol driver is still queuing I/O thinking
> offload support still exists. This patch toggles off corresponding FCoE
> netdev feature flags and notify the FCoE stack first, allowing FCoE
> protocol stack driver to update its flags upon NETDEV_FEAT_CHANGE so no
> I/O will be using offload.
> 
> Also, indicate FCoE offload flags in vlan_features in ixgbe_probe once
> and do not toggle them in ixgbe_fcoe_enable/disable so when FCoE is
> created on the VLAN interface, vlan_transfer_features() would properly
> update the VLAN netdev features flag and notify the FCoE protocol driver
> for NETDEV_FEAT_CHANGE.
> 
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-next-2.6 PATCH 4/5] ixgbe: use GFP_ATOMIC when allocating FCoE DDP context from the dma pool

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 17:00:24 -0700

> From: Yi Zou <yi.zou@intel.com>
> 
> The FCoE protocol stack may hold a lock when this gets called.
> 
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-next-2.6 PATCH 5/5] ixgbe: fix version string for ixgbe

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 17:00:47 -0700

> From: Don Skidmore <donald.c.skidmore@intel.com>
> 
> Bump the version string to better reflect what is in the driver.
> 
> Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Bill Fink]

On Mon, 19 Jul 2010, David Miller wrote:

> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon, 19 Jul 2010 16:59:27 -0700
> 
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > 
> > This change removes UDP from the supported protocols for RSS hashing.  The
> > reason for removing this protocol is because IP fragmentation was causing a
> > network flow to be broken into two streams, one for fragmented, and one for
> > non-fragmented and this in turn was causing out-of-order issues.
> > 
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > Acked-by: Don Skidmore <donald.c.skidmore@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> Applied.

Should there be a /proc or ethtool setting for whether or not to
use RSS hashing for UDP flows?  I would think that for many common
UDP applications, IP fragmentation would not be an issue because
they often tend to use sub-MTU sized datagrams.  And of course
UDP does not guarantee in-order delivery in any event.  Then a
remaining issue is what the default setting of such an option
should be.  I would lean to having it enabled by default, but
I can also see the safety argument for having it off by default.

						-Bill

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Eric Dumazet]

Le mardi 20 juillet 2010 à 02:07 -0400, Bill Fink a écrit :
> On Mon, 19 Jul 2010, David Miller wrote:
> 

> Should there be a /proc or ethtool setting for whether or not to
> use RSS hashing for UDP flows?  I would think that for many common
> UDP applications, IP fragmentation would not be an issue because
> they often tend to use sub-MTU sized datagrams.  And of course
> UDP does not guarantee in-order delivery in any event.  Then a
> remaining issue is what the default setting of such an option
> should be.  I would lean to having it enabled by default, but
> I can also see the safety argument for having it off by default.
> 

Their are several issues here.

1) Ability for the NIC to spread UDP loads on several queues.

2) Ability for the NIC to provide the hash to our stack, to speedup a
bit RPS.


If the patch is about 1), ie disables NIC ability to split UDP flows on
several RX queues, then yes : its probably _not_ wanted.



Commit message is not very clear on this topic.

By nature, UDP flows are subject to out of order issues, so what is this
patch tries to avoid ?

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Eric Dumazet]

Le lundi 19 juillet 2010 à 16:59 -0700, Jeff Kirsher a écrit :
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change removes UDP from the supported protocols for RSS hashing.  The
> reason for removing this protocol is because IP fragmentation was causing a
> network flow to be broken into two streams, one for fragmented, and one for
> non-fragmented and this in turn was causing out-of-order issues.
> 

Jeff, does it mean all UDP packets are going to be delivered to a single
queue ?

This would be a serious regression.

Many UDP applications try hard to not use fragments. 

They are going to pay the price because some application :
- Use big segments, fragmented.
- Is subject to OOO artifacts.

We would like some clarifications please :)

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 20 Jul 2010 08:30:00 +0200

> By nature, UDP flows are subject to out of order issues, so what is this
> patch tries to avoid ?

UDP being subject to out-of-order issues really doesn't matter one
bit.

We should never, _knowingly_ create out-of-order packets in our
networking stack.  And this is regardless of protocol.

If there is no way to make ixgbe respect in-order packet delivery for
UDP frames vis-a-vis fragmented frames, we must disable RX flow
spreading for UDP.

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 20 Jul 2010 08:39:40 +0200

> This would be a serious regression.

The regression is in the hardware Eric.

> Many UDP applications try hard to not use fragments. 
> 
> They are going to pay the price because some application :
> - Use big segments, fragmented.
> - Is subject to OOO artifacts.

None of this matters.  If the hardware can't flow seperate properly
it's tough cookies.

We never may reorder packets in our stack by our own doing.  The
only safe default is to turn UDP flow seperation off on chips
like ixgbe.

If you want an ethtool knob to turn it back on for your machines,
fine.  But never can it be enabled by default.

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Alexander Duyck]

Eric Dumazet wrote:
> Le lundi 19 juillet 2010 à 16:59 -0700, Jeff Kirsher a écrit :
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This change removes UDP from the supported protocols for RSS hashing.  The
>> reason for removing this protocol is because IP fragmentation was causing a
>> network flow to be broken into two streams, one for fragmented, and one for
>> non-fragmented and this in turn was causing out-of-order issues.
>>
> 
> Jeff, does it mean all UDP packets are going to be delivered to a single
> queue ?
> 
> This would be a serious regression.
> 
> Many UDP applications try hard to not use fragments. 
> 
> They are going to pay the price because some application :
> - Use big segments, fragmented.
> - Is subject to OOO artifacts.
> 
> We would like some clarifications please :)
> 
> 
> 

The packets will still be hashed on source and destination IPv4/IPv6 
addresses.  The change just drops reading the UDP source/destination 
ports since in the case of fragmented packets they are not available and 
as such were being parsed as IPv4/IPv6 packets.  By making this change 
the queue selection is consistent between all packets in the UDP stream.

The only regression I would expect to see would be in testing between 
two fixed systems since the IP addresses of the two systems would be 
fixed and so running multiple flows between the two would yield the same 
  RSS hash for multiple UDP streams.  As long as multiple ip addresses 
are used  you should see multiple RSS hashes generated and as such the 
load should still be distributed.

Thanks,

Alex

Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation

[Eric Dumazet]

Le mardi 20 juillet 2010 à 09:11 -0700, Alexander Duyck a écrit :

> The packets will still be hashed on source and destination IPv4/IPv6 
> addresses.  The change just drops reading the UDP source/destination 
> ports since in the case of fragmented packets they are not available and 
> as such were being parsed as IPv4/IPv6 packets.  By making this change 
> the queue selection is consistent between all packets in the UDP stream.
> 

Excellent, this is perfect IMHO.

> The only regression I would expect to see would be in testing between 
> two fixed systems since the IP addresses of the two systems would be 
> fixed and so running multiple flows between the two would yield the same 
>   RSS hash for multiple UDP streams.  As long as multiple ip addresses 
> are used  you should see multiple RSS hashes generated and as such the 
> load should still be distributed.
> 

Ack. Fortunately, one can still use RPS to spread load onto multiple
cpus in this case.

This until ixgpe fills skb->rxhash with a non null value.
If it happens one day, we shall remind _not_ filling it for UDP packets.

BTW, this reminds me a netdev discussion we had for bnx2x

http://www.kerneltrap.com/mailarchive/linux-netdev/2010/4/23/6275415/thread

And now, I understand why Toepliz hash doesnt use src port/dst port,
since this is not available on fragments, obviously...

Thanks !