Extensions to allow asynchronous TCP_INFO notifications based on congestion parameters

RFC PATCH New Sowmini Varadhan 1 participant 4 messages Oct 22, 2018

Subsystems (5)

Networking Network drivers BPF TCP +1 more subsystems

lore

Share & tools

lore

Sowmini Varadhan

author

2018-10-22 23:48 UTC #0 lore raw

cover Extensions to allow asynchronous TCP_INFO notifications based on congestion parameters

Problem statement:
We would like to monitor some subset of TCP sockets in user-space, (the monitoring application would define 4-tuples it wants to monitor) using TCP_INFO stats to analyze reported problems. The idea is to use those stats to see where the bottlenecks are likely to be ("is it application-limited?" or "is there evidence of BufferBloat in the path?" etc)

Today we can do this by periodically polling for tcp_info, but this could be made more efficient if the kernel would asynchronously notify the application via tcp_info when some "interesting" thresholds (e.g., "RTT variance > X", or "total_retrans > Y" etc) are reached. And to make this effective, it is better if we could apply the threshold check *before* constructing the tcp_info netlink notification, so that we don't waste resources constructing notifications that will be discarded by the filter.

One idea, implemented in this patchset, is to extend the tcp_call_bpf() infra so that the BPF kernel module (the sock_ops filter/callback) can examine the values in the sock_ops, apply any thresholds it wants, and return some new status ("BPF_TCP_INFO_NOTIFY"). Use this status in the tcp stack to queue up a tcp_info notification (similar to sock_diag_broadcast_destroy() today..)

Patch 1 in this set refactors the existing sock_diag code so that the functions can be reused for notifications from other states than CLOSE.

Patch 2 provides a minor extension to tcp_call_bpf() so that it will queue a tcp_info_notification if the BPF callout returns BPF_TCP_INFO_NOTIFY

Patch 3, provided strictly as a demonstration/PoC to aid in reviewing this proposal, shows a simple sample/bpf example where we trigger the tcp_info notification for an iperf connection if the number of retransmits exceeds 16.

Sowmini Varadhan (3):
sock_diag: Refactor inet_sock_diag_destroy code
tcp: BPF_TCP_INFO_NOTIFY support
bpf: Added a sample for tcp_info_notify callback

 include/linux/sock_diag.h      |   18 +++++++---
 include/net/tcp.h              |   15 +++++++-
 include/uapi/linux/bpf.h       |    4 ++
 include/uapi/linux/sock_diag.h |    2 +
 net/core/sock.c                |    4 +-
 net/core/sock_diag.c           |   11 +++---
 samples/bpf/Makefile           |    1 +
 samples/bpf/tcp_notify_kern.c  |   73 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 114 insertions(+), 14 deletions(-)
 create mode 100644 samples/bpf/tcp_notify_kern.c

Sowmini Varadhan

author

2018-10-22 23:48 UTC #1 lore raw

RFC PATCH 1/3 sock_diag: Refactor inet_sock_diag_destroy code

We want to use the inet_sock_diag_destroy code to send notifications for more types of TCP events than just socket_close(), so refactor the code to allow this.

Signed-off-by Sowmini Varadhan <[email protected]>

---
 include/linux/sock_diag.h      |   18 +++++++++++++-----
 include/uapi/linux/sock_diag.h |    2 ++
 net/core/sock.c                |    4 ++--
 net/core/sock_diag.c           |   11 ++++++-----
 4 files changed, 23 insertions(+), 12 deletions(-)

include/linux/sock_diag.h +13 -5

index 15fe980..df85767 100644
@@ -34,7 +34,7 @@ int sock_diag_put_filterinfo(bool may_report_filterinfo, struct sock *sk,
 			     struct sk_buff *skb, int attrtype);
 
 static inline
