netdev - Re: [PATCH net-next 0/5] qed/qede: Tunnel hardware GRO support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAKgT0UcG_QnxXJs9GU7MwgBtUNY4KNKqzPiRKHoRJYJsTaUZ6g@mail.gmail.com>
Date:	Wed, 22 Jun 2016 17:18:51 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Rick Jones <rick.jones2@....com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Yuval Mintz <Yuval.Mintz@...gic.com>,
	Manish Chopra <manish.chopra@...gic.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Ariel Elior <Ariel.Elior@...gic.com>,
	Tom Herbert <tom@...bertland.com>,
	Hannes Frederic Sowa <hannes@...hat.com>
Subject: Re: [PATCH net-next 0/5] qed/qede: Tunnel hardware GRO support

On Wed, Jun 22, 2016 at 4:52 PM, Rick Jones <rick.jones2@....com> wrote:
> On 06/22/2016 03:56 PM, Alexander Duyck wrote:
>>
>> On Wed, Jun 22, 2016 at 3:47 PM, Eric Dumazet <eric.dumazet@...il.com>
>> wrote:
>>>
>>> On Wed, 2016-06-22 at 14:52 -0700, Rick Jones wrote:
>>>>
>>>> Had the bnx2x-driven NICs' firmware not had that rather unfortunate
>>>> assumption about MSSes I probably would never have noticed.
>
>
>
>> It could be that you and Rick are running different firmware. I
>> believe you can expose that via "ethtool -i".  This is the ugly bit
>> about all this.  We are offloading GRO into the firmware of these
>> devices with no idea how any of it works and by linking GRO to LRO on
>> the same device you are stuck having to accept either the firmware
>> offload or nothing at all.  That is kind of the point Rick was trying
>> to get at.
>
>
> I think you are typing a bit too far ahead into my keyboard with that last
> sentence.  And I may not have been sufficiently complete in what I wrote.
> If the bnx2x-driven NICs' firmware had been coalescing more than two
> segments together, not only would I probably not have noticed, I probably
> would not have been upset to learn it was NIC-firmware GRO rather than
> stack.
>
> My complaint is the specific bug of coalescing only two segments when their
> size is unexpected, and the difficulty present in disabling the bnx2x-driven
> NICs' firmware GRO.  I don't have a problem necessarily with the existence
> of NIC-firmware GRO in general.  I just want to be able to enable/disable it
> easily.

Right.  Which is why I thought we were supposed to be using the LRO
flag when a NIC is performing the GRO instead of it being done by the
kernel.

I have no problem with us doing hardware GRO.  The only issue I have
is with us identifying it as GRO.

In my opinion the way I would have preferred to have seen the bnx2x
handle all this is to make it that the NETIF_F_LRO flag controlled if
the NIC performed aggregation or not and the module parameter
determine if the LRO conforms to the GRO type layout for the frame.

As it is now I can easily see this causing issues if someone updates
how GRO/GSO handles frames without realizing they now need to make
sure the bnx2x and qede drivers need to generate the same frame layout
as well.

- Alex