netdev - Re: [PATCH] e1000e: Cleanup handling of VLAN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5525B62A.1050601@gmail.com>
Date:	Wed, 08 Apr 2015 16:13:46 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Hisashi T Fujinaka <htodd@...fifty.com>
CC:	Alexander Duyck <alexander.h.duyck@...hat.com>,
	intel-wired-lan@...ts.osuosl.org, jeffrey.t.kirsher@...el.com,
	netdev@...r.kernel.org, mike@...tml.com
Subject: Re: [PATCH] e1000e: Cleanup handling of VLAN_HLEN as a part of max
 frame size

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@...hat.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html