* [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
[not found] <11846854701127-patch-mail.ibm.com>
@ 2007-07-17 15:17 ` Brian King
2007-07-18 22:33 ` Jeff Garzik
2007-07-17 15:18 ` [PATCH 3/4] ibmveth: Add ethtool TSO handlers Brian King
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Brian King @ 2007-07-17 15:17 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
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;
_
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 3/4] ibmveth: Add ethtool TSO handlers
[not found] <11846854701127-patch-mail.ibm.com>
2007-07-17 15:17 ` [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload Brian King
@ 2007-07-17 15:18 ` Brian King
2007-07-17 15:18 ` [PATCH 4/4] ibmveth: Add ethtool driver stats hooks Brian King
2007-07-18 22:32 ` [PATCH 1/4] ibmveth: Enable TCP checksum offload Jeff Garzik
3 siblings, 0 replies; 12+ messages in thread
From: Brian King @ 2007-07-17 15:18 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
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)
_
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 4/4] ibmveth: Add ethtool driver stats hooks
[not found] <11846854701127-patch-mail.ibm.com>
2007-07-17 15:17 ` [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload Brian King
2007-07-17 15:18 ` [PATCH 3/4] ibmveth: Add ethtool TSO handlers Brian King
@ 2007-07-17 15:18 ` Brian King
2007-07-18 22:34 ` Jeff Garzik
2007-07-18 22:32 ` [PATCH 1/4] ibmveth: Enable TCP checksum offload Jeff Garzik
3 siblings, 1 reply; 12+ messages in thread
From: Brian King @ 2007-07-17 15:18 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
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
_
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload
[not found] <11846854701127-patch-mail.ibm.com>
` (2 preceding siblings ...)
2007-07-17 15:18 ` [PATCH 4/4] ibmveth: Add ethtool driver stats hooks Brian King
@ 2007-07-18 22:32 ` Jeff Garzik
2007-07-19 13:35 ` Brian King
3 siblings, 1 reply; 12+ messages in thread
From: Jeff Garzik @ 2007-07-18 22:32 UTC (permalink / raw)
To: Brian King; +Cc: santil, rcjenn, netdev, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload
2007-07-18 22:32 ` [PATCH 1/4] ibmveth: Enable TCP checksum offload Jeff Garzik
@ 2007-07-19 13:35 ` Brian King
0 siblings, 0 replies; 12+ messages in thread
From: Brian King @ 2007-07-19 13:35 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linuxppc-dev, rcjenn, santil, netdev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
[not found] <1184860086366-patch-mail.ibm.com>
@ 2007-07-19 15:48 ` Brian King
2007-07-19 16:08 ` Ragner Magalhaes
0 siblings, 1 reply; 12+ messages in thread
From: Brian King @ 2007-07-19 15:48 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
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-19 08:38:27.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 08:39:08.000000000 -0500
@@ -641,12 +641,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)
@@ -1104,9 +1219,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-19 08:38:27.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-07-19 08:38:27.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;
_
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-07-19 15:48 ` [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable " Brian King
@ 2007-07-19 16:08 ` Ragner Magalhaes
2007-07-19 17:59 ` Brian King
0 siblings, 1 reply; 12+ messages in thread
From: Ragner Magalhaes @ 2007-07-19 16:08 UTC (permalink / raw)
To: ext Brian King; +Cc: santil, rcjenn, netdev, linuxppc-dev
ext Brian King wrote:
> +
> +static int ibmveth_set_rx_csum(struct net_device *dev, u32 data)
> +{
> + struct ibmveth_adapter *adapter = dev->priv;
> +
Why do not to do
if ((data && adapter->rx_csum) || (!data && !adapter->rx_csum))
return 0;
less two lines.
> + 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;
> +
here also, as above ...
> + 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;
> +}
> +
Best regards,
Ragner
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-07-19 16:08 ` Ragner Magalhaes
@ 2007-07-19 17:59 ` Brian King
2007-07-19 18:17 ` Ragner Magalhaes
0 siblings, 1 reply; 12+ messages in thread
From: Brian King @ 2007-07-19 17:59 UTC (permalink / raw)
To: Ragner Magalhaes; +Cc: santil, rcjenn, netdev, linuxppc-dev
Ragner Magalhaes wrote:
> ext Brian King wrote:
>
>> +
>> +static int ibmveth_set_rx_csum(struct net_device *dev, u32 data)
>> +{
>> + struct ibmveth_adapter *adapter = dev->priv;
>> +
>
> Why do not to do
>
> if ((data && adapter->rx_csum) || (!data && !adapter->rx_csum))
> return 0;
> less two lines.
Ok.
>
> here also, as above ...
>> + if (data && (dev->features & NETIF_F_IP_CSUM))
>> + return 0;
>> + if (!data && !(dev->features & NETIF_F_IP_CSUM))
>> + return 0;
This change would make the line > 80 columns, which I prefer to avoid.
Updated patch attached which addresses the first comment.
Thanks,
Brian
---
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>
---
drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/net/ibmveth.h | 1
2 files changed, 117 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-19 11:15:01.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:17:16.000000000 -0500
@@ -641,12 +641,125 @@ 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) || (!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)
@@ -1104,9 +1217,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-19 11:15:01.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-07-19 11:15: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;
_
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-07-19 17:59 ` Brian King
@ 2007-07-19 18:17 ` Ragner Magalhaes
2007-07-19 18:28 ` Brian King
0 siblings, 1 reply; 12+ messages in thread
From: Ragner Magalhaes @ 2007-07-19 18:17 UTC (permalink / raw)
To: brking; +Cc: santil, rcjenn, netdev, linuxppc-dev
ext Brian King wrote:
> Ragner Magalhaes wrote:
>> ext Brian King wrote:
>>
>>> +
>>> +static int ibmveth_set_rx_csum(struct net_device *dev, u32 data)
>>> +{
>>> + struct ibmveth_adapter *adapter = dev->priv;
>>> +
>> Why do not to do
>>
>> if ((data && adapter->rx_csum) || (!data && !adapter->rx_csum))
>> return 0;
>> less two lines.
>
> Ok.
>
>> here also, as above ...
>>> + if (data && (dev->features & NETIF_F_IP_CSUM))
>>> + return 0;
>>> + if (!data && !(dev->features & NETIF_F_IP_CSUM))
>>> + return 0;
>
> This change would make the line > 80 columns, which I prefer to avoid.
> Updated patch attached which addresses the first comment.
I think would not be ugly to make.
if ((data && (dev->features & NETIF_F_IP_CSUM)) ||
(!data && !(dev->features & NETIF_F_IP_CSUM)))
return 0;
>
> Thanks,
>
> Brian
>
> ---
Thanks,
Ragner
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-07-19 18:17 ` Ragner Magalhaes
@ 2007-07-19 18:28 ` Brian King
0 siblings, 0 replies; 12+ messages in thread
From: Brian King @ 2007-07-19 18:28 UTC (permalink / raw)
To: Ragner Magalhaes; +Cc: santil, rcjenn, netdev, linuxppc-dev
Ragner Magalhaes wrote:
> ext Brian King wrote:
>> Ragner Magalhaes wrote:
>>> here also, as above ...
>>>> + if (data && (dev->features & NETIF_F_IP_CSUM))
>>>> + return 0;
>>>> + if (!data && !(dev->features & NETIF_F_IP_CSUM))
>>>> + return 0;
>> This change would make the line > 80 columns, which I prefer to avoid.
>> Updated patch attached which addresses the first comment.
> I think would not be ugly to make.
>
> if ((data && (dev->features & NETIF_F_IP_CSUM)) ||
> (!data && !(dev->features & NETIF_F_IP_CSUM)))
> return 0;
I find that less readable than what I currently have.
-Brian
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 12+ messages in thread