| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250321041612.GA2679131@ax162>
Date: Thu, 20 Mar 2025 21:16:12 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>,
David Ahern <dsahern@...nel.org>, kuniyu@...zon.com
Subject: Re: [PATCH v4 net-next 2/2] udp_tunnel: use static call for GRO
hooks when possible
Hi Paolo,
On Tue, Mar 11, 2025 at 09:42:29PM +0100, Paolo Abeni wrote:
> It's quite common to have a single UDP tunnel type active in the
> whole system. In such a case we can replace the indirect call for
> the UDP tunnel GRO callback with a static call.
>
> Add the related accounting in the control path and switch to static
> call when possible. To keep the code simple use a static array for
> the registered tunnel types, and size such array based on the kernel
> config.
>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
> v3 -> v4:
> - fix CONFIG_FOE typo
> - avoid gaps in gro types array
> - drop WARN_ON in dummy_gro_rcv()
>
> v2 -> v3:
> - avoid unneeded checks in udp_tunnel_update_gro_rcv()
>
> v1 -> v2:
> - fix UDP_TUNNEL=n build
> ---
> include/net/udp_tunnel.h | 4 ++
> net/ipv4/udp_offload.c | 130 ++++++++++++++++++++++++++++++++++++-
> net/ipv4/udp_tunnel_core.c | 2 +
> 3 files changed, 135 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
> index eda0f3e2f65fa..a7b230867eb14 100644
> --- a/include/net/udp_tunnel.h
> +++ b/include/net/udp_tunnel.h
> @@ -205,9 +205,11 @@ static inline void udp_tunnel_encap_enable(struct sock *sk)
>
> #if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add);
> +void udp_tunnel_update_gro_rcv(struct sock *sk, bool add);
> #else
> static inline void udp_tunnel_update_gro_lookup(struct net *net,
> struct sock *sk, bool add) {}
> +static inline void udp_tunnel_update_gro_rcv(struct sock *sk, bool add) {}
> #endif
>
> static inline void udp_tunnel_cleanup_gro(struct sock *sk)
> @@ -215,6 +217,8 @@ static inline void udp_tunnel_cleanup_gro(struct sock *sk)
> struct udp_sock *up = udp_sk(sk);
> struct net *net = sock_net(sk);
>
> + udp_tunnel_update_gro_rcv(sk, false);
> +
> if (!up->tunnel_list.pprev)
> return;
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index e36d8a234848f..088aa8cb8ac0c 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -15,6 +15,37 @@
> #include <net/udp_tunnel.h>
>
> #if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> +
> +/*
> + * Dummy GRO tunnel callback, exists mainly to avoid dangling/NULL
> + * values for the udp tunnel static call.
> + */
> +static struct sk_buff *dummy_gro_rcv(struct sock *sk,
> + struct list_head *head,
> + struct sk_buff *skb)
> +{
> + NAPI_GRO_CB(skb)->flush = 1;
> + return NULL;
> +}
> +
> +typedef struct sk_buff *(*udp_tunnel_gro_rcv_t)(struct sock *sk,
> + struct list_head *head,
> + struct sk_buff *skb);
> +
> +struct udp_tunnel_type_entry {
> + udp_tunnel_gro_rcv_t gro_receive;
> + refcount_t count;
> +};
> +
> +#define UDP_MAX_TUNNEL_TYPES (IS_ENABLED(CONFIG_GENEVE) + \
> + IS_ENABLED(CONFIG_VXLAN) * 2 + \
> + IS_ENABLED(CONFIG_NET_FOU) * 2)
I am seeing a warning in one particular configuration in my matrix when
building with clang:
$ make -skj"$(nproc)" ARCH=mips LLVM=1 mrproper malta_defconfig net/ipv4/udp_offload.o
net/ipv4/udp_offload.c:130:8: warning: array index 0 is past the end of the array (that has type 'struct udp_tunnel_type_entry[0]') [-Warray-bounds]
130 | udp_tunnel_gro_types[0].gro_receive);
| ^ ~
include/linux/static_call.h:154:42: note: expanded from macro 'static_call_update'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~
net/ipv4/udp_offload.c:47:1: note: array 'udp_tunnel_gro_types' declared here
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^
1 warning generated.
GCC is more noisy but -Warray-bounds is not on by default yet.
$ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- KCFLAGS=-Warray-bounds mrproper malta_defconfig net/ipv4/udp_offload.o
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:125:44: warning: array subscript <unknown> is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
125 | *cur = udp_tunnel_gro_types[--udp_tunnel_gro_type_nr];
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:111:17: warning: array subscript <unknown> is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
111 | refcount_set(&cur->count, 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In file included from ./arch/mips/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:390,
from include/linux/array_size.h:5,
from include/linux/kernel.h:16,
from include/linux/skbuff.h:13,
from net/ipv4/udp_offload.c:9:
In function 'arch_atomic_set',
inlined from 'raw_atomic_set' at include/linux/atomic/atomic-arch-fallback.h:503:2,
inlined from 'atomic_set' at include/linux/atomic/atomic-instrumented.h:68:2,
inlined from 'refcount_set' at include/linux/refcount.h:134:2,
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:111:3,
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
include/asm-generic/rwonce.h:55:37: warning: array subscript [3, 536870912] is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
55 | *(volatile typeof(x) *)&(x) = (val); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
include/asm-generic/rwonce.h:61:9: note: in expansion of macro '__WRITE_ONCE'
61 | __WRITE_ONCE(x, val); \
| ^~~~~~~~~~~~
arch/mips/include/asm/atomic.h:34:9: note: in expansion of macro 'WRITE_ONCE'
34 | WRITE_ONCE(v->counter, i); \
| ^~~~~~~~~~
arch/mips/include/asm/atomic.h:37:1: note: in expansion of macro 'ATOMIC_OPS'
37 | ATOMIC_OPS(atomic, int)
| ^~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: at offset 12 into object 'udp_tunnel_gro_types' of size 0
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:110:44: warning: array subscript <unknown> is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
110 | cur = &udp_tunnel_gro_types[udp_tunnel_gro_type_nr++];
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In file included from include/linux/bpf.h:30,
from include/linux/security.h:35,
from include/net/scm.h:9,
from include/linux/netlink.h:9,
from include/uapi/linux/neighbour.h:6,
from include/linux/netdevice.h:44,
from include/net/sock.h:46,
from include/linux/tcp.h:19,
from include/linux/ipv6.h:102,
from include/net/gro.h:8,
from net/ipv4/udp_offload.c:10:
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:130:56: warning: array subscript 0 is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
130 | udp_tunnel_gro_types[0].gro_receive);
| ~~~~~~~~~~~~~~~~~~~~^~~
include/linux/static_call.h:154:49: note: in definition of macro 'static_call_update'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:89:41: warning: array subscript i is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
89 | if (udp_tunnel_gro_types[i].gro_receive == up->gro_receive)
| ~~~~~~~~~~~~~~~~~~~~^~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
In function 'udp_tunnel_update_gro_rcv',
inlined from 'udp_tunnel_update_gro_rcv' at net/ipv4/udp_offload.c:78:6:
net/ipv4/udp_offload.c:90:31: warning: array subscript i is outside array bounds of 'struct udp_tunnel_type_entry[0]' [-Warray-bounds=]
90 | cur = &udp_tunnel_gro_types[i];
| ^~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_tunnel_update_gro_rcv':
net/ipv4/udp_offload.c:47:37: note: while referencing 'udp_tunnel_gro_types'
47 | static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
| ^~~~~~~~~~~~~~~~~~~~
Should UDP_MAX_TUNNEL_TYPES be at least 1?
Cheers,
Nathan
> +DEFINE_STATIC_CALL(udp_tunnel_gro_rcv, dummy_gro_rcv);
> +static DEFINE_STATIC_KEY_FALSE(udp_tunnel_static_call);
> +static struct mutex udp_tunnel_gro_type_lock;
> +static struct udp_tunnel_type_entry udp_tunnel_gro_types[UDP_MAX_TUNNEL_TYPES];
> +static unsigned int udp_tunnel_gro_type_nr;
> static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
>
> void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
> @@ -43,6 +74,101 @@ void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
> spin_unlock(&udp_tunnel_gro_lock);
> }
> EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_lookup);
> +
> +void udp_tunnel_update_gro_rcv(struct sock *sk, bool add)
> +{
> + struct udp_tunnel_type_entry *cur = NULL;
> + struct udp_sock *up = udp_sk(sk);
> + int i, old_gro_type_nr;
> +
> + if (!up->gro_receive)
> + return;
> +
> + mutex_lock(&udp_tunnel_gro_type_lock);
> + for (i = 0; i < udp_tunnel_gro_type_nr; i++)
> + if (udp_tunnel_gro_types[i].gro_receive == up->gro_receive)
> + cur = &udp_tunnel_gro_types[i];
> +
> + old_gro_type_nr = udp_tunnel_gro_type_nr;
> + if (add) {
> + /*
> + * Update the matching entry, if found, or add a new one
> + * if needed
> + */
> + if (cur) {
> + refcount_inc(&cur->count);
> + goto out;
> + }
> +
> + if (unlikely(udp_tunnel_gro_type_nr == UDP_MAX_TUNNEL_TYPES)) {
> + pr_err_once("Too many UDP tunnel types, please increase UDP_MAX_TUNNEL_TYPES\n");
> + /* Ensure static call will never be enabled */
> + udp_tunnel_gro_type_nr = UDP_MAX_TUNNEL_TYPES + 2;
> + goto out;
> + }
> +
> + cur = &udp_tunnel_gro_types[udp_tunnel_gro_type_nr++];
> + refcount_set(&cur->count, 1);
> + cur->gro_receive = up->gro_receive;
> + } else {
> + /*
> + * The stack cleanups only successfully added tunnel, the
> + * lookup on removal should never fail.
> + */
> + if (WARN_ON_ONCE(!cur))
> + goto out;
> +
> + if (!refcount_dec_and_test(&cur->count))
> + goto out;
> +
> + /* avoid gaps, so that the enable tunnel has always id 0 */
> + *cur = udp_tunnel_gro_types[--udp_tunnel_gro_type_nr];
> + }
> +
> + if (udp_tunnel_gro_type_nr == 1) {
> + static_call_update(udp_tunnel_gro_rcv,
> + udp_tunnel_gro_types[0].gro_receive);
> + static_branch_enable(&udp_tunnel_static_call);
> + } else if (old_gro_type_nr == 1) {
> + static_branch_disable(&udp_tunnel_static_call);
> + static_call_update(udp_tunnel_gro_rcv, dummy_gro_rcv);
> + }
> +
> +out:
> + mutex_unlock(&udp_tunnel_gro_type_lock);
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_rcv);
> +
> +static void udp_tunnel_gro_init(void)
> +{
> + mutex_init(&udp_tunnel_gro_type_lock);
> +}
> +
> +static struct sk_buff *udp_tunnel_gro_rcv(struct sock *sk,
> + struct list_head *head,
> + struct sk_buff *skb)
> +{
> + if (static_branch_likely(&udp_tunnel_static_call)) {
> + if (unlikely(gro_recursion_inc_test(skb))) {
> + NAPI_GRO_CB(skb)->flush |= 1;
> + return NULL;
> + }
> + return static_call(udp_tunnel_gro_rcv)(sk, head, skb);
> + }
> + return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> +}
> +
> +#else
> +
> +static void udp_tunnel_gro_init(void) {}
> +
> +static struct sk_buff *udp_tunnel_gro_rcv(struct sock *sk,
> + struct list_head *head,
> + struct sk_buff *skb)
> +{
> + return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> +}
> +
> #endif
>
> static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> @@ -654,7 +780,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>
> skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
> skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> - pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> + pp = udp_tunnel_gro_rcv(sk, head, skb);
>
> out:
> skb_gro_flush_final(skb, pp, flush);
> @@ -804,5 +930,7 @@ int __init udpv4_offload_init(void)
> .gro_complete = udp4_gro_complete,
> },
> };
> +
> + udp_tunnel_gro_init();
> return inet_add_offload(&net_hotdata.udpv4_offload, IPPROTO_UDP);
> }
> diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
> index b5695826e57ad..c49fceea83139 100644
> --- a/net/ipv4/udp_tunnel_core.c
> +++ b/net/ipv4/udp_tunnel_core.c
> @@ -90,6 +90,8 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>
> udp_tunnel_encap_enable(sk);
>
> + udp_tunnel_update_gro_rcv(sock->sk, true);
> +
> if (!sk->sk_dport && !sk->sk_bound_dev_if && sk_saddr_any(sock->sk))
> udp_tunnel_update_gro_lookup(net, sock->sk, true);
> }
> --
> 2.48.1
>
Powered by blists - more mailing lists