[PATCH 1/4] ibmveth: Enable TCP checksum offload

[PATCH 1/4] ibmveth: Enable TCP checksum offload

[Brian King]

This patchset enables TCP checksum offload support for IPV4
on ibmveth. This completely eliminates the generation and checking of
the checksum for packets that are completely virtual and never
touch a physical network. A simple TCP_STREAM netperf run on
a virtual network with maximum mtu set yielded a ~30% increase
in throughput. This feature is enabled by default on systems that
support it, but can be disabled with a module option.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   58 ++++++++++++++++++++++++++++++++
 linux-2.6-bjking1/drivers/net/ibmveth.h |   41 +++++++++++++++++++++-
 2 files changed, 97 insertions(+), 2 deletions(-)

diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload	2007-07-12 08:27:47.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-12 09:35:55.000000000 -0500
@@ -47,6 +47,8 @@
 #include <linux/mm.h>
 #include <linux/ethtool.h>
 #include <linux/proc_fs.h>
+#include <linux/in.h>
+#include <linux/ip.h>
 #include <asm/semaphore.h>
 #include <asm/hvcall.h>
 #include <asm/atomic.h>
@@ -103,12 +105,15 @@ 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";
+static unsigned int ibmveth_csum_offload = 1;
 #define ibmveth_driver_version "1.03"
 
 MODULE_AUTHOR("Santiago Leon <santil@us.ibm.com>");
 MODULE_DESCRIPTION("IBM i/pSeries Virtual Ethernet Driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(ibmveth_driver_version);
+module_param_named(csum_offload, ibmveth_csum_offload, uint, 0);
+MODULE_PARM_DESC(csum_offload, "Checksum offload (0/1). Default: 1");
 
 /* simple methods of getting data from the current rxq entry */
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
@@ -131,6 +136,11 @@ static inline int ibmveth_rxq_frame_leng
 	return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].length);
 }
 
