From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [RFC PATCH v2 00/10] udp: implement GRO support Date: Mon, 22 Oct 2018 11:41:35 +0200 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Network Development , Willem de Bruijn , steffen.klassert@secunet.com To: Willem de Bruijn Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58860 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728448AbeJVR7X (ORCPT ); Mon, 22 Oct 2018 13:59:23 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: 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 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