Re: [PATCH] Allow TSO to be disabled for forcedeth driver

Re: [PATCH] Allow TSO to be disabled for forcedeth driver

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

Ayaz Abdulla wrote:
>
> However, please change your patch to account for the ifdef NETIF_F_TSO 
> and you don't need spin lock around this change. For example, here are 
> the snippets:

Done

[-- Attachment #2: forcedeth-tso-toggle --]
[-- Type: text/plain, Size: 1242 bytes --]

Implement get / set tso for forcedeth driver.

Signed-off-by: Zachary Amsden <zach@vmware.com>

Index: linux-2.6.17-rc/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.17-rc.orig/drivers/net/forcedeth.c	2006-05-18 13:31:55.000000000 -0700
+++ linux-2.6.17-rc/drivers/net/forcedeth.c	2006-06-01 16:00:58.000000000 -0700
@@ -2615,6 +2615,23 @@ static int nv_nway_reset(struct net_devi
 	return ret;
 }
 
+#ifdef NETIF_F_TSO
+static int nv_set_tso(struct net_device *dev, u32 value)
+{
+	struct fe_priv *np = netdev_priv(dev);
+	int ret;
+
+	spin_lock_irq(&np->lock);
+	if ((np->driver_data & DEV_HAS_CHECKSUM))
+		ret = ethtool_op_set_tso(dev, value);
+	else
+		ret = value ? -EOPNOTSUPP : 0;
+	spin_unlock_irq(&np->lock);
+
+	return ret;
+}
+#endif
+
 static struct ethtool_ops ops = {
 	.get_drvinfo = nv_get_drvinfo,
 	.get_link = ethtool_op_get_link,
@@ -2626,6 +2643,10 @@ static struct ethtool_ops ops = {
 	.get_regs = nv_get_regs,
 	.nway_reset = nv_nway_reset,
 	.get_perm_addr = ethtool_op_get_perm_addr,
+#ifdef NETIF_F_TSO
+	.get_tso = ethtool_op_get_tso,
+	.set_tso = nv_set_tso
+#endif
 };
 
 static void nv_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)

[PATCH] Allow TSO to be disabled for forcedeth driver

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]

TSO can cause performance problems in certain environments, and being 
able to turn it on or off is helpful for debugging network issues.  Most 
other network drivers that support TSO allow it to be toggled, so add 
this feature to forcedeth.  Tested by Harald Dunkel, who reported that 
this fixed his network performance issue with VMware.



[-- Attachment #2: forcedeth-tso-toggle --]
[-- Type: text/plain, Size: 1185 bytes --]

Implement get / set tso for forcedeth driver.

Signed-off-by: Zachary Amsden <zach@vmware.com>

Index: linux-2.6.17-rc/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.17-rc.orig/drivers/net/forcedeth.c	2006-05-18 13:31:55.000000000 -0700
+++ linux-2.6.17-rc/drivers/net/forcedeth.c	2006-05-31 14:34:53.000000000 -0700
@@ -2615,6 +2615,21 @@ static int nv_nway_reset(struct net_devi
 	return ret;
 }
 
+static int nv_set_tso(struct net_device *dev, u32 value)
+{
+	struct fe_priv *np = netdev_priv(dev);
+	int ret;
+
+	spin_lock_irq(&np->lock);
+	if ((np->driver_data & DEV_HAS_CHECKSUM))
+		ret = ethtool_op_set_tso(dev, value);
+	else
+		ret = value ? -EOPNOTSUPP : 0;
+	spin_unlock_irq(&np->lock);
+
+	return ret;
+}
+
 static struct ethtool_ops ops = {
 	.get_drvinfo = nv_get_drvinfo,
 	.get_link = ethtool_op_get_link,
@@ -2626,6 +2641,8 @@ static struct ethtool_ops ops = {
 	.get_regs = nv_get_regs,
 	.nway_reset = nv_nway_reset,
 	.get_perm_addr = ethtool_op_get_perm_addr,
+	.get_tso = ethtool_op_get_tso,
+	.set_tso = nv_set_tso
 };
 
 static void nv_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)

Re: [PATCH] Allow TSO to be disabled for forcedeth driver

[Andrew Morton]

Zachary Amsden <zach@vmware.com> wrote:
>
> TSO can cause performance problems in certain environments, and being 
> able to turn it on or off is helpful for debugging network issues.  Most 
> other network drivers that support TSO allow it to be toggled, so add 
> this feature to forcedeth.  Tested by Harald Dunkel, who reported that 
> this fixed his network performance issue with VMware.
> 

(This is regarding
http://www.vmware.com/community/thread.jspa?messageID=408893)


Why does TSO-with-forcedeth make vmware networking slow?

Is it specific to the forcedeth driver?

Re: [PATCH] Allow TSO to be disabled for forcedeth driver

[Zachary Amsden]

Andrew Morton wrote:
> Zachary Amsden <zach@vmware.com> wrote:
>   
>> TSO can cause performance problems in certain environments, and being 
>> able to turn it on or off is helpful for debugging network issues.  Most 
>> other network drivers that support TSO allow it to be toggled, so add 
>> this feature to forcedeth.  Tested by Harald Dunkel, who reported that 
>> this fixed his network performance issue with VMware.
>>
>>     
>
> (This is regarding
> http://www.vmware.com/community/thread.jspa?messageID=408893)
>
>
> Why does TSO-with-forcedeth make vmware networking slow?
>
> Is it specific to the forcedeth driver?
>   

No.  TSO is not good for bridged virtual networking in general, since 
even if the bridged networking module understood TSO, it would then have 
to split up any large packets into smaller packets to pass on to the 
guest virtual machine - or require that the guest virtual machine have 
and understand how to use a TSO compatible network interface as well.  
Both solutions are extremely problematic, and the easiest thing to do is 
just disable TSO.  It makes sense for any protocol bridge device, 
including some firewall configurations.

Zach