diff mbox series

[net] net/tcp: use deferred jump label for TCP acked data hook

Message ID 20190508234614.7751-1-jakub.kicinski@netronome.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] net/tcp: use deferred jump label for TCP acked data hook | expand

Commit Message

Jakub Kicinski May 8, 2019, 11:46 p.m. UTC 
User space can flip the clean_acked_data_enabled static branch
on and off with TLS offload when CONFIG_TLS_DEVICE is enabled.
jump_label.h suggests we use the delayed version in this case.

Deferred branches now also don't take the branch mutex on
decrement, so we avoid potential locking issues.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/tcp.h    |  2 +-
 net/ipv4/tcp_input.c | 16 +++++++++++-----
 net/tls/tls_device.c |  1 +
 3 files changed, 13 insertions(+), 6 deletions(-)

Comments

David Miller May 9, 2019, 4:46 p.m. UTC | #1 
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed,  8 May 2019 16:46:14 -0700

> User space can flip the clean_acked_data_enabled static branch
> on and off with TLS offload when CONFIG_TLS_DEVICE is enabled.
> jump_label.h suggests we use the delayed version in this case.
> 
> Deferred branches now also don't take the branch mutex on
> decrement, so we avoid potential locking issues.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

Eric, please review.
Eric Dumazet May 9, 2019, 5:08 p.m. UTC | #2 
On Thu, May 9, 2019 at 9:46 AM David Miller <davem@davemloft.net> wrote:
>
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed,  8 May 2019 16:46:14 -0700
>
> > User space can flip the clean_acked_data_enabled static branch
> > on and off with TLS offload when CONFIG_TLS_DEVICE is enabled.
> > jump_label.h suggests we use the delayed version in this case.
> >
> > Deferred branches now also don't take the branch mutex on
> > decrement, so we avoid potential locking issues.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Simon Horman <simon.horman@netronome.com>
>
> Eric, please review.

This looks fine (and nice), thanks.

Reviewed-by: Eric Dumazet <edumazet@google.com>
David Miller May 9, 2019, 6:14 p.m. UTC | #3 
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed,  8 May 2019 16:46:14 -0700

> User space can flip the clean_acked_data_enabled static branch
> on and off with TLS offload when CONFIG_TLS_DEVICE is enabled.
> jump_label.h suggests we use the delayed version in this case.
> 
> Deferred branches now also don't take the branch mutex on
> decrement, so we avoid potential locking issues.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

Applied, thanks Jakub.
diff mbox series

Patch

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 7cf1181630a3..985aa5db570c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2198,7 +2198,7 @@  extern struct static_key_false tcp_have_smc;
 void clean_acked_data_enable(struct inet_connection_sock *icsk,
 			     void (*cad)(struct sock *sk, u32 ack_seq));
 void clean_acked_data_disable(struct inet_connection_sock *icsk);
-
+void clean_acked_data_flush(void);
 #endif
 
 #endif	/* _TCP_H */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 077d9abdfcf5..20f6fac5882e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -77,7 +77,7 @@ 
 #include <asm/unaligned.h>
 #include <linux/errqueue.h>
 #include <trace/events/tcp.h>
-#include <linux/static_key.h>
+#include <linux/jump_label_ratelimit.h>
 #include <net/busy_poll.h>
 
 int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
@@ -113,22 +113,28 @@  int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
 #define REXMIT_NEW	2 /* FRTO-style transmit of unsent/new packets */
 
 #if IS_ENABLED(CONFIG_TLS_DEVICE)
-static DEFINE_STATIC_KEY_FALSE(clean_acked_data_enabled);
+static DEFINE_STATIC_KEY_DEFERRED_FALSE(clean_acked_data_enabled, HZ);
 
 void clean_acked_data_enable(struct inet_connection_sock *icsk,
 			     void (*cad)(struct sock *sk, u32 ack_seq))
 {
 	icsk->icsk_clean_acked = cad;
-	static_branch_inc(&clean_acked_data_enabled);
+	static_branch_inc(&clean_acked_data_enabled.key);
 }
 EXPORT_SYMBOL_GPL(clean_acked_data_enable);
 
 void clean_acked_data_disable(struct inet_connection_sock *icsk)
 {
-	static_branch_dec(&clean_acked_data_enabled);
+	static_branch_slow_dec_deferred(&clean_acked_data_enabled);
 	icsk->icsk_clean_acked = NULL;
 }
 EXPORT_SYMBOL_GPL(clean_acked_data_disable);
+
+void clean_acked_data_flush(void)
+{
+	static_key_deferred_flush(&clean_acked_data_enabled);
+}
+EXPORT_SYMBOL_GPL(clean_acked_data_flush);
 #endif
 
 static void tcp_gro_dev_warn(struct sock *sk, const struct sk_buff *skb,
@@ -3598,7 +3604,7 @@  static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		icsk->icsk_retransmits = 0;
 
 #if IS_ENABLED(CONFIG_TLS_DEVICE)
-		if (static_branch_unlikely(&clean_acked_data_enabled))
+		if (static_branch_unlikely(&clean_acked_data_enabled.key))
 			if (icsk->icsk_clean_acked)
 				icsk->icsk_clean_acked(sk, ack);
 #endif
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 7b2f40fe339f..5325607b5886 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1038,4 +1038,5 @@  void __exit tls_device_cleanup(void)
 {
 	unregister_netdevice_notifier(&tls_dev_notifier);
 	flush_work(&tls_device_gc_work);
+	clean_acked_data_flush();
 }