-enum sknetlink_groups sock_diag_destroy_group(const struct sock *sk)
+enum sknetlink_groups sock_diag_group(const struct sock *sk)
 {
 	switch (sk->sk_family) {
 	case AF_INET:

@@ -43,7 +43,15 @@ enum sknetlink_groups sock_diag_destroy_group(const struct sock *sk)
 
 		switch (sk->sk_protocol) {
 		case IPPROTO_TCP:
-			return SKNLGRP_INET_TCP_DESTROY;
+			switch (sk->sk_state) {
+			case TCP_ESTABLISHED:
+				return SKNLGRP_INET_TCP_CONNECTED;
+			case TCP_SYN_SENT:
+			case TCP_SYN_RECV:
+				return SKNLGRP_INET_TCP_3WH;
+			default:
+				return SKNLGRP_INET_TCP_DESTROY;
+			}
 		case IPPROTO_UDP:
 			return SKNLGRP_INET_UDP_DESTROY;
 		default:

@@ -67,15 +75,15 @@ enum sknetlink_groups sock_diag_destroy_group(const struct sock *sk)
 }
 
 static inline
-bool sock_diag_has_destroy_listeners(const struct sock *sk)
+bool sock_diag_has_listeners(const struct sock *sk)
 {
 	const struct net *n = sock_net(sk);
-	const enum sknetlink_groups group = sock_diag_destroy_group(sk);
+	const enum sknetlink_groups group = sock_diag_group(sk);
 
 	return group != SKNLGRP_NONE && n->diag_nlsk &&
 		netlink_has_listeners(n->diag_nlsk, group);
 }
-void sock_diag_broadcast_destroy(struct sock *sk);
+void sock_diag_broadcast(struct sock *sk);
 
 int sock_diag_destroy(struct sock *sk, int err);
 #endif

include/uapi/linux/sock_diag.h +2 -0

index e592500..4252674 100644
@@ -32,6 +32,8 @@ enum sknetlink_groups {
 	SKNLGRP_INET_UDP_DESTROY,
 	SKNLGRP_INET6_TCP_DESTROY,
 	SKNLGRP_INET6_UDP_DESTROY,
+	SKNLGRP_INET_TCP_3WH,
+	SKNLGRP_INET_TCP_CONNECTED,
 	__SKNLGRP_MAX,
 };
 #define SKNLGRP_MAX	(__SKNLGRP_MAX - 1)

net/core/sock.c +2 -2

index 7e8796a..6684840 100644
@@ -1600,8 +1600,8 @@ static void __sk_free(struct sock *sk)
 	if (likely(sk->sk_net_refcnt))
 		sock_inuse_add(sock_net(sk), -1);
 
-	if (unlikely(sk->sk_net_refcnt && sock_diag_has_destroy_listeners(sk)))
-		sock_diag_broadcast_destroy(sk);
+	if (unlikely(sk->sk_net_refcnt && sock_diag_has_listeners(sk)))
+		sock_diag_broadcast(sk);
 	else
 		sk_destruct(sk);
 }

net/core/sock_diag.c +6 -5

index 3312a58..dbd9e65 100644
@@ -116,14 +116,14 @@ static size_t sock_diag_nlmsg_size(void)
 	       + nla_total_size_64bit(sizeof(struct tcp_info))); /* INET_DIAG_INFO */
 }
 
-static void sock_diag_broadcast_destroy_work(struct work_struct *work)
+static void sock_diag_broadcast_work(struct work_struct *work)
 {
 	struct broadcast_sk *bsk =
 		container_of(work, struct broadcast_sk, work);
 	struct sock *sk = bsk->sk;
 	const struct sock_diag_handler *hndl;
 	struct sk_buff *skb;
-	const enum sknetlink_groups group = sock_diag_destroy_group(sk);
+	const enum sknetlink_groups group = sock_diag_group(sk);
 	int err = -1;
 
 	WARN_ON(group == SKNLGRP_NONE);

@@ -144,11 +144,12 @@ static void sock_diag_broadcast_destroy_work(struct work_struct *work)
 	else
 		kfree_skb(skb);
 out:
-	sk_destruct(sk);
+	if (group <= SKNLGRP_INET6_UDP_DESTROY)
+		sk_destruct(sk);
 	kfree(bsk);
 }
 
-void sock_diag_broadcast_destroy(struct sock *sk)
+void sock_diag_broadcast(struct sock *sk)
 {
 	/* Note, this function is often called from an interrupt context. */
 	struct broadcast_sk *bsk =

@@ -156,7 +157,7 @@ void sock_diag_broadcast_destroy(struct sock *sk)
 	if (!bsk)
 		return sk_destruct(sk);
 	bsk->sk = sk;
-	INIT_WORK(&bsk->work, sock_diag_broadcast_destroy_work);
+	INIT_WORK(&bsk->work, sock_diag_broadcast_work);
 	queue_work(broadcast_wq, &bsk->work);
 }