+static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
+{
+	return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].csum_good);
+}
+
 /* setup the initial settings for a buffer pool */
 static void ibmveth_init_buffer_pool(struct ibmveth_buff_pool *pool, u32 pool_index, u32 pool_size, u32 buff_size, u32 pool_active)
 {
@@ -684,6 +694,24 @@ static int ibmveth_start_xmit(struct sk_
 					desc[0].fields.length, DMA_TO_DEVICE);
 	desc[0].fields.valid   = 1;
 
+	if (skb->ip_summed == CHECKSUM_PARTIAL &&
+	    ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
+		ibmveth_error_printk("tx: failed to checksum packet\n");
+		tx_dropped++;
+		goto out;
+	}
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		unsigned char *buf = skb_transport_header(skb) + skb->csum_offset;
+
+		desc[0].fields.no_csum = 1;
+		desc[0].fields.csum_good = 1;
+
+		/* Need to zero out the checksum */
+		buf[0] = 0;
+		buf[1] = 0;
+	}
+
 	if(dma_mapping_error(desc[0].fields.address)) {
 		ibmveth_error_printk("tx: unable to map initial fragment\n");
 		tx_map_failed++;
@@ -702,6 +730,10 @@ static int ibmveth_start_xmit(struct sk_
 				frag->size, DMA_TO_DEVICE);
 		desc[curfrag+1].fields.length = frag->size;
 		desc[curfrag+1].fields.valid  = 1;
+		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			desc[curfrag+1].fields.no_csum = 1;
+			desc[curfrag+1].fields.csum_good = 1;
+		}
 
 		if(dma_mapping_error(desc[curfrag+1].fields.address)) {
 			ibmveth_error_printk("tx: unable to map fragment %d\n", curfrag);
@@ -792,7 +824,11 @@ static int ibmveth_poll(struct net_devic
 			} else {
 				int length = ibmveth_rxq_frame_length(adapter);
 				int offset = ibmveth_rxq_frame_offset(adapter);
+				int csum_good = ibmveth_rxq_csum_good(adapter);
+
 				skb = ibmveth_rxq_get_buffer(adapter);
+				if (csum_good)
+					skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 				ibmveth_rxq_harvest_buffer(adapter);
 
@@ -962,8 +998,10 @@ static void ibmveth_poll_controller(stru
 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 = NULL;
+	union ibmveth_illan_attributes set_attr, ret_attr;
 
 	unsigned char *mac_addr_p;
 	unsigned int *mcastFilterSize_p;
@@ -1058,6 +1096,26 @@ static int __devinit ibmveth_probe(struc
 
 	ibmveth_debug_printk("registering netdev...\n");
 
+	if (ibmveth_csum_offload) {
+		ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr.desc);
+
+		if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
+		    !ret_attr.fields.trunk_priority &&
+		    ret_attr.fields.csum_offload_padded_pkt_support) {
+			set_attr.desc = 0;
+			set_attr.fields.tcp_csum_offload_ipv4 = 1;
+
+			ret = h_illan_attributes(dev->unit_address, 0, set_attr.desc,
+						 &ret_attr.desc);
+
+			if (ret == H_SUCCESS)
+				netdev->features |= NETIF_F_IP_CSUM;
+			else
+				ret = h_illan_attributes(dev->unit_address, set_attr.desc,
+							 0, &ret_attr.desc);
+		}
+	}
+
 	rc = register_netdev(netdev);
 
 	if(rc) {
diff -puN drivers/net/ibmveth.h~ibmveth_csum_offload drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_csum_offload	2007-07-12 08:27:47.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h	2007-07-12 09:32:54.000000000 -0500
@@ -67,6 +67,21 @@ static inline long h_send_logical_lan(un
 	return rc;
 }
 
+static inline long h_illan_attributes(unsigned long unit_address,
+				      unsigned long reset_mask, unsigned long set_mask,
+				      unsigned long *ret_attributes)
+{
+	long rc;
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
+
+	rc = plpar_hcall(H_ILLAN_ATTRIBUTES, retbuf, unit_address,
+			 reset_mask, set_mask);
+
+	*ret_attributes = retbuf[0];
+
+	return rc;
+}
+
 #define h_multicast_ctrl(ua, cmd, mac) \
   plpar_hcall_norets(H_MULTICAST_CTRL, ua, cmd, mac)
 
@@ -144,7 +159,9 @@ struct ibmveth_adapter {
 struct ibmveth_buf_desc_fields {
     u32 valid : 1;
     u32 toggle : 1;
-    u32 reserved : 6;
+    u32 reserved : 4;
+    u32 no_csum : 1;
+    u32 csum_good : 1;
     u32 length : 24;
     u32 address;
 };
@@ -154,10 +171,30 @@ union ibmveth_buf_desc {
     struct ibmveth_buf_desc_fields fields;
 };
 
+struct ibmveth_illan_attributes_fields {
+	u32 reserved;
+	u32 reserved2 : 18;
+	u32 csum_offload_padded_pkt_support : 1;
+	u32 reserved3 : 1;
+	u32 trunk_priority : 4;
+	u32 reserved4 : 5;
+	u32 tcp_csum_offload_ipv6 : 1;
+	u32 tcp_csum_offload_ipv4 : 1;
+	u32 active_trunk : 1;
+};
+
+union ibmveth_illan_attributes {
+	u64 desc;
+	struct ibmveth_illan_attributes_fields fields;
+};
+
 struct ibmveth_rx_q_entry {
     u16 toggle : 1;
     u16 valid : 1;
-    u16 reserved : 14;
+    u16 reserved : 4;
+    u16 no_csum : 1;
+    u16 csum_good : 1;
+    u16 reserved2 : 8;
     u16 offset;
     u32 length;
     u64 correlator;
_

[PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload

[Brian King]

This patch adds the appropriate ethtool hooks to allow for enabling/disabling
of hypervisor assisted checksum offload for TCP.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |  120 +++++++++++++++++++++++++++++++-
 linux-2.6-bjking1/drivers/net/ibmveth.h |    1 
 2 files changed, 119 insertions(+), 2 deletions(-)

diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool	2007-07-12 09:36:01.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-12 09:41:15.000000000 -0500
@@ -644,12 +644,127 @@ static u32 netdev_get_link(struct net_de
 	return 1;
 }
 
+static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+
+	if (data)
+		adapter->rx_csum = 1;
+	else {
+		adapter->rx_csum = 0;
+		dev->features &= ~NETIF_F_IP_CSUM;
+	}
+}
+
+static void ibmveth_set_tx_csum_flags(struct net_device *dev, u32 data)
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+
+	if (data) {
+		dev->features |= NETIF_F_IP_CSUM;
+		adapter->rx_csum = 1;
+	} else
+		dev->features &= ~NETIF_F_IP_CSUM;
+}
+
+static int ibmveth_set_csum_offload(struct net_device *dev, u32 data,
+				    void (*done) (struct net_device *, u32))
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+	union ibmveth_illan_attributes set_attr, clr_attr, ret_attr;
+	long ret;
+	int rc1 = 0, rc2 = 0;
+	int restart = 0;
+
+	if (netif_running(dev)) {
+		restart = 1;
+		adapter->pool_config = 1;
+		ibmveth_close(dev);
+		adapter->pool_config = 0;
+	}
+
+	set_attr.desc = 0;
+	clr_attr.desc = 0;
+
+	if (data)
+		set_attr.fields.tcp_csum_offload_ipv4 = 1;
+	else
+		clr_attr.fields.tcp_csum_offload_ipv4 = 1;
+
+	ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr.desc);
+
+	if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
+	    !ret_attr.fields.trunk_priority &&
+	    ret_attr.fields.csum_offload_padded_pkt_support) {
+		ret = h_illan_attributes(adapter->vdev->unit_address, clr_attr.desc,
+					 set_attr.desc, &ret_attr.desc);
+
+		if (ret != H_SUCCESS) {
+			rc1 = -EIO;
+			ibmveth_error_printk("unable to change checksum offload settings."
+					     " %d rc=%ld\n", data, ret);
+
+			ret = h_illan_attributes(adapter->vdev->unit_address,
+						 set_attr.desc, clr_attr.desc, &ret_attr.desc);
+		} else
+			done(dev, data);
+	} else {
+		rc1 = -EIO;
+		ibmveth_error_printk("unable to change checksum offload settings."
+				     " %d rc=%ld ret_attr=%lx\n", data, ret, ret_attr.desc);
+	}
+
+	if (restart)
+		rc2 = ibmveth_open(dev);
+
+	return rc1 ? rc1 : rc2;
+}
+
+static int ibmveth_set_rx_csum(struct net_device *dev, u32 data)
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+
+	if (data && adapter->rx_csum)
+		return 0;
+	if (!data && !adapter->rx_csum)
+		return 0;
+
+	return ibmveth_set_csum_offload(dev, data, ibmveth_set_rx_csum_flags);
+}
+
+static int ibmveth_set_tx_csum(struct net_device *dev, u32 data)
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+	int rc = 0;
+
+	if (data && (dev->features & NETIF_F_IP_CSUM))
+		return 0;
+	if (!data && !(dev->features & NETIF_F_IP_CSUM))
+		return 0;
+
+	if (data && !adapter->rx_csum)
+		rc = ibmveth_set_csum_offload(dev, data, ibmveth_set_tx_csum_flags);
+	else
+		ibmveth_set_tx_csum_flags(dev, data);
+
+	return rc;
+}
+
+static u32 ibmveth_get_rx_csum(struct net_device *dev)
+{
+	struct ibmveth_adapter *adapter = dev->priv;
+	return adapter->rx_csum;
+}
+
 static const struct ethtool_ops netdev_ethtool_ops = {
 	.get_drvinfo		= netdev_get_drvinfo,
 	.get_settings		= netdev_get_settings,
 	.get_link		= netdev_get_link,
 	.get_sg			= ethtool_op_get_sg,
 	.get_tx_csum		= ethtool_op_get_tx_csum,
+	.set_tx_csum		= ibmveth_set_tx_csum,
+	.get_rx_csum		= ibmveth_get_rx_csum,
+	.set_rx_csum		= ibmveth_set_rx_csum
 };
 
 static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
