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 <pab...@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..
