Re: [net-next,v5,03/11] net: tunnel: convert udp_tunnel{6,}_xmit_skb to noref

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: tunnel: convert udp_tunnel{6,}_xmit_skb to noref

This commit changes udp_tunnel{6,}_xmit_skb to use noref dst handling,
removing the requirement that callers take a reference on the dst.
The key change is replacing skb_dst_set() with skb_dst_set_noref() in
udp_tunnel6_xmit_skb().

> diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
> index cef3e0210744..d58815db8182 100644
> --- a/net/ipv6/ip6_udp_tunnel.c
> +++ b/net/ipv6/ip6_udp_tunnel.c
> @@ -95,7 +95,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
>
>  	uh->len = htons(skb->len);
>
> -	skb_dst_set(skb, dst);
> +	skb_dst_set_noref(skb, dst);

skb_dst_set_noref() contains a WARN_ON check:

    WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());

Does this create an issue for the SCTP UDP encapsulation path?

Looking at sctp_v6_xmit() in net/sctp/ipv6.c, when SCTP sends via UDP
encapsulation, it calls:

    udp_tunnel6_xmit_skb(dst, sk, skb, NULL, &fl6->saddr, &fl6->daddr,
                         tclass, ip6_dst_hoplimit(dst), label,
                         sctp_sk(sk)->udp_port, t->encap_port, false, 0);

This call path can originate from user context via sendmsg():

    sctp_sendmsg()
      -> sctp_primitive_SEND()  [uses GFP_KERNEL - process context]
           -> sctp_do_sm()
                -> sctp_packet_transmit()
                     -> sctp_v6_xmit()

Unlike the network device transmit path (which runs under rcu_read_lock_bh()
from __dev_queue_xmit()), SCTP's control plane transmit path runs in process
context without RCU-BH protection.

Will this trigger the WARN_ON in skb_dst_set_noref() when SCTP sends UDP
encapsulated packets from process context?

>
>  	udp6_set_csum(nocheck, skb, saddr, daddr, skb->len);

[ ... ]
-- 
pw-bot: cr