Re: [RFC PATCH v2 03/10] udp: add support for UDP_GRO cmsg

[Paolo Abeni <pabeni@redhat.com>]

On Sun, 2018-10-21 at 16:07 -0400, Willem de Bruijn wrote:
> On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni <pabeni@redhat.com> 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 <pabeni@redhat.com>
> > ---
> > 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