netdev - Re: [RFC PATCH v2 00/10] udp: implement 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: <e1b77cd6d8fb66d5e99cad6438d64b6cd9ac4205.camel@redhat.com>
Date:   Mon, 22 Oct 2018 11:41:35 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        steffen.klassert@...unet.com
Subject: Re: [RFC PATCH v2 00/10] udp: implement GRO support

Hi all,

On Sun, 2018-10-21 at 16:05 -0400, Willem de Bruijn wrote:
> On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > 
> > This series implements GRO support for UDP sockets, as the RX counterpart
> > of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> > The core functionality is implemented by the second patch, introducing a new
> > sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> > segment size to the user space via a new cmsg.
> > UDP GRO performs a socket lookup for each ingress packets and aggregate datagram
> > directed to UDP GRO enabled sockets with constant l4 tuple.
> > 
> > UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT
> > rules, and that could potentially confuse existing applications.
> 
> Good catch.
> 
> > The solution adopted here is to de-segment the GRO packet before enqueuing
> > as needed. Since we must cope with packet reinsertion after de-segmentation,
> > the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed
> > to UDP usage.
> > 
> > While the current code can probably be improved, this safeguard ,implemented in
> > the patches 4-7, allows future enachements to enable UDP GSO offload on more
> > virtual devices eventually even on forwarded packets.
> > 
> > The last 4 for patches implement some performance and functional self-tests,
> > re-using the existing udpgso infrastructure. The problematic scenario described
> > above is explicitly tested.
> 
> This looks awesome! Impressive testing, too.
> 
> A few comments in the individual patches, mostly minor.

Thank you for the in-depth review! (in the WE ;)

I'll try to address the comments on each patch individually.

In the end I rushed a bit this RFC, because the misdirection issue (and
the tentative fix) bothered me more than a bit: I wanted to check I was
not completely out-of-track.

Cheers,

Paolo