Re: [PATCH net-next 01/10] udp: implement complete book-keeping for encap_needed
From: Eric Dumazet <hidden>
Date: 2018-11-14 17:10:51
On 11/07/2018 03:38 AM, Paolo Abeni wrote:
quoted hunk ↗ jump to hunk
The *encap_needed static keys are enabled by UDP tunnels and several UDP encapsulations type, but they are never turned off. This can cause unneeded overall performance degradation for systems where such features are used transiently. This patch introduces complete book-keeping for such keys, decreasing the usage at socket destruction time, if needed, and avoiding that the same socket could increase the key usage multiple times. rfc v3 -> v1: - add socket lock around udp_tunnel_encap_enable() rfc v2 -> rfc v3: - use udp_tunnel_encap_enable() in setsockopt() Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- include/linux/udp.h | 7 ++++++- include/net/udp_tunnel.h | 6 ++++++ net/ipv4/udp.c | 19 +++++++++++++------ net/ipv6/udp.c | 14 +++++++++----- 4 files changed, 34 insertions(+), 12 deletions(-)diff --git a/include/linux/udp.h b/include/linux/udp.h index 320d49d85484..a4dafff407fb 100644 --- a/include/linux/udp.h +++ b/include/linux/udp.h@@ -49,7 +49,12 @@ struct udp_sock { unsigned int corkflag; /* Cork is required */ __u8 encap_type; /* Is this an Encapsulation socket? */ unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */ - no_check6_rx:1;/* Allow zero UDP6 checksums on RX? */ + no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */ + encap_enabled:1; /* This socket enabled encap + * processing; UDP tunnels and + * different encapsulation layer set + * this + */ /* * Following member retains the information to create a UDP header * when the socket is uncorked.diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h index fe680ab6b15a..3fbe56430e3b 100644 --- a/include/net/udp_tunnel.h +++ b/include/net/udp_tunnel.h@@ -165,6 +165,12 @@ static inline int udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum) static inline void udp_tunnel_encap_enable(struct socket *sock) { + struct udp_sock *up = udp_sk(sock->sk); + + if (up->encap_enabled) + return; + + up->encap_enabled = 1; #if IS_ENABLED(CONFIG_IPV6) if (sock->sk->sk_family == PF_INET6) ipv6_stub->udpv6_encap_enable();diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 1976fddb9e00..0ed715a72249 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c@@ -115,6 +115,7 @@ #include "udp_impl.h" #include <net/sock_reuseport.h> #include <net/addrconf.h> +#include <net/udp_tunnel.h> struct udp_table udp_table __read_mostly; EXPORT_SYMBOL(udp_table);@@ -2398,11 +2399,15 @@ void udp_destroy_sock(struct sock *sk) bool slow = lock_sock_fast(sk); udp_flush_pending_frames(sk); unlock_sock_fast(sk, slow); - if (static_branch_unlikely(&udp_encap_needed_key) && up->encap_type) { - void (*encap_destroy)(struct sock *sk); - encap_destroy = READ_ONCE(up->encap_destroy); - if (encap_destroy) - encap_destroy(sk); + if (static_branch_unlikely(&udp_encap_needed_key)) { + if (up->encap_type) { + void (*encap_destroy)(struct sock *sk); + encap_destroy = READ_ONCE(up->encap_destroy); + if (encap_destroy) + encap_destroy(sk); + } + if (up->encap_enabled) + static_branch_disable(&udp_encap_needed_key); } }@@ -2447,7 +2452,9 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname, /* FALLTHROUGH */ case UDP_ENCAP_L2TPINUDP: up->encap_type = val; - udp_encap_enable(); + lock_sock(sk); + udp_tunnel_encap_enable(sk->sk_socket); + release_sock(sk); break; default: err = -ENOPROTOOPT;diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index d2d97d07ef27..fc0ce6c59ebb 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c@@ -1458,11 +1458,15 @@ void udpv6_destroy_sock(struct sock *sk) udp_v6_flush_pending_frames(sk); release_sock(sk); - if (static_branch_unlikely(&udpv6_encap_needed_key) && up->encap_type) { - void (*encap_destroy)(struct sock *sk); - encap_destroy = READ_ONCE(up->encap_destroy); - if (encap_destroy) - encap_destroy(sk); + if (static_branch_unlikely(&udpv6_encap_needed_key)) { + if (up->encap_type) { + void (*encap_destroy)(struct sock *sk); + encap_destroy = READ_ONCE(up->encap_destroy); + if (encap_destroy) + encap_destroy(sk); + } + if (up->encap_enabled) + static_branch_disable(&udpv6_encap_needed_key);
Where is the opposite operation done ? syzbot reported : WARNING: CPU: 0 PID: 7564 at kernel/jump_label.c:188 static_key_disable_cpuslocked+0x2bb/0x310 kernel/jump_label.c:188 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 7564 Comm: syz-executor2 Not tainted 4.20.0-rc1-next-20181109+ #110 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x244/0x39d lib/dump_stack.c:113 panic+0x2ad/0x55c kernel/panic.c:188 __warn.cold.8+0x20/0x45 kernel/panic.c:540 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969 RIP: 0010:static_key_disable_cpuslocked+0x2bb/0x310 kernel/jump_label.c:188 Code: ff eb db e8 07 da e3 ff 48 89 da 48 c7 c6 00 bf 31 88 48 c7 c7 e0 bb 31 88 e8 71 0d ad ff 0f 0b e9 0e fe ff ff e8 e5 d9 e3 ff <0f> 0b e9 2b ff ff ff 4c 89 f7 e8 56 32 27 00 e9 5e fe ff ff 48 89 RSP: 0018:ffff8801ba2b7928 EFLAGS: 00010293 RAX: ffff8801bb558340 RBX: 00000000ffffffff RCX: ffffffff819bcdf5 RDX: 0000000000000000 RSI: ffffffff819bcecb RDI: 0000000000000005 RBP: ffff8801ba2b79f0 R08: ffff8801bb558340 R09: fffffbfff17885c4 R10: fffffbfff17885c4 R11: ffffffff8bc42e23 R12: 1ffff10037456f25 R13: 1ffff10037456f31 R14: dffffc0000000000 R15: ffff8801ba2b79c8 static_key_disable+0x1a/0x30 kernel/jump_label.c:202 udpv6_destroy_sock+0x1e3/0x220 net/ipv6/udp.c:1492 sk_common_release+0x6d/0x320 net/core/sock.c:3001 udp_lib_close+0x15/0x20 include/net/udp.h:206 inet_release+0x104/0x1f0 net/ipv4/af_inet.c:428 inet6_release+0x50/0x70 net/ipv6/af_inet6.c:458 __sock_release+0xd7/0x250 net/socket.c:580 sock_close+0x19/0x20 net/socket.c:1142 __fput+0x3bc/0xa70 fs/file_table.c:279 ____fput+0x15/0x20 fs/file_table.c:312 task_work_run+0x1e8/0x2a0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166 prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline] syscall_return_slowpath arch/x86/entry/common.c:268 [inline] do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x411021 Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 34 19 00 00 c3 48 83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 RSP: 002b:00007ffeb03ec0a0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003 RAX: 0000000000000000 RBX: 0000000000000004 RCX: 0000000000411021 RDX: 0000000000000001 RSI: 0000000000731ce8 RDI: 0000000000000003 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 R10: 00007ffeb03ebfd0 R11: 0000000000000293 R12: 0000000000000000 R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000002 Kernel Offset: disabled Rebooting in 86400 seconds..