netdev - Re: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO

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]

Date:   Mon, 04 Dec 2017 11:26:24 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Miller <davem@...emloft.net>, alexander.duyck@...il.com
Cc:     michael.chan@...adcom.com, netdev@...r.kernel.org,
        Ariel.Elior@...ium.com, everest-linux-l2@...ium.com
Subject: Re: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO_HW

On Mon, 2017-12-04 at 13:59 -0500, David Miller wrote:
> From: Alexander Duyck <alexander.duyck@...il.com>
> Date: Mon, 4 Dec 2017 10:43:58 -0800
> 
> > On Mon, Dec 4, 2017 at 10:23 AM, Michael Chan <michael.chan@...adco
> m.com> wrote:
> >> On Mon, Dec 4, 2017 at 8:47 AM, Alexander Duyck
> >> <alexander.duyck@...il.com> wrote:
> >>> On Mon, Dec 4, 2017 at 3:12 AM, Michael Chan <michael.chan@...adc
> om.com> wrote:
> >>>> Introduce NETIF_F_GRO_HW feature flag for NICs that support
> hardware
> >>>> GRO.  With this flag, we can now independently turn on or off
> hardware
> >>>> GRO when GRO is on.  Hardware GRO guarantees that packets can be
> >>>> re-segmented by TSO/GSO to reconstruct the original packet
> stream.
> >>>>
> >>>> Cc: Ariel Elior <Ariel.Elior@...ium.com>
> >>>> Cc: everest-linux-l2@...ium.com
> >>>> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> >>>
> >>> Do we really need yet another feature bit for this? We already
> have
> >>> LRO and GRO and now we have to add something that isn't quite
> either
> >>> one?
> >>
> >> I think so, to be consistent with TSO/GSO on the transmit side. 
> On
> >> the receive side, we have LRO/GRO_HW/GRO.  There is difference
> between
> >> LRO/GRO_HW that we need to distinguish between the 2.
> > 
> > I don't really see the difference. Your GRO_HW likely doens't do
> all
> > of the stuff GRO can do. Neither does LRO. Both occur in the
> hardware
> > normally. It would make sense to reuse the LRO flag for this
> instead
> > of coming up with a new feature flag that makes things confusing by
> > saying you are doing a software offload in hardware.
> > 
> > I view LRO as a subset of what GRO can handle, that is performed in
> > hardware. From the stack perspective the only thing that really
> > matters is that the frames can be segmented back into what they
> were
> > before they were assembled. That is why I think it would be better
> to
> > add a flag indicating that the LRO is reversible instead of adding
> yet
> > another feature bit that the user has to toggle. That way if at
> some
> > point in the future an issue is found where your "GRO in hardware"
> > feature has a bug that isn't reversible it is just a matter of
> > clearing the privage flag bit and the mechanisms already in place
> for
> > dealing with assembly and routing can take over.
> 
> I don't think they should use the LRO flag.
> 
> If their HW GRO stream is fully reversible, which it is, then it's
> not
> LRO.
> 
> LRO gets disabled when bridging or routing is enabled, and HW GRO
> should not take this penalty.

Also having separate flags means that one can decide to disable HW GRO
and enable (linux) GRO if he wants to. Or the opposite.

I definitely like this idea.