@@ -1108,9 +1223,10 @@ static int __devinit ibmveth_probe(struc
 			ret = h_illan_attributes(dev->unit_address, 0, set_attr.desc,
 						 &ret_attr.desc);
 
-			if (ret == H_SUCCESS)
+			if (ret == H_SUCCESS) {
+				adapter->rx_csum = 1;
 				netdev->features |= NETIF_F_IP_CSUM;
-			else
+			} else
 				ret = h_illan_attributes(dev->unit_address, set_attr.desc,
 							 0, &ret_attr.desc);
 		}
diff -puN drivers/net/ibmveth.h~ibmveth_csum_offload_ethtool drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_csum_offload_ethtool	2007-07-12 09:36:01.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h	2007-07-12 09:36:01.000000000 -0500
@@ -140,6 +140,7 @@ struct ibmveth_adapter {
     struct ibmveth_buff_pool rx_buff_pool[IbmVethNumBufferPools];
     struct ibmveth_rx_q rx_queue;
     int pool_config;
+    int rx_csum;
 
     /* adapter specific stats */
     u64 replenish_task_cycles;
_

[PATCH 3/4] ibmveth: Add ethtool TSO handlers

[Brian King]

Add handlers for get_tso and get_ufo to prevent errors being printed
by ethtool.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_get_tso drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_get_tso	2007-07-12 09:39:20.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-12 09:39:20.000000000 -0500
@@ -764,7 +764,9 @@ static const struct ethtool_ops netdev_e
 	.get_tx_csum		= ethtool_op_get_tx_csum,
 	.set_tx_csum		= ibmveth_set_tx_csum,
 	.get_rx_csum		= ibmveth_get_rx_csum,
-	.set_rx_csum		= ibmveth_set_rx_csum
+	.set_rx_csum		= ibmveth_set_rx_csum,
+	.get_tso			= ethtool_op_get_tso,
+	.get_ufo			= ethtool_op_get_ufo
 };
 
 static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
