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

On Sun, 2018-10-21 at 16:07 -0400, Willem de Bruijn wrote:

On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni [off-list ref] wrote:

quoted

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.

quoted

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