| 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
| ||
|
Message-ID: <4E56DB6C.2020606@intel.com>
Date: Thu, 25 Aug 2011 16:31:56 -0700
From: Alexander Duyck <alexander.h.duyck@...el.com>
To: Ben Greear <greearb@...delatech.com>
CC: Alexander Duyck <alexander.duyck@...il.com>,
jeffrey.t.kirsher@...el.com, Jesse Gross <jesse@...ira.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags.
On 08/25/2011 11:51 AM, Ben Greear wrote:
> On 07/20/2011 11:35 PM, Alexander Duyck wrote:
>> On Wed, Jul 20, 2011 at 6:21 PM, Jeff Kirsher
>> <jeffrey.t.kirsher@...el.com> wrote:
>>> On Wed, 2011-07-20 at 17:27 -0700, Ben Greear wrote:
>>>> On 07/20/2011 05:18 PM, Jesse Gross wrote:
>>>>> On Thu, Feb 17, 2011 at 9:28 AM, Ben
>>>>> Greear<greearb@...delatech.com> wrote:
>>>>>> On 02/17/2011 03:04 AM, Jeff Kirsher wrote:
>>>>>>>
>>>>>>> On Thu, Feb 10, 2011 at 13:59,<greearb@...delatech.com> wrote:
>>>>>>>>
>>>>>>>> From: Ben Greear<greearb@...delatech.com>
>>>>>>>>
>>>>>>>> This allows the NIC to receive 1518 byte (not counting
>>>>>>>> FCS) packets when MTU is 1500, thus allowing 1500 MTU
>>>>>>>> VLAN frames to be received. Please note that no VLANs
>>>>>>>> were actually configured on the NIC...it was just acting
>>>>>>>> as pass-through device.
>>>>>>>>
>>>>>>>> Signed-off-by: Ben Greear<greearb@...delatech.com>
>>>>>>>> ---
>>>>>>>> :100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.c
>>>>>>>> drivers/net/igb/igb_main.c | 5 +++--
>>>>>>>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/igb/igb_main.c
>>>>>>>> b/drivers/net/igb/igb_main.c
>>>>>>>> index 58c665b..30c9cc6 100644
>>>>>>>> --- a/drivers/net/igb/igb_main.c
>>>>>>>> +++ b/drivers/net/igb/igb_main.c
>>>>>>>> @@ -2281,7 +2281,8 @@ static int __devinit igb_sw_init(struct
>>>>>>>> igb_adapter
>>>>>>>> *adapter)
>>>>>>>> adapter->rx_itr_setting = IGB_DEFAULT_ITR;
>>>>>>>> adapter->tx_itr_setting = IGB_DEFAULT_ITR;
>>>>>>>>
>>>>>>>> - adapter->max_frame_size = netdev->mtu + ETH_HLEN +
>>>>>>>> ETH_FCS_LEN;
>>>>>>>> + adapter->max_frame_size = (netdev->mtu + ETH_HLEN +
>>>>>>>> ETH_FCS_LEN
>>>>>>>> + + VLAN_HLEN);
>>>>>>>> adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
>>>>>>>>
>>>>>>>> spin_lock_init(&adapter->stats64_lock);
>>>>>>>> @@ -4303,7 +4304,7 @@ static int igb_change_mtu(struct net_device
>>>>>>>> *netdev, int new_mtu)
>>>>>>>> {
>>>>>>>> struct igb_adapter *adapter = netdev_priv(netdev);
>>>>>>>> struct pci_dev *pdev = adapter->pdev;
>>>>>>>> - int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
>>>>>>>> + int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN +
>>>>>>>> VLAN_HLEN;
>>>>>>>> u32 rx_buffer_len, i;
>>>>>>>>
>>>>>>>> if ((new_mtu< 68) || (max_frame>
>>>>>>>> MAX_JUMBO_FRAME_SIZE)) {
>>>>>>>
>>>>>>> While testing this patch, validation found that the patch
>>>>>>> reduces the
>>>>>>> maximum mtu size
>>>>>>> by 4 bytes (reduces it from 9216 to 9212). This is not a
>>>>>>> desired side
>>>>>>> effect of this patch.
>>>>>>
>>>>>> You could add handling for that case and have it act as it used
>>>>>> to when
>>>>>> new_mtu is greater than 9212?
>>>>>>
>>>>>> I tested e1000e and it worked w/out hacking at 1500 MTU, so maybe
>>>>>> check how it does it?
>>>>>
>>>>> I just wanted to bring this up again to see if any progress had been
>>>>> made. We were looking at this driver and trying to figure out the
>>>>> best way to convert it to use the new vlan model but I'm not familiar
>>>>
>>>> I've been watching :)
>>>>
>>>>> enough with the hardware to know. It seems that all of the other
>>>>> Intel drivers unconditionally add space for the vlan tag to the
>>>>> receive buffer (and would therefore have similar effects as this
>>>>> patch), is there something different about this card?
>>>>>
>>>>> I believe that Alex was working on something in this area (in the
>>>>> context of one of my patches from a long time ago) but I'm not sure
>>>>> what came of that.
>>>>
>>>> Truth is, I don't really see why it's a problem to decrease the
>>>> maximum MTU slightly in order to make it work with VLANs.
>>>>
>>>> I'm not sure if there is some way to make it work with VLANs
>>>> and not decrease the maximum MTU.
>>>
>>> This was the reason this did not get accepted. I was looking into what
>>> could be done so that we did not decease the maximum MTU, but I got
>>> side-tracked and have not done anything on it in several months.
>>>
>>
>> I can take a look at fixing this most likely tomorrow. I have some
>> work planned for igb anyway over the next few days.
>>
>> Odds are it is just a matter of where the VLAN_HLEN is added. As I
>> recall for our drivers the correct spot is in the setting of
>> rx_buffer_len since that is the area more concerned with maximum
>> receive frame size versus the mtu section which is more concerned with
>> the transmit side of things.
>
> Did a patch for this ever get posted? I'll be happy to test it
> if so...
>
> Thanks,
> Ben
>
We haven't posted one yet. I have one written up but it is currently
mixed in with a set of 30 patches that I am testing/cleaning
up/formatting before submitting to our formal validation team. I will
likely be submitting it to Jeff Kirsher sometime next week and the
patches will probably be available a few weeks after that.
Thanks,
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
Powered by blists - more mailing lists