Re: [PATCH] udp: let rx-udp-gro-forwarding aggregate under encapsulation

[Willem de Bruijn]

Eva Kurchatova wrote:
> The rx-udp-gro-forwarding checks of the veth kselftest fail with "got
> 10 packets, expected 1" on a host where something uses UDP
> encapsulation, a vxlan say, and pass on a host where nothing does.

Uses encapsulation in the same network namespace as where the test
runs? If different namespaces, the socket lookup should fail and
thus the behavior be the same as on a host without a tunnel.

In general, I think the fix here, if any, is to address the flakiness
of the test, not to change the lookup behavior, which is as is
intentionally.

> Since commit f2e9de210d50 ("udp: gro behind static key") the receiving
> socket is looked up only once a tunnel socket exists, and commit
> 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO
> packets") takes "no socket found" for "forwarded" and aggregates only
> then. So the device setting works on a host without a tunnel and stops
> working with one.

These heuristics were chosen for a reason:

to avoid expensive coalescing + resegmenting if packets are likely to
hit a normal (i.e., without UDP_GRO) local UDP socket.

> Aggregate whenever the device asks, socket found or not. That is what
> every host without a tunnel has done all along: a local socket without

without a tunnel, or with a tunnel?

> UDP_GRO gets the packets resegmented on delivery, and a tunnel socket
> keeps its own receive path.
> 
> Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets")
> Signed-off-by: Eva Kurchatova <eva.kurchatova@xxxxxxxxxxxxx>
> ---
>  net/ipv4/udp_offload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a..d4e0e6bb9959 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -804,7 +804,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>  		if (skb->dev->features & NETIF_F_GRO_FRAGLIST)
>  			NAPI_GRO_CB(skb)->is_flist = sk ? !udp_test_bit(GRO_ENABLED, sk) : 1;
>  
> -		if ((!sk && (skb->dev->features & NETIF_F_GRO_UDP_FWD)) ||
> +		if ((skb->dev->features & NETIF_F_GRO_UDP_FWD) ||
>  		    (sk && udp_test_bit(GRO_ENABLED, sk)) || NAPI_GRO_CB(skb)->is_flist)
>  			return call_gro_receive(udp_gro_receive_segment, head, skb);
>  
> -- 
> 2.56.0
>