diff mbox series

[bpf-next] selftests/bpf: Use prog_attach_type to attach in test_sockmap

Message ID e27d7d0c1e0e79b0acd22ac6ad5d8f9f00225303.1716372485.git.tanggeliang@kylinos.cn (mailing list archive)
State New
Headers show
Series [bpf-next] selftests/bpf: Use prog_attach_type to attach in test_sockmap | expand

Commit Message

Geliang Tang May 22, 2024, 10:08 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

Since prog_attach_type[] array is defined, it makes sense to use it paired
with prog_fd[] array for bpf_prog_attach() and bpf_prog_detach2() instead
of open-coding.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/test_sockmap.c | 44 +++++++++++-----------
 1 file changed, 22 insertions(+), 22 deletions(-)

Comments

Geliang Tang May 23, 2024, 7:03 a.m. UTC | #1 
This patch is "Rejected", according to Jakub's comments:

https://lore.kernel.org/bpf/87zfsiw3a3.fsf@cloudflare.com/

Thanks,
-Geliang

On Wed, 2024-05-22 at 18:08 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Since prog_attach_type[] array is defined, it makes sense to use it
> paired
> with prog_fd[] array for bpf_prog_attach() and bpf_prog_detach2()
> instead
> of open-coding.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/test_sockmap.c | 44 +++++++++++---------
> --
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c
> b/tools/testing/selftests/bpf/test_sockmap.c
> index 4499b3cfc3a6..8c8208b82c5e 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -65,6 +65,18 @@ int map_fd[9];
>  struct bpf_map *maps[9];
>  int prog_fd[9];
>  
> +int prog_attach_type[] = {
> +	BPF_SK_SKB_STREAM_PARSER,
> +	BPF_SK_SKB_STREAM_VERDICT,
> +	BPF_SK_SKB_STREAM_VERDICT,
> +	BPF_CGROUP_SOCK_OPS,
> +	BPF_SK_MSG_VERDICT,
> +	BPF_SK_MSG_VERDICT,
> +	BPF_SK_MSG_VERDICT,
> +	BPF_SK_MSG_VERDICT,
> +	BPF_SK_MSG_VERDICT,
> +};
> +
>  int txmsg_pass;
>  int txmsg_redir;
>  int txmsg_drop;
> @@ -961,7 +973,7 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  	/* Attach programs to sockmap */
>  	if (!txmsg_omit_skb_parser) {
>  		err = bpf_prog_attach(prog_fd[0], map_fd[0],
> -				      BPF_SK_SKB_STREAM_PARSER, 0);
> +				      prog_attach_type[0], 0);
>  		if (err) {
>  			fprintf(stderr,
>  				"ERROR: bpf_prog_attach (sockmap %i-
> >%i): %d (%s)\n",
> @@ -971,7 +983,7 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  	}
>  
>  	err = bpf_prog_attach(prog_fd[1], map_fd[0],
> -				BPF_SK_SKB_STREAM_VERDICT, 0);
> +			      prog_attach_type[1], 0);
>  	if (err) {
>  		fprintf(stderr, "ERROR: bpf_prog_attach (sockmap):
> %d (%s)\n",
>  			err, strerror(errno));
> @@ -982,7 +994,7 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  	if (txmsg_ktls_skb) {
>  		if (!txmsg_omit_skb_parser) {
>  			err = bpf_prog_attach(prog_fd[0], map_fd[8],
> -					     
> BPF_SK_SKB_STREAM_PARSER, 0);
> +					      prog_attach_type[0],
> 0);
>  			if (err) {
>  				fprintf(stderr,
>  					"ERROR: bpf_prog_attach (TLS
> sockmap %i->%i): %d (%s)\n",
> @@ -992,7 +1004,7 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  		}
>  
>  		err = bpf_prog_attach(prog_fd[2], map_fd[8],
> -				      BPF_SK_SKB_STREAM_VERDICT, 0);
> +				      prog_attach_type[2], 0);
>  		if (err) {
>  			fprintf(stderr, "ERROR: bpf_prog_attach (TLS
> sockmap): %d (%s)\n",
>  				err, strerror(errno));
> @@ -1001,7 +1013,7 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  	}
>  
>  	/* Attach to cgroups */
> -	err = bpf_prog_attach(prog_fd[3], cg_fd,
> BPF_CGROUP_SOCK_OPS, 0);
> +	err = bpf_prog_attach(prog_fd[3], cg_fd,
> prog_attach_type[3], 0);
>  	if (err) {
>  		fprintf(stderr, "ERROR: bpf_prog_attach (groups): %d
> (%s)\n",
>  			err, strerror(errno));
> @@ -1279,11 +1291,11 @@ static int run_options(struct sockmap_options
> *options, int cg_fd,  int test)
>  		fprintf(stderr, "unknown test\n");
>  out:
>  	/* Detatch and zero all the maps */
> -	bpf_prog_detach2(prog_fd[3], cg_fd, BPF_CGROUP_SOCK_OPS);
> -	bpf_prog_detach2(prog_fd[0], map_fd[0],
> BPF_SK_SKB_STREAM_PARSER);
> -	bpf_prog_detach2(prog_fd[1], map_fd[0],
> BPF_SK_SKB_STREAM_VERDICT);
> -	bpf_prog_detach2(prog_fd[0], map_fd[8],
> BPF_SK_SKB_STREAM_PARSER);
> -	bpf_prog_detach2(prog_fd[2], map_fd[8],
> BPF_SK_SKB_STREAM_VERDICT);
> +	bpf_prog_detach2(prog_fd[3], cg_fd, prog_attach_type[3]);
> +	bpf_prog_detach2(prog_fd[0], map_fd[0],
> prog_attach_type[0]);
> +	bpf_prog_detach2(prog_fd[1], map_fd[0],
> prog_attach_type[1]);
> +	bpf_prog_detach2(prog_fd[0], map_fd[8],
> prog_attach_type[0]);
> +	bpf_prog_detach2(prog_fd[2], map_fd[8],
> prog_attach_type[2]);
>  
>  	if (tx_prog_fd >= 0)
>  		bpf_prog_detach2(tx_prog_fd, map_fd[1],
> BPF_SK_MSG_VERDICT);
> @@ -1783,18 +1795,6 @@ char *map_names[] = {
>  	"tls_sock_map",
>  };
>  
> -int prog_attach_type[] = {
> -	BPF_SK_SKB_STREAM_PARSER,
> -	BPF_SK_SKB_STREAM_VERDICT,
> -	BPF_SK_SKB_STREAM_VERDICT,
> -	BPF_CGROUP_SOCK_OPS,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -};
> -
>  int prog_type[] = {
>  	BPF_PROG_TYPE_SK_SKB,
>  	BPF_PROG_TYPE_SK_SKB,
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 4499b3cfc3a6..8c8208b82c5e 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -65,6 +65,18 @@  int map_fd[9];
 struct bpf_map *maps[9];
 int prog_fd[9];
 
+int prog_attach_type[] = {
+	BPF_SK_SKB_STREAM_PARSER,
+	BPF_SK_SKB_STREAM_VERDICT,
+	BPF_SK_SKB_STREAM_VERDICT,
+	BPF_CGROUP_SOCK_OPS,
+	BPF_SK_MSG_VERDICT,
+	BPF_SK_MSG_VERDICT,
+	BPF_SK_MSG_VERDICT,
+	BPF_SK_MSG_VERDICT,
+	BPF_SK_MSG_VERDICT,
+};
+
 int txmsg_pass;
 int txmsg_redir;
 int txmsg_drop;
@@ -961,7 +973,7 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 	/* Attach programs to sockmap */
 	if (!txmsg_omit_skb_parser) {
 		err = bpf_prog_attach(prog_fd[0], map_fd[0],
-				      BPF_SK_SKB_STREAM_PARSER, 0);
+				      prog_attach_type[0], 0);
 		if (err) {
 			fprintf(stderr,
 				"ERROR: bpf_prog_attach (sockmap %i->%i): %d (%s)\n",
@@ -971,7 +983,7 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 	}
 
 	err = bpf_prog_attach(prog_fd[1], map_fd[0],
-				BPF_SK_SKB_STREAM_VERDICT, 0);
+			      prog_attach_type[1], 0);
 	if (err) {
 		fprintf(stderr, "ERROR: bpf_prog_attach (sockmap): %d (%s)\n",
 			err, strerror(errno));
@@ -982,7 +994,7 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 	if (txmsg_ktls_skb) {
 		if (!txmsg_omit_skb_parser) {
 			err = bpf_prog_attach(prog_fd[0], map_fd[8],
-					      BPF_SK_SKB_STREAM_PARSER, 0);
+					      prog_attach_type[0], 0);
 			if (err) {
 				fprintf(stderr,
 					"ERROR: bpf_prog_attach (TLS sockmap %i->%i): %d (%s)\n",
@@ -992,7 +1004,7 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 		}
 
 		err = bpf_prog_attach(prog_fd[2], map_fd[8],
-				      BPF_SK_SKB_STREAM_VERDICT, 0);
+				      prog_attach_type[2], 0);
 		if (err) {
 			fprintf(stderr, "ERROR: bpf_prog_attach (TLS sockmap): %d (%s)\n",
 				err, strerror(errno));
@@ -1001,7 +1013,7 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 	}
 
 	/* Attach to cgroups */
-	err = bpf_prog_attach(prog_fd[3], cg_fd, BPF_CGROUP_SOCK_OPS, 0);
+	err = bpf_prog_attach(prog_fd[3], cg_fd, prog_attach_type[3], 0);
 	if (err) {
 		fprintf(stderr, "ERROR: bpf_prog_attach (groups): %d (%s)\n",
 			err, strerror(errno));
@@ -1279,11 +1291,11 @@  static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 		fprintf(stderr, "unknown test\n");
 out:
 	/* Detatch and zero all the maps */
-	bpf_prog_detach2(prog_fd[3], cg_fd, BPF_CGROUP_SOCK_OPS);
-	bpf_prog_detach2(prog_fd[0], map_fd[0], BPF_SK_SKB_STREAM_PARSER);
-	bpf_prog_detach2(prog_fd[1], map_fd[0], BPF_SK_SKB_STREAM_VERDICT);
-	bpf_prog_detach2(prog_fd[0], map_fd[8], BPF_SK_SKB_STREAM_PARSER);
-	bpf_prog_detach2(prog_fd[2], map_fd[8], BPF_SK_SKB_STREAM_VERDICT);
+	bpf_prog_detach2(prog_fd[3], cg_fd, prog_attach_type[3]);
+	bpf_prog_detach2(prog_fd[0], map_fd[0], prog_attach_type[0]);
+	bpf_prog_detach2(prog_fd[1], map_fd[0], prog_attach_type[1]);
+	bpf_prog_detach2(prog_fd[0], map_fd[8], prog_attach_type[0]);
+	bpf_prog_detach2(prog_fd[2], map_fd[8], prog_attach_type[2]);
 
 	if (tx_prog_fd >= 0)
 		bpf_prog_detach2(tx_prog_fd, map_fd[1], BPF_SK_MSG_VERDICT);
@@ -1783,18 +1795,6 @@  char *map_names[] = {
 	"tls_sock_map",
 };
 
-int prog_attach_type[] = {
-	BPF_SK_SKB_STREAM_PARSER,
-	BPF_SK_SKB_STREAM_VERDICT,
-	BPF_SK_SKB_STREAM_VERDICT,
-	BPF_CGROUP_SOCK_OPS,
-	BPF_SK_MSG_VERDICT,
-	BPF_SK_MSG_VERDICT,
-	BPF_SK_MSG_VERDICT,
-	BPF_SK_MSG_VERDICT,
-	BPF_SK_MSG_VERDICT,
-};
-
 int prog_type[] = {
 	BPF_PROG_TYPE_SK_SKB,
 	BPF_PROG_TYPE_SK_SKB,