Skip to contentSkip to conversation

bpf: more sock_ops callbacks

PATCH In review Lawrence Brakmo 3 participants 15 messages Dec 19, 2017 – Dec 20, 2017

Subsystems (8)
Networking Network drivers Kselftest BPF +4 more subsystems
lore
Share & tools
lore

Lawrence Brakmo

author
#0 lore raw

cover bpf: more sock_ops callbacks

This patchset adds support for:

- direct R or R/W access to many tcp_sock fields
- passing up to 4 arguments to sock_ops BPF functions
- tcp_sock field bpf_sock_ops_flags for controlling callbacks
- optionally calling sock_ops BPF program when RTO fires
- optionally calling sock_ops BPF program when packet is retransmitted
- optionally calling sock_ops BPF program when TCP state changes
- access to tclass and sk_txhash
- new selftest

Signed-off-by Lawrence Brakmo <[email protected]>

Consists of the following patches:
[PATCH bpf 01/11] bpf: Make SOCK_OPS_GET_TCP size independent [PATCH bpf 02/11] bpf: Make SOCK_OPS_GET_TCP struct independent [PATCH bpf 03/11] bpf: Add write access to tcp_sock and sock fields [PATCH bpf 04/11] bpf: Support passing args to sock_ops bpf function [PATCH bpf 05/11] bpf: Adds field bpf_sock_ops_flags to tcp_sock [PATCH bpf 06/11] bpf: Add sock_ops RTO callback
[PATCH bpf 07/11] bpf: Add support for reading sk_state and more [PATCH bpf 08/11] bpf: Add sock_ops R/W access to tclass & sk_txhash [PATCH bpf 09/11] bpf: Add BPF_SOCK_OPS_RETRANS_CB
[PATCH bpf 10/11] bpf: Add BPF_SOCK_OPS_STATE_CB
[PATCH bpf 11/11] bpf: add selftest for tcpbpf

include/linux/filter.h                         |   4 +
include/linux/tcp.h                            |   8 ++
include/net/tcp.h                              |  66 +++++++++-
include/uapi/linux/bpf.h                       |  39 +++++-
include/uapi/linux/tcp.h                       |   5 +
net/core/filter.c                              | 212 ++++++++++++++++++++++++++++++--
net/ipv4/tcp.c                                 |   4 +-
net/ipv4/tcp_nv.c                              |   2 +-
net/ipv4/tcp_output.c                          |   5 +-
net/ipv4/tcp_timer.c                           |   9 ++
tools/include/uapi/linux/bpf.h                 |  45 ++++++-
tools/testing/selftests/bpf/Makefile           |   5 +-
tools/testing/selftests/bpf/tcp_client.py      |  57 +++++++++
tools/testing/selftests/bpf/tcp_server.py      |  83 +++++++++++++
tools/testing/selftests/bpf/test_tcpbpf_kern.c | 133 ++++++++++++++++++++
tools/testing/selftests/bpf/test_tcpbpf_user.c | 119 ++++++++++++++++++
16 files changed, 772 insertions(+), 24 deletions(-)
author
#1 lore raw

PATCH 1/11 bpf: Make SOCK_OPS_GET_TCP size independent

Make SOCK_OPS_GET_TCP helper macro size independent (before only worked with 4-byte fields.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 net/core/filter.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
net/core/filter.c +8 -5
index 754abe1..d47d126 100644
@@ -4448,9 +4448,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		break;
 
 /* Helper macro for adding read access to tcp_sock fields. */
-#define SOCK_OPS_GET_TCP32(FIELD_NAME)					      \
+#define SOCK_OPS_GET_TCP(FIELD_NAME)					      \
 	do {								      \
-		BUILD_BUG_ON(FIELD_SIZEOF(struct tcp_sock, FIELD_NAME) != 4); \
+		BUILD_BUG_ON(FIELD_SIZEOF(struct tcp_sock, FIELD_NAME) >      \
+			     FIELD_SIZEOF(struct bpf_sock_ops, FIELD_NAME));  \
 		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \
 						struct bpf_sock_ops_kern,     \
 						is_fullsock),		      \

@@ -4462,16 +4463,18 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 						struct bpf_sock_ops_kern, sk),\
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
-		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,        \
+		*insn++ = BPF_LDX_MEM(FIELD_SIZEOF(struct tcp_sock,	      \
+						   FIELD_NAME), si->dst_reg,  \
+				      si->dst_reg,			      \
 				      offsetof(struct tcp_sock, FIELD_NAME)); \
 	} while (0)
 
 	case offsetof(struct bpf_sock_ops, snd_cwnd):
-		SOCK_OPS_GET_TCP32(snd_cwnd);
+		SOCK_OPS_GET_TCP(snd_cwnd);
 		break;
 
 	case offsetof(struct bpf_sock_ops, srtt_us):
