On Wed, Jun 22, 2016 at 4:52 PM, Rick Jones <rick.jon...@hpe.com> wrote: > On 06/22/2016 03:56 PM, Alexander Duyck wrote: >> >> On Wed, Jun 22, 2016 at 3:47 PM, Eric Dumazet <eric.duma...@gmail.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
