[PATCH bpf-next v7 0/7] add BPF_F_PERMANENT flag for sockmap skmsg redirect

[PATCH bpf-next v7 0/7] add BPF_F_PERMANENT flag for sockmap skmsg redirect

[Liu Jian]

v6->v7: Rebase to latest bpf-next tree, and no changes.
v5->v6: Modified the description of the helper function.
v4->v5: Fix one refcount bug caused by patch1.
v3->v4: Change the two helpers's description.
	Let BPF_F_PERMANENT takes precedence over apply/cork_bytes.

Liu Jian (7):
  bpf, sockmap: add BPF_F_PERMANENT flag for skmsg redirect
  selftests/bpf: Add txmsg permanently test for sockmap
  selftests/bpf: Add txmsg redir permanently test for sockmap
  selftests/bpf: add skmsg verdict tests
  selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENT flag
  selftests/bpf: add tests for verdict skmsg to itself
  selftests/bpf: add tests for verdict skmsg to closed socket

 include/linux/skmsg.h                         |   1 +
 include/uapi/linux/bpf.h                      |  45 +++++--
 net/core/skmsg.c                              |   6 +-
 net/core/sock_map.c                           |   4 +-
 net/ipv4/tcp_bpf.c                            |  12 +-
 tools/include/uapi/linux/bpf.h                |  45 +++++--
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 122 ++++++++++++++++++
 .../selftests/bpf/progs/test_sockmap_kern.h   |   3 +-
 .../bpf/progs/test_sockmap_msg_verdict.c      |  25 ++++
 tools/testing/selftests/bpf/test_sockmap.c    |  41 +++++-
 10 files changed, 272 insertions(+), 32 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c

-- 
2.34.1

[PATCH bpf-next v7 1/7] bpf, sockmap: add BPF_F_PERMANENT flag for skmsg redirect

[Liu Jian]

If the sockmap msg redirection function is used only to forward packets
and no other operation, the execution result of the BPF_SK_MSG_VERDICT
program is the same each time. In this case, the BPF program only needs to
be run once. Add BPF_F_PERMANENT flag to bpf_msg_redirect_map() and
bpf_msg_redirect_hash() to implement this ability.

Then we can enable this function in the bpf program as follows:
bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENT);

Test results using netperf  TCP_STREAM mode:
for i in 1 64 128 512 1k 2k 32k 64k 100k 500k 1m;then
netperf -T 1,2 -t TCP_STREAM -H 127.0.0.1 -l 20 -- -m $i -s 100m,100m -S 100m,100m
done

before:
3.84 246.52 496.89 1885.03 3415.29 6375.03 40749.09 48764.40 51611.34 55678.26 55992.78
after:
4.43 279.20 555.82 2080.79 3870.70 7105.44 41836.41 49709.75 51861.56 55211.00 54566.85