-		SOCK_OPS_GET_TCP32(srtt_us);
+		SOCK_OPS_GET_TCP(srtt_us);
 		break;
 	}
 	return insn - insn_buf;
Signature · 1 line
2.9.5
author
#2 lore raw

PATCH 2/11 bpf: Make SOCK_OPS_GET_TCP struct independent

In reply to Lawrence Brakmo

Changed SOCK_OPS_GET_TCP to SOCK_OPS_GET_FIELD and added a new argument so now it can also work with struct sock fields.

Previous: SOCK_OPS_GET_TCP(FIELD_NAME)
New:      SOCK_OPS_GET_FIELD(FIELD_NAME, OBJ)

Where OBJ is either "struct tcp_sock" or "struct sock" (without quotation). Assumes FIELD_NAME is a field in the struct bpf_sock_ops and in the OBJ specified.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 net/core/filter.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
net/core/filter.c +9 -9
index d47d126..f808269 100644
@@ -4447,10 +4447,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 					       is_fullsock));
 		break;
 
-/* Helper macro for adding read access to tcp_sock fields. */
-#define SOCK_OPS_GET_TCP(FIELD_NAME)					      \
+/* Helper macro for adding read access to tcp_sock or sock fields. */
+#define SOCK_OPS_GET_FIELD(FIELD_NAME, OBJ)				      \
 	do {								      \
-		BUILD_BUG_ON(FIELD_SIZEOF(struct tcp_sock, FIELD_NAME) >      \
+		BUILD_BUG_ON(FIELD_SIZEOF(OBJ, FIELD_NAME) >		      \
 			     FIELD_SIZEOF(struct bpf_sock_ops, FIELD_NAME));  \
 		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \
 						struct bpf_sock_ops_kern,     \

@@ -4463,18 +4463,18 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 						struct bpf_sock_ops_kern, sk),\
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
-		*insn++ = BPF_LDX_MEM(FIELD_SIZEOF(struct tcp_sock,	      \
-						   FIELD_NAME), si->dst_reg,  \
-				      si->dst_reg,			      \
-				      offsetof(struct tcp_sock, FIELD_NAME)); \
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(OBJ,		      \
+						       FIELD_NAME),	      \
+				      si->dst_reg, si->dst_reg,		      \
+				      offsetof(OBJ, FIELD_NAME));	      \
 	} while (0)
 
 	case offsetof(struct bpf_sock_ops, snd_cwnd):
-		SOCK_OPS_GET_TCP(snd_cwnd);
+		SOCK_OPS_GET_FIELD(snd_cwnd, struct tcp_sock);
 		break;
 
 	case offsetof(struct bpf_sock_ops, srtt_us):
-		SOCK_OPS_GET_TCP(srtt_us);
+		SOCK_OPS_GET_FIELD(srtt_us, struct tcp_sock);
 		break;
 	}
 	return insn - insn_buf;
Signature · 1 line
2.9.5
author
#3 lore raw

PATCH 3/11 bpf: Add write access to tcp_sock and sock fields

In reply to Lawrence Brakmo

This patch adds a macro, SOCK_OPS_SET_FIELD, for writing to struct tcp_sock or struct sock fields. This required adding a new field "temp" to struct bpf_sock_ops_kern for temporary storage that is used by sock_ops_convert_ctx_access. It is used to store and recover the contents of a register, so the register can be used to store the address of the sk. Since we cannot overwrite the dst_reg because it contains the pointer to ctx, nor the src_reg since it contains the value we want to store, we need an extra register to contain the address of the sk.

Also adds the macro SOCK_OPS_GET_OR_SET_FIELD that calls one of the GET or SET macros depending on the value of the TYPE field.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/linux/filter.h |  3 +++
 include/net/tcp.h      |  2 +-
 net/core/filter.c      | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
include/linux/filter.h +3 -0
index 5feb441..8929162 100644
@@ -987,6 +987,9 @@ struct bpf_sock_ops_kern {
 		u32 replylong[4];
 	};
 	u32	is_fullsock;
+	u64	temp;			/* Used by sock_ops_convert_ctx_access
+					 * as temporary storaage of a register
+					 */
 };
 
 #endif /* __LINUX_FILTER_H__ */
include/net/tcp.h +1 -1
index 6cc205c..e0213f1 100644
@@ -2011,7 +2011,7 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
 	struct bpf_sock_ops_kern sock_ops;
 	int ret;
 