Signature · 1 line

1.7.1

Sowmini Varadhan

author

2018-10-22 23:48 UTC #2 lore raw

RFC PATCH 2/3 tcp: BPF_TCP_INFO_NOTIFY support

In reply to Sowmini Varadhan

We want to be able to set up the monitoring application so that it can be aysnchronously notified when "interesting" events happen, e.g., when application-determined thresholds on parameters like RTT estimate, number of retransmissions, RTO are reached.

The bpf_sock_ops infrastructure provided as part of Commit 40304b2a1567 ("bpf: BPF support for sock_ops") provides an elegant way to trigger this asynchronous notification. The BPF program can examine the current TCP state reported in the bpf_sock_ops and conditionally return a (new) status BPF_TCP_INFO_NOTIFY. The return status is used by the caller to queue up a tcp_info notification for the application.

Signed-off-by Sowmini Varadhan <[email protected]>

---
 include/net/tcp.h        |   15 +++++++++++++--
 include/uapi/linux/bpf.h |    4 ++++
 2 files changed, 17 insertions(+), 2 deletions(-)

include/net/tcp.h +13 -2

index 0d29292..df06a9f 100644
@@ -47,6 +47,7 @@
 #include <linux/seq_file.h>
 #include <linux/memcontrol.h>
 #include <linux/bpf-cgroup.h>
+#include <linux/sock_diag.h>
 
 extern struct inet_hashinfo tcp_hashinfo;

@@ -2065,6 +2066,12 @@ struct tcp_ulp_ops {
 	__MODULE_INFO(alias, alias_userspace, name);		\
 	__MODULE_INFO(alias, alias_tcp_ulp, "tcp-ulp-" name)
 
+#define	TCPDIAG_CB(sk)							\
+do {									\
+	if (unlikely(sk->sk_net_refcnt && sock_diag_has_listeners(sk)))	\
+		sock_diag_broadcast(sk);				\
+} while (0)
+
 /* Call BPF_SOCK_OPS program that returns an int. If the return value
  * is < 0, then the BPF op failed (for example if the loaded BPF
  * program does not support the chosen operation or there is no BPF

@@ -2088,9 +2095,13 @@ static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
 		memcpy(sock_ops.args, args, nargs * sizeof(*args));
 
 	ret = BPF_CGROUP_RUN_PROG_SOCK_OPS(&sock_ops);
-	if (ret == 0)
+	if (ret == 0) {
 		ret = sock_ops.reply;
-	else
+
+		/* XXX would be nice if we could use replylong[1] here */
+		if (ret == BPF_TCP_INFO_NOTIFY)
+			TCPDIAG_CB(sk);
+	} else
 		ret = -1;
 	return ret;
 }

include/uapi/linux/bpf.h +4 -0

index aa5ccd2..bc45e5e 100644
@@ -2678,6 +2678,10 @@ enum {
 	BPF_TCP_MAX_STATES	/* Leave at the end! */
 };
 
+enum {
+	BPF_TCP_INFO_NOTIFY = 2
+};
+
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
 #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */

Signature · 1 line

1.7.1

Sowmini Varadhan

author

2018-10-22 23:48 UTC #3 lore raw

RFC PATCH 3/3 bpf: Added a sample for tcp_info_notify callback

In reply to Sowmini Varadhan

Simple Proof-Of-Concept test program for BPF_TCP_INFO_NOTIFY (will move this to testing/selftests/net later)

Signed-off-by Sowmini Varadhan <[email protected]>

---
 samples/bpf/Makefile          |    1 +
 samples/bpf/tcp_notify_kern.c |   73 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 samples/bpf/tcp_notify_kern.c

samples/bpf/Makefile +1 -0

index be0a961..d937bd2 100644
@@ -152,6 +152,7 @@ always += tcp_bufs_kern.o
 always += tcp_cong_kern.o
 always += tcp_iw_kern.o
 always += tcp_clamp_kern.o
+always += tcp_notify_kern.o
 always += tcp_basertt_kern.o
 always += tcp_tos_reflect_kern.o
 always += xdp_redirect_kern.o

samples/bpf/tcp_notify_kern.c new file +73 -0

73 lines not on this page · raw message

Signature · 1 line

1.7.1