Signed-off-by: Liu Jian <liujian56@huawei.com>
Tested-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 include/linux/skmsg.h          |  1 +
 include/uapi/linux/bpf.h       | 45 ++++++++++++++++++++++++++--------
 net/core/skmsg.c               |  6 ++++-
 net/core/sock_map.c            |  4 +--
 net/ipv4/tcp_bpf.c             | 12 +++++----
 tools/include/uapi/linux/bpf.h | 45 ++++++++++++++++++++++++++--------
 6 files changed, 85 insertions(+), 28 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index c1637515a8a4..acd7de85608b 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -83,6 +83,7 @@ struct sk_psock {
 	u32				cork_bytes;
 	u32				eval;
 	bool				redir_ingress; /* undefined if sk_redir is null */
+	bool				redir_permanent;
 	struct sk_msg			*cork;
 	struct sk_psock_progs		progs;
 #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0f6cdf52b1da..ad18b0ffe88f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3044,11 +3044,23 @@ union bpf_attr {
  * 		socket level. If the message *msg* is allowed to pass (i.e. if
  * 		the verdict eBPF program returns **SK_PASS**), redirect it to
  * 		the socket referenced by *map* (of type
- * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*. Both ingress and
- * 		egress interfaces can be used for redirection. The
- * 		**BPF_F_INGRESS** value in *flags* is used to make the
- * 		distinction (ingress path is selected if the flag is present,
- * 		egress path otherwise). This is the only flag supported for now.
+ * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*.
+ *
+ *		The following *flags* are supported:
+ *
+ *		**BPF_F_INGRESS**
+ *		        Both ingress and egress interfaces can be used for redirection.
+ *		        The **BPF_F_INGRESS** value in *flags* is used to make the
+ *		        distinction. Ingress path is selected if the flag is present,
+ *		        egress path otherwise.
+ *		**BPF_F_PERMANENT**
+ *		        Indicates that redirect verdict and the target socket should be
+ *		        remembered. The verdict program will not be run for subsequent
+ *		        packets.
+ *
+ *		        **BPF_F_PERMANENT** cannot be use together with
+ *		        **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If
+ *			**BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored.
  * 	Return
  * 		**SK_PASS** on success, or **SK_DROP** on error.
  *
@@ -3321,11 +3333,23 @@ union bpf_attr {
  *		socket level. If the message *msg* is allowed to pass (i.e. if
  *		the verdict eBPF program returns **SK_PASS**), redirect it to
  *		the socket referenced by *map* (of type
- *		**BPF_MAP_TYPE_SOCKHASH**) using hash *key*. Both ingress and
- *		egress interfaces can be used for redirection. The
- *		**BPF_F_INGRESS** value in *flags* is used to make the
- *		distinction (ingress path is selected if the flag is present,
- *		egress path otherwise). This is the only flag supported for now.
+ *		**BPF_MAP_TYPE_SOCKHASH**) using hash *key*.
+ *
+ *		The following *flags* are supported:
+ *
+ *		**BPF_F_INGRESS**
+ *		        Both ingress and egress interfaces can be used for redirection.
+ *		        The **BPF_F_INGRESS** value in *flags* is used to make the
+ *		        distinction. Ingress path is selected if the flag is present,
+ *		        egress path otherwise.
+ *		**BPF_F_PERMANENT**
+ *		        Indicates that redirect verdict and the target socket should be
+ *		        remembered. The verdict program will not be run for subsequent
+ *		        packets.
+ *
+ *		        **BPF_F_PERMANENT** cannot be use together with
+ *		        **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If
+ *			**BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored.
  *	Return
  *		**SK_PASS** on success, or **SK_DROP** on error.
  *
@@ -5928,6 +5952,7 @@ enum {
 /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
 enum {
 	BPF_F_INGRESS			= (1ULL << 0),
+	BPF_F_PERMANENT			= (1ULL << 1),
 };
 
 /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 6c31eefbd777..22e55eec3089 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -878,7 +878,11 @@ int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock,
 	msg->sk = sk;
 	ret = bpf_prog_run_pin_on_cpu(prog, msg);
 	ret = sk_psock_map_verd(ret, msg->sk_redir);
-	psock->apply_bytes = msg->apply_bytes;
+	psock->redir_permanent = msg->flags & BPF_F_PERMANENT;
+	if (psock->redir_permanent)
+		msg->cork_bytes = msg->apply_bytes = 0;
+	else
+		psock->apply_bytes = msg->apply_bytes;
 	if (ret == __SK_REDIRECT) {
 		if (psock->sk_redir) {
 			sock_put(psock->sk_redir);
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 4292c2ed1828..b716b42486c5 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -662,7 +662,7 @@ BPF_CALL_4(bpf_msg_redirect_map, struct sk_msg *, msg,
 {
 	struct sock *sk;
 
-	if (unlikely(flags & ~(BPF_F_INGRESS)))
+	if (unlikely(flags & ~(BPF_F_INGRESS | BPF_F_PERMANENT)))
 		return SK_DROP;
 
 	sk = __sock_map_lookup_elem(map, key);
@@ -1263,7 +1263,7 @@ BPF_CALL_4(bpf_msg_redirect_hash, struct sk_msg *, msg,
 {
 	struct sock *sk;
 
-	if (unlikely(flags & ~(BPF_F_INGRESS)))
+	if (unlikely(flags & ~(BPF_F_INGRESS | BPF_F_PERMANENT)))
 		return SK_DROP;
 
 	sk = __sock_hash_lookup_elem(map, key);
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 53b0d62fd2c2..3b3719a37a26 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -433,8 +433,10 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 		if (!psock->apply_bytes) {
 			/* Clean up before releasing the sock lock. */
 			eval = psock->eval;
-			psock->eval = __SK_NONE;
-			psock->sk_redir = NULL;
+			if (!psock->redir_permanent) {
+				psock->eval = __SK_NONE;
+				psock->sk_redir = NULL;
+			}
 		}
 		if (psock->cork) {
 			cork = true;
@@ -448,7 +450,7 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 					    msg, tosend, flags);
 		sent = origsize - msg->sg.size;
 
-		if (eval == __SK_REDIRECT)
+		if (!psock->redir_permanent && eval == __SK_REDIRECT)
 			sock_put(sk_redir);
 
 		lock_sock(sk);
@@ -474,8 +476,8 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 	}
 
 	if (likely(!ret)) {
-		if (!psock->apply_bytes) {
-			psock->eval =  __SK_NONE;
+		if (!psock->apply_bytes && !psock->redir_permanent) {
+			psock->eval = __SK_NONE;
 			if (psock->sk_redir) {
 				sock_put(psock->sk_redir);
 				psock->sk_redir = NULL;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0f6cdf52b1da..ad18b0ffe88f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3044,11 +3044,23 @@ union bpf_attr {
  * 		socket level. If the message *msg* is allowed to pass (i.e. if
  * 		the verdict eBPF program returns **SK_PASS**), redirect it to
  * 		the socket referenced by *map* (of type
- * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*. Both ingress and
- * 		egress interfaces can be used for redirection. The
- * 		**BPF_F_INGRESS** value in *flags* is used to make the
- * 		distinction (ingress path is selected if the flag is present,
- * 		egress path otherwise). This is the only flag supported for now.
+ * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*.
+ *
+ *		The following *flags* are supported:
+ *
+ *		**BPF_F_INGRESS**
+ *		        Both ingress and egress interfaces can be used for redirection.
+ *		        The **BPF_F_INGRESS** value in *flags* is used to make the
+ *		        distinction. Ingress path is selected if the flag is present,
+ *		        egress path otherwise.
+ *		**BPF_F_PERMANENT**
+ *		        Indicates that redirect verdict and the target socket should be
+ *		        remembered. The verdict program will not be run for subsequent
+ *		        packets.
+ *
+ *		        **BPF_F_PERMANENT** cannot be use together with
+ *		        **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If
+ *			**BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored.
  * 	Return
  * 		**SK_PASS** on success, or **SK_DROP** on error.
  *
@@ -3321,11 +3333,23 @@ union bpf_attr {
  *		socket level. If the message *msg* is allowed to pass (i.e. if
  *		the verdict eBPF program returns **SK_PASS**), redirect it to
  *		the socket referenced by *map* (of type
- *		**BPF_MAP_TYPE_SOCKHASH**) using hash *key*. Both ingress and
- *		egress interfaces can be used for redirection. The
- *		**BPF_F_INGRESS** value in *flags* is used to make the
- *		distinction (ingress path is selected if the flag is present,
- *		egress path otherwise). This is the only flag supported for now.
+ *		**BPF_MAP_TYPE_SOCKHASH**) using hash *key*.
+ *
+ *		The following *flags* are supported:
+ *
+ *		**BPF_F_INGRESS**
+ *		        Both ingress and egress interfaces can be used for redirection.
+ *		        The **BPF_F_INGRESS** value in *flags* is used to make the
+ *		        distinction. Ingress path is selected if the flag is present,
+ *		        egress path otherwise.
+ *		**BPF_F_PERMANENT**
+ *		        Indicates that redirect verdict and the target socket should be
+ *		        remembered. The verdict program will not be run for subsequent
+ *		        packets.
+ *
+ *		        **BPF_F_PERMANENT** cannot be use together with
+ *		        **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If
+ *			**BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored.
  *	Return
  *		**SK_PASS** on success, or **SK_DROP** on error.
  *
@@ -5928,6 +5952,7 @@ enum {
 /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
 enum {
 	BPF_F_INGRESS			= (1ULL << 0),
+	BPF_F_PERMANENT			= (1ULL << 1),
 };
 
 /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */
-- 
2.34.1

[PATCH bpf-next v7 2/7] selftests/bpf: Add txmsg permanently test for sockmap

[Liu Jian]

Add one test for txmsg ingress permanently test for sockmap.

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 tools/testing/selftests/bpf/test_sockmap.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 024a0faafb3b..bf2cef7ae3ae 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -77,6 +77,7 @@ int txmsg_end_push;
 int txmsg_start_pop;
 int txmsg_pop;
 int txmsg_ingress;
+int txmsg_permanent;
 int txmsg_redir_skb;
 int txmsg_ktls_skb;
 int txmsg_ktls_skb_drop;
@@ -107,6 +108,7 @@ static const struct option long_options[] = {
 	{"txmsg_start_pop",  required_argument,	NULL, 'w'},
 	{"txmsg_pop",	     required_argument,	NULL, 'x'},
 	{"txmsg_ingress", no_argument,		&txmsg_ingress, 1 },
+	{"txmsg_permanent", no_argument,	&txmsg_permanent, 1 },
 	{"txmsg_redir_skb", no_argument,	&txmsg_redir_skb, 1 },
 	{"ktls", no_argument,			&ktls, 1 },
 	{"peek", no_argument,			&peek_flag, 1 },
@@ -175,7 +177,7 @@ static void test_reset(void)
 	txmsg_start_push = txmsg_end_push = 0;
 	txmsg_pass = txmsg_drop = txmsg_redir = 0;
 	txmsg_apply = txmsg_cork = 0;
-	txmsg_ingress = txmsg_redir_skb = 0;
+	txmsg_ingress = txmsg_permanent = txmsg_redir_skb = 0;
 	txmsg_ktls_skb = txmsg_ktls_skb_drop = txmsg_ktls_skb_redir = 0;
 	txmsg_omit_skb_parser = 0;
 	skb_use_parser = 0;
@@ -1165,10 +1167,13 @@ static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 		}
 
 		if (txmsg_ingress) {
-			int in = BPF_F_INGRESS;
+			int txmsg_flag = BPF_F_INGRESS;
+
+			if (txmsg_permanent)
+				txmsg_flag |= BPF_F_PERMANENT;
 
 			i = 0;
-			err = bpf_map_update_elem(map_fd[6], &i, &in, BPF_ANY);
+			err = bpf_map_update_elem(map_fd[6], &i, &txmsg_flag, BPF_ANY);
 			if (err) {
 				fprintf(stderr,
 					"ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
@@ -1506,6 +1511,14 @@ static void test_txmsg_ingress_redir(int cgrp, struct sockmap_options *opt)
 	test_send(opt, cgrp);
 }
 
+static void test_txmsg_ingress_redir_permanent(int cgrp, struct sockmap_options *opt)
+{
+	txmsg_pass = txmsg_drop = 0;
+	txmsg_ingress = txmsg_redir = 1;
+	txmsg_permanent = 1;
+	test_send(opt, cgrp);
+}
+
 static void test_txmsg_skb(int cgrp, struct sockmap_options *opt)
 {
 	bool data = opt->data_test;
@@ -1862,6 +1875,7 @@ struct _test test[] = {
 	{"txmsg test redirect wait send mem", test_txmsg_redir_wait_sndmem},
 	{"txmsg test drop", test_txmsg_drop},
 	{"txmsg test ingress redirect", test_txmsg_ingress_redir},
+	{"txmsg test ingress redirect permanent", test_txmsg_ingress_redir_permanent},
 	{"txmsg test skb", test_txmsg_skb},
 	{"txmsg test apply", test_txmsg_apply},
 	{"txmsg test cork", test_txmsg_cork},
-- 
2.34.1

[PATCH bpf-next v7 3/7] selftests/bpf: Add txmsg redir permanently test for sockmap

[Liu Jian]

Add one test for txmsg redir permanently test for sockmap.

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 .../selftests/bpf/progs/test_sockmap_kern.h   |  3 ++-
 tools/testing/selftests/bpf/test_sockmap.c    | 21 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
index 99d2ea9fb658..b0a2ddd55b83 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
@@ -298,8 +298,9 @@ int bpf_prog6(struct sk_msg_md *msg)
 
 	f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
 	if (f && *f) {
-		key = 2;
 		flags = *f;
+		if (flags & BPF_F_INGRESS)
+			key = 2;
 	}
 #ifdef SOCKMAP
 	return bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index bf2cef7ae3ae..c602ac8780a8 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -1166,6 +1166,19 @@ static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 
 		}
 
+		if (txmsg_permanent) {
+			int txmsg_flag = BPF_F_PERMANENT;
+
+			i = 0;
+			err = bpf_map_update_elem(map_fd[6], &i, &txmsg_flag, BPF_ANY);
+			if (err) {
+				fprintf(stderr,
+					"ERROR: bpf_map_update_elem (txmsg_permanent): %d (%s)\n",
+					err, strerror(errno));
+				goto out;
+			}
+		}
+
 		if (txmsg_ingress) {
 			int txmsg_flag = BPF_F_INGRESS;
 
@@ -1490,6 +1503,13 @@ static void test_txmsg_redir(int cgrp, struct sockmap_options *opt)
 	test_send(opt, cgrp);
 }
 
+static void test_txmsg_redir_permanent(int cgrp, struct sockmap_options *opt)
+{
+	txmsg_redir = 1;
+	txmsg_permanent = 1;
+	test_send(opt, cgrp);
+}
+
 static void test_txmsg_redir_wait_sndmem(int cgrp, struct sockmap_options *opt)
 {
 	txmsg_redir = 1;
@@ -1872,6 +1892,7 @@ static int populate_progs(char *bpf_file)
 struct _test test[] = {
 	{"txmsg test passthrough", test_txmsg_pass},
 	{"txmsg test redirect", test_txmsg_redir},
+	{"txmsg test redirect permanent", test_txmsg_redir_permanent},
 	{"txmsg test redirect wait send mem", test_txmsg_redir_wait_sndmem},
 	{"txmsg test drop", test_txmsg_drop},
 	{"txmsg test ingress redirect", test_txmsg_ingress_redir},
-- 
2.34.1

[PATCH bpf-next v7 4/7] selftests/bpf: add skmsg verdict tests

[Liu Jian]

Add two normal skmsg verdict tests in sockmap_basic.c

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 71 +++++++++++++++++++
 .../bpf/progs/test_sockmap_msg_verdict.c      | 25 +++++++
 2 files changed, 96 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index f75f84d0b3d7..8e90664569e3 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -12,6 +12,7 @@
 #include "test_sockmap_progs_query.skel.h"
 #include "test_sockmap_pass_prog.skel.h"
 #include "test_sockmap_drop_prog.skel.h"
+#include "test_sockmap_msg_verdict.skel.h"
 #include "bpf_iter_sockmap.skel.h"
 
 #include "sockmap_helpers.h"
@@ -524,6 +525,72 @@ static void test_sockmap_skb_verdict_peek(void)
 	test_sockmap_pass_prog__destroy(pass);
 }
 
+static void test_sockmap_msg_verdict(bool is_ingress)
+{
+	int key, sent, recvd, recv_fd;
+	int err, map, verdict, s, c0, c1, p0, p1;
+	struct test_sockmap_msg_verdict *skel;
+	char buf[256] = "0123456789";
+
+	skel = test_sockmap_msg_verdict__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	verdict = bpf_program__fd(skel->progs.prog_skmsg_verdict);
+	map = bpf_map__fd(skel->maps.sock_map);
+
+
+	err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
+	if (!ASSERT_OK(err, "bpf_prog_attach"))
+		goto out;
+
+	s = socket_loopback(AF_INET, SOCK_STREAM);
+	if (!ASSERT_GT(s, -1, "socket_loopback(s)"))
+		goto out;
+	err = create_socket_pairs(s, AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+	if (!ASSERT_OK(err, "create_socket_pairs(s)"))
+		goto out;
+
+	key = 0;
+	err = bpf_map_update_elem(map, &key, &p1, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(key0)"))
+		goto out_close;
+	key = 1;
+	err = bpf_map_update_elem(map, &key, &c1, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(key1)"))
+		goto out_close;
+	key = 2;
+	err = bpf_map_update_elem(map, &key, &p0, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(key2)"))
+		goto out_close;
+	key = 3;
+	err = bpf_map_update_elem(map, &key, &c0, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(key3)"))
+		goto out_close;
+
+	if (is_ingress) {
+		recv_fd = c1;
+		skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
+		skel->bss->skmsg_redir_key = 1;
+	} else {
+		recv_fd = c0;
+		skel->bss->skmsg_redir_flags = 0;
+		skel->bss->skmsg_redir_key = 2;
+	}
+
+	sent = xsend(p1, &buf, sizeof(buf), 0);
+	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+	recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
+	ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
+
+out_close:
+	close(c0);
+	close(p0);
+	close(c1);
+	close(p1);
+out:
+	test_sockmap_msg_verdict__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -566,4 +633,8 @@ void test_sockmap_basic(void)
 		test_sockmap_skb_verdict_fionread(false);
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
 		test_sockmap_skb_verdict_peek();
+	if (test__start_subtest("sockmap msg_verdict"))
+		test_sockmap_msg_verdict(false);
+	if (test__start_subtest("sockmap msg_verdict ingress"))
+		test_sockmap_msg_verdict(true);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
new file mode 100644
index 000000000000..002b76a1ae35
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, 4);
+	__type(key, int);
+	__type(value, int);
+} sock_map SEC(".maps");
+
+u64 skmsg_redir_flags = 0;
+u32 skmsg_redir_key = 0;
+
+SEC("sk_msg")
+int prog_skmsg_verdict(struct sk_msg_md *msg)
+{
+	u64 flags = skmsg_redir_flags;
+	int key = skmsg_redir_key;
+
+	bpf_msg_redirect_map(msg, &sock_map, key, flags);
+	return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.34.1

[PATCH bpf-next v7 5/7] selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENT flag

[Liu Jian]

Add two tests for BPF_F_PERMANENT flag in sockmap_basic.c.

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c    | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 8e90664569e3..ef147d73ff47 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -525,7 +525,7 @@ static void test_sockmap_skb_verdict_peek(void)
 	test_sockmap_pass_prog__destroy(pass);
 }
 
-static void test_sockmap_msg_verdict(bool is_ingress)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent)
 {
 	int key, sent, recvd, recv_fd;
 	int err, map, verdict, s, c0, c1, p0, p1;
@@ -577,11 +577,18 @@ static void test_sockmap_msg_verdict(bool is_ingress)
 		skel->bss->skmsg_redir_key = 2;
 	}
 
+	if (is_permanent)
+		skel->bss->skmsg_redir_flags |= BPF_F_PERMANENT;
+
 	sent = xsend(p1, &buf, sizeof(buf), 0);
 	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
 	recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
 	ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
 
+	sent = xsend(p1, &buf, sizeof(buf), 0);
+	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+	recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
+	ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
 out_close:
 	close(c0);
 	close(p0);
@@ -634,7 +641,11 @@ void test_sockmap_basic(void)
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
 		test_sockmap_skb_verdict_peek();
 	if (test__start_subtest("sockmap msg_verdict"))
-		test_sockmap_msg_verdict(false);
+		test_sockmap_msg_verdict(false, false);
 	if (test__start_subtest("sockmap msg_verdict ingress"))
-		test_sockmap_msg_verdict(true);
+		test_sockmap_msg_verdict(true, false);
+	if (test__start_subtest("sockmap msg_verdict permanent"))
+		test_sockmap_msg_verdict(false, true);
+	if (test__start_subtest("sockmap msg_verdict ingress permanent"))
+		test_sockmap_msg_verdict(true, true);
 }
-- 
2.34.1

[PATCH bpf-next v7 6/7] selftests/bpf: add tests for verdict skmsg to itself

[Liu Jian]

Add tests for verdict skmsg to itself in sockmap_basic.c

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index ef147d73ff47..75107762a86e 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -525,7 +525,7 @@ static void test_sockmap_skb_verdict_peek(void)
 	test_sockmap_pass_prog__destroy(pass);
 }
 
-static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent, bool is_self)
 {
 	int key, sent, recvd, recv_fd;
 	int err, map, verdict, s, c0, c1, p0, p1;
@@ -568,13 +568,23 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent)
 		goto out_close;
 
 	if (is_ingress) {
-		recv_fd = c1;
 		skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
-		skel->bss->skmsg_redir_key = 1;
+		if (is_self) {
+			skel->bss->skmsg_redir_key = 0;
+			recv_fd = p1;
+		} else {
+			skel->bss->skmsg_redir_key = 1;
+			recv_fd = c1;
+		}
 	} else {
-		recv_fd = c0;
 		skel->bss->skmsg_redir_flags = 0;
-		skel->bss->skmsg_redir_key = 2;
+		if (is_self) {
+			skel->bss->skmsg_redir_key = 0;
+			recv_fd = c1;
+		} else {
+			skel->bss->skmsg_redir_key = 2;
+			recv_fd = c0;
+		}
 	}
 
 	if (is_permanent)
@@ -641,11 +651,15 @@ void test_sockmap_basic(void)
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
 		test_sockmap_skb_verdict_peek();
 	if (test__start_subtest("sockmap msg_verdict"))
-		test_sockmap_msg_verdict(false, false);
+		test_sockmap_msg_verdict(false, false, false);
 	if (test__start_subtest("sockmap msg_verdict ingress"))
-		test_sockmap_msg_verdict(true, false);
+		test_sockmap_msg_verdict(true, false, false);
 	if (test__start_subtest("sockmap msg_verdict permanent"))
-		test_sockmap_msg_verdict(false, true);
+		test_sockmap_msg_verdict(false, true, false);
 	if (test__start_subtest("sockmap msg_verdict ingress permanent"))
-		test_sockmap_msg_verdict(true, true);
+		test_sockmap_msg_verdict(true, true, false);
+	if (test__start_subtest("sockmap msg_verdict permanent self"))
+		test_sockmap_msg_verdict(false, true, true);
+	if (test__start_subtest("sockmap msg_verdict ingress permanent self"))
+		test_sockmap_msg_verdict(true, true, true);
 }
-- 
2.34.1

[PATCH bpf-next v7 7/7] selftests/bpf: add tests for verdict skmsg to closed socket

[Liu Jian]

Add four tests for verdict skmsg to closed socket in sockmap_basic.c.

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 42 +++++++++++++++----
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 75107762a86e..4d49129cdd6b 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -525,9 +525,10 @@ static void test_sockmap_skb_verdict_peek(void)
 	test_sockmap_pass_prog__destroy(pass);
 }
 
-static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent, bool is_self)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent, bool is_self,
+				     bool target_shutdown)
 {
-	int key, sent, recvd, recv_fd;
+	int key, sent, recvd, recv_fd, target_fd;
 	int err, map, verdict, s, c0, c1, p0, p1;
 	struct test_sockmap_msg_verdict *skel;
 	char buf[256] = "0123456789";
@@ -571,18 +572,22 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent, bool is
 		skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
 		if (is_self) {
 			skel->bss->skmsg_redir_key = 0;
+			target_fd = p1;
 			recv_fd = p1;
 		} else {
 			skel->bss->skmsg_redir_key = 1;
+			target_fd = c1;
 			recv_fd = c1;
 		}
 	} else {
 		skel->bss->skmsg_redir_flags = 0;
 		if (is_self) {
 			skel->bss->skmsg_redir_key = 0;
+			target_fd = p1;
 			recv_fd = c1;
 		} else {
 			skel->bss->skmsg_redir_key = 2;
+			target_fd = p0;
 			recv_fd = c0;
 		}
 	}
@@ -595,6 +600,19 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanent, bool is
 	recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
 	ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
 
+	if (target_shutdown) {
+		signal(SIGPIPE, SIG_IGN);
+		close(target_fd);
+		sent = send(p1, &buf, sizeof(buf), 0);
+		if (is_permanent) {
+			ASSERT_EQ(sent, -1, "xsend(p1)");
+			ASSERT_EQ(errno, EPIPE, "xsend(p1)");
+		} else {
+			ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+		}
+		goto out_close;
+	}
+
 	sent = xsend(p1, &buf, sizeof(buf), 0);
 	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
 	recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
@@ -651,15 +669,23 @@ void test_sockmap_basic(void)
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
 		test_sockmap_skb_verdict_peek();
 	if (test__start_subtest("sockmap msg_verdict"))
-		test_sockmap_msg_verdict(false, false, false);
+		test_sockmap_msg_verdict(false, false, false, false);
 	if (test__start_subtest("sockmap msg_verdict ingress"))
-		test_sockmap_msg_verdict(true, false, false);
+		test_sockmap_msg_verdict(true, false, false, false);
 	if (test__start_subtest("sockmap msg_verdict permanent"))
-		test_sockmap_msg_verdict(false, true, false);
+		test_sockmap_msg_verdict(false, true, false, false);
 	if (test__start_subtest("sockmap msg_verdict ingress permanent"))
-		test_sockmap_msg_verdict(true, true, false);
+		test_sockmap_msg_verdict(true, true, false, false);
 	if (test__start_subtest("sockmap msg_verdict permanent self"))
-		test_sockmap_msg_verdict(false, true, true);
+		test_sockmap_msg_verdict(false, true, true, false);
 	if (test__start_subtest("sockmap msg_verdict ingress permanent self"))
-		test_sockmap_msg_verdict(true, true, true);
+		test_sockmap_msg_verdict(true, true, true, false);
+	if (test__start_subtest("sockmap msg_verdict permanent shutdown"))
+		test_sockmap_msg_verdict(false, true, false, true);
+	if (test__start_subtest("sockmap msg_verdict ingress permanent shutdown"))
+		test_sockmap_msg_verdict(true, true, false, true);
+	if (test__start_subtest("sockmap msg_verdict shutdown"))
+		test_sockmap_msg_verdict(false, false, false, true);
+	if (test__start_subtest("sockmap msg_verdict ingress shutdown"))
+		test_sockmap_msg_verdict(true, false, false, true);
 }
-- 
2.34.1

Re: [PATCH bpf-next v7 7/7] selftests/bpf: add tests for verdict skmsg to closed socket

[Jakub Sitnicki]

On Sat, Oct 28, 2023 at 06:05 PM +08, Liu Jian wrote:
> Add four tests for verdict skmsg to closed socket in sockmap_basic.c.
>
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
>  .../selftests/bpf/prog_tests/sockmap_basic.c  | 42 +++++++++++++++----
>  1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index 75107762a86e..4d49129cdd6b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c

[...]

> @@ -651,15 +669,23 @@ void test_sockmap_basic(void)
>  	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
>  		test_sockmap_skb_verdict_peek();
>  	if (test__start_subtest("sockmap msg_verdict"))
> -		test_sockmap_msg_verdict(false, false, false);
> +		test_sockmap_msg_verdict(false, false, false, false);
>  	if (test__start_subtest("sockmap msg_verdict ingress"))
> -		test_sockmap_msg_verdict(true, false, false);
> +		test_sockmap_msg_verdict(true, false, false, false);
>  	if (test__start_subtest("sockmap msg_verdict permanent"))
> -		test_sockmap_msg_verdict(false, true, false);
> +		test_sockmap_msg_verdict(false, true, false, false);
>  	if (test__start_subtest("sockmap msg_verdict ingress permanent"))
> -		test_sockmap_msg_verdict(true, true, false);
> +		test_sockmap_msg_verdict(true, true, false, false);
>  	if (test__start_subtest("sockmap msg_verdict permanent self"))
> -		test_sockmap_msg_verdict(false, true, true);
> +		test_sockmap_msg_verdict(false, true, true, false);
>  	if (test__start_subtest("sockmap msg_verdict ingress permanent self"))
> -		test_sockmap_msg_verdict(true, true, true);
> +		test_sockmap_msg_verdict(true, true, true, false);
> +	if (test__start_subtest("sockmap msg_verdict permanent shutdown"))
> +		test_sockmap_msg_verdict(false, true, false, true);
> +	if (test__start_subtest("sockmap msg_verdict ingress permanent shutdown"))
> +		test_sockmap_msg_verdict(true, true, false, true);
> +	if (test__start_subtest("sockmap msg_verdict shutdown"))
> +		test_sockmap_msg_verdict(false, false, false, true);
> +	if (test__start_subtest("sockmap msg_verdict ingress shutdown"))
> +		test_sockmap_msg_verdict(true, false, false, true);
>  }

I appreciate the split up of test changes into commits. Thanks.

As you see, the args for test_sockmap_msg_verdict became quite cryptic.
I think having dedicated aliases for 'true' would make it more readable:

        const bool INGRESS = true;
        const bool PERMANENT = true;
        const bool TO_SELF = true;
        const bool TARGET_SHUTDOWN = true;

Then invocations as:

        test_sockmap_msg_verdict(true, false, false, true);

become:

        test_sockmap_msg_verdict(INGRESS, !PERMANENT, !TO_SELF, TARGET_SHUTDOWN);

Re: [PATCH bpf-next v7 0/7] add BPF_F_PERMANENT flag for sockmap skmsg redirect

[Jakub Sitnicki]

On Sat, Oct 28, 2023 at 06:05 PM +08, Liu Jian wrote:
> v6->v7: Rebase to latest bpf-next tree, and no changes.
> v5->v6: Modified the description of the helper function.
> v4->v5: Fix one refcount bug caused by patch1.
> v3->v4: Change the two helpers's description.
> 	Let BPF_F_PERMANENT takes precedence over apply/cork_bytes.
>
> Liu Jian (7):
>   bpf, sockmap: add BPF_F_PERMANENT flag for skmsg redirect
>   selftests/bpf: Add txmsg permanently test for sockmap
>   selftests/bpf: Add txmsg redir permanently test for sockmap
>   selftests/bpf: add skmsg verdict tests
>   selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENT flag
>   selftests/bpf: add tests for verdict skmsg to itself
>   selftests/bpf: add tests for verdict skmsg to closed socket
>
>  include/linux/skmsg.h                         |   1 +
>  include/uapi/linux/bpf.h                      |  45 +++++--
>  net/core/skmsg.c                              |   6 +-
>  net/core/sock_map.c                           |   4 +-
>  net/ipv4/tcp_bpf.c                            |  12 +-
>  tools/include/uapi/linux/bpf.h                |  45 +++++--
>  .../selftests/bpf/prog_tests/sockmap_basic.c  | 122 ++++++++++++++++++
>  .../selftests/bpf/progs/test_sockmap_kern.h   |   3 +-
>  .../bpf/progs/test_sockmap_msg_verdict.c      |  25 ++++
>  tools/testing/selftests/bpf/test_sockmap.c    |  41 +++++-
>  10 files changed, 272 insertions(+), 32 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c

I gave it one last look. For the series:

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

Re: [PATCH bpf-next v7 0/7] add BPF_F_PERMANENT flag for sockmap skmsg redirect

[John Fastabend]

Jakub Sitnicki wrote:
> On Sat, Oct 28, 2023 at 06:05 PM +08, Liu Jian wrote:
> > v6->v7: Rebase to latest bpf-next tree, and no changes.
> > v5->v6: Modified the description of the helper function.
> > v4->v5: Fix one refcount bug caused by patch1.
> > v3->v4: Change the two helpers's description.
> > 	Let BPF_F_PERMANENT takes precedence over apply/cork_bytes.
> >
> > Liu Jian (7):
> >   bpf, sockmap: add BPF_F_PERMANENT flag for skmsg redirect
> >   selftests/bpf: Add txmsg permanently test for sockmap
> >   selftests/bpf: Add txmsg redir permanently test for sockmap
> >   selftests/bpf: add skmsg verdict tests
> >   selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENT flag
> >   selftests/bpf: add tests for verdict skmsg to itself
> >   selftests/bpf: add tests for verdict skmsg to closed socket
> >
> >  include/linux/skmsg.h                         |   1 +
> >  include/uapi/linux/bpf.h                      |  45 +++++--
> >  net/core/skmsg.c                              |   6 +-
> >  net/core/sock_map.c                           |   4 +-
> >  net/ipv4/tcp_bpf.c                            |  12 +-
> >  tools/include/uapi/linux/bpf.h                |  45 +++++--
> >  .../selftests/bpf/prog_tests/sockmap_basic.c  | 122 ++++++++++++++++++
> >  .../selftests/bpf/progs/test_sockmap_kern.h   |   3 +-
> >  .../bpf/progs/test_sockmap_msg_verdict.c      |  25 ++++
> >  tools/testing/selftests/bpf/test_sockmap.c    |  41 +++++-
> >  10 files changed, 272 insertions(+), 32 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
> 
> I gave it one last look. For the series:
> 
> Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

Ah I assumed the reviewed-bys were just carried through. So one more time,

Reviewed-by: John Fastabend <john.fastabend@gmail.com>