-	memset(&sock_ops, 0, sizeof(sock_ops));
+	memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, is_fullsock));
 	if (sk_fullsock(sk)) {
 		sock_ops.is_fullsock = 1;
 		sock_owned_by_me(sk);
net/core/filter.c +46 -0

52 lines not on this page · raw message

Signature · 1 line
2.9.5
maintainer
#4 lore raw

Re: [PATCH bpf 03/11] bpf: Add write access to tcp_sock and sock fields

Quoted 39 lines · Lawrence Brakmo

This patch adds a macro, SOCK_OPS_SET_FIELD, for writing to struct tcp_sock or struct sock fields. This required adding a new field "temp" to struct bpf_sock_ops_kern for temporary storage that is used by sock_ops_convert_ctx_access. It is used to store and recover the contents of a register, so the register can be used to store the address of the sk. Since we cannot overwrite the dst_reg because it contains the pointer to ctx, nor the src_reg since it contains the value we want to store, we need an extra register to contain the address of the sk.

Also adds the macro SOCK_OPS_GET_OR_SET_FIELD that calls one of the GET or SET macros depending on the value of the TYPE field.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/linux/filter.h |  3 +++
 include/net/tcp.h      |  2 +-
 net/core/filter.c      | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
include/linux/filter.h +3 -0
index 5feb441..8929162 100644
@@ -987,6 +987,9 @@ struct bpf_sock_ops_kern {
 		u32 replylong[4];
 	};
 	u32	is_fullsock;
+	u64	temp;			/* Used by sock_ops_convert_ctx_access
+					 * as temporary storaage of a register
+					 */
 };
 
 #endif /* __LINUX_FILTER_H__ */
include/net/tcp.h +0 -0
index 6cc205c..e0213f1 100644

include/net/tcp.h static inline int tcp_call_bpf(struct sock *sk, int op) · line 2014

Quoted 5 lines
include/net/tcp.h +1 -1
@@ -2011,7 +2011,7 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
 	struct bpf_sock_ops_kern sock_ops;
 	int ret;
 
-	memset(&sock_ops, 0, sizeof(sock_ops));
+	memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, is_fullsock));

I don't think this is correct. sock_ops is on stack, so above you only zero up to the offset of is_fullsock, but not including it, so when you have !sk_fullsock(sk), then your BPF prog will still act as if the sock_ops.is_fullsock was set in case prior stack garbage said so.

include/net/tcp.h static inline int tcp_call_bpf(struct sock *sk, int op) · line 2017

Quoted 3 lines
include/net/tcp.h +0 -0
 	if (sk_fullsock(sk)) {
 		sock_ops.is_fullsock = 1;
 		sock_owned_by_me(sk);

Thanks,
Daniel

#5 lore raw

Re: [PATCH bpf 03/11] bpf: Add write access to tcp_sock and sock fields

In reply to Lawrence Brakmo
Quoted 18 lines · Lawrence Brakmo
net/core/filter.c +18 -0
+#define SOCK_OPS_SET_FIELD(FIELD_NAME, OBJ)				      \
+	do {								      \
+		int reg = BPF_REG_9;					      \
+		BUILD_BUG_ON(FIELD_SIZEOF(OBJ, FIELD_NAME) >		      \
+			     FIELD_SIZEOF(struct bpf_sock_ops, FIELD_NAME));  \
+		while (si->dst_reg == reg || si->src_reg == reg)	      \
+			reg--;						      \
+		*insn++ = BPF_STX_MEM(BPF_DW, si->dst_reg, reg,		      \
+				      offsetof(struct bpf_sock_ops_kern,      \
+					       temp));			      \
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \
+						struct bpf_sock_ops_kern,     \
+						is_fullsock),		      \
+				      reg, si->dst_reg,			      \
+				      offsetof(struct bpf_sock_ops_kern,      \
+					       is_fullsock));		      \
+		*insn++ = BPF_JMP_IMM(BPF_JEQ, reg, 0, 2);		      \
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \

net/core/filter.c static u32 sock_ops_convert_ctx_access(enum bpf_access_type type, · line 4508

Quoted 10 lines
net/core/filter.c +10 -0
+						struct bpf_sock_ops_kern, sk),\
+				      reg, si->dst_reg,			      \
+				      offsetof(struct bpf_sock_ops_kern, sk));\
+		*insn++ = BPF_STX_MEM(BPF_FIELD_SIZEOF(OBJ, FIELD_NAME),      \
+				      reg, si->src_reg,			      \
+				      offsetof(OBJ, FIELD_NAME));	      \
+		*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->dst_reg,		      \
+				      offsetof(struct bpf_sock_ops_kern,      \
+					       temp));			      \
+	} while (0)

that's neat. I like it.
I guess the prog can check is_fullsock on its own to see whether writes will fail or not, so JEQ above is ok.
Only while() loop looks a bit scary.
May be replace with two 'if' ?
if (si->dst_reg == reg || si->src_reg == reg)
reg --;
if (si->dst_reg == reg || si->src_reg == reg)
reg --;
so it's clear that tmp reg will be reg_7, 8 or 9.

