* [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
@ 2019-09-04 16:25 Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 1/6] selftests/bpf: test_progs: add test__join_cgroup helper Stanislav Fomichev
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Now that test_progs is shaping into more generic test framework,
let's convert sockopt tests to it. This requires adding
a helper to create and join a cgroup first (test__join_cgroup).
Since we already hijack stdout/stderr that shouldn't be
a problem (cgroup helpers log to stderr).
The rest of the patches just move sockopt tests files under prog_tests/
and do the required small adjustments.
Stanislav Fomichev (6):
selftests/bpf: test_progs: add test__join_cgroup helper
selftests/bpf: test_progs: convert test_sockopt
selftests/bpf: test_progs: convert test_sockopt_sk
selftests/bpf: test_progs: convert test_sockopt_multi
selftests/bpf: test_progs: convert test_sockopt_inherit
selftests/bpf: test_progs: convert test_tcp_rtt
tools/testing/selftests/bpf/.gitignore | 5 -
tools/testing/selftests/bpf/Makefile | 12 +--
.../{test_sockopt.c => prog_tests/sockopt.c} | 50 ++-------
.../sockopt_inherit.c} | 102 ++++++++----------
.../sockopt_multi.c} | 62 ++---------
.../sockopt_sk.c} | 60 +++--------
.../{test_tcp_rtt.c => prog_tests/tcp_rtt.c} | 83 +++++---------
tools/testing/selftests/bpf/test_progs.c | 38 +++++++
tools/testing/selftests/bpf/test_progs.h | 4 +-
9 files changed, 142 insertions(+), 274 deletions(-)
rename tools/testing/selftests/bpf/{test_sockopt.c => prog_tests/sockopt.c} (96%)
rename tools/testing/selftests/bpf/{test_sockopt_inherit.c => prog_tests/sockopt_inherit.c} (72%)
rename tools/testing/selftests/bpf/{test_sockopt_multi.c => prog_tests/sockopt_multi.c} (83%)
rename tools/testing/selftests/bpf/{test_sockopt_sk.c => prog_tests/sockopt_sk.c} (79%)
rename tools/testing/selftests/bpf/{test_tcp_rtt.c => prog_tests/tcp_rtt.c} (76%)
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 1/6] selftests/bpf: test_progs: add test__join_cgroup helper
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 2/6] selftests/bpf: test_progs: convert test_sockopt Stanislav Fomichev
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
test__join_cgroup() combines the following operations that usually
go hand in hand and returns cgroup fd:
* setup cgroup environment (make sure cgroupfs is mounted)
* mkdir cgroup
* join cgroup
It also marks a test as a "cgroup cleanup needed" and removes cgroup
state after the test is done.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/Makefile | 4 +--
tools/testing/selftests/bpf/test_progs.c | 38 ++++++++++++++++++++++++
tools/testing/selftests/bpf/test_progs.h | 1 +
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c7595b4ed55d..e145954d3765 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -102,7 +102,7 @@ $(OUTPUT)/test_socket_cookie: cgroup_helpers.c
$(OUTPUT)/test_sockmap: cgroup_helpers.c
$(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c
$(OUTPUT)/test_tcpnotify_user: cgroup_helpers.c trace_helpers.c
-$(OUTPUT)/test_progs: trace_helpers.c
+$(OUTPUT)/test_progs: cgroup_helpers.c trace_helpers.c
$(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c
$(OUTPUT)/test_cgroup_storage: cgroup_helpers.c
$(OUTPUT)/test_netcnt: cgroup_helpers.c
@@ -196,7 +196,7 @@ $(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\
| $(ALU32_BUILD_DIR)
$(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \
-o $(ALU32_BUILD_DIR)/test_progs_32 \
- test_progs.c test_stub.c trace_helpers.c prog_tests/*.c \
+ test_progs.c test_stub.c cgroup_helpers.c trace_helpers.c prog_tests/*.c \
$(OUTPUT)/libbpf.a $(LDLIBS)
$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index e8616e778cb5..af75a1c7a458 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017 Facebook
*/
#include "test_progs.h"
+#include "cgroup_helpers.h"
#include "bpf_rlimit.h"
#include <argp.h>
#include <string.h>
@@ -17,6 +18,7 @@ struct prog_test_def {
int error_cnt;
int skip_cnt;
bool tested;
+ bool need_cgroup_cleanup;
const char *subtest_name;
int subtest_num;
@@ -122,6 +124,39 @@ void test__fail(void)
env.test->error_cnt++;
}
+int test__join_cgroup(const char *path)
+{
+ int fd;
+
+ if (!env.test->need_cgroup_cleanup) {
+ if (setup_cgroup_environment()) {
+ fprintf(stderr,
+ "#%d %s: Failed to setup cgroup environment\n",
+ env.test->test_num, env.test->test_name);
+ return -1;
+ }
+
+ env.test->need_cgroup_cleanup = true;
+ }
+
+ fd = create_and_get_cgroup(path);
+ if (fd < 0) {
+ fprintf(stderr,
+ "#%d %s: Failed to create cgroup '%s' (errno=%d)\n",
+ env.test->test_num, env.test->test_name, path, errno);
+ return fd;
+ }
+
+ if (join_cgroup(path)) {
+ fprintf(stderr,
+ "#%d %s: Failed to join cgroup '%s' (errno=%d)\n",
+ env.test->test_num, env.test->test_name, path, errno);
+ return -1;
+ }
+
+ return fd;
+}
+
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -530,6 +565,9 @@ int main(int argc, char **argv)
fprintf(env.stdout, "#%d %s:%s\n",
test->test_num, test->test_name,
test->error_cnt ? "FAIL" : "OK");
+
+ if (test->need_cgroup_cleanup)
+ cleanup_cgroup_environment();
}
stdio_restore();
printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index c8edb9464ba6..e518bd5da3e2 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -71,6 +71,7 @@ extern void test__force_log();
extern bool test__start_subtest(const char *name);
extern void test__skip(void);
extern void test__fail(void);
+extern int test__join_cgroup(const char *path);
#define MAGIC_BYTES 123
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/6] selftests/bpf: test_progs: convert test_sockopt
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 1/6] selftests/bpf: test_progs: add test__join_cgroup helper Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 3/6] selftests/bpf: test_progs: convert test_sockopt_sk Stanislav Fomichev
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Move the files, adjust includes, remove entry from Makefile & .gitignore
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../{test_sockopt.c => prog_tests/sockopt.c} | 50 +++----------------
3 files changed, 8 insertions(+), 46 deletions(-)
rename tools/testing/selftests/bpf/{test_sockopt.c => prog_tests/sockopt.c} (96%)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 60c9338cd9b4..0315120eac8f 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,7 +39,6 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
-test_sockopt
test_sockopt_sk
test_sockopt_multi
test_sockopt_inherit
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e145954d3765..08e2183974d5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -28,7 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
+ test_btf_dump test_cgroup_attach xdping test_sockopt_sk \
test_sockopt_multi test_sockopt_inherit test_tcp_rtt
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
@@ -109,7 +109,6 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
-$(OUTPUT)/test_sockopt: cgroup_helpers.c
$(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
$(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
$(OUTPUT)/test_sockopt_inherit: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_sockopt.c b/tools/testing/selftests/bpf/prog_tests/sockopt.c
similarity index 96%
rename from tools/testing/selftests/bpf/test_sockopt.c
rename to tools/testing/selftests/bpf/prog_tests/sockopt.c
index 23bd0819382d..3e8517a8395a 100644
--- a/tools/testing/selftests/bpf/test_sockopt.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt.c
@@ -1,22 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
-
-#include <errno.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-
-#include <linux/filter.h>
-#include <bpf/bpf.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_rlimit.h"
-#include "bpf_util.h"
+#include <test_progs.h>
#include "cgroup_helpers.h"
-#define CG_PATH "/sockopt"
-
static char bpf_log_buf[4096];
static bool verbose;
@@ -983,39 +968,18 @@ static int run_test(int cgroup_fd, struct sockopt_test *test)
return ret;
}
-int main(int args, char **argv)
+void test_sockopt(void)
{
- int err = EXIT_FAILURE, error_cnt = 0;
int cgroup_fd, i;
- if (setup_cgroup_environment())
- goto cleanup_obj;
-
- cgroup_fd = create_and_get_cgroup(CG_PATH);
- if (cgroup_fd < 0)
- goto cleanup_cgroup_env;
-
- if (join_cgroup(CG_PATH))
- goto cleanup_cgroup;
+ cgroup_fd = test__join_cgroup("/sockopt");
+ if (CHECK_FAIL(cgroup_fd < 0))
+ return;
for (i = 0; i < ARRAY_SIZE(tests); i++) {
- int err = run_test(cgroup_fd, &tests[i]);
-
- if (err)
- error_cnt++;
-
- printf("#%d %s: %s\n", i, err ? "FAIL" : "PASS",
- tests[i].descr);
+ test__start_subtest(tests[i].descr);
+ CHECK_FAIL(run_test(cgroup_fd, &tests[i]));
}
- printf("Summary: %ld PASSED, %d FAILED\n",
- ARRAY_SIZE(tests) - error_cnt, error_cnt);
- err = error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
-
-cleanup_cgroup:
close(cgroup_fd);
-cleanup_cgroup_env:
- cleanup_cgroup_environment();
-cleanup_obj:
- return err;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 3/6] selftests/bpf: test_progs: convert test_sockopt_sk
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 1/6] selftests/bpf: test_progs: add test__join_cgroup helper Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 2/6] selftests/bpf: test_progs: convert test_sockopt Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 4/6] selftests/bpf: test_progs: convert test_sockopt_multi Stanislav Fomichev
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Move the files, adjust includes, remove entry from Makefile & .gitignore
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../sockopt_sk.c} | 60 ++++---------------
tools/testing/selftests/bpf/test_progs.h | 3 +-
4 files changed, 15 insertions(+), 52 deletions(-)
rename tools/testing/selftests/bpf/{test_sockopt_sk.c => prog_tests/sockopt_sk.c} (79%)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 0315120eac8f..bc83c1a7ea1b 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,7 +39,6 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
-test_sockopt_sk
test_sockopt_multi
test_sockopt_inherit
test_tcp_rtt
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 08e2183974d5..ea790901297c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -28,7 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping test_sockopt_sk \
+ test_btf_dump test_cgroup_attach xdping \
test_sockopt_multi test_sockopt_inherit test_tcp_rtt
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
@@ -109,7 +109,6 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
-$(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
$(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
$(OUTPUT)/test_sockopt_inherit: cgroup_helpers.c
$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
similarity index 79%
rename from tools/testing/selftests/bpf/test_sockopt_sk.c
rename to tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index e4f6055d92e9..2061a6beac0f 100644
--- a/tools/testing/selftests/bpf/test_sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
@@ -1,23 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
-
-#include <errno.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <netinet/tcp.h>
-
-#include <linux/filter.h>
-#include <bpf/bpf.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_rlimit.h"
-#include "bpf_util.h"
+#include <test_progs.h>
#include "cgroup_helpers.h"
-#define CG_PATH "/sockopt"
-
#define SOL_CUSTOM 0xdeadbeef
static int getsetsockopt(void)
@@ -176,7 +160,7 @@ static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
return 0;
}
-static int run_test(int cgroup_fd)
+static void run_test(int cgroup_fd)
{
struct bpf_prog_load_attr attr = {
.file = "./sockopt_sk.o",
@@ -186,51 +170,31 @@ static int run_test(int cgroup_fd)
int err;
err = bpf_prog_load_xattr(&attr, &obj, &ignored);
- if (err) {
- log_err("Failed to load BPF object");
- return -1;
- }
+ if (CHECK_FAIL(err))
+ return;
err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt");
- if (err)
+ if (CHECK_FAIL(err))
goto close_bpf_object;
err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt");
- if (err)
+ if (CHECK_FAIL(err))
goto close_bpf_object;
- err = getsetsockopt();
+ CHECK_FAIL(getsetsockopt());
close_bpf_object:
bpf_object__close(obj);
- return err;
}
-int main(int args, char **argv)
+void test_sockopt_sk(void)
{
int cgroup_fd;
- int err = EXIT_SUCCESS;
-
- if (setup_cgroup_environment())
- goto cleanup_obj;
-
- cgroup_fd = create_and_get_cgroup(CG_PATH);
- if (cgroup_fd < 0)
- goto cleanup_cgroup_env;
-
- if (join_cgroup(CG_PATH))
- goto cleanup_cgroup;
-
- if (run_test(cgroup_fd))
- err = EXIT_FAILURE;
- printf("test_sockopt_sk: %s\n",
- err == EXIT_SUCCESS ? "PASSED" : "FAILED");
+ cgroup_fd = test__join_cgroup("/sockopt_sk");
+ if (CHECK_FAIL(cgroup_fd < 0))
+ return;
-cleanup_cgroup:
+ run_test(cgroup_fd);
close(cgroup_fd);
-cleanup_cgroup_env:
- cleanup_cgroup_environment();
-cleanup_obj:
- return err;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index e518bd5da3e2..0c48f64f732b 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -16,9 +16,10 @@ typedef __u16 __sum16;
#include <linux/if_packet.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
-#include <linux/tcp.h>
+#include <netinet/tcp.h>
#include <linux/filter.h>
#include <linux/perf_event.h>
+#include <linux/socket.h>
#include <linux/unistd.h>
#include <sys/ioctl.h>
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 4/6] selftests/bpf: test_progs: convert test_sockopt_multi
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
` (2 preceding siblings ...)
2019-09-04 16:25 ` [PATCH bpf-next 3/6] selftests/bpf: test_progs: convert test_sockopt_sk Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 5/6] selftests/bpf: test_progs: convert test_sockopt_inherit Stanislav Fomichev
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Move the files, adjust includes, remove entry from Makefile & .gitignore
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../sockopt_multi.c} | 62 +++----------------
3 files changed, 11 insertions(+), 55 deletions(-)
rename tools/testing/selftests/bpf/{test_sockopt_multi.c => prog_tests/sockopt_multi.c} (83%)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index bc83c1a7ea1b..4143add5a11e 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,6 +39,5 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
-test_sockopt_multi
test_sockopt_inherit
test_tcp_rtt
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ea790901297c..271f8ce89c97 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -29,7 +29,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
test_btf_dump test_cgroup_attach xdping \
- test_sockopt_multi test_sockopt_inherit test_tcp_rtt
+ test_sockopt_inherit test_tcp_rtt
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -109,7 +109,6 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
-$(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
$(OUTPUT)/test_sockopt_inherit: cgroup_helpers.c
$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_sockopt_multi.c b/tools/testing/selftests/bpf/prog_tests/sockopt_multi.c
similarity index 83%
rename from tools/testing/selftests/bpf/test_sockopt_multi.c
rename to tools/testing/selftests/bpf/prog_tests/sockopt_multi.c
index 4be3441db867..29188d6f5c8d 100644
--- a/tools/testing/selftests/bpf/test_sockopt_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_multi.c
@@ -1,19 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-
-#include <error.h>
-#include <errno.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-
-#include <linux/filter.h>
-#include <bpf/bpf.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_rlimit.h"
-#include "bpf_util.h"
+#include <test_progs.h>
#include "cgroup_helpers.h"
static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
@@ -308,7 +294,7 @@ static int run_setsockopt_test(struct bpf_object *obj, int cg_parent,
return err;
}
-int main(int argc, char **argv)
+void test_sockopt_multi(void)
{
struct bpf_prog_load_attr attr = {
.file = "./sockopt_multi.o",
@@ -319,56 +305,28 @@ int main(int argc, char **argv)
int err = -1;
int ignored;
- if (setup_cgroup_environment()) {
- log_err("Failed to setup cgroup environment\n");
- goto out;
- }
-
- cg_parent = create_and_get_cgroup("/parent");
- if (cg_parent < 0) {
- log_err("Failed to create cgroup /parent\n");
- goto out;
- }
-
- cg_child = create_and_get_cgroup("/parent/child");
- if (cg_child < 0) {
- log_err("Failed to create cgroup /parent/child\n");
+ cg_parent = test__join_cgroup("/parent");
+ if (CHECK_FAIL(cg_parent < 0))
goto out;
- }
- if (join_cgroup("/parent/child")) {
- log_err("Failed to join cgroup /parent/child\n");
+ cg_child = test__join_cgroup("/parent/child");
+ if (CHECK_FAIL(cg_child < 0))
goto out;
- }
err = bpf_prog_load_xattr(&attr, &obj, &ignored);
- if (err) {
- log_err("Failed to load BPF object");
+ if (CHECK_FAIL(err))
goto out;
- }
sock_fd = socket(AF_INET, SOCK_STREAM, 0);
- if (sock_fd < 0) {
- log_err("Failed to create socket");
+ if (CHECK_FAIL(sock_fd < 0))
goto out;
- }
- if (run_getsockopt_test(obj, cg_parent, cg_child, sock_fd))
- err = -1;
- printf("test_sockopt_multi: getsockopt %s\n",
- err ? "FAILED" : "PASSED");
-
- if (run_setsockopt_test(obj, cg_parent, cg_child, sock_fd))
- err = -1;
- printf("test_sockopt_multi: setsockopt %s\n",
- err ? "FAILED" : "PASSED");
+ CHECK_FAIL(run_getsockopt_test(obj, cg_parent, cg_child, sock_fd));
+ CHECK_FAIL(run_setsockopt_test(obj, cg_parent, cg_child, sock_fd));
out:
close(sock_fd);
bpf_object__close(obj);
close(cg_child);
close(cg_parent);
-
- printf("test_sockopt_multi: %s\n", err ? "FAILED" : "PASSED");
- return err ? EXIT_FAILURE : EXIT_SUCCESS;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 5/6] selftests/bpf: test_progs: convert test_sockopt_inherit
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
` (3 preceding siblings ...)
2019-09-04 16:25 ` [PATCH bpf-next 4/6] selftests/bpf: test_progs: convert test_sockopt_multi Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 6/6] selftests/bpf: test_progs: convert test_tcp_rtt Stanislav Fomichev
2019-09-04 23:03 ` [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Alexei Starovoitov
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Move the files, adjust includes, remove entry from Makefile & .gitignore
I also added pthread_cond_wait for the server thread startup. We don't
want to connect to the server that's not yet up (for some reason
this existing race is now more prominent with test_progs).
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 4 +-
.../sockopt_inherit.c} | 102 ++++++++----------
3 files changed, 43 insertions(+), 64 deletions(-)
rename tools/testing/selftests/bpf/{test_sockopt_inherit.c => prog_tests/sockopt_inherit.c} (72%)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 4143add5a11e..5b06bb45b500 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,5 +39,4 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
-test_sockopt_inherit
test_tcp_rtt
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 271f8ce89c97..fe786df1174b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -28,8 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping \
- test_sockopt_inherit test_tcp_rtt
+ test_btf_dump test_cgroup_attach xdping test_tcp_rtt
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -109,7 +108,6 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
-$(OUTPUT)/test_sockopt_inherit: cgroup_helpers.c
$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/test_sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
similarity index 72%
rename from tools/testing/selftests/bpf/test_sockopt_inherit.c
rename to tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index 1bf699815b9b..6cbeea7b4bf1 100644
--- a/tools/testing/selftests/bpf/test_sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -1,22 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
-#include <error.h>
-#include <errno.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <pthread.h>
-
-#include <linux/filter.h>
-#include <bpf/bpf.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_rlimit.h"
-#include "bpf_util.h"
+#include <test_progs.h>
#include "cgroup_helpers.h"
-#define CG_PATH "/sockopt_inherit"
#define SOL_CUSTOM 0xdeadbeef
#define CUSTOM_INHERIT1 0
#define CUSTOM_INHERIT2 1
@@ -74,6 +59,9 @@ static int verify_sockopt(int fd, int optname, const char *msg, char expected)
return 0;
}
+static pthread_mutex_t server_started_mtx = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t server_started = PTHREAD_COND_INITIALIZER;
+
static void *server_thread(void *arg)
{
struct sockaddr_storage addr;
@@ -82,16 +70,26 @@ static void *server_thread(void *arg)
int client_fd;
int err = 0;
- if (listen(fd, 1) < 0)
- error(1, errno, "Failed to listed on socket");
+ err = listen(fd, 1);
+
+ pthread_mutex_lock(&server_started_mtx);
+ pthread_cond_signal(&server_started);
+ pthread_mutex_unlock(&server_started_mtx);
+
+ if (CHECK_FAIL(err < 0)) {
+ perror("Failed to listed on socket");
+ return NULL;
+ }
err += verify_sockopt(fd, CUSTOM_INHERIT1, "listen", 1);
err += verify_sockopt(fd, CUSTOM_INHERIT2, "listen", 1);
err += verify_sockopt(fd, CUSTOM_LISTENER, "listen", 1);
client_fd = accept(fd, (struct sockaddr *)&addr, &len);
- if (client_fd < 0)
- error(1, errno, "Failed to accept client");
+ if (CHECK_FAIL(client_fd < 0)) {
+ perror("Failed to accept client");
+ return NULL;
+ }
err += verify_sockopt(client_fd, CUSTOM_INHERIT1, "accept", 1);
err += verify_sockopt(client_fd, CUSTOM_INHERIT2, "accept", 1);
@@ -167,7 +165,7 @@ static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
return 0;
}
-static int run_test(int cgroup_fd)
+static void run_test(int cgroup_fd)
{
struct bpf_prog_load_attr attr = {
.file = "./sockopt_inherit.o",
@@ -180,40 +178,41 @@ static int run_test(int cgroup_fd)
int err;
err = bpf_prog_load_xattr(&attr, &obj, &ignored);
- if (err) {
- log_err("Failed to load BPF object");
- return -1;
- }
+ if (CHECK_FAIL(err))
+ return;
err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt");
- if (err)
+ if (CHECK_FAIL(err))
goto close_bpf_object;
err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt");
- if (err)
+ if (CHECK_FAIL(err))
goto close_bpf_object;
server_fd = start_server();
- if (server_fd < 0) {
- err = -1;
+ if (CHECK_FAIL(server_fd < 0))
+ goto close_bpf_object;
+
+ if (CHECK_FAIL(pthread_create(&tid, NULL, server_thread,
+ (void *)&server_fd)))
goto close_bpf_object;
- }
- pthread_create(&tid, NULL, server_thread, (void *)&server_fd);
+ pthread_mutex_lock(&server_started_mtx);
+ pthread_cond_wait(&server_started, &server_started_mtx);
+ pthread_mutex_unlock(&server_started_mtx);
client_fd = connect_to_server(server_fd);
- if (client_fd < 0) {
- err = -1;
+ if (CHECK_FAIL(client_fd < 0))
goto close_server_fd;
- }
- err += verify_sockopt(client_fd, CUSTOM_INHERIT1, "connect", 0);
- err += verify_sockopt(client_fd, CUSTOM_INHERIT2, "connect", 0);
- err += verify_sockopt(client_fd, CUSTOM_LISTENER, "connect", 0);
+ CHECK_FAIL(verify_sockopt(client_fd, CUSTOM_INHERIT1, "connect", 0));
+ CHECK_FAIL(verify_sockopt(client_fd, CUSTOM_INHERIT2, "connect", 0));
+ CHECK_FAIL(verify_sockopt(client_fd, CUSTOM_LISTENER, "connect", 0));
pthread_join(tid, &server_err);
- err += (int)(long)server_err;
+ err = (int)(long)server_err;
+ CHECK_FAIL(err);
close(client_fd);
@@ -221,33 +220,16 @@ static int run_test(int cgroup_fd)
close(server_fd);
close_bpf_object:
bpf_object__close(obj);
- return err;
}
-int main(int args, char **argv)
+void test_sockopt_inherit(void)
{
int cgroup_fd;
- int err = EXIT_SUCCESS;
-
- if (setup_cgroup_environment())
- return err;
-
- cgroup_fd = create_and_get_cgroup(CG_PATH);
- if (cgroup_fd < 0)
- goto cleanup_cgroup_env;
-
- if (join_cgroup(CG_PATH))
- goto cleanup_cgroup;
-
- if (run_test(cgroup_fd))
- err = EXIT_FAILURE;
- printf("test_sockopt_inherit: %s\n",
- err == EXIT_SUCCESS ? "PASSED" : "FAILED");
+ cgroup_fd = test__join_cgroup("/sockopt_inherit");
+ if (CHECK_FAIL(cgroup_fd < 0))
+ return;
-cleanup_cgroup:
+ run_test(cgroup_fd);
close(cgroup_fd);
-cleanup_cgroup_env:
- cleanup_cgroup_environment();
- return err;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 6/6] selftests/bpf: test_progs: convert test_tcp_rtt
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
` (4 preceding siblings ...)
2019-09-04 16:25 ` [PATCH bpf-next 5/6] selftests/bpf: test_progs: convert test_sockopt_inherit Stanislav Fomichev
@ 2019-09-04 16:25 ` Stanislav Fomichev
2019-09-04 23:03 ` [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Alexei Starovoitov
6 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-04 16:25 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Move the files, adjust includes, remove entry from Makefile & .gitignore
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../{test_tcp_rtt.c => prog_tests/tcp_rtt.c} | 83 ++++++-------------
3 files changed, 28 insertions(+), 59 deletions(-)
rename tools/testing/selftests/bpf/{test_tcp_rtt.c => prog_tests/tcp_rtt.c} (76%)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 5b06bb45b500..7470327edcfe 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,4 +39,3 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
-test_tcp_rtt
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index fe786df1174b..811f1b24d02b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -28,7 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping test_tcp_rtt
+ test_btf_dump test_cgroup_attach xdping
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -108,7 +108,6 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
-$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/test_tcp_rtt.c b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
similarity index 76%
rename from tools/testing/selftests/bpf/test_tcp_rtt.c
rename to tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
index 93916a69823e..fdc0b3614a9e 100644
--- a/tools/testing/selftests/bpf/test_tcp_rtt.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
@@ -1,24 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
-#include <error.h>
-#include <errno.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <netinet/tcp.h>
-#include <pthread.h>
-
-#include <linux/filter.h>
-#include <bpf/bpf.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_rlimit.h"
-#include "bpf_util.h"
+#include <test_progs.h>
#include "cgroup_helpers.h"
-#define CG_PATH "/tcp_rtt"
-
struct tcp_rtt_storage {
__u32 invoked;
__u32 dsack_dups;
@@ -31,8 +14,8 @@ static void send_byte(int fd)
{
char b = 0x55;
- if (write(fd, &b, sizeof(b)) != 1)
- error(1, errno, "Failed to send single byte");
+ if (CHECK_FAIL(write(fd, &b, sizeof(b)) != 1))
+ perror("Failed to send single byte");
}
static int wait_for_ack(int fd, int retries)
@@ -66,8 +49,10 @@ static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 invoked,
int err = 0;
struct tcp_rtt_storage val;
- if (bpf_map_lookup_elem(map_fd, &client_fd, &val) < 0)
- error(1, errno, "Failed to read socket storage");
+ if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &client_fd, &val) < 0)) {
+ perror("Failed to read socket storage");
+ return -1;
+ }
if (val.invoked != invoked) {
log_err("%s: unexpected bpf_tcp_sock.invoked %d != %d",
@@ -225,61 +210,47 @@ static void *server_thread(void *arg)
int fd = *(int *)arg;
int client_fd;
- if (listen(fd, 1) < 0)
- error(1, errno, "Failed to listed on socket");
+ if (CHECK_FAIL(listen(fd, 1)) < 0) {
+ perror("Failed to listed on socket");
+ return NULL;
+ }
client_fd = accept(fd, (struct sockaddr *)&addr, &len);
- if (client_fd < 0)
- error(1, errno, "Failed to accept client");
+ if (CHECK_FAIL(client_fd < 0)) {
+ perror("Failed to accept client");
+ return NULL;
+ }
/* Wait for the next connection (that never arrives)
* to keep this thread alive to prevent calling
* close() on client_fd.
*/
- if (accept(fd, (struct sockaddr *)&addr, &len) >= 0)
- error(1, errno, "Unexpected success in second accept");
+ if (CHECK_FAIL(accept(fd, (struct sockaddr *)&addr, &len) >= 0)) {
+ perror("Unexpected success in second accept");
+ return NULL;
+ }
close(client_fd);
return NULL;
}
-int main(int args, char **argv)
+void test_tcp_rtt(void)
{
int server_fd, cgroup_fd;
- int err = EXIT_SUCCESS;
pthread_t tid;
- if (setup_cgroup_environment())
- goto cleanup_obj;
-
- cgroup_fd = create_and_get_cgroup(CG_PATH);
- if (cgroup_fd < 0)
- goto cleanup_cgroup_env;
-
- if (join_cgroup(CG_PATH))
- goto cleanup_cgroup;
+ cgroup_fd = test__join_cgroup("/tcp_rtt");
+ if (CHECK_FAIL(cgroup_fd < 0))
+ return;
server_fd = start_server();
- if (server_fd < 0) {
- err = EXIT_FAILURE;
- goto cleanup_cgroup;
- }
+ if (CHECK_FAIL(server_fd < 0))
+ goto close_cgroup_fd;
pthread_create(&tid, NULL, server_thread, (void *)&server_fd);
-
- if (run_test(cgroup_fd, server_fd))
- err = EXIT_FAILURE;
-
+ CHECK_FAIL(run_test(cgroup_fd, server_fd));
close(server_fd);
-
- printf("test_sockopt_sk: %s\n",
- err == EXIT_SUCCESS ? "PASSED" : "FAILED");
-
-cleanup_cgroup:
+close_cgroup_fd:
close(cgroup_fd);
-cleanup_cgroup_env:
- cleanup_cgroup_environment();
-cleanup_obj:
- return err;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
` (5 preceding siblings ...)
2019-09-04 16:25 ` [PATCH bpf-next 6/6] selftests/bpf: test_progs: convert test_tcp_rtt Stanislav Fomichev
@ 2019-09-04 23:03 ` Alexei Starovoitov
2019-09-06 9:32 ` Andrii Nakryiko
6 siblings, 1 reply; 12+ messages in thread
From: Alexei Starovoitov @ 2019-09-04 23:03 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, davem, ast, daniel
On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> Now that test_progs is shaping into more generic test framework,
> let's convert sockopt tests to it. This requires adding
> a helper to create and join a cgroup first (test__join_cgroup).
> Since we already hijack stdout/stderr that shouldn't be
> a problem (cgroup helpers log to stderr).
>
> The rest of the patches just move sockopt tests files under prog_tests/
> and do the required small adjustments.
Looks good. Thank you for working on it.
Could you de-verbose setsockopt test a bit?
#23/32 setsockopt: deny write ctx->retval:OK
#23/33 setsockopt: deny read ctx->retval:OK
#23/34 setsockopt: deny writing to ctx->optval:OK
#23/35 setsockopt: deny writing to ctx->optval_end:OK
#23/36 setsockopt: allow IP_TOS <= 128:OK
#23/37 setsockopt: deny IP_TOS > 128:OK
37 subtests is a bit too much spam.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
2019-09-04 23:03 ` [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Alexei Starovoitov
@ 2019-09-06 9:32 ` Andrii Nakryiko
2019-09-06 15:18 ` Stanislav Fomichev
0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2019-09-06 9:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, Networking, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On Wed, Sep 4, 2019 at 4:03 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> > Now that test_progs is shaping into more generic test framework,
> > let's convert sockopt tests to it. This requires adding
> > a helper to create and join a cgroup first (test__join_cgroup).
> > Since we already hijack stdout/stderr that shouldn't be
> > a problem (cgroup helpers log to stderr).
> >
> > The rest of the patches just move sockopt tests files under prog_tests/
> > and do the required small adjustments.
>
> Looks good. Thank you for working on it.
> Could you de-verbose setsockopt test a bit?
> #23/32 setsockopt: deny write ctx->retval:OK
> #23/33 setsockopt: deny read ctx->retval:OK
> #23/34 setsockopt: deny writing to ctx->optval:OK
> #23/35 setsockopt: deny writing to ctx->optval_end:OK
> #23/36 setsockopt: allow IP_TOS <= 128:OK
> #23/37 setsockopt: deny IP_TOS > 128:OK
> 37 subtests is a bit too much spam.
If we merged test_btf into test_progs, we'd have >150 subtests, which
would be pretty verbose as well. But the benefit of subtest is that
you can run just that sub-test and debug/verify just it, without all
the rest stuff.
So I'm wondering, if too many lines of default output is the only
problem, should we just not output per-subtest line by default?
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
2019-09-06 9:32 ` Andrii Nakryiko
@ 2019-09-06 15:18 ` Stanislav Fomichev
2019-09-06 16:42 ` Alexei Starovoitov
0 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2019-09-06 15:18 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Stanislav Fomichev, Networking, bpf,
David S. Miller, Alexei Starovoitov, Daniel Borkmann
On 09/06, Andrii Nakryiko wrote:
> On Wed, Sep 4, 2019 at 4:03 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> > > Now that test_progs is shaping into more generic test framework,
> > > let's convert sockopt tests to it. This requires adding
> > > a helper to create and join a cgroup first (test__join_cgroup).
> > > Since we already hijack stdout/stderr that shouldn't be
> > > a problem (cgroup helpers log to stderr).
> > >
> > > The rest of the patches just move sockopt tests files under prog_tests/
> > > and do the required small adjustments.
> >
> > Looks good. Thank you for working on it.
> > Could you de-verbose setsockopt test a bit?
> > #23/32 setsockopt: deny write ctx->retval:OK
> > #23/33 setsockopt: deny read ctx->retval:OK
> > #23/34 setsockopt: deny writing to ctx->optval:OK
> > #23/35 setsockopt: deny writing to ctx->optval_end:OK
> > #23/36 setsockopt: allow IP_TOS <= 128:OK
> > #23/37 setsockopt: deny IP_TOS > 128:OK
> > 37 subtests is a bit too much spam.
>
> If we merged test_btf into test_progs, we'd have >150 subtests, which
> would be pretty verbose as well. But the benefit of subtest is that
> you can run just that sub-test and debug/verify just it, without all
> the rest stuff.
>
> So I'm wondering, if too many lines of default output is the only
> problem, should we just not output per-subtest line by default?
Ack, we can output per-subtest line if it fails so it's easy to re-run;
otherwise, hiding by default sounds good. I'll prepare a v3 sometime
today; Alexei, let us know if you disagree.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
2019-09-06 15:18 ` Stanislav Fomichev
@ 2019-09-06 16:42 ` Alexei Starovoitov
2019-09-06 17:02 ` Alexei Starovoitov
0 siblings, 1 reply; 12+ messages in thread
From: Alexei Starovoitov @ 2019-09-06 16:42 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, Stanislav Fomichev, Networking, bpf,
David S. Miller, Alexei Starovoitov, Daniel Borkmann
On Fri, Sep 06, 2019 at 08:18:08AM -0700, Stanislav Fomichev wrote:
> On 09/06, Andrii Nakryiko wrote:
> > On Wed, Sep 4, 2019 at 4:03 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> > > > Now that test_progs is shaping into more generic test framework,
> > > > let's convert sockopt tests to it. This requires adding
> > > > a helper to create and join a cgroup first (test__join_cgroup).
> > > > Since we already hijack stdout/stderr that shouldn't be
> > > > a problem (cgroup helpers log to stderr).
> > > >
> > > > The rest of the patches just move sockopt tests files under prog_tests/
> > > > and do the required small adjustments.
> > >
> > > Looks good. Thank you for working on it.
> > > Could you de-verbose setsockopt test a bit?
> > > #23/32 setsockopt: deny write ctx->retval:OK
> > > #23/33 setsockopt: deny read ctx->retval:OK
> > > #23/34 setsockopt: deny writing to ctx->optval:OK
> > > #23/35 setsockopt: deny writing to ctx->optval_end:OK
> > > #23/36 setsockopt: allow IP_TOS <= 128:OK
> > > #23/37 setsockopt: deny IP_TOS > 128:OK
> > > 37 subtests is a bit too much spam.
> >
> > If we merged test_btf into test_progs, we'd have >150 subtests, which
> > would be pretty verbose as well. But the benefit of subtest is that
> > you can run just that sub-test and debug/verify just it, without all
> > the rest stuff.
> >
> > So I'm wondering, if too many lines of default output is the only
> > problem, should we just not output per-subtest line by default?
> Ack, we can output per-subtest line if it fails so it's easy to re-run;
> otherwise, hiding by default sounds good. I'll prepare a v3 sometime
> today; Alexei, let us know if you disagree.
If the subtests are runnable and useful individually it's good to have
them as subtests.
I think in the above I misread them as a sequence of sub-checks that needs
to happen before actual test result.
Looking at test_sockopt.c I see that they're separate tests,
so yeah keep them.
No need to hide by default or extra flags.
Let me look at v1 and v2 again...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
2019-09-06 16:42 ` Alexei Starovoitov
@ 2019-09-06 17:02 ` Alexei Starovoitov
0 siblings, 0 replies; 12+ messages in thread
From: Alexei Starovoitov @ 2019-09-06 17:02 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, Stanislav Fomichev, Networking, bpf,
David S. Miller, Alexei Starovoitov, Daniel Borkmann
On Fri, Sep 6, 2019 at 9:42 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Sep 06, 2019 at 08:18:08AM -0700, Stanislav Fomichev wrote:
> > On 09/06, Andrii Nakryiko wrote:
> > > On Wed, Sep 4, 2019 at 4:03 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> > > > > Now that test_progs is shaping into more generic test framework,
> > > > > let's convert sockopt tests to it. This requires adding
> > > > > a helper to create and join a cgroup first (test__join_cgroup).
> > > > > Since we already hijack stdout/stderr that shouldn't be
> > > > > a problem (cgroup helpers log to stderr).
> > > > >
> > > > > The rest of the patches just move sockopt tests files under prog_tests/
> > > > > and do the required small adjustments.
> > > >
> > > > Looks good. Thank you for working on it.
> > > > Could you de-verbose setsockopt test a bit?
> > > > #23/32 setsockopt: deny write ctx->retval:OK
> > > > #23/33 setsockopt: deny read ctx->retval:OK
> > > > #23/34 setsockopt: deny writing to ctx->optval:OK
> > > > #23/35 setsockopt: deny writing to ctx->optval_end:OK
> > > > #23/36 setsockopt: allow IP_TOS <= 128:OK
> > > > #23/37 setsockopt: deny IP_TOS > 128:OK
> > > > 37 subtests is a bit too much spam.
> > >
> > > If we merged test_btf into test_progs, we'd have >150 subtests, which
> > > would be pretty verbose as well. But the benefit of subtest is that
> > > you can run just that sub-test and debug/verify just it, without all
> > > the rest stuff.
> > >
> > > So I'm wondering, if too many lines of default output is the only
> > > problem, should we just not output per-subtest line by default?
> > Ack, we can output per-subtest line if it fails so it's easy to re-run;
> > otherwise, hiding by default sounds good. I'll prepare a v3 sometime
> > today; Alexei, let us know if you disagree.
>
> If the subtests are runnable and useful individually it's good to have
> them as subtests.
> I think in the above I misread them as a sequence of sub-checks that needs
> to happen before actual test result.
> Looking at test_sockopt.c I see that they're separate tests,
> so yeah keep them.
> No need to hide by default or extra flags.
> Let me look at v1 and v2 again...
I applied v1 set. Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-09-06 17:02 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04 16:25 [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 1/6] selftests/bpf: test_progs: add test__join_cgroup helper Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 2/6] selftests/bpf: test_progs: convert test_sockopt Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 3/6] selftests/bpf: test_progs: convert test_sockopt_sk Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 4/6] selftests/bpf: test_progs: convert test_sockopt_multi Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 5/6] selftests/bpf: test_progs: convert test_sockopt_inherit Stanislav Fomichev
2019-09-04 16:25 ` [PATCH bpf-next 6/6] selftests/bpf: test_progs: convert test_tcp_rtt Stanislav Fomichev
2019-09-04 23:03 ` [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs Alexei Starovoitov
2019-09-06 9:32 ` Andrii Nakryiko
2019-09-06 15:18 ` Stanislav Fomichev
2019-09-06 16:42 ` Alexei Starovoitov
2019-09-06 17:02 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).