: e1000: allow VLAN devices to use TSO and CSUM offload

RFC PATCH In review Patrick McHardy 4 participants 9 messages Jul 16, 2008 – Oct 11, 2008

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Patrick McHardy

author

2008-07-16 10:05 UTC #0 lore raw

RFC PATCH 1/1 : e1000: allow VLAN devices to use TSO and CSUM offload

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.

commit 9648c6ef22e4f6e14ed5b5d91d9809779bb520c7

Author: Patrick McHardy <[email protected]>
Date:   Wed Jul 16 11:45:09 2008 +0200

e1000: allow VLAN devices to use TSO and CSUM offload

Signed-off-by Patrick McHardy <[email protected]>

drivers/net/e1000/e1000_main.c +5 -0

index f8df8bd..b6b3135 100644
@@ -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 */

Patrick McHardy

author

2008-10-10 14:39 UTC #1 lore raw

Quoted 7 lines · Patrick McHardy

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?

commit 9648c6ef22e4f6e14ed5b5d91d9809779bb520c7

Author: Patrick McHardy <[email protected]>
Date:   Wed Jul 16 11:45:09 2008 +0200

e1000: allow VLAN devices to use TSO and CSUM offload

Signed-off-by Patrick McHardy <[email protected]>

drivers/net/e1000/e1000_main.c +5 -0

index f8df8bd..b6b3135 100644
@@ -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 */

Brandeburg, Jesse

2008-10-10 17:49 UTC #2 lore raw

Quoted 10 lines · Patrick McHardy

Quoted 7 lines · Patrick McHardy
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

Ben Hutchings

2008-10-10 17:59 UTC #3 lore raw

Quoted 10 lines · Jesse

Quoted 9 lines · Patrick McHardy
Quoted 7 lines · Patrick McHardy
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.

Quoted 5 lines

Quoted 1 line
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.

Signature · 3 lines

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.

David Miller

maintainer

2008-10-10 19:09 UTC #4 lore raw

From: Ben Hutchings <[email protected]>
Date: Fri, 10 Oct 2008 18:59:36 +0100

Quoted 13 lines · Jesse

Quoted 12 lines
Quoted 10 lines · Patrick McHardy
Quoted 7 lines · Patrick McHardy
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?

Quoted 7 lines

Quoted 3 lines
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=/

Brandeburg, Jesse

2008-10-10 22:01 UTC #5 lore raw

Quoted 8 lines · David Miller

Quoted 3 lines
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?

David Miller

maintainer

2008-10-10 22:12 UTC #6 lore raw

From: "Brandeburg, Jesse" <[email protected]>
Date: Fri, 10 Oct 2008 15:01:09 -0700

Quoted 5 lines

Quoted 4 lines · David Miller
Quoted 3 lines
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.

Quoted 8 lines

Quoted 4 lines
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.

Brandeburg, Jesse

2008-10-10 17:54 UTC #7 lore raw

In reply to Patrick McHardy

Quoted 16 lines · Jesse

Quoted 10 lines · Patrick McHardy
Quoted 7 lines · Patrick McHardy
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.

Quoted 9 lines

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)

Quoted 6 lines

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.

Patrick McHardy

author

2008-10-11 14:35 UTC #8 lore raw

Quoted 5 lines · Jesse

Quoted 3 lines
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)

Quoted 8 lines

Quoted 5 lines
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).