author
#6 lore raw

PATCH 4/11 bpf: Support passing args to sock_ops bpf function

In reply to Lawrence Brakmo

Adds support for passing up to 4 arguments to sock_ops bpf functions. It reusues the reply union, so the bpf_sock_ops structures are not increased in size.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/linux/filter.h   |  1 +
 include/net/tcp.h        | 64 ++++++++++++++++++++++++++++++++++++++++++++----
 include/uapi/linux/bpf.h |  5 ++--
 net/ipv4/tcp.c           |  2 +-
 net/ipv4/tcp_nv.c        |  2 +-
 net/ipv4/tcp_output.c    |  2 +-
 6 files changed, 66 insertions(+), 10 deletions(-)
include/linux/filter.h +1 -0
index 8929162..2a09f27 100644
@@ -983,6 +983,7 @@ struct bpf_sock_ops_kern {
 	struct	sock *sk;
 	u32	op;
 	union {
+		u32 args[4];
 		u32 reply;
 		u32 replylong[4];
 	};
include/net/tcp.h +59 -5

104 lines not on this page · raw message

include/uapi/linux/bpf.h +3 -2
index 595bda1..addd849 100644
@@ -936,8 +936,9 @@ struct bpf_map_info {
 struct bpf_sock_ops {
 	__u32 op;
 	union {
-		__u32 reply;
-		__u32 replylong[4];
+		__u32 args[4];		/* Optionally passed to bpf program */
+		__u32 reply;		/* Returned by bpf program	    */
+		__u32 replylong[4];	/* Optionally returned by bpf prog  */
 	};
 	__u32 family;
 	__u32 remote_ip4;	/* Stored in network byte order */
net/ipv4/tcp.c +1 -1
index 1803116..817df3f 100644
@@ -465,7 +465,7 @@ void tcp_init_transfer(struct sock *sk, int bpf_op)
 	tcp_mtup_init(sk);
 	icsk->icsk_af_ops->rebuild_header(sk);
 	tcp_init_metrics(sk);
-	tcp_call_bpf(sk, bpf_op);
+	tcp_call_bpf(sk, bpf_op, 0, NULL);
 	tcp_init_congestion_control(sk);
 	tcp_init_buffer_space(sk);
 }
net/ipv4/tcp_nv.c +1 -1
index 0b5a05b..ddbce73 100644
@@ -146,7 +146,7 @@ static void tcpnv_init(struct sock *sk)
 	 * within a datacenter, where we have reasonable estimates of
 	 * RTTs
 	 */
-	base_rtt = tcp_call_bpf(sk, BPF_SOCK_OPS_BASE_RTT);
+	base_rtt = tcp_call_bpf(sk, BPF_SOCK_OPS_BASE_RTT, 0, NULL);
 	if (base_rtt > 0) {
 		ca->nv_base_rtt = base_rtt;
 		ca->nv_lower_bound_rtt = (base_rtt * 205) >> 8; /* 80% */
net/ipv4/tcp_output.c +1 -1
index a4d214c..50cb242 100644
@@ -3471,7 +3471,7 @@ int tcp_connect(struct sock *sk)
 	struct sk_buff *buff;
 	int err;
 
-	tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB);
+	tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB, 0, NULL);
 
 	if (inet_csk(sk)->icsk_af_ops->rebuild_header(sk))
 		return -EHOSTUNREACH; /* Routing failure or similar. */
Signature · 1 line
2.9.5
author
#7 lore raw

PATCH 5/11 bpf: Adds field bpf_sock_ops_flags to tcp_sock

In reply to Lawrence Brakmo

Adds field bpf_sock_ops_flags to tcp_sock and bpf_sock_ops. Its primary use is to determine if there should be calls to sock_ops bpf program at various points in the TCP code. The field is initialized to zero, disabling the calls. A sock_ops BPF program can set, per connection and as necessary, when the connection is established.

It also adds support for reading and writting the field within a sock_ops BPF program.

Examples of where to call the bpf program:

1) When RTO fires
2) When a packet is retransmitted
3) When the connection terminates
4) When a packet is sent
5) When a packet is received

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/linux/tcp.h      | 8 ++++++++
 include/uapi/linux/bpf.h | 1 +
 net/core/filter.c        | 6 ++++++
 3 files changed, 15 insertions(+)
