Message ID | 48EF690F.5070105@trash.net |
---|---|
State | Awaiting Upstream, archived |
Delegated to: | David Miller |
Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */