| Message ID | 20150408204630.4643.37880.stgit@ahduyck-vm-fedora22 |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 8 Apr 2015, Alexander Duyck wrote: > Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when changing interface MTU") > Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> > --- > > I have only build tested this though I am 99% sure the fixes here are > correct. This patch should fix issues on 82573 and ich8 w/ setting an MTU > of 1500, and for the PCH series w/ setting an MTU of 9000. Since the original fix was something submitted by Red Hat, can you check that you're not re-breaking whatever it was that Red Hat thought they were fixing? -- 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 04/08/2015 02:15 PM, Hisashi T Fujinaka wrote: > On Wed, 8 Apr 2015, Alexander Duyck wrote: > >> Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when >> changing interface MTU") >> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> >> --- >> >> I have only build tested this though I am 99% sure the fixes here are >> correct. This patch should fix issues on 82573 and ich8 w/ setting >> an MTU >> of 1500, and for the PCH series w/ setting an MTU of 9000. > > Since the original fix was something submitted by Red Hat, can you check > that you're not re-breaking whatever it was that Red Hat thought they > were fixing? The original issue is referenced in the patch that this fixes. The problem was that the VLAN header wasn't being considered when computing the Rx buffer size, so you could change the MTU to 1504 and the if statement at the end of e1000_change_mtu was still using a 1522 Rx buffer size and max frame even though we had technically just configured things for 1526. The updated logic is correctly taking the VLAN header into account so if you bump the MTU 1504 it will switch over to jumbo frames mode w/ 2K buffers. The bit I am fixing is that there were several spots including the backend value for max_hw_frame_size that didn't take VLAN header length into account. There were cases where 1500 MTU was being treated as a jumbo frame, or we were coming up 4 bytes shy as in the pch2, ich8, and 82573 e1000_info structures. - Alex -- 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 Wed, 8 Apr 2015, Alexander Duyck wrote: > On 04/08/2015 02:15 PM, Hisashi T Fujinaka wrote: >> On Wed, 8 Apr 2015, Alexander Duyck wrote: >> >>> Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when >>> changing interface MTU") >>> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> >>> --- >>> >>> I have only build tested this though I am 99% sure the fixes here are >>> correct. This patch should fix issues on 82573 and ich8 w/ setting >>> an MTU >>> of 1500, and for the PCH series w/ setting an MTU of 9000. >> >> Since the original fix was something submitted by Red Hat, can you check >> that you're not re-breaking whatever it was that Red Hat thought they >> were fixing? > > The original issue is referenced in the patch that this fixes. The > problem was that the VLAN header wasn't being considered when computing > the Rx buffer size, so you could change the MTU to 1504 and the if > statement at the end of e1000_change_mtu was still using a 1522 Rx > buffer size and max frame even though we had technically just configured > things for 1526. > > The updated logic is correctly taking the VLAN header into account so if > you bump the MTU 1504 it will switch over to jumbo frames mode w/ 2K > buffers. > > The bit I am fixing is that there were several spots including the > backend value for max_hw_frame_size that didn't take VLAN header length > into account. There were cases where 1500 MTU was being treated as a > jumbo frame, or we were coming up 4 bytes shy as in the pch2, ich8, and > 82573 e1000_info structures. The max_hw_frame_size should still be limited to 9018. -- 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 04/08/2015 04:05 PM, Hisashi T Fujinaka wrote: > On Wed, 8 Apr 2015, Alexander Duyck wrote: > >> On 04/08/2015 02:15 PM, Hisashi T Fujinaka wrote: >>> On Wed, 8 Apr 2015, Alexander Duyck wrote: >>> >>>> Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when >>>> changing interface MTU") >>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> >>>> --- >>>> >>>> I have only build tested this though I am 99% sure the fixes here are >>>> correct. This patch should fix issues on 82573 and ich8 w/ setting >>>> an MTU >>>> of 1500, and for the PCH series w/ setting an MTU of 9000. >>> >>> Since the original fix was something submitted by Red Hat, can you >>> check >>> that you're not re-breaking whatever it was that Red Hat thought they >>> were fixing? >> >> The original issue is referenced in the patch that this fixes. The >> problem was that the VLAN header wasn't being considered when computing >> the Rx buffer size, so you could change the MTU to 1504 and the if >> statement at the end of e1000_change_mtu was still using a 1522 Rx >> buffer size and max frame even though we had technically just configured >> things for 1526. >> >> The updated logic is correctly taking the VLAN header into account so if >> you bump the MTU 1504 it will switch over to jumbo frames mode w/ 2K >> buffers. >> >> The bit I am fixing is that there were several spots including the >> backend value for max_hw_frame_size that didn't take VLAN header length >> into account. There were cases where 1500 MTU was being treated as a >> jumbo frame, or we were coming up 4 bytes shy as in the pch2, ich8, and >> 82573 e1000_info structures. > > The max_hw_frame_size should still be limited to 9018. It is but it isn't. If you look in e1000_change_mtu you will find the node about "Jumbo frame workaround on 82579 and newer requires CRC be stripped". With that being the case I'm wondering if the 9018 doesn't include CRC but instead includes VLAN header. So as a result the actual hardware is processing frames that are 9022 in size, but the buffer only ever receives at most 9018 since the CRC is stripped. I suspect that is why the Windows driver has had no issues reporting support for a size of 9014 (excluding VLAN and CRC) on these parts and hasn't had any issues. - Alex -- 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 Wed, 2015-04-08 at 14:02 -0700, Alexander Duyck wrote: > When the VLAN_HLEN was added to the calculation for the maximum frame > size > there seems to have been a number of issues added to the driver. > > The first issue is that in some cases the maximum frame size for a > device > never really reached the actual maximum frame size as the VLAN header > length was not included the calculation for that value. As a result > some > parts only supported a maximum frame size of either 1496 in the case > of > parts that didn't support jumbo frames, and 8996 in the case of the > parts > that do. > > The second issue is the fact that there were several checks that > weren't > updated so as a result setting an MTU of 1500 was treated as enabling > jumbo > frames as the calculated value was 1522 instead of 1518. I have > addressed > those by replacing ETH_FRAME_LEN with VLAN_ETH_FRAME_LEN where > appropriate. > > The final issue was the fact that lowering the MTU below 1500 would > cause > the driver to allocate 2K buffers for the rings. This is an old issue > that > was fixed several years ago in igb/ixgbe and I am addressing now by > just > replacing == with a <= so that we always just round up to 1522 for > anything > that isn't a jumbo frame. > > Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when > changing interface MTU") > Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> > --- > > I have only build tested this though I am 99% sure the fixes here are > correct. This patch should fix issues on 82573 and ich8 w/ setting an > MTU > of 1500, and for the PCH series w/ setting an MTU of 9000. > > I assume I can get away with bumping the max_hw_frame_size for the PCH > parts from 9018 to 9022 based on the fact that the Windows INF for the > parts > lists supporting either 1514, 4088, and 9014 all of which exclude the > 8 > bytes for CRC and VLAN header. > > drivers/net/ethernet/intel/e1000e/82571.c | 2 +- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 10 +++++----- > drivers/net/ethernet/intel/e1000e/netdev.c | 18 ++++++++---------- > 3 files changed, 14 insertions(+), 16 deletions(-) I have applied your patch to my queue, thanks Alex.
On Wed, 8 Apr 2015, Alexander Duyck wrote: > On 04/08/2015 04:05 PM, Hisashi T Fujinaka wrote: >> On Wed, 8 Apr 2015, Alexander Duyck wrote: >> >>> On 04/08/2015 02:15 PM, Hisashi T Fujinaka wrote: >>>> On Wed, 8 Apr 2015, Alexander Duyck wrote: >>>> >>>>> Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when >>>>> changing interface MTU") >>>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> >>>>> --- >>>>> >>>>> I have only build tested this though I am 99% sure the fixes here are >>>>> correct. This patch should fix issues on 82573 and ich8 w/ setting >>>>> an MTU >>>>> of 1500, and for the PCH series w/ setting an MTU of 9000. >>>> >>>> Since the original fix was something submitted by Red Hat, can you >>>> check >>>> that you're not re-breaking whatever it was that Red Hat thought they >>>> were fixing? >>> >>> The original issue is referenced in the patch that this fixes. The >>> problem was that the VLAN header wasn't being considered when computing >>> the Rx buffer size, so you could change the MTU to 1504 and the if >>> statement at the end of e1000_change_mtu was still using a 1522 Rx >>> buffer size and max frame even though we had technically just configured >>> things for 1526. >>> >>> The updated logic is correctly taking the VLAN header into account so if >>> you bump the MTU 1504 it will switch over to jumbo frames mode w/ 2K >>> buffers. >>> >>> The bit I am fixing is that there were several spots including the >>> backend value for max_hw_frame_size that didn't take VLAN header length >>> into account. There were cases where 1500 MTU was being treated as a >>> jumbo frame, or we were coming up 4 bytes shy as in the pch2, ich8, and >>> 82573 e1000_info structures. >> >> The max_hw_frame_size should still be limited to 9018. > > It is but it isn't. If you look in e1000_change_mtu you will find the > node about "Jumbo frame workaround on 82579 and newer requires CRC be > stripped". With that being the case I'm wondering if the 9018 doesn't > include CRC but instead includes VLAN header. So as a result the actual > hardware is processing frames that are 9022 in size, but the buffer only > ever receives at most 9018 since the CRC is stripped. > > I suspect that is why the Windows driver has had no issues reporting > support for a size of 9014 (excluding VLAN and CRC) on these parts and > hasn't had any issues. I can only tell you what was told to me by Dave Ertman, which is that there is a hard hardware limit of 9018 bytes. I wouldn't know if we do have Windows issues because it's a completely different division and there's no reason for any of those issues to be routed to us. In fact, the problem with different max MTU across the drivers in Linux has only ever been reported by one user. I'm still looking for the HW documentation and would like Jeff to hold off until we find it. -- 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 Wed, 8 Apr 2015, Hisashi T Fujinaka wrote: > On Wed, 8 Apr 2015, Alexander Duyck wrote: >> >> It is but it isn't. If you look in e1000_change_mtu you will find the >> node about "Jumbo frame workaround on 82579 and newer requires CRC be >> stripped". With that being the case I'm wondering if the 9018 doesn't >> include CRC but instead includes VLAN header. So as a result the actual >> hardware is processing frames that are 9022 in size, but the buffer only >> ever receives at most 9018 since the CRC is stripped. >> >> I suspect that is why the Windows driver has had no issues reporting >> support for a size of 9014 (excluding VLAN and CRC) on these parts and >> hasn't had any issues. > > I can only tell you what was told to me by Dave Ertman, which is that > there is a hard hardware limit of 9018 bytes. I wouldn't know if we do > have Windows issues because it's a completely different division and > there's no reason for any of those issues to be routed to us. > > In fact, the problem with different max MTU across the drivers in Linux > has only ever been reported by one user. > > I'm still looking for the HW documentation and would like Jeff to hold > off until we find it. OK. So the data sheet states: LPE controls whether long packet reception is permitted. Hardware discards long packets if LPE is 0. A long packet is one longer than 1522 bytes. If LPE is 1, the maximum packet size that the device can receive is 9018 bytes. So if you're pre-stripping the CRC, then it will be less than 9018. I guess I'd like to hear what Dave thinks. -- 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 04/08/2015 05:26 PM, Hisashi T Fujinaka wrote: > On Wed, 8 Apr 2015, Hisashi T Fujinaka wrote: > >> On Wed, 8 Apr 2015, Alexander Duyck wrote: >>> >>> It is but it isn't. If you look in e1000_change_mtu you will find the >>> node about "Jumbo frame workaround on 82579 and newer requires CRC be >>> stripped". With that being the case I'm wondering if the 9018 doesn't >>> include CRC but instead includes VLAN header. So as a result the >>> actual >>> hardware is processing frames that are 9022 in size, but the buffer >>> only >>> ever receives at most 9018 since the CRC is stripped. >>> >>> I suspect that is why the Windows driver has had no issues reporting >>> support for a size of 9014 (excluding VLAN and CRC) on these parts and >>> hasn't had any issues. >> >> I can only tell you what was told to me by Dave Ertman, which is that >> there is a hard hardware limit of 9018 bytes. I wouldn't know if we do >> have Windows issues because it's a completely different division and >> there's no reason for any of those issues to be routed to us. >> >> In fact, the problem with different max MTU across the drivers in Linux >> has only ever been reported by one user. >> >> I'm still looking for the HW documentation and would like Jeff to hold >> off until we find it. > > OK. So the data sheet states: > > LPE controls whether long packet reception is permitted. Hardware > discards long packets if LPE is 0. A long packet is one longer than 1522 > bytes. If LPE is 1, the maximum packet size that the device can receive > is 9018 bytes. Yeah, that is mostly clear except it doesn't indicate if that includes or excludes the CRC and/or VLAN header. All the documentation I can find seems to indicate it is likely skipping one or the other since it recommends setting any switches to support 9022 in order to account for both. > So if you're pre-stripping the CRC, then it will be less than 9018. Right so the argument is probably moot since you cannot enable jumbo frames unless CRC stripping is enabled. Ugh, but it looks like nothing prevents disabling CRC stripping once jumbo frames has been enabled. I'll patch that shortly. > I guess I'd like to hear what Dave thinks. The problem is this pre-dates when Dave Ertman started working on e1000e. All of this went into effect back during the introduction of 82579 and i217/i218 and the two patches from Bruce that reduced the size down to 9018 didn't specify where the number came from. I suspect it is the same data sheet we have been looking at which is a bit vague about what is included in that size. -- 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 04/08/2015 04:02 PM, Alexander Duyck wrote: > When the VLAN_HLEN was added to the calculation for the maximum frame size > there seems to have been a number of issues added to the driver. > > The first issue is that in some cases the maximum frame size for a device > never really reached the actual maximum frame size as the VLAN header > length was not included the calculation for that value. As a result some > parts only supported a maximum frame size of either 1496 in the case of > parts that didn't support jumbo frames, and 8996 in the case of the parts > that do. > > The second issue is the fact that there were several checks that weren't > updated so as a result setting an MTU of 1500 was treated as enabling jumbo > frames as the calculated value was 1522 instead of 1518. I have addressed > those by replacing ETH_FRAME_LEN with VLAN_ETH_FRAME_LEN where appropriate. > > The final issue was the fact that lowering the MTU below 1500 would cause > the driver to allocate 2K buffers for the rings. This is an old issue that > was fixed several years ago in igb/ixgbe and I am addressing now by just > replacing == with a <= so that we always just round up to 1522 for anything > that isn't a jumbo frame. Alex, Thanks taking the time to work on a patch. I have tested this patch in 4.0 on i218-v hardware and it is working. I see 9000 in tcpdump (tso/gso off) and my switch blocks packets if I set the max frame size to 9017 down from 9018. I also played with a VLAN real quick, and did not encounter any problems, but I don't normally use VLANs so it may need further testing. Thanks, Michael -- 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
We will look into this with the HW architect and get back to you on what the HW limitations actually are on this issue. -----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Alexander Duyck Sent: Thursday, April 09, 2015 04:19 To: Hisashi T Fujinaka Cc: netdev@vger.kernel.org; mike@cchtml.com; intel-wired-lan@lists.osuosl.org; Ertman, David M Subject: Re: [Intel-wired-lan] [PATCH] e1000e: Cleanup handling of VLAN_HLEN as a part of max frame size On 04/08/2015 05:26 PM, Hisashi T Fujinaka wrote: > On Wed, 8 Apr 2015, Hisashi T Fujinaka wrote: > >> On Wed, 8 Apr 2015, Alexander Duyck wrote: >>> >>> It is but it isn't. If you look in e1000_change_mtu you will find the >>> node about "Jumbo frame workaround on 82579 and newer requires CRC be >>> stripped". With that being the case I'm wondering if the 9018 doesn't >>> include CRC but instead includes VLAN header. So as a result the >>> actual >>> hardware is processing frames that are 9022 in size, but the buffer >>> only >>> ever receives at most 9018 since the CRC is stripped. >>> >>> I suspect that is why the Windows driver has had no issues reporting >>> support for a size of 9014 (excluding VLAN and CRC) on these parts and >>> hasn't had any issues. >> >> I can only tell you what was told to me by Dave Ertman, which is that >> there is a hard hardware limit of 9018 bytes. I wouldn't know if we do >> have Windows issues because it's a completely different division and >> there's no reason for any of those issues to be routed to us. >> >> In fact, the problem with different max MTU across the drivers in Linux >> has only ever been reported by one user. >> >> I'm still looking for the HW documentation and would like Jeff to hold >> off until we find it. > > OK. So the data sheet states: > > LPE controls whether long packet reception is permitted. Hardware > discards long packets if LPE is 0. A long packet is one longer than 1522 > bytes. If LPE is 1, the maximum packet size that the device can receive > is 9018 bytes. Yeah, that is mostly clear except it doesn't indicate if that includes or excludes the CRC and/or VLAN header. All the documentation I can find seems to indicate it is likely skipping one or the other since it recommends setting any switches to support 9022 in order to account for both. > So if you're pre-stripping the CRC, then it will be less than 9018. Right so the argument is probably moot since you cannot enable jumbo frames unless CRC stripping is enabled. Ugh, but it looks like nothing prevents disabling CRC stripping once jumbo frames has been enabled. I'll patch that shortly. > I guess I'd like to hear what Dave thinks. The problem is this pre-dates when Dave Ertman started working on e1000e. All of this went into effect back during the introduction of 82579 and i217/i218 and the two patches from Bruce that reduced the size down to 9018 didn't specify where the number came from. I suspect it is the same data sheet we have been looking at which is a bit vague about what is included in that size.
On 04/08/2015 11:17 PM, Templeman, Chaim wrote:
> We will look into this with the HW architect and get back to you on what the HW limitations actually are on this issue.
Chaim, thanks for looking into this. The key piece that would be useful
to know is if the 9018 should really be 9022 if you account for both
VLAN header and CRC. Based on the documentation for the Windows driver
that seems to be the case as they support an MTU of 9014 excluding the 8
bytes for CRC and VLAN header.
- Alex
--
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
I checked with the HW guys, and they confirmed that a total packet size of 9022 is acceptable. The original size of 9018 was defined as: Frame size (9018) = DST (6) + SRC (6) + VLAN (4) + TYPE (2) + MTU (x) + CRC (4). If an additional 4 bytes is required, that is ok. -----Original Message----- From: Alexander Duyck [mailto:alexander.duyck@gmail.com] Sent: Thursday, April 09, 2015 18:51 To: Templeman, Chaim; Hisashi T Fujinaka Cc: netdev@vger.kernel.org; mike@cchtml.com; intel-wired-lan@lists.osuosl.org; Ertman, David M; Lubetkin, YanirX Subject: Re: [Intel-wired-lan] [PATCH] e1000e: Cleanup handling of VLAN_HLEN as a part of max frame size On 04/08/2015 11:17 PM, Templeman, Chaim wrote: > We will look into this with the HW architect and get back to you on what the HW limitations actually are on this issue. Chaim, thanks for looking into this. The key piece that would be useful to know is if the 9018 should really be 9022 if you account for both VLAN header and CRC. Based on the documentation for the Windows driver that seems to be the case as they support an MTU of 9014 excluding the 8 bytes for CRC and VLAN header. - Alex --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- 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: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On > Behalf Of Alexander Duyck > Sent: Wednesday, April 08, 2015 2:03 PM > To: intel-wired-lan@lists.osuosl.org; Kirsher, Jeffrey T > Cc: netdev@vger.kernel.org; mike@cchtml.com; htodd@twofifty.com > Subject: [Intel-wired-lan] [PATCH] e1000e: Cleanup handling of VLAN_HLEN > as a part of max frame size > > When the VLAN_HLEN was added to the calculation for the maximum frame size > there seems to have been a number of issues added to the driver. > > The first issue is that in some cases the maximum frame size for a device > never really reached the actual maximum frame size as the VLAN header > length was not included the calculation for that value. As a result some > parts only supported a maximum frame size of either 1496 in the case of > parts that didn't support jumbo frames, and 8996 in the case of the parts > that do. > > The second issue is the fact that there were several checks that weren't > updated so as a result setting an MTU of 1500 was treated as enabling > jumbo > frames as the calculated value was 1522 instead of 1518. I have addressed > those by replacing ETH_FRAME_LEN with VLAN_ETH_FRAME_LEN where > appropriate. > > The final issue was the fact that lowering the MTU below 1500 would cause > the driver to allocate 2K buffers for the rings. This is an old issue > that > was fixed several years ago in igb/ixgbe and I am addressing now by just > replacing == with a <= so that we always just round up to 1522 for > anything > that isn't a jumbo frame. > > Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when changing > interface MTU") > Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> > --- > > I have only build tested this though I am 99% sure the fixes here are > correct. This patch should fix issues on 82573 and ich8 w/ setting an MTU > of 1500, and for the PCH series w/ setting an MTU of 9000. > > I assume I can get away with bumping the max_hw_frame_size for the PCH > parts from 9018 to 9022 based on the fact that the Windows INF for the > parts > lists supporting either 1514, 4088, and 9014 all of which exclude the 8 > bytes for CRC and VLAN header. > > drivers/net/ethernet/intel/e1000e/82571.c | 2 +- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 10 +++++----- > drivers/net/ethernet/intel/e1000e/netdev.c | 18 ++++++++---------- > 3 files changed, 14 insertions(+), 16 deletions(-) Tested-by: Aaron Brown <aaron.f.brown@intel.com> -- 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/ethernet/intel/e1000e/82571.c b/drivers/net/ethernet/intel/e1000e/82571.c index dc79ed85030b..32e77755a9c6 100644 --- a/drivers/net/ethernet/intel/e1000e/82571.c +++ b/drivers/net/ethernet/intel/e1000e/82571.c @@ -2010,7 +2010,7 @@ const struct e1000_info e1000_82573_info = { .flags2 = FLAG2_DISABLE_ASPM_L1 | FLAG2_DISABLE_ASPM_L0S, .pba = 20, - .max_hw_frame_size = ETH_FRAME_LEN + ETH_FCS_LEN, + .max_hw_frame_size = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN, .get_variants = e1000_get_variants_82571, .mac_ops = &e82571_mac_ops, .phy_ops = &e82_phy_ops_m88, diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c index 9d81c0317433..e2498dbf3c3b 100644 --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c @@ -1563,7 +1563,7 @@ static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter) ((adapter->hw.mac.type >= e1000_pch2lan) && (!(er32(CTRL_EXT) & E1000_CTRL_EXT_LSECCK)))) { adapter->flags &= ~FLAG_HAS_JUMBO_FRAMES; - adapter->max_hw_frame_size = ETH_FRAME_LEN + ETH_FCS_LEN; + adapter->max_hw_frame_size = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN; hw->mac.ops.blink_led = NULL; } @@ -5681,7 +5681,7 @@ const struct e1000_info e1000_ich8_info = { | FLAG_HAS_FLASH | FLAG_APME_IN_WUC, .pba = 8, - .max_hw_frame_size = ETH_FRAME_LEN + ETH_FCS_LEN, + .max_hw_frame_size = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN, .get_variants = e1000_get_variants_ich8lan, .mac_ops = &ich8_mac_ops, .phy_ops = &ich8_phy_ops, @@ -5754,7 +5754,7 @@ const struct e1000_info e1000_pch2_info = { .flags2 = FLAG2_HAS_PHY_STATS | FLAG2_HAS_EEE, .pba = 26, - .max_hw_frame_size = 9018, + .max_hw_frame_size = 9022, .get_variants = e1000_get_variants_ich8lan, .mac_ops = &ich8_mac_ops, .phy_ops = &ich8_phy_ops, @@ -5774,7 +5774,7 @@ const struct e1000_info e1000_pch_lpt_info = { .flags2 = FLAG2_HAS_PHY_STATS | FLAG2_HAS_EEE, .pba = 26, - .max_hw_frame_size = 9018, + .max_hw_frame_size = 9022, .get_variants = e1000_get_variants_ich8lan, .mac_ops = &ich8_mac_ops, .phy_ops = &ich8_phy_ops, @@ -5794,7 +5794,7 @@ const struct e1000_info e1000_pch_spt_info = { .flags2 = FLAG2_HAS_PHY_STATS | FLAG2_HAS_EEE, .pba = 26, - .max_hw_frame_size = 9018, + .max_hw_frame_size = 9022, .get_variants = e1000_get_variants_ich8lan, .mac_ops = &ich8_mac_ops, .phy_ops = &ich8_phy_ops, diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 74ec185a697f..f77db9304060 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -3807,7 +3807,7 @@ void e1000e_reset(struct e1000_adapter *adapter) /* reset Packet Buffer Allocation to default */ ew32(PBA, pba); - if (adapter->max_frame_size > ETH_FRAME_LEN + ETH_FCS_LEN) { + if (adapter->max_frame_size > (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) { /* To maintain wire speed transmits, the Tx FIFO should be * large enough to accommodate two full transmit packets, * rounded up to the next 1KB and expressed in KB. Likewise, @@ -4196,9 +4196,9 @@ static int e1000_sw_init(struct e1000_adapter *adapter) { struct net_device *netdev = adapter->netdev; - adapter->rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN; + adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN; adapter->rx_ps_bsize0 = 128; - adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN; + adapter->max_frame_size = netdev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN; adapter->tx_ring_count = E1000_DEFAULT_TXD; adapter->rx_ring_count = E1000_DEFAULT_RXD; @@ -5781,17 +5781,17 @@ struct rtnl_link_stats64 *e1000e_get_stats64(struct net_device *netdev, static int e1000_change_mtu(struct net_device *netdev, int new_mtu) { struct e1000_adapter *adapter = netdev_priv(netdev); - int max_frame = new_mtu + VLAN_HLEN + ETH_HLEN + ETH_FCS_LEN; + int max_frame = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; /* Jumbo frame support */ - if ((max_frame > ETH_FRAME_LEN + ETH_FCS_LEN) && + if ((max_frame > (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) && !(adapter->flags & FLAG_HAS_JUMBO_FRAMES)) { e_err("Jumbo Frames not supported.\n"); return -EINVAL; } /* Supported frame sizes */ - if ((new_mtu < ETH_ZLEN + ETH_FCS_LEN + VLAN_HLEN) || + if ((new_mtu < (VLAN_ETH_ZLEN + ETH_FCS_LEN)) || (max_frame > adapter->max_hw_frame_size)) { e_err("Unsupported MTU setting\n"); return -EINVAL; @@ -5831,10 +5831,8 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu) adapter->rx_buffer_len = 4096; /* adjust allocation if LPE protects us, and we aren't using SBP */ - if ((max_frame == ETH_FRAME_LEN + ETH_FCS_LEN) || - (max_frame == ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN)) - adapter->rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN - + ETH_FCS_LEN; + if (max_frame <= (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) + adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN; if (netif_running(netdev)) e1000e_up(adapter);
When the VLAN_HLEN was added to the calculation for the maximum frame size there seems to have been a number of issues added to the driver. The first issue is that in some cases the maximum frame size for a device never really reached the actual maximum frame size as the VLAN header length was not included the calculation for that value. As a result some parts only supported a maximum frame size of either 1496 in the case of parts that didn't support jumbo frames, and 8996 in the case of the parts that do. The second issue is the fact that there were several checks that weren't updated so as a result setting an MTU of 1500 was treated as enabling jumbo frames as the calculated value was 1522 instead of 1518. I have addressed those by replacing ETH_FRAME_LEN with VLAN_ETH_FRAME_LEN where appropriate. The final issue was the fact that lowering the MTU below 1500 would cause the driver to allocate 2K buffers for the rings. This is an old issue that was fixed several years ago in igb/ixgbe and I am addressing now by just replacing == with a <= so that we always just round up to 1522 for anything that isn't a jumbo frame. Fixes: c751a3d58cf2d ("e1000e: Correctly include VLAN_HLEN when changing interface MTU") Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com> --- I have only build tested this though I am 99% sure the fixes here are correct. This patch should fix issues on 82573 and ich8 w/ setting an MTU of 1500, and for the PCH series w/ setting an MTU of 9000. I assume I can get away with bumping the max_hw_frame_size for the PCH parts from 9018 to 9022 based on the fact that the Windows INF for the parts lists supporting either 1514, 4088, and 9014 all of which exclude the 8 bytes for CRC and VLAN header. drivers/net/ethernet/intel/e1000e/82571.c | 2 +- drivers/net/ethernet/intel/e1000e/ich8lan.c | 10 +++++----- drivers/net/ethernet/intel/e1000e/netdev.c | 18 ++++++++---------- 3 files changed, 14 insertions(+), 16 deletions(-) -- 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