* [PATCH bpf-next 0/4] Parallelize verif_scale selftests
@ 2021-10-22 22:32 Andrii Nakryiko
2021-10-22 22:32 ` [PATCH bpf-next 1/4] selftests/bpf: normalize selftest entry points Andrii Nakryiko
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2021-10-22 22:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Reduce amount of waiting time when running test_progs in parallel mode (-j) by
splitting bpf_verif_scale selftests into multiple tests. Previously it was
structured as a test with multiple subtests, but subtests are not easily
parallelizable with test_progs' infra. Also in practice each scale subtest is
really an independent test with nothing shared across all substest.
This patch set changes how test_progs test discovery works. Now it is possible
to define multiple tests within a single source code file. One of the patches
also marks tc_redirect selftests as serial, because it's extremely harmful to
the test system when run in parallel mode.
Andrii Nakryiko (4):
selftests/bpf: normalize selftest entry points
selftests/bpf: support multiple tests per file
selftests/bpf: mark tc_redirect selftest as serial
selftests/bpf: split out bpf_verif_scale selftests into multiple tests
tools/testing/selftests/bpf/Makefile | 7 +-
.../bpf/prog_tests/bpf_verif_scale.c | 220 ++++++++++++------
.../selftests/bpf/prog_tests/btf_dump.c | 2 +-
.../selftests/bpf/prog_tests/resolve_btfids.c | 10 +-
.../selftests/bpf/prog_tests/signal_pending.c | 2 +-
.../selftests/bpf/prog_tests/snprintf.c | 4 +-
.../selftests/bpf/prog_tests/tc_redirect.c | 2 +-
.../bpf/prog_tests/xdp_adjust_tail.c | 6 +-
.../bpf/prog_tests/xdp_devmap_attach.c | 4 +-
9 files changed, 169 insertions(+), 88 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH bpf-next 1/4] selftests/bpf: normalize selftest entry points 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko @ 2021-10-22 22:32 ` Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file Andrii Nakryiko ` (3 subsequent siblings) 4 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-22 22:32 UTC (permalink / raw) To: bpf, ast, daniel; +Cc: andrii, kernel-team Ensure that all test entry points are global void functions with no input arguments. Mark few subtest entry points as static. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 +- .../testing/selftests/bpf/prog_tests/resolve_btfids.c | 10 ++++------ .../testing/selftests/bpf/prog_tests/signal_pending.c | 2 +- tools/testing/selftests/bpf/prog_tests/snprintf.c | 4 ++-- .../testing/selftests/bpf/prog_tests/xdp_adjust_tail.c | 6 +++--- .../selftests/bpf/prog_tests/xdp_devmap_attach.c | 4 ++-- 6 files changed, 13 insertions(+), 15 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c index 12f457b6786d..d8961054c05b 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c @@ -133,7 +133,7 @@ static char *dump_buf; static size_t dump_buf_sz; static FILE *dump_buf_file; -void test_btf_dump_incremental(void) +static void test_btf_dump_incremental(void) { struct btf *btf = NULL; struct btf_dump *d = NULL; diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c index f62361306f6d..c7cb25334d78 100644 --- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c @@ -117,14 +117,14 @@ static int resolve_symbols(void) return 0; } -int test_resolve_btfids(void) +void test_resolve_btfids(void) { __u32 *test_list, *test_lists[] = { test_list_local, test_list_global }; unsigned int i, j; int ret = 0; if (resolve_symbols()) - return -1; + return; /* Check BTF_ID_LIST(test_list_local) and * BTF_ID_LIST_GLOBAL(test_list_global) IDs @@ -138,7 +138,7 @@ int test_resolve_btfids(void) test_symbols[i].name, test_list[i], test_symbols[i].id); if (ret) - return ret; + return; } } @@ -161,9 +161,7 @@ int test_resolve_btfids(void) if (i > 0) { if (!ASSERT_LE(test_set.ids[i - 1], test_set.ids[i], "sort_check")) - return -1; + return; } } - - return ret; } diff --git a/tools/testing/selftests/bpf/prog_tests/signal_pending.c b/tools/testing/selftests/bpf/prog_tests/signal_pending.c index dfcbddcbe4d3..fdfdcff6cbef 100644 --- a/tools/testing/selftests/bpf/prog_tests/signal_pending.c +++ b/tools/testing/selftests/bpf/prog_tests/signal_pending.c @@ -42,7 +42,7 @@ static void test_signal_pending_by_type(enum bpf_prog_type prog_type) signal(SIGALRM, SIG_DFL); } -void test_signal_pending(enum bpf_prog_type prog_type) +void test_signal_pending(void) { test_signal_pending_by_type(BPF_PROG_TYPE_SOCKET_FILTER); test_signal_pending_by_type(BPF_PROG_TYPE_FLOW_DISSECTOR); diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c b/tools/testing/selftests/bpf/prog_tests/snprintf.c index 8fd1b4b29a0e..394ebfc3bbf3 100644 --- a/tools/testing/selftests/bpf/prog_tests/snprintf.c +++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c @@ -33,7 +33,7 @@ #define EXP_NO_BUF_RET 29 -void test_snprintf_positive(void) +static void test_snprintf_positive(void) { char exp_addr_out[] = EXP_ADDR_OUT; char exp_sym_out[] = EXP_SYM_OUT; @@ -103,7 +103,7 @@ static int load_single_snprintf(char *fmt) return ret; } -void test_snprintf_negative(void) +static void test_snprintf_negative(void) { ASSERT_OK(load_single_snprintf("valid %d"), "valid usage"); diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c index d5c98f2cb12f..f529e3c923ae 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c @@ -2,7 +2,7 @@ #include <test_progs.h> #include <network_helpers.h> -void test_xdp_adjust_tail_shrink(void) +static void test_xdp_adjust_tail_shrink(void) { const char *file = "./test_xdp_adjust_tail_shrink.o"; __u32 duration, retval, size, expect_sz; @@ -30,7 +30,7 @@ void test_xdp_adjust_tail_shrink(void) bpf_object__close(obj); } -void test_xdp_adjust_tail_grow(void) +static void test_xdp_adjust_tail_grow(void) { const char *file = "./test_xdp_adjust_tail_grow.o"; struct bpf_object *obj; @@ -58,7 +58,7 @@ void test_xdp_adjust_tail_grow(void) bpf_object__close(obj); } -void test_xdp_adjust_tail_grow2(void) +static void test_xdp_adjust_tail_grow2(void) { const char *file = "./test_xdp_adjust_tail_grow.o"; char buf[4096]; /* avoid segfault: large buf to hold grow results */ diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c index d4e9a9972a67..3079d5568f8f 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c @@ -8,7 +8,7 @@ #define IFINDEX_LO 1 -void test_xdp_with_devmap_helpers(void) +static void test_xdp_with_devmap_helpers(void) { struct test_xdp_with_devmap_helpers *skel; struct bpf_prog_info info = {}; @@ -60,7 +60,7 @@ void test_xdp_with_devmap_helpers(void) test_xdp_with_devmap_helpers__destroy(skel); } -void test_neg_xdp_devmap_helpers(void) +static void test_neg_xdp_devmap_helpers(void) { struct test_xdp_devmap_helpers *skel; -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 1/4] selftests/bpf: normalize selftest entry points Andrii Nakryiko @ 2021-10-22 22:32 ` Andrii Nakryiko 2021-10-25 20:12 ` sunyucong 2021-10-22 22:32 ` [PATCH bpf-next 3/4] selftests/bpf: mark tc_redirect selftest as serial Andrii Nakryiko ` (2 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-22 22:32 UTC (permalink / raw) To: bpf, ast, daniel; +Cc: andrii, kernel-team Revamp how test discovery works for test_progs and allow multiple test entries per file. Any global void function with no arguments and serial_test_ or test_ prefix is considered a test. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/testing/selftests/bpf/Makefile | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 498222543c37..ac47cf9760fc 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) $(TRUNNER_TESTS_DIR)-tests-hdr := y $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@) - $$(shell ( cd $(TRUNNER_TESTS_DIR); \ - echo '/* Generated header, do not edit */'; \ - ls *.c 2> /dev/null | \ - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ + $$(shell (echo '/* Generated header, do not edit */'; \ + sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \ + $(TRUNNER_TESTS_DIR)/*.c | sort ; \ ) > $$@) endif -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file 2021-10-22 22:32 ` [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file Andrii Nakryiko @ 2021-10-25 20:12 ` sunyucong 2021-10-25 20:39 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: sunyucong @ 2021-10-25 20:12 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > Revamp how test discovery works for test_progs and allow multiple test > entries per file. Any global void function with no arguments and > serial_test_ or test_ prefix is considered a test. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > tools/testing/selftests/bpf/Makefile | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 498222543c37..ac47cf9760fc 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > $(TRUNNER_TESTS_DIR)-tests-hdr := y > $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@) > - $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > - echo '/* Generated header, do not edit */'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > + $$(shell (echo '/* Generated header, do not edit */'; \ > + sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \ probably not that important : allow \s* before void and after void. Or, maybe we can just (?!static) instead of anchoring to line start. > + $(TRUNNER_TESTS_DIR)/*.c | sort ; \ to be super safe : maybe add a check here to ensure each file contains at least one test function. > ) > $$@) > endif > > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file 2021-10-25 20:12 ` sunyucong @ 2021-10-25 20:39 ` Andrii Nakryiko 2021-10-25 20:55 ` sunyucong 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-25 20:39 UTC (permalink / raw) To: sunyucong Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team On Mon, Oct 25, 2021 at 1:13 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > Revamp how test discovery works for test_progs and allow multiple test > > entries per file. Any global void function with no arguments and > > serial_test_ or test_ prefix is considered a test. > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > tools/testing/selftests/bpf/Makefile | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > index 498222543c37..ac47cf9760fc 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > > $(TRUNNER_TESTS_DIR)-tests-hdr := y > > $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > > $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@) > > - $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > > - echo '/* Generated header, do not edit */'; \ > > - ls *.c 2> /dev/null | \ > > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > > + $$(shell (echo '/* Generated header, do not edit */'; \ > > + sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \ > > probably not that important : allow \s* before void and after void. > Or, maybe we can just (?!static) instead of anchoring to line > start. Selftests source code is pretty strict with formatting, so I don't think we'll deviate from the strict `^void <name>` pattern (and we certainly don't want to deviate). So I didn't want to overcomplicate regexes unnecessarily. > > > + $(TRUNNER_TESTS_DIR)/*.c | sort ; \ > > to be super safe : maybe add a check here to ensure each file contains > at least one test function. It's actually a useful property to have .c files that don't have tests. This can be used for adding various shared helpers. Currently all *_helpers.c are in selftests/bpf/ directory and have to be explicitly wired in Makefile, which is a bit annoying. With this setup we can just put a new .c file in the selftests/bpf/prog_tests/ and it will be automatically compiled and linked. It also will significantly hurt readability to add some sort of per-file check in there, do you think it's worth it? > > > ) > $$@) > > endif > > > > -- > > 2.30.2 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file 2021-10-25 20:39 ` Andrii Nakryiko @ 2021-10-25 20:55 ` sunyucong 2021-10-25 21:09 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: sunyucong @ 2021-10-25 20:55 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team On Mon, Oct 25, 2021 at 1:39 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Oct 25, 2021 at 1:13 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > > > On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > > > Revamp how test discovery works for test_progs and allow multiple test > > > entries per file. Any global void function with no arguments and > > > serial_test_ or test_ prefix is considered a test. > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > --- > > > tools/testing/selftests/bpf/Makefile | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > index 498222543c37..ac47cf9760fc 100644 > > > --- a/tools/testing/selftests/bpf/Makefile > > > +++ b/tools/testing/selftests/bpf/Makefile > > > @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > > > $(TRUNNER_TESTS_DIR)-tests-hdr := y > > > $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > > > $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@) > > > - $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > > > - echo '/* Generated header, do not edit */'; \ > > > - ls *.c 2> /dev/null | \ > > > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > > > + $$(shell (echo '/* Generated header, do not edit */'; \ > > > + sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \ > > > > probably not that important : allow \s* before void and after void. > > Or, maybe we can just (?!static) instead of anchoring to line > > start. > > Selftests source code is pretty strict with formatting, so I don't > think we'll deviate from the strict `^void <name>` pattern (and we > certainly don't want to deviate). So I didn't want to overcomplicate > regexes unnecessarily. > > > > > > + $(TRUNNER_TESTS_DIR)/*.c | sort ; \ > > > > to be super safe : maybe add a check here to ensure each file contains > > at least one test function. > > It's actually a useful property to have .c files that don't have > tests. This can be used for adding various shared helpers. Currently > all *_helpers.c are in selftests/bpf/ directory and have to be > explicitly wired in Makefile, which is a bit annoying. With this setup > we can just put a new .c file in the selftests/bpf/prog_tests/ and it > will be automatically compiled and linked. > > It also will significantly hurt readability to add some sort of > per-file check in there, do you think it's worth it? You are right, probably not really worth it. we just have to watch the total test numbers, it should always goes up :-D > > > > > > ) > $$@) > > > endif > > > > > > -- > > > 2.30.2 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file 2021-10-25 20:55 ` sunyucong @ 2021-10-25 21:09 ` Andrii Nakryiko 0 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-25 21:09 UTC (permalink / raw) To: sunyucong Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team On Mon, Oct 25, 2021 at 1:56 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > On Mon, Oct 25, 2021 at 1:39 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Mon, Oct 25, 2021 at 1:13 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > > > > > On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > > > > > Revamp how test discovery works for test_progs and allow multiple test > > > > entries per file. Any global void function with no arguments and > > > > serial_test_ or test_ prefix is considered a test. > > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > > --- > > > > tools/testing/selftests/bpf/Makefile | 7 +++---- > > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > > index 498222543c37..ac47cf9760fc 100644 > > > > --- a/tools/testing/selftests/bpf/Makefile > > > > +++ b/tools/testing/selftests/bpf/Makefile > > > > @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > > > > $(TRUNNER_TESTS_DIR)-tests-hdr := y > > > > $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > > > > $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@) > > > > - $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > > > > - echo '/* Generated header, do not edit */'; \ > > > > - ls *.c 2> /dev/null | \ > > > > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > > > > + $$(shell (echo '/* Generated header, do not edit */'; \ > > > > + sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \ > > > > > > probably not that important : allow \s* before void and after void. > > > Or, maybe we can just (?!static) instead of anchoring to line > > > start. > > > > Selftests source code is pretty strict with formatting, so I don't > > think we'll deviate from the strict `^void <name>` pattern (and we > > certainly don't want to deviate). So I didn't want to overcomplicate > > regexes unnecessarily. > > > > > > > > > + $(TRUNNER_TESTS_DIR)/*.c | sort ; \ > > > > > > to be super safe : maybe add a check here to ensure each file contains > > > at least one test function. > > > > It's actually a useful property to have .c files that don't have > > tests. This can be used for adding various shared helpers. Currently > > all *_helpers.c are in selftests/bpf/ directory and have to be > > explicitly wired in Makefile, which is a bit annoying. With this setup > > we can just put a new .c file in the selftests/bpf/prog_tests/ and it > > will be automatically compiled and linked. > > > > It also will significantly hurt readability to add some sort of > > per-file check in there, do you think it's worth it? > > You are right, probably not really worth it. we just have to watch the > total test numbers, it should always goes up :-D Yep. We should be able to automate this once we have some sort of baseline comparison functionality in BPF CI. > > > > > > > > > > ) > $$@) > > > > endif > > > > > > > > -- > > > > 2.30.2 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 3/4] selftests/bpf: mark tc_redirect selftest as serial 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 1/4] selftests/bpf: normalize selftest entry points Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file Andrii Nakryiko @ 2021-10-22 22:32 ` Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 4/4] selftests/bpf: split out bpf_verif_scale selftests into multiple tests Andrii Nakryiko 2021-10-25 20:15 ` [PATCH bpf-next 0/4] Parallelize verif_scale selftests sunyucong 4 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-22 22:32 UTC (permalink / raw) To: bpf, ast, daniel; +Cc: andrii, kernel-team, Jussi Maki It seems to cause a lot of harm to kprobe/tracepoint selftests. Yucong mentioned before that it does manipulate sysfs, which might be the reason. So let's mark it as serial, though ideally it would be less intrusive on the system at test. Cc: Jussi Maki <joamaki@gmail.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/testing/selftests/bpf/prog_tests/tc_redirect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c index e87bc4466d9a..53672634bc52 100644 --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c @@ -769,7 +769,7 @@ static void *test_tc_redirect_run_tests(void *arg) return NULL; } -void test_tc_redirect(void) +void serial_test_tc_redirect(void) { pthread_t test_thread; int err; -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 4/4] selftests/bpf: split out bpf_verif_scale selftests into multiple tests 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko ` (2 preceding siblings ...) 2021-10-22 22:32 ` [PATCH bpf-next 3/4] selftests/bpf: mark tc_redirect selftest as serial Andrii Nakryiko @ 2021-10-22 22:32 ` Andrii Nakryiko 2021-10-25 20:15 ` [PATCH bpf-next 0/4] Parallelize verif_scale selftests sunyucong 4 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-22 22:32 UTC (permalink / raw) To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yucong Sun Instead of using subtests in bpf_verif_scale selftest, turn each scale sub-test into its own test. Each subtest is compltely independent and just reuses a bit of common test running logic, so the conversion is trivial. For convenience, keep all of BPF verifier scale tests in one file. This conversion shaves off a significant amount of time when running test_progs in parallel mode. E.g., just running scale tests (-t verif_scale): BEFORE ====== Summary: 24/0 PASSED, 0 SKIPPED, 0 FAILED real 0m22.894s user 0m0.012s sys 0m22.797s AFTER ===== Summary: 24/0 PASSED, 0 SKIPPED, 0 FAILED real 0m12.044s user 0m0.024s sys 0m27.869s Ten second saving right there. test_progs -j is not yet ready to be turned on by default, unfortunately, and some tests fail almost every time, but this is a good improvement nevertheless. Ignoring few failures, here is sequential vs parallel run times when running all tests now: SEQUENTIAL ========== Summary: 206/953 PASSED, 4 SKIPPED, 0 FAILED real 1m5.625s user 0m4.211s sys 0m31.650s PARALLEL ======== Summary: 204/952 PASSED, 4 SKIPPED, 2 FAILED real 0m35.550s user 0m4.998s sys 0m39.890s Cc: Yucong Sun <sunyucong@gmail.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- .../bpf/prog_tests/bpf_verif_scale.c | 220 ++++++++++++------ 1 file changed, 152 insertions(+), 68 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c index 3d002c245d2b..867349e4ed9e 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c @@ -39,82 +39,166 @@ struct scale_test_def { bool fails; }; -void test_bpf_verif_scale(void) -{ - struct scale_test_def tests[] = { - { "loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT, true /* fails */ }, - - { "test_verif_scale1.o", BPF_PROG_TYPE_SCHED_CLS }, - { "test_verif_scale2.o", BPF_PROG_TYPE_SCHED_CLS }, - { "test_verif_scale3.o", BPF_PROG_TYPE_SCHED_CLS }, - - { "pyperf_global.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "pyperf_subprogs.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - /* full unroll by llvm */ - { "pyperf50.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "pyperf100.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "pyperf180.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - /* partial unroll. llvm will unroll loop ~150 times. - * C loop count -> 600. - * Asm loop count -> 4. - * 16k insns in loop body. - * Total of 5 such loops. Total program size ~82k insns. - */ - { "pyperf600.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - /* no unroll at all. - * C loop count -> 600. - * ASM loop count -> 600. - * ~110 insns in loop body. - * Total of 5 such loops. Total program size ~1500 insns. - */ - { "pyperf600_nounroll.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - { "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "loop4.o", BPF_PROG_TYPE_SCHED_CLS }, - { "loop5.o", BPF_PROG_TYPE_SCHED_CLS }, - { "loop6.o", BPF_PROG_TYPE_KPROBE }, - - /* partial unroll. 19k insn in a loop. - * Total program size 20.8k insn. - * ~350k processed_insns - */ - { "strobemeta.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - /* no unroll, tiny loops */ - { "strobemeta_nounroll1.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - { "strobemeta_nounroll2.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - /* non-inlined subprogs */ - { "strobemeta_subprogs.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, - - { "test_sysctl_loop1.o", BPF_PROG_TYPE_CGROUP_SYSCTL }, - { "test_sysctl_loop2.o", BPF_PROG_TYPE_CGROUP_SYSCTL }, - - { "test_xdp_loop.o", BPF_PROG_TYPE_XDP }, - { "test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL }, - }; +static void scale_test(const char *file, + enum bpf_prog_type attach_type, + bool should_fail) +{ libbpf_print_fn_t old_print_fn = NULL; - int err, i; + int err; if (env.verifier_stats) { test__force_log(); old_print_fn = libbpf_set_print(libbpf_debug_print); } - for (i = 0; i < ARRAY_SIZE(tests); i++) { - const struct scale_test_def *test = &tests[i]; - - if (!test__start_subtest(test->file)) - continue; - - err = check_load(test->file, test->attach_type); - CHECK_FAIL(err && !test->fails); - } + err = check_load(file, attach_type); + if (should_fail) + ASSERT_ERR(err, "expect_error"); + else + ASSERT_OK(err, "expect_success"); if (env.verifier_stats) libbpf_set_print(old_print_fn); } + +void test_verif_scale1() +{ + scale_test("test_verif_scale1.o", BPF_PROG_TYPE_SCHED_CLS, false); +} + +void test_verif_scale2() +{ + scale_test("test_verif_scale2.o", BPF_PROG_TYPE_SCHED_CLS, false); +} + +void test_verif_scale3() +{ + scale_test("test_verif_scale3.o", BPF_PROG_TYPE_SCHED_CLS, false); +} + +void test_verif_scale_pyperf_global() +{ + scale_test("pyperf_global.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf_subprogs() +{ + scale_test("pyperf_subprogs.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf50() +{ + /* full unroll by llvm */ + scale_test("pyperf50.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf100() +{ + /* full unroll by llvm */ + scale_test("pyperf100.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf180() +{ + /* full unroll by llvm */ + scale_test("pyperf180.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf600() +{ + /* partial unroll. llvm will unroll loop ~150 times. + * C loop count -> 600. + * Asm loop count -> 4. + * 16k insns in loop body. + * Total of 5 such loops. Total program size ~82k insns. + */ + scale_test("pyperf600.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_pyperf600_nounroll() +{ + /* no unroll at all. + * C loop count -> 600. + * ASM loop count -> 600. + * ~110 insns in loop body. + * Total of 5 such loops. Total program size ~1500 insns. + */ + scale_test("pyperf600_nounroll.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_loop1() +{ + scale_test("loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_loop2() +{ + scale_test("loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_loop3_fail() +{ + scale_test("loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT, true /* fails */); +} + +void test_verif_scale_loop4() +{ + scale_test("loop4.o", BPF_PROG_TYPE_SCHED_CLS, false); +} + +void test_verif_scale_loop5() +{ + scale_test("loop5.o", BPF_PROG_TYPE_SCHED_CLS, false); +} + +void test_verif_scale_loop6() +{ + scale_test("loop6.o", BPF_PROG_TYPE_KPROBE, false); +} + +void test_verif_scale_strobemeta() +{ + /* partial unroll. 19k insn in a loop. + * Total program size 20.8k insn. + * ~350k processed_insns + */ + scale_test("strobemeta.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_strobemeta_nounroll1() +{ + /* no unroll, tiny loops */ + scale_test("strobemeta_nounroll1.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_strobemeta_nounroll2() +{ + /* no unroll, tiny loops */ + scale_test("strobemeta_nounroll2.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_strobemeta_subprogs() +{ + /* non-inlined subprogs */ + scale_test("strobemeta_subprogs.o", BPF_PROG_TYPE_RAW_TRACEPOINT, false); +} + +void test_verif_scale_sysctl_loop1() +{ + scale_test("test_sysctl_loop1.o", BPF_PROG_TYPE_CGROUP_SYSCTL, false); +} + +void test_verif_scale_sysctl_loop2() +{ + scale_test("test_sysctl_loop2.o", BPF_PROG_TYPE_CGROUP_SYSCTL, false); +} + +void test_verif_scale_xdp_loop() +{ + scale_test("test_xdp_loop.o", BPF_PROG_TYPE_XDP, false); +} + +void test_verif_scale_seg6_loop() +{ + scale_test("test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL, false); +} -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 0/4] Parallelize verif_scale selftests 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko ` (3 preceding siblings ...) 2021-10-22 22:32 ` [PATCH bpf-next 4/4] selftests/bpf: split out bpf_verif_scale selftests into multiple tests Andrii Nakryiko @ 2021-10-25 20:15 ` sunyucong 2021-10-26 1:12 ` Alexei Starovoitov 4 siblings, 1 reply; 11+ messages in thread From: sunyucong @ 2021-10-25 20:15 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team Thanks, this patch is awesome! Acked-by: Yucong Sun <sunyucong@gmail.com> On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > Reduce amount of waiting time when running test_progs in parallel mode (-j) by > splitting bpf_verif_scale selftests into multiple tests. Previously it was > structured as a test with multiple subtests, but subtests are not easily > parallelizable with test_progs' infra. Also in practice each scale subtest is > really an independent test with nothing shared across all substest. > > This patch set changes how test_progs test discovery works. Now it is possible > to define multiple tests within a single source code file. One of the patches > also marks tc_redirect selftests as serial, because it's extremely harmful to > the test system when run in parallel mode. > > Andrii Nakryiko (4): > selftests/bpf: normalize selftest entry points > selftests/bpf: support multiple tests per file > selftests/bpf: mark tc_redirect selftest as serial > selftests/bpf: split out bpf_verif_scale selftests into multiple tests > > tools/testing/selftests/bpf/Makefile | 7 +- > .../bpf/prog_tests/bpf_verif_scale.c | 220 ++++++++++++------ > .../selftests/bpf/prog_tests/btf_dump.c | 2 +- > .../selftests/bpf/prog_tests/resolve_btfids.c | 10 +- > .../selftests/bpf/prog_tests/signal_pending.c | 2 +- > .../selftests/bpf/prog_tests/snprintf.c | 4 +- > .../selftests/bpf/prog_tests/tc_redirect.c | 2 +- > .../bpf/prog_tests/xdp_adjust_tail.c | 6 +- > .../bpf/prog_tests/xdp_devmap_attach.c | 4 +- > 9 files changed, 169 insertions(+), 88 deletions(-) > > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 0/4] Parallelize verif_scale selftests 2021-10-25 20:15 ` [PATCH bpf-next 0/4] Parallelize verif_scale selftests sunyucong @ 2021-10-26 1:12 ` Alexei Starovoitov 0 siblings, 0 replies; 11+ messages in thread From: Alexei Starovoitov @ 2021-10-26 1:12 UTC (permalink / raw) To: sunyucong Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team On Mon, Oct 25, 2021 at 1:15 PM sunyucong@gmail.com <sunyucong@gmail.com> wrote: > > Thanks, this patch is awesome! > > Acked-by: Yucong Sun <sunyucong@gmail.com> > > > On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > Reduce amount of waiting time when running test_progs in parallel mode (-j) by > > splitting bpf_verif_scale selftests into multiple tests. Previously it was > > structured as a test with multiple subtests, but subtests are not easily > > parallelizable with test_progs' infra. Also in practice each scale subtest is > > really an independent test with nothing shared across all substest. > > > > This patch set changes how test_progs test discovery works. Now it is possible > > to define multiple tests within a single source code file. One of the patches > > also marks tc_redirect selftests as serial, because it's extremely harmful to > > the test system when run in parallel mode. The patch set was applied. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-26 1:12 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-22 22:32 [PATCH bpf-next 0/4] Parallelize verif_scale selftests Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 1/4] selftests/bpf: normalize selftest entry points Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file Andrii Nakryiko 2021-10-25 20:12 ` sunyucong 2021-10-25 20:39 ` Andrii Nakryiko 2021-10-25 20:55 ` sunyucong 2021-10-25 21:09 ` Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 3/4] selftests/bpf: mark tc_redirect selftest as serial Andrii Nakryiko 2021-10-22 22:32 ` [PATCH bpf-next 4/4] selftests/bpf: split out bpf_verif_scale selftests into multiple tests Andrii Nakryiko 2021-10-25 20:15 ` [PATCH bpf-next 0/4] Parallelize verif_scale selftests sunyucong 2021-10-26 1:12 ` 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).