include/linux/tcp.h +8 -0
index df5d97a..c46553f 100644
@@ -372,6 +372,14 @@ struct tcp_sock {
 	 */
 	struct request_sock *fastopen_rsk;
 	u32	*saved_syn;
+
+/* Sock_ops bpf program related variables */
+#ifdef CONFIG_BPF
+	u32     bpf_sock_ops_flags;     /* values defined in uapi/linux/tcp.h */
+#define BPF_SOCK_OPS_TEST_FLAG(TP, ARG) (TP->bpf_sock_ops_flags & ARG)
+#else
+#define BPF_SOCK_OPS_TEST_FLAG(TP, ARG) 0
+#endif
 };
 
 enum tsq_enum {
include/uapi/linux/bpf.h +1 -0
index addd849..dfbf43a 100644
@@ -953,6 +953,7 @@ struct bpf_sock_ops {
 				 */
 	__u32 snd_cwnd;
 	__u32 srtt_us;		/* Averaged RTT << 3 in usecs */
+	__u32 bpf_sock_ops_flags; /* flags defined in uapi/linux/tcp.h */
 };
 
 /* List of known BPF sock_ops operators.
net/core/filter.c +6 -0
index 97e65df..2692514 100644
@@ -3842,6 +3842,7 @@ static bool sock_ops_is_valid_access(int off, int size,
 		switch (off) {
 		case offsetof(struct bpf_sock_ops, op) ...
 		     offsetof(struct bpf_sock_ops, replylong[3]):
+		case offsetof(struct bpf_sock_ops, bpf_sock_ops_flags):
 			break;
 		default:
 			return false;

@@ -4522,6 +4523,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 	case offsetof(struct bpf_sock_ops, srtt_us):
 		SOCK_OPS_GET_FIELD(srtt_us, struct tcp_sock);
 		break;
+
+	case offsetof(struct bpf_sock_ops, bpf_sock_ops_flags):
+		SOCK_OPS_GET_OR_SET_FIELD(bpf_sock_ops_flags, struct tcp_sock,
+					  type);
+		break;
 	}
 	return insn - insn_buf;
 }
Signature · 1 line
2.9.5
author
#8 lore raw

PATCH 6/11 bpf: Add sock_ops RTO callback

In reply to Lawrence Brakmo

Adds an optional call to sock_ops BPF program based on whether the BPF_SOCK_OPS_RTO_CB_FLAG is set in bpf_sock_ops_flags.
The BPF program is passed 2 arguments: icsk_retransmits and whether the RTO has expired.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/uapi/linux/bpf.h | 5 +++++
 include/uapi/linux/tcp.h | 3 +++
 net/ipv4/tcp_timer.c     | 9 +++++++++
 3 files changed, 17 insertions(+)
include/uapi/linux/bpf.h +5 -0
index dfbf43a..1c36795 100644
@@ -989,6 +989,11 @@ enum {
 					 * a congestion threshold. RTTs above
 					 * this indicate congestion
 					 */
+	BPF_SOCK_OPS_RTO_CB,		/* Called when an RTO has triggered.
+					 * Arg1: value of icsk_retransmits
+					 * Arg2: value of icsk_rto
+					 * Arg3: whether RTO has expired
+					 */
 };
 
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
include/uapi/linux/tcp.h +3 -0
index b4a4f64..089c19e 100644
@@ -259,6 +259,9 @@ struct tcp_md5sig {
 	__u8	tcpm_key[TCP_MD5SIG_MAXKEYLEN];		/* key (binary) */
 };
 
+/* Definitions for bpf_sock_ops_flags */
+#define BPF_SOCK_OPS_RTO_CB_FLAG	(1<<0)
+
 /* INET_DIAG_MD5SIG */
 struct tcp_diag_md5sig {
 	__u8	tcpm_family;
net/ipv4/tcp_timer.c +9 -0
index 16df6dd..e6afd93 100644
@@ -230,9 +230,18 @@ static int tcp_write_timeout(struct sock *sk)
 	}
 	if (expired) {
 		/* Has it gone just too far? */
+		if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))
+			tcp_call_bpf_3arg(sk, BPF_SOCK_OPS_RTO_CB,
+					  icsk->icsk_retransmits,
+					  icsk->icsk_rto, 1);
 		tcp_write_err(sk);
 		return 1;
 	}
+
+	if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))
+		tcp_call_bpf_3arg(sk, BPF_SOCK_OPS_RTO_CB,
+				  icsk->icsk_retransmits,
+				  icsk->icsk_rto, 0);
 	return 0;
 }
Signature · 1 line
2.9.5
author
#9 lore raw

PATCH 7/11 bpf: Add support for reading sk_state and more

In reply to Lawrence Brakmo

Add support for reading many more tcp_sock fields

