[PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Patrick McHardy]

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

This patch changes e1000 to set vlan_features so TSO and CSUM
offload can be used by VLAN devices, similar as with the other
Intel drivers.

Only RFC because I don't know whether there is buggy hardware
that doesn't support this properly, but feel free to apply in
case there isn't.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 795 bytes --]

commit 9648c6ef22e4f6e14ed5b5d91d9809779bb520c7
Author: Patrick McHardy <kaber@trash.net>
Date:   Wed Jul 16 11:45:09 2008 +0200

    e1000: allow VLAN devices to use TSO and CSUM offload
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index f8df8bd..b6b3135 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1011,6 +1011,11 @@ e1000_probe(struct pci_dev *pdev,
 
 	netdev->features |= NETIF_F_LLTX;
 
+	netdev->vlan_features |= NETIF_F_TSO;
+	netdev->vlan_features |= NETIF_F_TSO6;
+	netdev->vlan_features |= NETIF_F_HW_CSUM;
+	netdev->vlan_features |= NETIF_F_SG;
+
 	adapter->en_mng_pt = e1000_enable_mng_pass_thru(&adapter->hw);
 
 	/* initialize eeprom parameters */

Re: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Patrick McHardy]

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

Patrick McHardy wrote:
> This patch changes e1000 to set vlan_features so TSO and CSUM
> offload can be used by VLAN devices, similar as with the other
> Intel drivers.
> 
> Only RFC because I don't know whether there is buggy hardware
> that doesn't support this properly, but feel free to apply in
> case there isn't.

I never received a response to this. Any comments?




[-- Attachment #2: x --]
[-- Type: text/plain, Size: 795 bytes --]

commit 9648c6ef22e4f6e14ed5b5d91d9809779bb520c7
Author: Patrick McHardy <kaber@trash.net>
Date:   Wed Jul 16 11:45:09 2008 +0200

    e1000: allow VLAN devices to use TSO and CSUM offload
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index f8df8bd..b6b3135 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1011,6 +1011,11 @@ e1000_probe(struct pci_dev *pdev,
 
 	netdev->features |= NETIF_F_LLTX;
 
+	netdev->vlan_features |= NETIF_F_TSO;
+	netdev->vlan_features |= NETIF_F_TSO6;
+	netdev->vlan_features |= NETIF_F_HW_CSUM;
+	netdev->vlan_features |= NETIF_F_SG;
+
 	adapter->en_mng_pt = e1000_enable_mng_pass_thru(&adapter->hw);
 
 	/* initialize eeprom parameters */

RE: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Brandeburg, Jesse]

Patrick McHardy wrote:
> Patrick McHardy wrote:
>> This patch changes e1000 to set vlan_features so TSO and CSUM
>> offload can be used by VLAN devices, similar as with the other Intel
>> drivers. 
>> 
>> Only RFC because I don't know whether there is buggy hardware
>> that doesn't support this properly, but feel free to apply in
>> case there isn't.
> 
> I never received a response to this. Any comments?

Sorry to delay the reply, the only one that stands out to me as a
possible problem with the devices that e1000 still supports is the
NETIF_F_TSO6.

I know that some of our older parts like 82544 don't support TSO6.

As I read: these should work:
82546GB/EB, 82545GM/EM, 82541(PI/GI/EI), 82541ER, 82547GI/EI, and
82540EP/EM

These parts should all work (at least as advertised in the software
reference manual available at
http://superb-east.dl.sourceforge.net/sourceforge/e1000/OpenSDM_8254x-37
.pdf)

with the exception of 82544 GC/EI

So as it stands your patch will not work on some hardware.  Also, would
we need to hook the ethtool methods such that if a user disables TSO it
will disable vlan_features(TSO) as well?

Jesse

RE: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Ben Hutchings]

On Fri, 2008-10-10 at 10:48 -0700, Brandeburg, Jesse wrote:
> Patrick McHardy wrote:
> > Patrick McHardy wrote:
> >> This patch changes e1000 to set vlan_features so TSO and CSUM
> >> offload can be used by VLAN devices, similar as with the other Intel
> >> drivers. 
> >> 
> >> Only RFC because I don't know whether there is buggy hardware
> >> that doesn't support this properly, but feel free to apply in
> >> case there isn't.
> > 
> > I never received a response to this. Any comments?
> 
> Sorry to delay the reply, the only one that stands out to me as a
> possible problem with the devices that e1000 still supports is the
> NETIF_F_TSO6.

vlan_features is a mask for features.  So long as all parts that do
support TSO-IPv6 also support it for VLAN-tagged packets, it should be
safe to set NETIF_F_TSO6 in vlan_features unconditionally.

