ixgbe: dcb, set DPF bit when PFC is enabled

From: John Fastabend <[email protected]>

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.

---

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

index 4f7a26a..25b02fb 100644
@@ -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;

From: Alexander Duyck <[email protected]>

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.

---

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

index b235aa1..813d2cb 100644
@@ -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);
 

From: Jeff Kirsher <[email protected]>
Date: Mon, 19 Jul 2010 16:59:27 -0700

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

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 ?

From: Eric Dumazet <[email protected]>
Date: Tue, 20 Jul 2010 08:30:00 +0200

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.

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 :)

From: Eric Dumazet <[email protected]>
Date: Tue, 20 Jul 2010 08:39:40 +0200

The regression is in the hardware Eric.

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.

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

Excellent, this is perfect IMHO.

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 !

From: Yi Zou <[email protected]>

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.

---

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

index f6ef4cd..1737d2b 100644
@@ -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);

index 813d2cb..7d619d6 100644
@@ -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;

From: Jeff Kirsher <[email protected]>
Date: Mon, 19 Jul 2010 16:59:52 -0700

Applied.

From: Yi Zou <[email protected]>

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

---

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

index 1737d2b..072327c 100644
@@ -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;

From: Jeff Kirsher <[email protected]>
Date: Mon, 19 Jul 2010 17:00:24 -0700

Applied.

From: Don Skidmore <[email protected]>

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

---

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

index 7d619d6..9203759 100644
@@ -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.";
 

From: Jeff Kirsher <[email protected]>
Date: Mon, 19 Jul 2010 17:00:47 -0700

Applied.

From: Jeff Kirsher <[email protected]>
Date: Mon, 19 Jul 2010 16:59:03 -0700

Applied.