Re: [PATCH] udp: resubmit encapsulation packets on all multicast listeners

Mariano Baragiola wrote:
> UDP encapsulation handlers (FOU/GUE and similar) return a positive
> protocol number from udp_queue_rcv_skb()/udpv6_queue_rcv_skb() when the
> UDP header has been consumed and the packet must be resubmitted to the
> IP protocol handler. Unicast paths already propagate that return value.
> 
> Multicast delivery called consume_skb() on every positive return, so the
> inner packet was dropped instead of being reinjected.
> 
> Commit 3cb8d4b9bfeb ("udp: fix encapsulation packet resubmit in multicast
> deliver") fixed only the primary ("first") socket path on net-next and is
> not yet in net.

It is now, so this will conflict.

> Secondary listeners still clone the skb and drop it on a
> positive return, and net itself still drops the primary socket path too.

When I asked about additional listeners Anton responded:

"
The clone loop is not reachable for encapsulation sockets, so there is
no remaining gap.

FOU/GUE tunnel sockets are created via udp_sock_create() /
setup_udp_tunnel_sock() and do not set SO_REUSEADDR or SO_REUSEPORT.
Without either, a UDP socket cannot share its port, so an encap socket
is always the only socket bound to its port. In
__udp[46]_lib_mcast_deliver() it is therefore always delivered as
'first', and the clone loop -- which handles the second and subsequent
sockets in the group -- never runs for it. The positive (resubmit)
return from udp_queue_rcv_skb() only happens for encap sockets; plain
UDP sockets return 0 or a negative value there. So the resubmit case
in the clone loop cannot occur.
"
https://lore.kernel.org/netdev/alAAXv60KUZ9KYx1@dau-home-pc/

> Resubmit secondary clones inline via ip_protocol_deliver_rcu() /

So is this a real path?

Small caveat on the SO_REUSEADDR/SO_REUSEPORT: SO_BINDTODEVICE
is another way to skip the equality check in udp_lib_lport_inuse2.
> ip6_protocol_deliver_rcu() (matching the GSO segment path in
> udp_queue_rcv_skb()/udpv6_queue_rcv_skb()), and propagate the primary
> socket return value with the same IPv4/IPv6 sign convention as the
> unicast helpers.
> 
> Fixes: ca065d0cf80f ("udp: no longer use SLAB_DESTROY_BY_RCU")

Fixes should be the above patch if that is incomplete.

> Signed-off-by: Mariano Baragiola <[email protected]>
> ---
>  net/ipv4/udp.c | 16 ++++++++++++----
>  net/ipv6/udp.c | 16 ++++++++++++----
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 70f6cbd4ef73..d6a17b0462b6 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -2475,6 +2475,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
>  	struct udp_hslot *hslot;
>  	struct sk_buff *nskb;
>  	bool use_hash2;
> +	int ret;
>  
>  	hash2_any = 0;
>  	hash2 = 0;
> @@ -2508,8 +2509,13 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
>  			__UDP_INC_STATS(net, UDP_MIB_INERRORS);
>  			continue;
>  		}
> -		if (udp_queue_rcv_skb(sk, nskb) > 0)
> -			consume_skb(nskb);
> +		/* >0 means the encap handler wants IP-level resubmit. Do that
> +		 * inline for secondary listeners; only the first socket can
> +		 * propagate the protocol number to the caller.
> +		 */
> +		ret = udp_queue_rcv_skb(sk, nskb);
> +		if (ret > 0)
> +			ip_protocol_deliver_rcu(net, nskb, ret);
>  	}