_

[PATCH 4/4] ibmveth: Add ethtool driver stats hooks

[Brian King]

Add ethtool hooks to ibmveth to retrieve driver statistics.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   53 +++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats	2007-07-12 09:39:23.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-12 09:39:23.000000000 -0500
@@ -115,6 +115,28 @@ MODULE_VERSION(ibmveth_driver_version);
 module_param_named(csum_offload, ibmveth_csum_offload, uint, 0);
 MODULE_PARM_DESC(csum_offload, "Checksum offload (0/1). Default: 1");
 
+struct ibmveth_stat {
+	char name[ETH_GSTRING_LEN];
+	int offset;
+};
+
+#define IBMVETH_STAT_OFF(stat) offsetof(struct ibmveth_adapter, stat)
+#define IBMVETH_GET_STAT(a, off) *((u64 *)(((unsigned long)(a)) + off))
+
+struct ibmveth_stat ibmveth_stats[] = {
+	{ "replenish_task_cycles", IBMVETH_STAT_OFF(replenish_task_cycles) },
+	{ "replenish_no_mem", IBMVETH_STAT_OFF(replenish_no_mem) },
+	{ "replenish_add_buff_failure", IBMVETH_STAT_OFF(replenish_add_buff_failure) },
+	{ "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
+	{ "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
+	{ "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
+	{ "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
+	{ "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
+	{ "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
+	{ "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
+	{ "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
+};
+
 /* simple methods of getting data from the current rxq entry */
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
 {
@@ -756,6 +778,32 @@ static u32 ibmveth_get_rx_csum(struct ne
 	return adapter->rx_csum;
 }
 
+static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+	int i;
+
+	if (stringset != ETH_SS_STATS)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++, data += ETH_GSTRING_LEN)
+		memcpy(data, ibmveth_stats[i].name, ETH_GSTRING_LEN);
+}
+
+static int ibmveth_get_stats_count(struct net_device *dev)
+{
+	return ARRAY_SIZE(ibmveth_stats);
+}
+
+static void ibmveth_get_ethtool_stats(struct net_device *dev,
+				      struct ethtool_stats *stats, u64 *data)
+{
+	int i;
+	struct ibmveth_adapter *adapter = dev->priv;
+
+	for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++)
+		data[i] = IBMVETH_GET_STAT(adapter, ibmveth_stats[i].offset);
+}
+
 static const struct ethtool_ops netdev_ethtool_ops = {
 	.get_drvinfo		= netdev_get_drvinfo,
 	.get_settings		= netdev_get_settings,
@@ -766,7 +814,10 @@ static const struct ethtool_ops netdev_e
 	.get_rx_csum		= ibmveth_get_rx_csum,
 	.set_rx_csum		= ibmveth_set_rx_csum,
 	.get_tso			= ethtool_op_get_tso,
-	.get_ufo			= ethtool_op_get_ufo
+	.get_ufo			= ethtool_op_get_ufo,
+	.get_strings		= ibmveth_get_strings,
+	.get_stats_count		= ibmveth_get_stats_count,
+	.get_ethtool_stats	= ibmveth_get_ethtool_stats
 };
 
 static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
diff -puN drivers/net/ibmveth.h~ibmveth_ethtool_driver_stats drivers/net/ibmveth.h
_

Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload

[Jeff Garzik]

Brian King wrote:
> This patchset enables TCP checksum offload support for IPV4
> on ibmveth. This completely eliminates the generation and checking of
> the checksum for packets that are completely virtual and never
> touch a physical network. A simple TCP_STREAM netperf run on
> a virtual network with maximum mtu set yielded a ~30% increase
> in throughput. This feature is enabled by default on systems that
> support it, but can be disabled with a module option.
> 
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>

this should be controllable purely via ethtool...


> diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload	2007-07-12 08:27:47.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-12 09:35:55.000000000 -0500
> @@ -47,6 +47,8 @@
>  #include <linux/mm.h>
>  #include <linux/ethtool.h>
>  #include <linux/proc_fs.h>
> +#include <linux/in.h>
> +#include <linux/ip.h>
>  #include <asm/semaphore.h>
>  #include <asm/hvcall.h>
>  #include <asm/atomic.h>
> @@ -103,12 +105,15 @@ 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";
> +static unsigned int ibmveth_csum_offload = 1;
>  #define ibmveth_driver_version "1.03"
>  
>  MODULE_AUTHOR("Santiago Leon <santil@us.ibm.com>");
>  MODULE_DESCRIPTION("IBM i/pSeries Virtual Ethernet Driver");
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(ibmveth_driver_version);
> +module_param_named(csum_offload, ibmveth_csum_offload, uint, 0);
> +MODULE_PARM_DESC(csum_offload, "Checksum offload (0/1). Default: 1");
>  
>  /* simple methods of getting data from the current rxq entry */
>  static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
> @@ -131,6 +136,11 @@ static inline int ibmveth_rxq_frame_leng
>  	return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].length);
>  }
>  
> +static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
> +{
> +	return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].csum_good);
> +}
> +
>  /* setup the initial settings for a buffer pool */
>  static void ibmveth_init_buffer_pool(struct ibmveth_buff_pool *pool, u32 pool_index, u32 pool_size, u32 buff_size, u32 pool_active)
>  {
> @@ -684,6 +694,24 @@ static int ibmveth_start_xmit(struct sk_
>  					desc[0].fields.length, DMA_TO_DEVICE);
>  	desc[0].fields.valid   = 1;
>  
> +	if (skb->ip_summed == CHECKSUM_PARTIAL &&
> +	    ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
> +		ibmveth_error_printk("tx: failed to checksum packet\n");
> +		tx_dropped++;
> +		goto out;
> +	}
> +
> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		unsigned char *buf = skb_transport_header(skb) + skb->csum_offset;
> +
> +		desc[0].fields.no_csum = 1;
> +		desc[0].fields.csum_good = 1;
> +
> +		/* Need to zero out the checksum */
> +		buf[0] = 0;
> +		buf[1] = 0;
> +	}
> +
>  	if(dma_mapping_error(desc[0].fields.address)) {
>  		ibmveth_error_printk("tx: unable to map initial fragment\n");
>  		tx_map_failed++;
> @@ -702,6 +730,10 @@ static int ibmveth_start_xmit(struct sk_
>  				frag->size, DMA_TO_DEVICE);
>  		desc[curfrag+1].fields.length = frag->size;
>  		desc[curfrag+1].fields.valid  = 1;
> +		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +			desc[curfrag+1].fields.no_csum = 1;
> +			desc[curfrag+1].fields.csum_good = 1;
> +		}
>  
>  		if(dma_mapping_error(desc[curfrag+1].fields.address)) {
>  			ibmveth_error_printk("tx: unable to map fragment %d\n", curfrag);
> @@ -792,7 +824,11 @@ static int ibmveth_poll(struct net_devic
>  			} else {
>  				int length = ibmveth_rxq_frame_length(adapter);
>  				int offset = ibmveth_rxq_frame_offset(adapter);
> +				int csum_good = ibmveth_rxq_csum_good(adapter);
> +
>  				skb = ibmveth_rxq_get_buffer(adapter);
> +				if (csum_good)
> +					skb->ip_summed = CHECKSUM_UNNECESSARY;
>  
>  				ibmveth_rxq_harvest_buffer(adapter);
>  
> @@ -962,8 +998,10 @@ static void ibmveth_poll_controller(stru
>  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 = NULL;
> +	union ibmveth_illan_attributes set_attr, ret_attr;
>  
>  	unsigned char *mac_addr_p;
>  	unsigned int *mcastFilterSize_p;
> @@ -1058,6 +1096,26 @@ static int __devinit ibmveth_probe(struc
>  
>  	ibmveth_debug_printk("registering netdev...\n");
>  
> +	if (ibmveth_csum_offload) {
> +		ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr.desc);
> +
> +		if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
> +		    !ret_attr.fields.trunk_priority &&
> +		    ret_attr.fields.csum_offload_padded_pkt_support) {
> +			set_attr.desc = 0;
> +			set_attr.fields.tcp_csum_offload_ipv4 = 1;
> +
> +			ret = h_illan_attributes(dev->unit_address, 0, set_attr.desc,
> +						 &ret_attr.desc);
> +
> +			if (ret == H_SUCCESS)
> +				netdev->features |= NETIF_F_IP_CSUM;
> +			else
> +				ret = h_illan_attributes(dev->unit_address, set_attr.desc,
> +							 0, &ret_attr.desc);
> +		}
> +	}
> +
>  	rc = register_netdev(netdev);
>  
>  	if(rc) {
> diff -puN drivers/net/ibmveth.h~ibmveth_csum_offload drivers/net/ibmveth.h
> --- linux-2.6/drivers/net/ibmveth.h~ibmveth_csum_offload	2007-07-12 08:27:47.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.h	2007-07-12 09:32:54.000000000 -0500
> @@ -67,6 +67,21 @@ static inline long h_send_logical_lan(un
>  	return rc;
>  }
>  
> +static inline long h_illan_attributes(unsigned long unit_address,
> +				      unsigned long reset_mask, unsigned long set_mask,
> +				      unsigned long *ret_attributes)
> +{
> +	long rc;
> +	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> +
> +	rc = plpar_hcall(H_ILLAN_ATTRIBUTES, retbuf, unit_address,
> +			 reset_mask, set_mask);
> +
> +	*ret_attributes = retbuf[0];
> +
> +	return rc;
> +}
> +
>  #define h_multicast_ctrl(ua, cmd, mac) \
>    plpar_hcall_norets(H_MULTICAST_CTRL, ua, cmd, mac)
>  
> @@ -144,7 +159,9 @@ struct ibmveth_adapter {
>  struct ibmveth_buf_desc_fields {
>      u32 valid : 1;
>      u32 toggle : 1;
> -    u32 reserved : 6;
> +    u32 reserved : 4;
> +    u32 no_csum : 1;
> +    u32 csum_good : 1;
>      u32 length : 24;
>      u32 address;
>  };
> @@ -154,10 +171,30 @@ union ibmveth_buf_desc {
>      struct ibmveth_buf_desc_fields fields;
>  };
>  
> +struct ibmveth_illan_attributes_fields {
> +	u32 reserved;
> +	u32 reserved2 : 18;
> +	u32 csum_offload_padded_pkt_support : 1;
> +	u32 reserved3 : 1;
> +	u32 trunk_priority : 4;
> +	u32 reserved4 : 5;
> +	u32 tcp_csum_offload_ipv6 : 1;
> +	u32 tcp_csum_offload_ipv4 : 1;
> +	u32 active_trunk : 1;
> +};
> +
> +union ibmveth_illan_attributes {
> +	u64 desc;
> +	struct ibmveth_illan_attributes_fields fields;
> +};
> +
>  struct ibmveth_rx_q_entry {
>      u16 toggle : 1;
>      u16 valid : 1;
> -    u16 reserved : 14;
> +    u16 reserved : 4;
> +    u16 no_csum : 1;
> +    u16 csum_good : 1;
> +    u16 reserved2 : 8;
>      u16 offset;

As a general rule, it would be nice to start moving away from bitfields 
in this driver

Re: [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload

[Jeff Garzik]

Brian King wrote:
> This patch adds the appropriate ethtool hooks to allow for enabling/disabling
> of hypervisor assisted checksum offload for TCP.
> 
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
> 
>  linux-2.6-bjking1/drivers/net/ibmveth.c |  120 +++++++++++++++++++++++++++++++-
>  linux-2.6-bjking1/drivers/net/ibmveth.h |    1 
>  2 files changed, 119 insertions(+), 2 deletions(-)

ACK.  As noted in previous email, with this, module option is not needed

Re: [PATCH 4/4] ibmveth: Add ethtool driver stats hooks

[Jeff Garzik]

Brian King wrote:
> Add ethtool hooks to ibmveth to retrieve driver statistics.
> 
> 
ACK patches 3-4

Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload

[Brian King]

Jeff Garzik wrote:
> Brian King wrote:
>> This patchset enables TCP checksum offload support for IPV4
>> on ibmveth. This completely eliminates the generation and checking of
>> the checksum for packets that are completely virtual and never
>> touch a physical network. A simple TCP_STREAM netperf run on
>> a virtual network with maximum mtu set yielded a ~30% increase
>> in throughput. This feature is enabled by default on systems that
>> support it, but can be disabled with a module option.
>>
>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> 
> this should be controllable purely via ethtool...

I'll remove the module parameter and resend.

>>  struct ibmveth_rx_q_entry {
>>      u16 toggle : 1;
>>      u16 valid : 1;
>> -    u16 reserved : 14;
>> +    u16 reserved : 4;
>> +    u16 no_csum : 1;
>> +    u16 csum_good : 1;
>> +    u16 reserved2 : 8;
>>      u16 offset;
> 
> As a general rule, it would be nice to start moving away from bitfields 
> in this driver

I'll take a look at that for a future cleanup patch.

Thanks,

Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

[PATCH 4/4] ibmveth: Add ethtool driver stats hooks

[Brian King]

Add ethtool hooks to ibmveth to retrieve driver statistics.

Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   53 +++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats	2007-07-19 08:39:35.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c	2007-07-19 08:39:35.000000000 -0500
@@ -112,6 +112,28 @@ MODULE_DESCRIPTION("IBM i/pSeries Virtua
 MODULE_LICENSE("GPL");
 MODULE_VERSION(ibmveth_driver_version);
 
+struct ibmveth_stat {
+	char name[ETH_GSTRING_LEN];
+	int offset;
+};
+
+#define IBMVETH_STAT_OFF(stat) offsetof(struct ibmveth_adapter, stat)
+#define IBMVETH_GET_STAT(a, off) *((u64 *)(((unsigned long)(a)) + off))
+
+struct ibmveth_stat ibmveth_stats[] = {
+	{ "replenish_task_cycles", IBMVETH_STAT_OFF(replenish_task_cycles) },
+	{ "replenish_no_mem", IBMVETH_STAT_OFF(replenish_no_mem) },
+	{ "replenish_add_buff_failure", IBMVETH_STAT_OFF(replenish_add_buff_failure) },
+	{ "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
+	{ "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
+	{ "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
+	{ "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
+	{ "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
+	{ "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
+	{ "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
+	{ "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
+};
+
 /* simple methods of getting data from the current rxq entry */
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
 {
@@ -753,6 +775,32 @@ static u32 ibmveth_get_rx_csum(struct ne
 	return adapter->rx_csum;
 }
 
+static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+	int i;
+
+	if (stringset != ETH_SS_STATS)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++, data += ETH_GSTRING_LEN)
+		memcpy(data, ibmveth_stats[i].name, ETH_GSTRING_LEN);
+}
+
+static int ibmveth_get_stats_count(struct net_device *dev)
+{
+	return ARRAY_SIZE(ibmveth_stats);
+}
+
+static void ibmveth_get_ethtool_stats(struct net_device *dev,
+				      struct ethtool_stats *stats, u64 *data)
+{
+	int i;
+	struct ibmveth_adapter *adapter = dev->priv;
+
+	for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++)
+		data[i] = IBMVETH_GET_STAT(adapter, ibmveth_stats[i].offset);
+}
+
 static const struct ethtool_ops netdev_ethtool_ops = {
 	.get_drvinfo		= netdev_get_drvinfo,
 	.get_settings		= netdev_get_settings,
@@ -763,7 +811,10 @@ static const struct ethtool_ops netdev_e
 	.get_rx_csum		= ibmveth_get_rx_csum,
 	.set_rx_csum		= ibmveth_set_rx_csum,
 	.get_tso			= ethtool_op_get_tso,
-	.get_ufo			= ethtool_op_get_ufo
+	.get_ufo			= ethtool_op_get_ufo,
+	.get_strings		= ibmveth_get_strings,
+	.get_stats_count		= ibmveth_get_stats_count,
+	.get_ethtool_stats	= ibmveth_get_ethtool_stats
 };
 
 static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
diff -puN drivers/net/ibmveth.h~ibmveth_ethtool_driver_stats drivers/net/ibmveth.h
_