Ben.

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

Re: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 10 Oct 2008 18:59:36 +0100

> On Fri, 2008-10-10 at 10:48 -0700, Brandeburg, Jesse wrote:
> > Patrick McHardy wrote:
> > > Patrick McHardy wrote:
> > >> This patch changes e1000 to set vlan_features so TSO and CSUM
> > >> offload can be used by VLAN devices, similar as with the other Intel
> > >> drivers. 
> > >> 
> > >> Only RFC because I don't know whether there is buggy hardware
> > >> that doesn't support this properly, but feel free to apply in
> > >> case there isn't.
> > > 
> > > I never received a response to this. Any comments?
> > 
> > Sorry to delay the reply, the only one that stands out to me as a
> > possible problem with the devices that e1000 still supports is the
> > NETIF_F_TSO6.
> 
> vlan_features is a mask for features.  So long as all parts that do
> support TSO-IPv6 also support it for VLAN-tagged packets, it should be
> safe to set NETIF_F_TSO6 in vlan_features unconditionally.

Right, it's an additional mask which gets and'd with the normal
device feature bits.

Jesse, any objections, given that?

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Brandeburg, Jesse]

David Miller wrote:
>> vlan_features is a mask for features.  So long as all parts that do
>> support TSO-IPv6 also support it for VLAN-tagged packets, it should
>> be safe to set NETIF_F_TSO6 in vlan_features unconditionally.
> 
> Right, it's an additional mask which gets and'd with the normal
> device feature bits.
> 
> Jesse, any objections, given that?

No, once that is explained I think it gets rid of any objections.  We
are going to test the patch now, so would you mind if you waited to take
it until jkirsher pushes it to you?

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[David Miller]

From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Date: Fri, 10 Oct 2008 15:01:09 -0700

> David Miller wrote:
> >> vlan_features is a mask for features.  So long as all parts that do
> >> support TSO-IPv6 also support it for VLAN-tagged packets, it should
> >> be safe to set NETIF_F_TSO6 in vlan_features unconditionally.
> > 
> > Right, it's an additional mask which gets and'd with the normal
> > device feature bits.
> > 
> > Jesse, any objections, given that?
> 
> No, once that is explained I think it gets rid of any objections.  We
> are going to test the patch now, so would you mind if you waited to take
> it until jkirsher pushes it to you?

Sure, no problem.

RE: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Brandeburg, Jesse]

Brandeburg, Jesse wrote:
> Patrick McHardy wrote:
>> Patrick McHardy wrote:
>>> This patch changes e1000 to set vlan_features so TSO and CSUM
>>> offload can be used by VLAN devices, similar as with the other
>>> Intel drivers. 
>>> 
>>> Only RFC because I don't know whether there is buggy hardware
>>> that doesn't support this properly, but feel free to apply in
>>> case there isn't.
>> 
>> I never received a response to this. Any comments?
> 
> Sorry to delay the reply, the only one that stands out to me as a
> possible problem with the devices that e1000 still supports is the
> NETIF_F_TSO6.  
> 
> I know that some of our older parts like 82544 don't support TSO6.
> 
> As I read: these should work:
> 82546GB/EB, 82545GM/EM, 82541(PI/GI/EI), 82541ER, 82547GI/EI, and
> 82540EP/EM 
> 
> These parts should all work (at least as advertised in the software
> reference manual available at
>
http://superb-east.dl.sourceforge.net/sourceforge/e1000/OpenSDM_8254x-37
.pdf)
> 
> with the exception of 82544 GC/EI
> 
> So as it stands your patch will not work on some hardware.  Also,
> would we need to hook the ethtool methods such that if a user
> disables TSO it will disable vlan_features(TSO) as well?  

Oh, and the 82542 and 82543 based parts don't do this either, they
aren't covered by the manual above.

Re: [PATCH RFC]: e1000: allow VLAN devices to use TSO and CSUM offload

[Patrick McHardy]

Brandeburg, Jesse wrote:
>> These parts should all work (at least as advertised in the software
>> reference manual available at
>>
> http://superb-east.dl.sourceforge.net/sourceforge/e1000/OpenSDM_8254x-37
> .pdf)
>> with the exception of 82544 GC/EI
>>
>> So as it stands your patch will not work on some hardware.  Also,
>> would we need to hook the ethtool methods such that if a user
>> disables TSO it will disable vlan_features(TSO) as well?  
> 
> Oh, and the 82542 and 82543 based parts don't do this either, they
> aren't covered by the manual above.

Thanks for the pointer (and for pushing the patch already).