state, same as sk->sk_state
rtt_min same as sk->rtt_min.s[0].v (current rtt_min)
snd_ssthresh
rcv_nxt
snd_nxt
snd_una
mss_cache
ecn_flags
rate_delivered
rate_interval_us
packets_out
retrans_out
total_retrans
segs_in
data_segs_in
segs_out
data_segs_out
bytes_received (__u64)
bytes_acked (__u64)

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/uapi/linux/bpf.h | 19 ++++++++++
 net/core/filter.c        | 96 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 114 insertions(+), 1 deletion(-)
include/uapi/linux/bpf.h +19 -0
index 1c36795..19a0b1b 100644
@@ -954,6 +954,25 @@ struct bpf_sock_ops {
 	__u32 snd_cwnd;
 	__u32 srtt_us;		/* Averaged RTT << 3 in usecs */
 	__u32 bpf_sock_ops_flags; /* flags defined in uapi/linux/tcp.h */
+	__u32 state;
+	__u32 rtt_min;
+	__u32 snd_ssthresh;
+	__u32 rcv_nxt;
+	__u32 snd_nxt;
+	__u32 snd_una;
+	__u32 mss_cache;
+	__u32 ecn_flags;
+	__u32 rate_delivered;
+	__u32 rate_interval_us;
+	__u32 packets_out;
+	__u32 retrans_out;
+	__u32 total_retrans;
+	__u32 segs_in;
+	__u32 data_segs_in;
+	__u32 segs_out;
+	__u32 data_segs_out;
+	__u64 bytes_received;
+	__u64 bytes_acked;
 };
 
 /* List of known BPF sock_ops operators.
net/core/filter.c +95 -1

114 lines not on this page · raw message

Signature · 1 line
2.9.5
author
#10 lore raw

PATCH 8/11 bpf: Add sock_ops R/W access to tclass & sk_txhash

In reply to Lawrence Brakmo

Adds direct R/W access to sk_txhash and access to tclass for ipv6 flows through getsockopt and setsockopt. Sample usage for tclass:

bpf_getsockopt(skops, SOL_IPV6, IPV6_TCLASS, &v, sizeof(v))

where skops is a pointer to the ctx (struct bpf_sock_ops).

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/uapi/linux/bpf.h |  1 +
 net/core/filter.c        | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)
include/uapi/linux/bpf.h +1 -0
index 19a0b1b..fe2b692 100644
@@ -973,6 +973,7 @@ struct bpf_sock_ops {
 	__u32 data_segs_out;
 	__u64 bytes_received;
 	__u64 bytes_acked;
+	__u32 sk_txhash;
 };
 
 /* List of known BPF sock_ops operators.
net/core/filter.c +46 -1

77 lines not on this page · raw message

Signature · 1 line
2.9.5
author
#11 lore raw

PATCH 9/11 bpf: Add BPF_SOCK_OPS_RETRANS_CB

In reply to Lawrence Brakmo

Adds support for calling sock_ops BPF program when there is a retransmission. Two arguments are used; one for the sequence number and other for the number of segments retransmitted. Does not include syn-ack retransmissions.

New op: BPF_SOCK_OPS_RETRANS_CB.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/uapi/linux/bpf.h | 4 ++++
 include/uapi/linux/tcp.h | 1 +
 net/ipv4/tcp_output.c    | 3 +++
 3 files changed, 8 insertions(+)
include/uapi/linux/bpf.h +4 -0
index fe2b692..7165619 100644
@@ -1014,6 +1014,10 @@ enum {
 					 * Arg2: value of icsk_rto
 					 * Arg3: whether RTO has expired
 					 */
+	BPF_SOCK_OPS_RETRANS_CB,	/* Called when skb is retransmitted.
+					 * Arg1: sequence number of 1st byte
+					 * Arg2: # segments
+					 */
 };
 
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
include/uapi/linux/tcp.h +1 -0
index 089c19e..dc36d3c 100644
@@ -261,6 +261,7 @@ struct tcp_md5sig {
 
 /* Definitions for bpf_sock_ops_flags */
 #define BPF_SOCK_OPS_RTO_CB_FLAG	(1<<0)
+#define BPF_SOCK_OPS_RETRANS_CB_FLAG	(1<<1)
 
 /* INET_DIAG_MD5SIG */
 struct tcp_diag_md5sig {
net/ipv4/tcp_output.c +3 -0
index 50cb242..b8ad088 100644
@@ -2910,6 +2910,9 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
 	if (likely(!err)) {
 		TCP_SKB_CB(skb)->sacked |= TCPCB_EVER_RETRANS;
 		trace_tcp_retransmit_skb(sk, skb);
+		if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RETRANS_CB_FLAG))
+			tcp_call_bpf_2arg(sk, BPF_SOCK_OPS_RETRANS_CB,
+					  TCP_SKB_CB(skb)->seq, segs);
 	} else if (err != -EBUSY) {
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRETRANSFAIL);
 	}
