* [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 15:56 [PATCH net-next 0/2] udp_tunnel: GRO optimizations Paolo Abeni
@ 2025-03-06 15:56 ` Paolo Abeni
2025-03-06 16:35 ` Eric Dumazet
2025-03-06 19:46 ` Willem de Bruijn
2025-03-06 15:56 ` [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible Paolo Abeni
2025-03-06 18:50 ` [PATCH net-next 0/2] udp_tunnel: GRO optimizations Jakub Kicinski
2 siblings, 2 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-03-06 15:56 UTC (permalink / raw)
To: netdev
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, David Ahern
Most UDP tunnels bind a socket to a local port, with ANY address, no
peer and no interface index specified.
Additionally it's quite common to have a single tunnel device per
namespace.
Track in each namespace the UDP tunnel socket respecting the above.
When only a single one is present, store a reference in the netns.
When such reference is not NULL, UDP tunnel GRO lookup just need to
match the incoming packet destination port vs the socket local port.
The tunnel socket never set the reuse[port] flag[s], when bound to no
address and interface, no other socket can exist in the same netns
matching the specified local port.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/udp.h | 16 ++++++++++++++++
include/net/netns/ipv4.h | 11 +++++++++++
include/net/udp.h | 1 +
include/net/udp_tunnel.h | 18 ++++++++++++++++++
net/ipv4/udp.c | 15 +++++++++++++--
net/ipv4/udp_offload.c | 37 +++++++++++++++++++++++++++++++++++++
net/ipv4/udp_tunnel_core.c | 12 ++++++++++++
net/ipv6/udp.c | 2 ++
net/ipv6/udp_offload.c | 5 +++++
9 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 0807e21cfec95..895240177f4f4 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -101,6 +101,13 @@ struct udp_sock {
/* Cache friendly copy of sk->sk_peek_off >= 0 */
bool peeking_with_offset;
+
+ /*
+ * Accounting for the tunnel GRO fastpath.
+ * Unprotected by compilers guard, as it uses space available in
+ * the last UDP socket cacheline.
+ */
+ struct hlist_node tunnel_list;
};
#define udp_test_bit(nr, sk) \
@@ -219,4 +226,13 @@ static inline void udp_allow_gso(struct sock *sk)
#define IS_UDPLITE(__sk) (__sk->sk_protocol == IPPROTO_UDPLITE)
+static inline struct sock *udp_tunnel_sk(const struct net *net, bool is_ipv6)
+{
+#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+ return rcu_dereference(net->ipv4.udp_tunnel_gro[is_ipv6].sk);
+#else
+ return NULL;
+#endif
+}
+
#endif /* _LINUX_UDP_H */
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 650b2dc9199f4..6373e3f17da84 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -47,6 +47,11 @@ struct sysctl_fib_multipath_hash_seed {
};
#endif
+struct udp_tunnel_gro {
+ struct sock __rcu *sk;
+ struct hlist_head list;
+};
+
struct netns_ipv4 {
/* Cacheline organization can be found documented in
* Documentation/networking/net_cachelines/netns_ipv4_sysctl.rst.
@@ -85,6 +90,11 @@ struct netns_ipv4 {
struct inet_timewait_death_row tcp_death_row;
struct udp_table *udp_table;
+#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+ /* Not in a pernet subsys because need to be available at GRO stage */
+ struct udp_tunnel_gro udp_tunnel_gro[2];
+#endif
+
#ifdef CONFIG_SYSCTL
struct ctl_table_header *forw_hdr;
struct ctl_table_header *frags_hdr;
@@ -277,4 +287,5 @@ struct netns_ipv4 {
struct hlist_head *inet_addr_lst;
struct delayed_work addr_chk_work;
};
+
#endif
diff --git a/include/net/udp.h b/include/net/udp.h
index 6e89520e100dc..a772510b2aa58 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -290,6 +290,7 @@ static inline void udp_lib_init_sock(struct sock *sk)
struct udp_sock *up = udp_sk(sk);
skb_queue_head_init(&up->reader_queue);
+ INIT_HLIST_NODE(&up->tunnel_list);
up->forward_threshold = sk->sk_rcvbuf >> 2;
set_bit(SOCK_CUSTOM_SOCKOPT, &sk->sk_socket->flags);
}
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index a93dc51f6323e..eda0f3e2f65fa 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -203,6 +203,24 @@ static inline void udp_tunnel_encap_enable(struct sock *sk)
udp_encap_enable();
}
+#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add);
+#else
+static inline void udp_tunnel_update_gro_lookup(struct net *net,
+ struct sock *sk, bool add) {}
+#endif
+
+static inline void udp_tunnel_cleanup_gro(struct sock *sk)
+{
+ struct udp_sock *up = udp_sk(sk);
+ struct net *net = sock_net(sk);
+
+ if (!up->tunnel_list.pprev)
+ return;
+
+ udp_tunnel_update_gro_lookup(net, sk, false);
+}
+
#define UDP_TUNNEL_NIC_MAX_TABLES 4
enum udp_tunnel_nic_info_flags {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 17c7736d83494..d1aa96ac52888 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2891,8 +2891,10 @@ void udp_destroy_sock(struct sock *sk)
if (encap_destroy)
encap_destroy(sk);
}
- if (udp_test_bit(ENCAP_ENABLED, sk))
- static_branch_dec(&udp_encap_needed_key);
+ if (udp_test_bit(ENCAP_ENABLED, sk)) {
+ udp_tunnel_cleanup_gro(sk);
+ udp_encap_disable();
+ }
}
}
@@ -3804,6 +3806,15 @@ static void __net_init udp_set_table(struct net *net)
static int __net_init udp_pernet_init(struct net *net)
{
+#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+ int i;
+
+ /* No tunnel is configured */
+ for (i = 0; i < ARRAY_SIZE(net->ipv4.udp_tunnel_gro); ++i) {
+ INIT_HLIST_HEAD(&net->ipv4.udp_tunnel_gro[i].list);
+ rcu_assign_pointer(net->ipv4.udp_tunnel_gro[1].sk, NULL);
+ }
+#endif
udp_sysctl_init(net);
udp_set_table(net);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index c1a85b300ee87..ac6dd2703190e 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -12,6 +12,38 @@
#include <net/udp.h>
#include <net/protocol.h>
#include <net/inet_common.h>
+#include <net/udp_tunnel.h>
+
+#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
+
+void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
+{
+ bool is_ipv6 = sk->sk_family == AF_INET6;
+ struct udp_sock *tup, *up = udp_sk(sk);
+ struct udp_tunnel_gro *udp_tunnel_gro;
+
+ spin_lock(&udp_tunnel_gro_lock);
+ udp_tunnel_gro = &net->ipv4.udp_tunnel_gro[is_ipv6];
+ if (add)
+ hlist_add_head(&up->tunnel_list, &udp_tunnel_gro->list);
+ else
+ hlist_del_init(&up->tunnel_list);
+
+ if (udp_tunnel_gro->list.first &&
+ !udp_tunnel_gro->list.first->next) {
+ tup = hlist_entry(udp_tunnel_gro->list.first, struct udp_sock,
+ tunnel_list);
+
+ rcu_assign_pointer(udp_tunnel_gro->sk, (struct sock *)tup);
+ } else {
+ rcu_assign_pointer(udp_tunnel_gro->sk, NULL);
+ }
+
+ spin_unlock(&udp_tunnel_gro_lock);
+}
+EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_lookup);
+#endif
static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
netdev_features_t features,
@@ -631,8 +663,13 @@ static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
{
const struct iphdr *iph = skb_gro_network_header(skb);
struct net *net = dev_net_rcu(skb->dev);
+ struct sock *sk;
int iif, sdif;
+ sk = udp_tunnel_sk(net, false);
+ if (sk && dport == htons(sk->sk_num))
+ return sk;
+
inet_get_iif_sdif(skb, &iif, &sdif);
return __udp4_lib_lookup(net, iph->saddr, sport,
diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
index 619a53eb672da..b969c997c89c7 100644
--- a/net/ipv4/udp_tunnel_core.c
+++ b/net/ipv4/udp_tunnel_core.c
@@ -58,6 +58,15 @@ int udp_sock_create4(struct net *net, struct udp_port_cfg *cfg,
}
EXPORT_SYMBOL(udp_sock_create4);
+static inline bool sk_saddr_any(struct sock *sk)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+ return ipv6_addr_any(&sk->sk_v6_rcv_saddr);
+#else
+ return !sk->sk_rcv_saddr;
+#endif
+}
+
void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
struct udp_tunnel_sock_cfg *cfg)
{
@@ -80,6 +89,9 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
udp_sk(sk)->gro_complete = cfg->gro_complete;
udp_tunnel_encap_enable(sk);
+
+ if (!sk->sk_dport && !sk->sk_bound_dev_if && sk_saddr_any(sock->sk))
+ udp_tunnel_update_gro_lookup(net, sock->sk, true);
}
EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 3a0d6c5a8286b..3087022d18a55 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -46,6 +46,7 @@
#include <net/tcp_states.h>
#include <net/ip6_checksum.h>
#include <net/ip6_tunnel.h>
+#include <net/udp_tunnel.h>
#include <net/xfrm.h>
#include <net/inet_hashtables.h>
#include <net/inet6_hashtables.h>
@@ -1824,6 +1825,7 @@ void udpv6_destroy_sock(struct sock *sk)
}
if (udp_test_bit(ENCAP_ENABLED, sk)) {
static_branch_dec(&udpv6_encap_needed_key);
+ udp_tunnel_cleanup_gro(sk);
udp_encap_disable();
}
}
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 404212dfc99ab..d8445ac1b2e43 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -118,8 +118,13 @@ static struct sock *udp6_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
{
const struct ipv6hdr *iph = skb_gro_network_header(skb);
struct net *net = dev_net_rcu(skb->dev);
+ struct sock *sk;
int iif, sdif;
+ sk = udp_tunnel_sk(net, true);
+ if (sk && dport == htons(sk->sk_num))
+ return sk;
+
inet6_get_iif_sdif(skb, &iif, &sdif);
return __udp6_lib_lookup(net, &iph->saddr, sport,
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 15:56 ` [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup Paolo Abeni
@ 2025-03-06 16:35 ` Eric Dumazet
2025-03-06 17:31 ` Paolo Abeni
2025-03-06 19:46 ` Willem de Bruijn
1 sibling, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2025-03-06 16:35 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, David Ahern
On Thu, Mar 6, 2025 at 4:57 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Most UDP tunnels bind a socket to a local port, with ANY address, no
> peer and no interface index specified.
> Additionally it's quite common to have a single tunnel device per
> namespace.
>
> Track in each namespace the UDP tunnel socket respecting the above.
> When only a single one is present, store a reference in the netns.
>
> When such reference is not NULL, UDP tunnel GRO lookup just need to
> match the incoming packet destination port vs the socket local port.
>
> The tunnel socket never set the reuse[port] flag[s], when bound to no
> address and interface, no other socket can exist in the same netns
> matching the specified local port.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
...
> static int __net_init udp_pernet_init(struct net *net)
> {
> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> + int i;
> +
> + /* No tunnel is configured */
> + for (i = 0; i < ARRAY_SIZE(net->ipv4.udp_tunnel_gro); ++i) {
> + INIT_HLIST_HEAD(&net->ipv4.udp_tunnel_gro[i].list);
> + rcu_assign_pointer(net->ipv4.udp_tunnel_gro[1].sk, NULL);
typo : [i] is what you meant (instead of [1])
> + }
> +#endif
> udp_sysctl_init(net);
> udp_set_table(net);
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index c1a85b300ee87..ac6dd2703190e 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -12,6 +12,38 @@
> #include <net/udp.h>
> #include <net/protocol.h>
> #include <net/inet_common.h>
> +#include <net/udp_tunnel.h>
> +
> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> +static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
> +
> +void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
> +{
> + bool is_ipv6 = sk->sk_family == AF_INET6;
> + struct udp_sock *tup, *up = udp_sk(sk);
> + struct udp_tunnel_gro *udp_tunnel_gro;
> +
> + spin_lock(&udp_tunnel_gro_lock);
> + udp_tunnel_gro = &net->ipv4.udp_tunnel_gro[is_ipv6];
> + if (add)
> + hlist_add_head(&up->tunnel_list, &udp_tunnel_gro->list);
> + else
> + hlist_del_init(&up->tunnel_list);
> +
> + if (udp_tunnel_gro->list.first &&
> + !udp_tunnel_gro->list.first->next) {
> + tup = hlist_entry(udp_tunnel_gro->list.first, struct udp_sock,
> + tunnel_list);
> +
> + rcu_assign_pointer(udp_tunnel_gro->sk, (struct sock *)tup);
> + } else {
> + rcu_assign_pointer(udp_tunnel_gro->sk, NULL);
> + }
> +
> + spin_unlock(&udp_tunnel_gro_lock);
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_lookup);
> +#endif
>
> static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> netdev_features_t features,
> @@ -631,8 +663,13 @@ static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
> {
> const struct iphdr *iph = skb_gro_network_header(skb);
> struct net *net = dev_net_rcu(skb->dev);
> + struct sock *sk;
> int iif, sdif;
>
> + sk = udp_tunnel_sk(net, false);
> + if (sk && dport == htons(sk->sk_num))
> + return sk;
> +
> inet_get_iif_sdif(skb, &iif, &sdif);
>
> return __udp4_lib_lookup(net, iph->saddr, sport,
> diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
> index 619a53eb672da..b969c997c89c7 100644
> --- a/net/ipv4/udp_tunnel_core.c
> +++ b/net/ipv4/udp_tunnel_core.c
> @@ -58,6 +58,15 @@ int udp_sock_create4(struct net *net, struct udp_port_cfg *cfg,
> }
> EXPORT_SYMBOL(udp_sock_create4);
>
> +static inline bool sk_saddr_any(struct sock *sk)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> + return ipv6_addr_any(&sk->sk_v6_rcv_saddr);
> +#else
> + return !sk->sk_rcv_saddr;
> +#endif
> +}
> +
> void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
> struct udp_tunnel_sock_cfg *cfg)
> {
> @@ -80,6 +89,9 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
> udp_sk(sk)->gro_complete = cfg->gro_complete;
>
> udp_tunnel_encap_enable(sk);
> +
> + if (!sk->sk_dport && !sk->sk_bound_dev_if && sk_saddr_any(sock->sk))
> + udp_tunnel_update_gro_lookup(net, sock->sk, true);
> }
> EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 3a0d6c5a8286b..3087022d18a55 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -46,6 +46,7 @@
> #include <net/tcp_states.h>
> #include <net/ip6_checksum.h>
> #include <net/ip6_tunnel.h>
> +#include <net/udp_tunnel.h>
> #include <net/xfrm.h>
> #include <net/inet_hashtables.h>
> #include <net/inet6_hashtables.h>
> @@ -1824,6 +1825,7 @@ void udpv6_destroy_sock(struct sock *sk)
> }
> if (udp_test_bit(ENCAP_ENABLED, sk)) {
> static_branch_dec(&udpv6_encap_needed_key);
In ipv4, you removed the static_branch_dec(&udp_encap_needed_key);
> + udp_tunnel_cleanup_gro(sk);
> udp_encap_disable();
> }
> }
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index 404212dfc99ab..d8445ac1b2e43 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -118,8 +118,13 @@ static struct sock *udp6_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
> {
> const struct ipv6hdr *iph = skb_gro_network_header(skb);
> struct net *net = dev_net_rcu(skb->dev);
> + struct sock *sk;
> int iif, sdif;
>
> + sk = udp_tunnel_sk(net, true);
> + if (sk && dport == htons(sk->sk_num))
> + return sk;
> +
> inet6_get_iif_sdif(skb, &iif, &sdif);
>
> return __udp6_lib_lookup(net, &iph->saddr, sport,
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 16:35 ` Eric Dumazet
@ 2025-03-06 17:31 ` Paolo Abeni
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-03-06 17:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, David Ahern
Hi,
On 3/6/25 5:35 PM, Eric Dumazet wrote:
>> static int __net_init udp_pernet_init(struct net *net)
>> {
>> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
>> + int i;
>> +
>> + /* No tunnel is configured */
>> + for (i = 0; i < ARRAY_SIZE(net->ipv4.udp_tunnel_gro); ++i) {
>> + INIT_HLIST_HEAD(&net->ipv4.udp_tunnel_gro[i].list);
>> + rcu_assign_pointer(net->ipv4.udp_tunnel_gro[1].sk, NULL);
>
> typo : [i] is what you meant (instead of [1])
Whoops... I guess testing did not discovered that because the pointer is
only touched after a tunnel is initialized. I'll fix in v2.
>> @@ -1824,6 +1825,7 @@ void udpv6_destroy_sock(struct sock *sk)
>> }
>> if (udp_test_bit(ENCAP_ENABLED, sk)) {
>> static_branch_dec(&udpv6_encap_needed_key);
>
> In ipv4, you removed the static_branch_dec(&udp_encap_needed_key);
I replaced such statement with udp_encap_disable(), which in turn does
the same. Still is a left-over from a previous revision of the patch,
when I intended to do all the cleanup in udp_encap_disable().
In v2 I'll avoid the unneeded ipv4 change.
Thanks for the review,
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 15:56 ` [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup Paolo Abeni
2025-03-06 16:35 ` Eric Dumazet
@ 2025-03-06 19:46 ` Willem de Bruijn
2025-03-06 21:20 ` Paolo Abeni
1 sibling, 1 reply; 12+ messages in thread
From: Willem de Bruijn @ 2025-03-06 19:46 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, David Ahern
Paolo Abeni wrote:
> Most UDP tunnels bind a socket to a local port, with ANY address, no
> peer and no interface index specified.
> Additionally it's quite common to have a single tunnel device per
> namespace.
>
> Track in each namespace the UDP tunnel socket respecting the above.
> When only a single one is present, store a reference in the netns.
>
> When such reference is not NULL, UDP tunnel GRO lookup just need to
> match the incoming packet destination port vs the socket local port.
>
> The tunnel socket never set the reuse[port] flag[s], when bound to no
> address and interface, no other socket can exist in the same netns
> matching the specified local port.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index c1a85b300ee87..ac6dd2703190e 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -12,6 +12,38 @@
> #include <net/udp.h>
> #include <net/protocol.h>
> #include <net/inet_common.h>
> +#include <net/udp_tunnel.h>
> +
> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> +static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
> +
> +void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
> +{
> + bool is_ipv6 = sk->sk_family == AF_INET6;
> + struct udp_sock *tup, *up = udp_sk(sk);
> + struct udp_tunnel_gro *udp_tunnel_gro;
> +
> + spin_lock(&udp_tunnel_gro_lock);
> + udp_tunnel_gro = &net->ipv4.udp_tunnel_gro[is_ipv6];
It's a bit odd to have an ipv6 member in netns.ipv4. Does it
significantly simplify the code vs a separate entry in netns.ipv6?
> + if (add)
> + hlist_add_head(&up->tunnel_list, &udp_tunnel_gro->list);
> + else
> + hlist_del_init(&up->tunnel_list);
> +
> + if (udp_tunnel_gro->list.first &&
> + !udp_tunnel_gro->list.first->next) {
> + tup = hlist_entry(udp_tunnel_gro->list.first, struct udp_sock,
> + tunnel_list);
> +
> + rcu_assign_pointer(udp_tunnel_gro->sk, (struct sock *)tup);
> + } else {
> + rcu_assign_pointer(udp_tunnel_gro->sk, NULL);
> + }
> +
> + spin_unlock(&udp_tunnel_gro_lock);
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_lookup);
> +#endif
>
> static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> netdev_features_t features,
> @@ -631,8 +663,13 @@ static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
> {
> const struct iphdr *iph = skb_gro_network_header(skb);
> struct net *net = dev_net_rcu(skb->dev);
> + struct sock *sk;
> int iif, sdif;
>
> + sk = udp_tunnel_sk(net, false);
> + if (sk && dport == htons(sk->sk_num))
> + return sk;
> +
This improves tunnel performance at a slight cost to everything else,
by having the tunnel check before the normal socket path.
Does a 5% best case gain warrant that? Not snark, I don't have a
good answer.
> inet_get_iif_sdif(skb, &iif, &sdif);
>
> return __udp4_lib_lookup(net, iph->saddr, sport,
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 19:46 ` Willem de Bruijn
@ 2025-03-06 21:20 ` Paolo Abeni
2025-03-06 22:25 ` Willem de Bruijn
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Abeni @ 2025-03-06 21:20 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Simon Horman,
David Ahern
On 3/6/25 8:46 PM, Willem de Bruijn wrote:
> Paolo Abeni wrote:
>> Most UDP tunnels bind a socket to a local port, with ANY address, no
>> peer and no interface index specified.
>> Additionally it's quite common to have a single tunnel device per
>> namespace.
>>
>> Track in each namespace the UDP tunnel socket respecting the above.
>> When only a single one is present, store a reference in the netns.
>>
>> When such reference is not NULL, UDP tunnel GRO lookup just need to
>> match the incoming packet destination port vs the socket local port.
>>
>> The tunnel socket never set the reuse[port] flag[s], when bound to no
>> address and interface, no other socket can exist in the same netns
>> matching the specified local port.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index c1a85b300ee87..ac6dd2703190e 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -12,6 +12,38 @@
>> #include <net/udp.h>
>> #include <net/protocol.h>
>> #include <net/inet_common.h>
>> +#include <net/udp_tunnel.h>
>> +
>> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
>> +static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
>> +
>> +void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
>> +{
>> + bool is_ipv6 = sk->sk_family == AF_INET6;
>> + struct udp_sock *tup, *up = udp_sk(sk);
>> + struct udp_tunnel_gro *udp_tunnel_gro;
>> +
>> + spin_lock(&udp_tunnel_gro_lock);
>> + udp_tunnel_gro = &net->ipv4.udp_tunnel_gro[is_ipv6];
>
> It's a bit odd to have an ipv6 member in netns.ipv4. Does it
> significantly simplify the code vs a separate entry in netns.ipv6?
The code complexity should not change much. I place both the ipv4 and
ipv6 data there to allow cache-line based optimization, as all the netns
fast-path fields are under struct netns_ipv4.
Currently the UDP tunnel related fields share the same cache-line of
`udp_table`.
>> @@ -631,8 +663,13 @@ static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
>> {
>> const struct iphdr *iph = skb_gro_network_header(skb);
>> struct net *net = dev_net_rcu(skb->dev);
>> + struct sock *sk;
>> int iif, sdif;
>>
>> + sk = udp_tunnel_sk(net, false);
>> + if (sk && dport == htons(sk->sk_num))
>> + return sk;
>> +
>
> This improves tunnel performance at a slight cost to everything else,
> by having the tunnel check before the normal socket path.
>
> Does a 5% best case gain warrant that? Not snark, I don't have a
> good answer.
We enter this function only when udp_encap_needed_key is true: ~an UDP
tunnel has been configured[1].
When tunnels are enabled, AFAIK the single tunnel device is the most
common and most relevant scenario, and in such setup this gives
measurable performance improvement. Other tunnel-based scenarios will
see the additional cost of a single conditional (using data on an
already hot cacheline, due to the above layout).
If you are concerned about such cost, I can add an additional static
branch protecting the above code chunk, so that the conditional will be
performed only when there is a single UDP tunnel configured. Please, let
me know.
Thanks,
Paolo
[1] to be more accurate: an UDP tunnel or an UDP socket with GRO enabled
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup.
2025-03-06 21:20 ` Paolo Abeni
@ 2025-03-06 22:25 ` Willem de Bruijn
0 siblings, 0 replies; 12+ messages in thread
From: Willem de Bruijn @ 2025-03-06 22:25 UTC (permalink / raw)
To: Paolo Abeni
Cc: Willem de Bruijn, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Simon Horman, David Ahern
On Thu, Mar 6, 2025 at 4:20 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 3/6/25 8:46 PM, Willem de Bruijn wrote:
> > Paolo Abeni wrote:
> >> Most UDP tunnels bind a socket to a local port, with ANY address, no
> >> peer and no interface index specified.
> >> Additionally it's quite common to have a single tunnel device per
> >> namespace.
> >>
> >> Track in each namespace the UDP tunnel socket respecting the above.
> >> When only a single one is present, store a reference in the netns.
> >>
> >> When such reference is not NULL, UDP tunnel GRO lookup just need to
> >> match the incoming packet destination port vs the socket local port.
> >>
> >> The tunnel socket never set the reuse[port] flag[s], when bound to no
> >> address and interface, no other socket can exist in the same netns
> >> matching the specified local port.
> >>
> >> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >
> >> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> >> index c1a85b300ee87..ac6dd2703190e 100644
> >> --- a/net/ipv4/udp_offload.c
> >> +++ b/net/ipv4/udp_offload.c
> >> @@ -12,6 +12,38 @@
> >> #include <net/udp.h>
> >> #include <net/protocol.h>
> >> #include <net/inet_common.h>
> >> +#include <net/udp_tunnel.h>
> >> +
> >> +#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
> >> +static DEFINE_SPINLOCK(udp_tunnel_gro_lock);
> >> +
> >> +void udp_tunnel_update_gro_lookup(struct net *net, struct sock *sk, bool add)
> >> +{
> >> + bool is_ipv6 = sk->sk_family == AF_INET6;
> >> + struct udp_sock *tup, *up = udp_sk(sk);
> >> + struct udp_tunnel_gro *udp_tunnel_gro;
> >> +
> >> + spin_lock(&udp_tunnel_gro_lock);
> >> + udp_tunnel_gro = &net->ipv4.udp_tunnel_gro[is_ipv6];
> >
> > It's a bit odd to have an ipv6 member in netns.ipv4. Does it
> > significantly simplify the code vs a separate entry in netns.ipv6?
>
> The code complexity should not change much. I place both the ipv4 and
> ipv6 data there to allow cache-line based optimization, as all the netns
> fast-path fields are under struct netns_ipv4.
>
> Currently the UDP tunnel related fields share the same cache-line of
> `udp_table`.
That's reason enough. Since you have to respin, please add a comment
in the commit message. It looked surprising at first read.
> >> @@ -631,8 +663,13 @@ static struct sock *udp4_gro_lookup_skb(struct sk_buff *skb, __be16 sport,
> >> {
> >> const struct iphdr *iph = skb_gro_network_header(skb);
> >> struct net *net = dev_net_rcu(skb->dev);
> >> + struct sock *sk;
> >> int iif, sdif;
> >>
> >> + sk = udp_tunnel_sk(net, false);
> >> + if (sk && dport == htons(sk->sk_num))
> >> + return sk;
> >> +
> >
> > This improves tunnel performance at a slight cost to everything else,
> > by having the tunnel check before the normal socket path.
> >
> > Does a 5% best case gain warrant that? Not snark, I don't have a
> > good answer.
>
> We enter this function only when udp_encap_needed_key is true: ~an UDP
> tunnel has been configured[1].
>
> When tunnels are enabled, AFAIK the single tunnel device is the most
> common and most relevant scenario, and in such setup this gives
> measurable performance improvement. Other tunnel-based scenarios will
> see the additional cost of a single conditional (using data on an
> already hot cacheline, due to the above layout).
>
> If you are concerned about such cost, I can add an additional static
> branch protecting the above code chunk, so that the conditional will be
> performed only when there is a single UDP tunnel configured. Please, let
> me know.
>
> Thanks,
>
> Paolo
>
> [1] to be more accurate: an UDP tunnel or an UDP socket with GRO enabled
Oh right, not all regular UDP GRO. That is okay then.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible
2025-03-06 15:56 [PATCH net-next 0/2] udp_tunnel: GRO optimizations Paolo Abeni
2025-03-06 15:56 ` [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup Paolo Abeni
@ 2025-03-06 15:56 ` Paolo Abeni
2025-03-07 10:53 ` kernel test robot
2025-03-07 12:05 ` kernel test robot
2025-03-06 18:50 ` [PATCH net-next 0/2] udp_tunnel: GRO optimizations Jakub Kicinski
2 siblings, 2 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-03-06 15:56 UTC (permalink / raw)
To: netdev
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, David Ahern
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@redhat.com>
---
include/net/udp_tunnel.h | 4 ++
net/ipv4/udp_offload.c | 132 ++++++++++++++++++++++++++++++++++++-
net/ipv4/udp_tunnel_core.c | 2 +
3 files changed, 137 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 ac6dd2703190e..485c5ad3f7510 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -15,6 +15,39 @@
#include <net/udp_tunnel.h>
#if IS_ENABLED(CONFIG_NET_UDP_TUNNEL)
+
+/*
+ * Dummy GRO tunnel callback; should never be invoked, 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)
+{
+ WARN_ON_ONCE(1);
+ 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_FOE) * 2)
+
+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 +76,102 @@ 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, *avail = NULL;
+ struct udp_sock *up = udp_sk(sk);
+ bool enabled, old_enabled;
+ int i;
+
+ if (!up->gro_receive)
+ return;
+
+ mutex_lock(&udp_tunnel_gro_type_lock);
+ for (i = 0; i < UDP_MAX_TUNNEL_TYPES; i++) {
+ if (!refcount_read(&udp_tunnel_gro_types[i].count))
+ avail = &udp_tunnel_gro_types[i];
+ else if (udp_tunnel_gro_types[i].gro_receive == up->gro_receive)
+ cur = &udp_tunnel_gro_types[i];
+ }
+ old_enabled = udp_tunnel_gro_type_nr == 1;
+ 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(!avail)) {
+ /* Ensure static call will never be enabled */
+ pr_err_once("Unexpected amount of UDP tunnel types, please update UDP_MAX_TUNNEL_TYPES\n");
+ udp_tunnel_gro_type_nr = UDP_MAX_TUNNEL_TYPES + 1;
+ goto out;
+ }
+
+ refcount_set(&avail->count, 1);
+ avail->gro_receive = up->gro_receive;
+ udp_tunnel_gro_type_nr++;
+ } 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;
+ udp_tunnel_gro_type_nr--;
+ }
+
+ /* Update the static call only when switching status */
+ enabled = udp_tunnel_gro_type_nr == 1;
+ if (enabled && !old_enabled) {
+ for (i = 0; i < UDP_MAX_TUNNEL_TYPES; i++) {
+ cur = &udp_tunnel_gro_types[i];
+ if (refcount_read(&cur->count)) {
+ static_call_update(udp_tunnel_gro_rcv,
+ cur->gro_receive);
+ static_branch_enable(&udp_tunnel_static_call);
+ }
+ }
+ } else if (!enabled && old_enabled) {
+ 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 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 struct skbuff *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,
@@ -650,7 +779,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);
@@ -800,5 +929,6 @@ int __init udpv4_offload_init(void)
.gro_complete = udp4_gro_complete,
},
};
+ mutex_init(&udp_tunnel_gro_type_lock);
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 b969c997c89c7..1ebc5daff5bc8 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible
2025-03-06 15:56 ` [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible Paolo Abeni
@ 2025-03-07 10:53 ` kernel test robot
2025-03-07 12:05 ` kernel test robot
1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-03-07 10:53 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: oe-kbuild-all, Willem de Bruijn, Eric Dumazet, Jakub Kicinski,
Simon Horman, David Ahern
Hi Paolo,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/udp_tunnel-create-a-fast-path-GRO-lookup/20250306-235952
base: net-next/main
patch link: https://lore.kernel.org/r/740cd03d2982943c313de334977e18cc9de1bc3e.1741275846.git.pabeni%40redhat.com
patch subject: [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible
config: arm-wpcm450_defconfig (https://download.01.org/0day-ci/archive/20250307/202503071846.PFKkO7SH-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503071846.PFKkO7SH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503071846.PFKkO7SH-lkp@intel.com/
All errors (new ones prefixed by >>):
net/ipv4/udp_offload.c: In function 'udp_tunnel_gro_rcv':
>> net/ipv4/udp_offload.c:172:16: error: returning 'struct sk_buff *' from a function with incompatible return type 'struct skbuff *' [-Wincompatible-pointer-types]
172 | return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function 'udp_gro_receive':
>> net/ipv4/udp_offload.c:782:12: error: assignment to 'struct sk_buff *' from incompatible pointer type 'struct skbuff *' [-Wincompatible-pointer-types]
782 | pp = udp_tunnel_gro_rcv(sk, head, skb);
| ^
In file included from include/linux/seqlock.h:19,
from include/linux/dcache.h:11,
from include/linux/fs.h:8,
from include/linux/highmem.h:5,
from include/linux/bvec.h:10,
from include/linux/skbuff.h:17,
from net/ipv4/udp_offload.c:9:
net/ipv4/udp_offload.c: In function 'udpv4_offload_init':
>> net/ipv4/udp_offload.c:932:21: error: 'udp_tunnel_gro_type_lock' undeclared (first use in this function); did you mean 'udp_tunnel_gro_rcv'?
932 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/mutex.h:64:23: note: in definition of macro 'mutex_init'
64 | __mutex_init((mutex), #mutex, &__key); \
| ^~~~~
net/ipv4/udp_offload.c:932:21: note: each undeclared identifier is reported only once for each function it appears in
932 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/mutex.h:64:23: note: in definition of macro 'mutex_init'
64 | __mutex_init((mutex), #mutex, &__key); \
| ^~~~~
vim +172 net/ipv4/udp_offload.c
167
168 static struct skbuff *udp_tunnel_gro_rcv(struct sock *sk,
169 struct list_head *head,
170 struct sk_buff *skb)
171 {
> 172 return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
173 }
174
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible
2025-03-06 15:56 ` [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible Paolo Abeni
2025-03-07 10:53 ` kernel test robot
@ 2025-03-07 12:05 ` kernel test robot
1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-03-07 12:05 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: llvm, oe-kbuild-all, Willem de Bruijn, Eric Dumazet,
Jakub Kicinski, Simon Horman, David Ahern
Hi Paolo,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/udp_tunnel-create-a-fast-path-GRO-lookup/20250306-235952
base: net-next/main
patch link: https://lore.kernel.org/r/740cd03d2982943c313de334977e18cc9de1bc3e.1741275846.git.pabeni%40redhat.com
patch subject: [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible
config: mips-rt305x_defconfig (https://download.01.org/0day-ci/archive/20250307/202503071931.FDaDRKvW-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503071931.FDaDRKvW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503071931.FDaDRKvW-lkp@intel.com/
All errors (new ones prefixed by >>):
>> net/ipv4/udp_offload.c:172:9: error: incompatible pointer types returning 'struct sk_buff *' from a function with result type 'struct skbuff *' [-Werror,-Wincompatible-pointer-types]
172 | return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> net/ipv4/udp_offload.c:782:5: error: incompatible pointer types assigning to 'struct sk_buff *' from 'struct skbuff *' [-Werror,-Wincompatible-pointer-types]
782 | pp = udp_tunnel_gro_rcv(sk, head, skb);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> net/ipv4/udp_offload.c:932:14: error: use of undeclared identifier 'udp_tunnel_gro_type_lock'; did you mean 'udp_tunnel_gro_rcv'?
932 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~
| udp_tunnel_gro_rcv
include/linux/mutex.h:64:16: note: expanded from macro 'mutex_init'
64 | __mutex_init((mutex), #mutex, &__key); \
| ^
net/ipv4/udp_offload.c:168:23: note: 'udp_tunnel_gro_rcv' declared here
168 | static struct skbuff *udp_tunnel_gro_rcv(struct sock *sk,
| ^
>> net/ipv4/udp_offload.c:932:2: error: incompatible pointer types passing 'struct skbuff *(*)(struct sock *, struct list_head *, struct sk_buff *)' to parameter of type 'struct mutex *' [-Werror,-Wincompatible-pointer-types]
932 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/mutex.h:64:15: note: expanded from macro 'mutex_init'
64 | __mutex_init((mutex), #mutex, &__key); \
| ^~~~~~~
include/linux/mutex.h:89:40: note: passing argument to parameter 'lock' here
89 | extern void __mutex_init(struct mutex *lock, const char *name,
| ^
4 errors generated.
vim +172 net/ipv4/udp_offload.c
167
> 168 static struct skbuff *udp_tunnel_gro_rcv(struct sock *sk,
169 struct list_head *head,
170 struct sk_buff *skb)
171 {
> 172 return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
173 }
174
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 0/2] udp_tunnel: GRO optimizations
2025-03-06 15:56 [PATCH net-next 0/2] udp_tunnel: GRO optimizations Paolo Abeni
2025-03-06 15:56 ` [PATCH net-next 1/2] udp_tunnel: create a fast-path GRO lookup Paolo Abeni
2025-03-06 15:56 ` [PATCH net-next 2/2] udp_tunnel: use static call for GRO hooks when possible Paolo Abeni
@ 2025-03-06 18:50 ` Jakub Kicinski
2025-03-06 20:59 ` Paolo Abeni
2 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-03-06 18:50 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Willem de Bruijn, David S. Miller, Eric Dumazet,
Simon Horman, David Ahern
On Thu, 6 Mar 2025 16:56:51 +0100 Paolo Abeni wrote:
> The UDP tunnel GRO stage is source of measurable overhead for workload
> based on UDP-encapsulated traffic: each incoming packets requires a full
> UDP socket lookup and an indirect call.
>
> In the most common setups a single UDP tunnel device is used. In such
> case we can optimize both the lookup and the indirect call.
>
> Patch 1 tracks per netns the active UDP tunnels and replaces the socket
> lookup with a single destination port comparison when possible.
>
> Patch 2 tracks the different types of UDP tunnels and replaces the
> indirect call with a static one when there is a single UDP tunnel type
> active.
>
> I measure ~5% performance improvement in TCP over UDP tunnel stream
> tests on top of this series.
Breaks the build with NET_UDP_TUNNEL=n (in contest) :(
net/ipv4/udp_offload.c: In function ‘udp_tunnel_gro_rcv’:
net/ipv4/udp_offload.c:172:16: error: returning ‘struct sk_buff *’ from a function with incompatible return type ‘struct skbuff *’ [-Werror=incompatible-pointer-types]
172 | return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/udp_offload.c: In function ‘udp_gro_receive’:
net/ipv4/udp_offload.c:786:12: error: assignment to ‘struct sk_buff *’ from incompatible pointer type ‘struct skbuff *’ [-Werror=incompatible-pointer-types]
786 | pp = udp_tunnel_gro_rcv(sk, head, skb);
| ^
In file included from ./include/linux/seqlock.h:19,
from ./include/linux/dcache.h:11,
from ./include/linux/fs.h:8,
from ./include/linux/highmem.h:5,
from ./include/linux/bvec.h:10,
from ./include/linux/skbuff.h:17,
from net/ipv4/udp_offload.c:9:
net/ipv4/udp_offload.c: In function ‘udpv4_offload_init’:
net/ipv4/udp_offload.c:936:21: error: ‘udp_tunnel_gro_type_lock’ undeclared (first use in this function); did you mean ‘udp_tunnel_gro_rcv’?
936 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/mutex.h:64:23: note: in definition of macro ‘mutex_init’
64 | __mutex_init((mutex), #mutex, &__key); \
| ^~~~~
net/ipv4/udp_offload.c:936:21: note: each undeclared identifier is reported only once for each function it appears in
936 | mutex_init(&udp_tunnel_gro_type_lock);
| ^~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/mutex.h:64:23: note: in definition of macro ‘mutex_init’
64 | __mutex_init((mutex), #mutex, &__key); \
| ^~~~~
cc1: all warnings being treated as errors
--
pw-bot: cr
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 0/2] udp_tunnel: GRO optimizations
2025-03-06 18:50 ` [PATCH net-next 0/2] udp_tunnel: GRO optimizations Jakub Kicinski
@ 2025-03-06 20:59 ` Paolo Abeni
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-03-06 20:59 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Willem de Bruijn, David S. Miller, Eric Dumazet,
Simon Horman, David Ahern
On 3/6/25 7:50 PM, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 16:56:51 +0100 Paolo Abeni wrote:
>> The UDP tunnel GRO stage is source of measurable overhead for workload
>> based on UDP-encapsulated traffic: each incoming packets requires a full
>> UDP socket lookup and an indirect call.
>>
>> In the most common setups a single UDP tunnel device is used. In such
>> case we can optimize both the lookup and the indirect call.
>>
>> Patch 1 tracks per netns the active UDP tunnels and replaces the socket
>> lookup with a single destination port comparison when possible.
>>
>> Patch 2 tracks the different types of UDP tunnels and replaces the
>> indirect call with a static one when there is a single UDP tunnel type
>> active.
>>
>> I measure ~5% performance improvement in TCP over UDP tunnel stream
>> tests on top of this series.
>
> Breaks the build with NET_UDP_TUNNEL=n (in contest) :(
>
> net/ipv4/udp_offload.c: In function ‘udp_tunnel_gro_rcv’:
> net/ipv4/udp_offload.c:172:16: error: returning ‘struct sk_buff *’ from a function with incompatible return type ‘struct skbuff *’ [-Werror=incompatible-pointer-types]
> 172 | return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/ipv4/udp_offload.c: In function ‘udp_gro_receive’:
> net/ipv4/udp_offload.c:786:12: error: assignment to ‘struct sk_buff *’ from incompatible pointer type ‘struct skbuff *’ [-Werror=incompatible-pointer-types]
> 786 | pp = udp_tunnel_gro_rcv(sk, head, skb);
> | ^
> In file included from ./include/linux/seqlock.h:19,
> from ./include/linux/dcache.h:11,
> from ./include/linux/fs.h:8,
> from ./include/linux/highmem.h:5,
> from ./include/linux/bvec.h:10,
> from ./include/linux/skbuff.h:17,
> from net/ipv4/udp_offload.c:9:
> net/ipv4/udp_offload.c: In function ‘udpv4_offload_init’:
> net/ipv4/udp_offload.c:936:21: error: ‘udp_tunnel_gro_type_lock’ undeclared (first use in this function); did you mean ‘udp_tunnel_gro_rcv’?
> 936 | mutex_init(&udp_tunnel_gro_type_lock);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/mutex.h:64:23: note: in definition of macro ‘mutex_init’
> 64 | __mutex_init((mutex), #mutex, &__key); \
> | ^~~~~
> net/ipv4/udp_offload.c:936:21: note: each undeclared identifier is reported only once for each function it appears in
> 936 | mutex_init(&udp_tunnel_gro_type_lock);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/mutex.h:64:23: note: in definition of macro ‘mutex_init’
> 64 | __mutex_init((mutex), #mutex, &__key); \
> | ^~~~~
> cc1: all warnings being treated as errors
whoops.. thanks will fix in v2!
/P
^ permalink raw reply [flat|nested] 12+ messages in thread