Skip to contentSkip to conversation

ibmveth: Remove duplicate checksum offload setup code

PATCH In review Robert Jennings 2 participants 4 messages Aug 18, 2010 – Aug 20, 2010

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Robert Jennings

author
#0 lore raw

PATCH 1/3 ibmveth: Remove duplicate checksum offload setup code

Remove code in the device probe function where we set up the checksum offload feature and replace it with a call to an existing function that is doing the same. This is done to clean up the driver in preparation of adding IPv6 checksum offload support.

Signed-off-by Robert Jennings <[email protected]>
---
 drivers/net/ibmveth.c |   18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)
drivers/net/ibmveth.c +1 -17
Index: b/drivers/net/ibmveth.c
===================================================================
@@ -1222,10 +1222,8 @@ static const struct net_device_ops ibmve
 static int __devinit ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 {
 	int rc, i;
-	long ret;
 	struct net_device *netdev;
 	struct ibmveth_adapter *adapter;
-	unsigned long set_attr, ret_attr;
 
 	unsigned char *mac_addr_p;
 	unsigned int *mcastFilterSize_p;

@@ -1310,21 +1308,7 @@ static int __devinit ibmveth_probe(struc
 
 	ibmveth_debug_printk("registering netdev...\n");
 
-	ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr);
-
-	if (ret == H_SUCCESS && !(ret_attr & IBMVETH_ILLAN_ACTIVE_TRUNK) &&
-	    !(ret_attr & IBMVETH_ILLAN_TRUNK_PRI_MASK) &&
-	    (ret_attr & IBMVETH_ILLAN_PADDED_PKT_CSUM)) {
-		set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
-
-		ret = h_illan_attributes(dev->unit_address, 0, set_attr, &ret_attr);
-
-		if (ret == H_SUCCESS) {
-			adapter->rx_csum = 1;
-			netdev->features |= NETIF_F_IP_CSUM;
-		} else
-			ret = h_illan_attributes(dev->unit_address, set_attr, 0, &ret_attr);
-	}
+	ibmveth_set_csum_offload(netdev, 1, ibmveth_set_tx_csum_flags);
 
 	rc = register_netdev(netdev);
author
#1 lore raw

PATCH 2/3 ibmveth: Enable IPv6 checksum offload

This patch enables TCP checksum offload support for IPv6 on ibmveth. This completely eliminates the generation and checking of the checksum for IPv6 packets that are completely virtual and never touch a physical network. A basic TCPIPV6_STREAM netperf run showed a ~30% throughput improvement when an MTU of 64000 was used.

This featured is enabled by default, as is the case for IPv4 checksum offload. When checksum offload is enabled the driver will negotiate IPv4 and IPv6 offload with the firmware separately and enable what is available. As long as either IPv4 or IPv6 offload is supported and enabled the device will report that checksum offload is enabled. The device stats, available through ethtool, will display which checksum offload features are supported/enabled by firmware.

Performance testing against a stock kernel shows no regression for IPv4 or IPv6 in terms of throughput or processor utilization with checksum disabled or enabled.

Signed-off-by Robert Jennings <[email protected]>
---
 drivers/net/ibmveth.c |   59 +++++++++++++++++++++++++++++++++++++++++---------
 drivers/net/ibmveth.h |    2 ++
 2 files changed, 50 insertions(+), 11 deletions(-)
drivers/net/ibmveth.c +48 -11
Index: b/drivers/net/ibmveth.c
===================================================================
@@ -50,6 +50,7 @@
 #include <linux/proc_fs.h>
 #include <linux/in.h>
 #include <linux/ip.h>
+#include <linux/ipv6.h>
 #include <linux/slab.h>
 #include <net/net_namespace.h>
 #include <asm/hvcall.h>

@@ -134,6 +135,8 @@ struct ibmveth_stat ibmveth_stats[] = {
 	{ "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
 	{ "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
 	{ "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) },
+	{ "fw_enabled_ipv4_csum", IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
+	{ "fw_enabled_ipv6_csum", IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
 };
 
 /* simple methods of getting data from the current rxq entry */

@@ -739,6 +742,7 @@ static void ibmveth_set_rx_csum_flags(st
 		 */
 		adapter->rx_csum = 0;
 		dev->features &= ~NETIF_F_IP_CSUM;
+		dev->features &= ~NETIF_F_IPV6_CSUM;
 	}
 }
 

@@ -747,10 +751,15 @@ static void ibmveth_set_tx_csum_flags(st
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 
 	if (data) {
-		dev->features |= NETIF_F_IP_CSUM;
+		if (adapter->fw_ipv4_csum_support)
+			dev->features |= NETIF_F_IP_CSUM;
+		if (adapter->fw_ipv6_csum_support)
+			dev->features |= NETIF_F_IPV6_CSUM;
 		adapter->rx_csum = 1;
-	} else
+	} else {
 		dev->features &= ~NETIF_F_IP_CSUM;
+		dev->features &= ~NETIF_F_IPV6_CSUM;
+	}
 }
 
 static int ibmveth_set_csum_offload(struct net_device *dev, u32 data,

@@ -758,7 +767,8 @@ static int ibmveth_set_csum_offload(stru
 {
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 	unsigned long set_attr, clr_attr, ret_attr;
-	long ret;
+	unsigned long set_attr6, clr_attr6;
+	long ret, ret6;
 	int rc1 = 0, rc2 = 0;
 	int restart = 0;
 

@@ -772,10 +782,13 @@ static int ibmveth_set_csum_offload(stru
 	set_attr = 0;
 	clr_attr = 0;
 
-	if (data)
+	if (data) {
 		set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
-	else
+		set_attr6 = IBMVETH_ILLAN_IPV6_TCP_CSUM;
+	} else {
 		clr_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
+		clr_attr6 = IBMVETH_ILLAN_IPV6_TCP_CSUM;
+	}
 
 	ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr);
 

@@ -786,14 +799,33 @@ static int ibmveth_set_csum_offload(stru
 					 set_attr, &ret_attr);
 
 		if (ret != H_SUCCESS) {
-			rc1 = -EIO;
-			ibmveth_error_printk("unable to change checksum offload settings."
-					     " %d rc=%ld\n", data, ret);
+			ibmveth_error_printk("unable to change IPv4 checksum "
+					     "offload settings. %d rc=%ld\n",
+					     data, ret);
 
 			ret = h_illan_attributes(adapter->vdev->unit_address,
 						 set_attr, clr_attr, &ret_attr);
 		} else
+			adapter->fw_ipv4_csum_support = data;
+
+		ret6 = h_illan_attributes(adapter->vdev->unit_address,
+					 clr_attr6, set_attr6, &ret_attr);
+
+		if (ret6 != H_SUCCESS) {
+			ibmveth_error_printk("unable to change IPv6 checksum "
+					     "offload settings. %d rc=%ld\n",
+					     data, ret);
+
+			ret = h_illan_attributes(adapter->vdev->unit_address,
+						 set_attr6, clr_attr6,
+						 &ret_attr);
+		} else
+			adapter->fw_ipv6_csum_support = data;
+
+		if (ret == H_SUCCESS || ret6 == H_SUCCESS)
 			done(dev, data);
+		else
+			rc1 = -EIO;
 	} else {
 		rc1 = -EIO;
 		ibmveth_error_printk("unable to change checksum offload settings."

@@ -821,9 +853,9 @@ static int ibmveth_set_tx_csum(struct ne
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 	int rc = 0;
 
-	if (data && (dev->features & NETIF_F_IP_CSUM))
+	if (data && (dev->features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		return 0;
-	if (!data && !(dev->features & NETIF_F_IP_CSUM))
+	if (!data && !(dev->features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		return 0;
 
 	if (data && !adapter->rx_csum)

@@ -910,7 +942,12 @@ static netdev_tx_t ibmveth_start_xmit(st
 	desc.fields.flags_len = IBMVETH_BUF_VALID | skb->len;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL &&
-	    ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
+	    ((skb->protocol == htons(ETH_P_IP) &&
+	      ip_hdr(skb)->protocol != IPPROTO_TCP) ||
+	     (skb->protocol == htons(ETH_P_IPV6) &&
+	      ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)) &&
+	    skb_checksum_help(skb)) {
+
 		ibmveth_error_printk("tx: failed to checksum packet\n");
 		tx_dropped++;
 		goto out;
drivers/net/ibmveth.h +2 -0
Index: b/drivers/net/ibmveth.h
===================================================================
@@ -149,6 +149,8 @@ struct ibmveth_adapter {
     void *bounce_buffer;
     dma_addr_t bounce_buffer_dma;
 
+    u64 fw_ipv6_csum_support;
+    u64 fw_ipv4_csum_support;
     /* adapter specific stats */
     u64 replenish_task_cycles;
     u64 replenish_no_mem;
author
#2 lore raw

PATCH 3/3 ibmveth: Driver version 1.04

In reply to Robert Jennings

Bump driver version.

Signed-off-by Robert Jennings <[email protected]>
---
 drivers/net/ibmveth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ibmveth.c +1 -1
Index: b/drivers/net/ibmveth.c
===================================================================
@@ -111,7 +111,7 @@ static struct proc_dir_entry *ibmveth_pr
 
 static const char ibmveth_driver_name[] = "ibmveth";
 static const char ibmveth_driver_string[] = "IBM i/pSeries Virtual Ethernet Driver";
-#define ibmveth_driver_version "1.03"
+#define ibmveth_driver_version "1.04"
 
 MODULE_AUTHOR("Santiago Leon <[email protected]>");
 MODULE_DESCRIPTION("IBM i/pSeries Virtual Ethernet Driver");
Acked-by on v1 1/3
#3 lore raw
In reply to Robert Jennings

Patches 1-3

Acked-by Brian King <[email protected]>
Quoted 53 lines · Robert Jennings

Remove code in the device probe function where we set up the checksum offload feature and replace it with a call to an existing function that is doing the same. This is done to clean up the driver in preparation of adding IPv6 checksum offload support.

Signed-off-by Robert Jennings <[email protected]>
---
 drivers/net/ibmveth.c |   18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

Index: b/drivers/net/ibmveth.c
===================================================================

drivers/net/ibmveth.c +1 -18
@@ -1222,10 +1222,8 @@ static const struct net_device_ops ibmve
 static int __devinit ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 {
 	int rc, i;
-	long ret;
 	struct net_device *netdev;
 	struct ibmveth_adapter *adapter;
-	unsigned long set_attr, ret_attr;
 
 	unsigned char *mac_addr_p;
 	unsigned int *mcastFilterSize_p;

@@ -1310,21 +1308,7 @@ static int __devinit ibmveth_probe(struc
 
 	ibmveth_debug_printk("registering netdev...\n");
 
-	ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr);
-
-	if (ret == H_SUCCESS && !(ret_attr & IBMVETH_ILLAN_ACTIVE_TRUNK) &&
-	    !(ret_attr & IBMVETH_ILLAN_TRUNK_PRI_MASK) &&
-	    (ret_attr & IBMVETH_ILLAN_PADDED_PKT_CSUM)) {
-		set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
-
-		ret = h_illan_attributes(dev->unit_address, 0, set_attr, &ret_attr);
-
-		if (ret == H_SUCCESS) {
-			adapter->rx_csum = 1;
-			netdev->features |= NETIF_F_IP_CSUM;
-		} else
-			ret = h_illan_attributes(dev->unit_address, set_attr, 0, &ret_attr);
-	}
+	ibmveth_set_csum_offload(netdev, 1, ibmveth_set_tx_csum_flags);
 
 	rc = register_netdev(netdev);
 
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 3 lines
Brian King
Linux on Power Virtualization
IBM Linux Technology Center