Signature · 1 line
2.9.5
author
#12 lore raw

PATCH 10/11 bpf: Add BPF_SOCK_OPS_STATE_CB

In reply to Lawrence Brakmo

Adds support for calling sock_ops BPF program when there is a TCP state change. Two arguments are used; one for the old state and another for the new state.

New op: BPF_SOCK_OPS_STATE_CB.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 include/uapi/linux/bpf.h | 4 ++++
 include/uapi/linux/tcp.h | 1 +
 net/ipv4/tcp.c           | 2 ++
 3 files changed, 7 insertions(+)
include/uapi/linux/bpf.h +4 -0
index 7165619..b018d6f 100644
@@ -1018,6 +1018,10 @@ enum {
 					 * Arg1: sequence number of 1st byte
 					 * Arg2: # segments
 					 */
+	BPF_SOCK_OPS_STATE_CB,		/* Called when TCP changes state.
+					 * Arg1: old_state
+					 * Arg2: new_state
+					 */
 };
 
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
include/uapi/linux/tcp.h +1 -0
index dc36d3c..211322c 100644
@@ -262,6 +262,7 @@ struct tcp_md5sig {
 /* Definitions for bpf_sock_ops_flags */
 #define BPF_SOCK_OPS_RTO_CB_FLAG	(1<<0)
 #define BPF_SOCK_OPS_RETRANS_CB_FLAG	(1<<1)
+#define BPF_SOCK_OPS_STATE_CB_FLAG	(1<<2)
 
 /* INET_DIAG_MD5SIG */
 struct tcp_diag_md5sig {
net/ipv4/tcp.c +2 -0
index 817df3f..e70dd2f 100644
@@ -2041,6 +2041,8 @@ void tcp_set_state(struct sock *sk, int state)
 	int oldstate = sk->sk_state;
 
 	trace_tcp_set_state(sk, oldstate, state);
+	if (BPF_SOCK_OPS_TEST_FLAG(tcp_sk(sk), BPF_SOCK_OPS_STATE_CB_FLAG))
+		tcp_call_bpf_2arg(sk, BPF_SOCK_OPS_STATE_CB, oldstate, state);
 
 	switch (state) {
 	case TCP_ESTABLISHED:
Signature · 1 line
2.9.5
author
#13 lore raw

PATCH 11/11 bpf: add selftest for tcpbpf

In reply to Lawrence Brakmo

Added a selftest for tcpbpf (sock_ops) that checks that the appropriate callbacks occured and that it can access tcp_sock fields and that their values are correct.

Signed-off-by Lawrence Brakmo <[email protected]>
---
 tools/include/uapi/linux/bpf.h                 |  45 ++++++++-
 tools/testing/selftests/bpf/Makefile           |   5 +-
 tools/testing/selftests/bpf/tcp_client.py      |  57 +++++++++++
 tools/testing/selftests/bpf/tcp_server.py      |  83 +++++++++++++++
 tools/testing/selftests/bpf/test_tcpbpf_kern.c | 133 +++++++++++++++++++++++++
 tools/testing/selftests/bpf/test_tcpbpf_user.c | 119 ++++++++++++++++++++++
 6 files changed, 438 insertions(+), 4 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/tcp_client.py
 create mode 100755 tools/testing/selftests/bpf/tcp_server.py
 create mode 100644 tools/testing/selftests/bpf/test_tcpbpf_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_tcpbpf_user.c
tools/include/uapi/linux/bpf.h +43 -2
index cf446c2..b018d6f 100644
@@ -936,8 +936,9 @@ struct bpf_map_info {
 struct bpf_sock_ops {
 	__u32 op;
 	union {
-		__u32 reply;
-		__u32 replylong[4];
+		__u32 args[4];		/* Optionally passed to bpf program */
+		__u32 reply;		/* Returned by bpf program	    */
+		__u32 replylong[4];	/* Optionally returned by bpf prog  */
 	};
 	__u32 family;
 	__u32 remote_ip4;	/* Stored in network byte order */

@@ -946,6 +947,33 @@ struct bpf_sock_ops {
 	__u32 local_ip6[4];	/* Stored in network byte order */
 	__u32 remote_port;	/* Stored in network byte order */
 	__u32 local_port;	/* stored in host byte order */
+	__u32 is_fullsock;	/* Some TCP fields are only valid if
+				 * there is a full socket. If not, the
+				 * fields read as zero.
+				 */
+	__u32 snd_cwnd;
+	__u32 srtt_us;		/* Averaged RTT << 3 in usecs */
+	__u32 bpf_sock_ops_flags; /* flags defined in uapi/linux/tcp.h */
+	__u32 state;
+	__u32 rtt_min;
+	__u32 snd_ssthresh;
+	__u32 rcv_nxt;
+	__u32 snd_nxt;
+	__u32 snd_una;
+	__u32 mss_cache;
+	__u32 ecn_flags;
+	__u32 rate_delivered;
+	__u32 rate_interval_us;
+	__u32 packets_out;
+	__u32 retrans_out;
+	__u32 total_retrans;
+	__u32 segs_in;
+	__u32 data_segs_in;
+	__u32 segs_out;
+	__u32 data_segs_out;
+	__u64 bytes_received;
+	__u64 bytes_acked;
+	__u32 sk_txhash;
 };
 
 /* List of known BPF sock_ops operators.

@@ -981,6 +1009,19 @@ enum {
 					 * a congestion threshold. RTTs above
 					 * this indicate congestion
 					 */
+	BPF_SOCK_OPS_RTO_CB,		/* Called when an RTO has triggered.
+					 * Arg1: value of icsk_retransmits
+					 * Arg2: value of icsk_rto
+					 * Arg3: whether RTO has expired
+					 */
+	BPF_SOCK_OPS_RETRANS_CB,	/* Called when skb is retransmitted.
+					 * Arg1: sequence number of 1st byte
+					 * Arg2: # segments
+					 */
+	BPF_SOCK_OPS_STATE_CB,		/* Called when TCP changes state.
+					 * Arg1: old_state
+					 * Arg2: new_state
+					 */
 };
 
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
tools/testing/selftests/bpf/Makefile +3 -2
index 255fb1f..f3632b2 100644
@@ -13,11 +13,12 @@ CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../i
 LDLIBS += -lcap -lelf
 
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
-	test_align test_verifier_log test_dev_cgroup
+	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user
 
 TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
 	test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o     \
-	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o
+	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
+	test_tcpbpf_kern.o
 
 TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh \
 	test_offload.py
tools/testing/selftests/bpf/tcp_client.py new file +57 -0

57 lines not on this page · raw message

tools/testing/selftests/bpf/tcp_server.py new file +83 -0

83 lines not on this page · raw message

tools/testing/selftests/bpf/test_tcpbpf_kern.c new file +133 -0

133 lines not on this page · raw message

tools/testing/selftests/bpf/test_tcpbpf_user.c new file +119 -0

119 lines not on this page · raw message

Signature · 1 line
2.9.5
#14 lore raw

Re: [PATCH bpf 11/11] bpf: add selftest for tcpbpf

tools/testing/selftests/bpf/Makefile CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../i · line 21

Quoted 3 lines · Lawrence Brakmo
tools/testing/selftests/bpf/Makefile +2 -1
-	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o
+	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
+	test_tcpbpf_kern.o

it won't apply. please base patches on bpf-next tree

tools/testing/selftests/bpf/tcp_client.py line 7

Quoted 7 lines
tools/testing/selftests/bpf/tcp_client.py new file +7 -0
+#!/usr/local/bin/python
+#
+# Copyright (c) 2017 Facebook
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of version 2 of the GNU General Public
+# License as published by the Free Software Foundation.

the license should be in SPDX format.

tools/testing/selftests/bpf/test_tcpbpf_kern.c line 6

Quoted 8 lines

+++ b/tools/testing/selftests/bpf/test_tcpbpf_kern.c

tools/testing/selftests/bpf/test_tcpbpf_kern.c new file +6 -0
@@ -0,0 +1,133 @@
+/* Copyright (c) 2017 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */

same here.

tools/testing/selftests/bpf/test_tcpbpf_kern.c line 114

Quoted 11 lines
tools/testing/selftests/bpf/test_tcpbpf_kern.c new file +11 -0
+		case BPF_SOCK_OPS_STATE_CB:
+			if (skops->args[1] == 7) {
+				__u32 key = 0;
+				struct globals g, *gp;
+
+				gp = bpf_map_lookup_elem(&global_map, &key);
+				if (gp == NULL) {
+				} else {
+					g = *gp;
+					g.total_retrans = skops->total_retrans;
+					g.data_segs_in = skops->data_segs_in;

you can reduce indent by doing
if (!gp)
break;
g = *gp;
g.total_retrans = skops->total_retrans;

Quoted 16 lines
tools/testing/selftests/bpf/test_tcpbpf_user.c new file +15 -0
+++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c

@@ -0,0 +1,119 @@
+/* Copyright (c) 2017 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <signal.h>
+#include <string.h>
+#include <assert.h>

tools/testing/selftests/bpf/test_tcpbpf_user.c line 24

Quoted 10 lines
tools/testing/selftests/bpf/test_tcpbpf_user.c new file +10 -0
+#include <linux/perf_event.h>
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+//#include "bpf_load.h"

please remove left over comments.