From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [RFC PATCH v2 03/10] udp: add support for UDP_GRO cmsg Date: Mon, 22 Oct 2018 17:44:29 +0200 Message-ID: <5d3ac231b85eb7d4630f84519fbc5babbb668946.camel@redhat.com> 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]:23888 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727587AbeJWADd (ORCPT ); Mon, 22 Oct 2018 20:03:33 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sun, 2018-10-21 at 16:07 -0400, Willem de Bruijn wrote: > On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni wrote: > > > > When UDP GRO is enabled, the UDP_GRO cmsg will carry the ingress > > datagram size. User-space can use such info to compute the original > > packets layout. > > > > Signed-off-by: Paolo Abeni > > --- > > CHECK: should we use a separate setsockopt to explicitly enable > > gso_size cmsg reception? So that user space can enable UDP_GRO and > > fetch cmsg without forcefully receiving GRO related info. > > A user can avoid the message by not passing control data. Though in > most practical cases it seems unsafe to do so, anyway, as the path MTU > can be lower than the expected device MTU. > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > > index 3c277378814f..2331ac9de954 100644 > > --- a/net/ipv4/udp.c > > +++ b/net/ipv4/udp.c > > @@ -1714,6 +1714,10 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, > > memset(sin->sin_zero, 0, sizeof(sin->sin_zero)); > > *addr_len = sizeof(*sin); > > } > > + > > + if (udp_sk(sk)->gro_enabled) > > + udp_cmsg_recv(msg, sk, skb); > > + > > Perhaps we can avoid adding a branch by setting a bit in > inet->cmsg_flags for gso_size to let the below branch handle the cmsg > processing. Uhmm... I think that for ipv6 sockets we need to set a bit in rxopt instead (and we already have some conditionals we could for ipv6 socket recv cmsg processing). > I'd still set that as part of the UDP_GRO setsockopt. Though if you > insist it could be a value 2 instead of 1, effectively allowing the > above mentioned opt-out. I'm ok with the current impl (no additional value to opt-in the UDP GRO cmsg). Cheers, Paolo