[RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization

[RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization

[Shan Wei]

This patch is not a bug fix.
Just add GSO to vlan_features initialization, and update comments.


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 net/core/dev.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6b6ef14..becc1e5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5478,11 +5478,12 @@ int register_netdevice(struct net_device *dev)
 		dev->features |= NETIF_F_NOCACHE_COPY;
 	}
 
-	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
-	 * vlan_dev_init() will do the dev->features check, so these features
-	 * are enabled only if supported by underlying device.
+	/* Enable GSO, GRO and NETIF_F_HIGHDMA for vlans by default,
+	 * vlan_dev_fix_features() will do the features check,
+	 * so NETIF_F_HIGHDMA feature is enabled only if supported
+	 * by underlying device.
 	 */
-	dev->vlan_features |= (NETIF_F_GRO | NETIF_F_HIGHDMA);
+	dev->vlan_features |= (NETIF_F_SOFT_FEATURES | NETIF_F_HIGHDMA);
 
 	ret = call_netdevice_notifiers(NETDEV_POST_INIT, dev);
 	ret = notifier_to_errno(ret);
-- 
1.7.4.1

Re: [RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization

[Ben Hutchings]

On Fri, 2011-06-24 at 11:38 +0800, Shan Wei wrote:
> This patch is not a bug fix.
> Just add GSO to vlan_features initialization, and update comments.
> 
> 
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
> ---
>  net/core/dev.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6b6ef14..becc1e5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5478,11 +5478,12 @@ int register_netdevice(struct net_device *dev)
>  		dev->features |= NETIF_F_NOCACHE_COPY;
>  	}
>  
> -	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
> -	 * vlan_dev_init() will do the dev->features check, so these features
> -	 * are enabled only if supported by underlying device.
> +	/* Enable GSO, GRO and NETIF_F_HIGHDMA for vlans by default,
> +	 * vlan_dev_fix_features() will do the features check,
> +	 * so NETIF_F_HIGHDMA feature is enabled only if supported
> +	 * by underlying device.
>  	 */
> -	dev->vlan_features |= (NETIF_F_GRO | NETIF_F_HIGHDMA);
> +	dev->vlan_features |= (NETIF_F_SOFT_FEATURES | NETIF_F_HIGHDMA);
>  
>  	ret = call_netdevice_notifiers(NETDEV_POST_INIT, dev);
>  	ret = notifier_to_errno(ret);

Have you verified that GSO works correctly for VLAN devices if the
underlying device does not support VLAN tag insertion?
skb_gso_segment() has code to handle this case, but I suspect it's not
actually being exercised at the moment.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization

[Shan Wei]

Hi Ben:

Ben Hutchings wrote, at 06/25/2011 12:51 AM:
> Have you verified that GSO works correctly for VLAN devices if the
> underlying device does not support VLAN tag insertion?

OK. I will do some test in future days.
Then report test results.

-- 
Best Regards
-----
Shan Wei

Re: [RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization

[Shan Wei]

Hi Ben:

Shan Wei wrote, at 06/28/2011 11:47 AM:
> Hi Ben:
> 
> Ben Hutchings wrote, at 06/25/2011 12:51 AM:
>> Have you verified that GSO works correctly for VLAN devices if the
>> underlying device does not support VLAN tag insertion?
> 
> OK. I will do some test in future days.
> Then report test results.

Results is coming....
GSO works correctly for VLAN devices when
underlying device is Realtek NIC with 8139too driver.

Network tracepoint shows as following:
<Begin:
       tcpclient-5915  [000]  2114.246780: net_dev_queue: dev=eth1.44 skbaddr=f3051bc0 len=74
       tcpclient-5915  [000]  2114.246790: net_dev_queue: dev=eth1 skbaddr=f3051bc0 len=74
       tcpclient-5915  [000]  2114.246798: net_dev_xmit: dev=eth1 skbaddr=f3051bc0 len=78 rc=0
       tcpclient-5915  [000]  2114.246799: net_dev_xmit: dev=eth1.44 skbaddr=f3051bc0 len=74 rc=0
          <idle>-0     [000]  2114.246905: net_dev_queue: dev=eth1.44 skbaddr=f73cc480 len=66
          <idle>-0     [000]  2114.246908: net_dev_queue: dev=eth1 skbaddr=f73cc480 len=66
          <idle>-0     [000]  2114.246911: net_dev_xmit: dev=eth1 skbaddr=f73cc480 len=70 rc=0
          <idle>-0     [000]  2114.246912: net_dev_xmit: dev=eth1.44 skbaddr=f73cc480 len=66 rc=0
       tcpclient-5915  [000]  2114.247109: net_dev_queue: dev=eth1.44 skbaddr=f3051bc0 len=2962
       tcpclient-5915  [000]  2114.247121: net_dev_queue: dev=eth1 skbaddr=f73cc380 len=1514
       tcpclient-5915  [000]  2114.247130: net_dev_xmit: dev=eth1 skbaddr=f73cc380 len=1518 rc=0
       tcpclient-5915  [000]  2114.247131: net_dev_xmit: dev=eth1.44 skbaddr=f73cc380 len=1514 rc=0
       tcpclient-5915  [000]  2114.247132: net_dev_queue: dev=eth1 skbaddr=f73cc480 len=1514
       tcpclient-5915  [000]  2114.247138: net_dev_xmit: dev=eth1 skbaddr=f73cc480 len=1518 rc=0
       tcpclient-5915  [000]  2114.247139: net_dev_xmit: dev=eth1.44 skbaddr=f73cc480 len=1514 rc=0
end:>

But, test case requires following two patches:
1. Fix broken vlan GSO setting

[PATCH 1/2] net: vlan: enable GSO by default
http://patchwork.ozlabs.org/patch/101714/

2. Fix my 8139too NIC driver that does not initial vlan_features.
diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -993,6 +993,7 @@ static int __devinit rtl8139_init_one (struct pci_dev *pdev,
         * features
         */
        dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA;
+       dev->vlan_features = dev->features;

-- 
Best Regards
-----
Shan Wei