From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net-next 01/10] udp: implement complete book-keeping for encap_needed Date: Tue, 13 Nov 2018 23:08:50 -0800 Message-ID: References: <7690feb3a9d55edf3fffe16f5710fa374a3c03ca.1541588248.git.pabeni@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "David S. Miller" , Willem de Bruijn , Steffen Klassert , Subash Abhinov Kasiviswanathan To: Paolo Abeni , netdev@vger.kernel.org Return-path: Received: from mail-pl1-f194.google.com ([209.85.214.194]:36369 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726927AbeKNRKv (ORCPT ); Wed, 14 Nov 2018 12:10:51 -0500 Received: by mail-pl1-f194.google.com with SMTP id w24-v6so7339692plq.3 for ; Tue, 13 Nov 2018 23:08:54 -0800 (PST) In-Reply-To: <7690feb3a9d55edf3fffe16f5710fa374a3c03ca.1541588248.git.pabeni@redhat.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/07/2018 03:38 AM, Paolo Abeni wrote: > 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 > --- > 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 > #include > +#include > > 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..