public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Allow TSO to be disabled for forcedeth driver
@ 2006-06-01 19:27 Zachary Amsden
  2006-06-01 19:53 ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Zachary Amsden @ 2006-06-01 19:27 UTC (permalink / raw)
  To: Andrew Morton, Manfred Spraul, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow TSO to be disabled for forcedeth driver
  2006-06-01 19:27 [PATCH] Allow TSO to be disabled for forcedeth driver Zachary Amsden
@ 2006-06-01 19:53 ` Andrew Morton
  2006-06-01 20:07   ` Zachary Amsden
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2006-06-01 19:53 UTC (permalink / raw)
  To: Zachary Amsden; +Cc: manfred, linux-kernel, Ayaz Abdulla

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?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow TSO to be disabled for forcedeth driver
  2006-06-01 19:53 ` Andrew Morton
@ 2006-06-01 20:07   ` Zachary Amsden
  0 siblings, 0 replies; 4+ messages in thread
From: Zachary Amsden @ 2006-06-01 20:07 UTC (permalink / raw)
  To: Andrew Morton; +Cc: manfred, linux-kernel, Ayaz Abdulla

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow TSO to be disabled for forcedeth driver
       [not found]     ` <447F472E.5060808@nvidia.com>
@ 2006-06-02  5:49       ` Zachary Amsden
  0 siblings, 0 replies; 4+ messages in thread
From: Zachary Amsden @ 2006-06-02  5:49 UTC (permalink / raw)
  To: Ayaz Abdulla, Andrew Morton, Linux Kernel Mailing List; +Cc: manfred

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-06-02  5:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-01 19:27 [PATCH] Allow TSO to be disabled for forcedeth driver Zachary Amsden
2006-06-01 19:53 ` Andrew Morton
2006-06-01 20:07   ` Zachary Amsden
     [not found] <DBFABB80F7FD3143A911F9E6CFD477B00BA5E36B@hqemmail02.nvidia.com>
     [not found] ` <20060601142909.369cf12f.akpm@osdl.org>
     [not found]   ` <447F6E13.8030904@vmware.com>
     [not found]     ` <447F472E.5060808@nvidia.com>
2006-06-02  5:49       ` Zachary Amsden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox