| Message ID | 20220206043107.18549-2-houtao1@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | selftests: add test for kfunc call | expand |
On 2/5/22 8:31 PM, Hou Tao wrote: > Two subtests in ksyms_module.c are not qualified as static, so these > subtests are exported as standalone tests in tests.h and lead to > confusion for the output of "./test_progs -t ksyms_module". > > By using the following command: > > grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \ > tools/testing/selftests/bpf/prog_tests/*.c | \ > awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1' > > Find out that other tests also have the similar problem, so fix > these tests by marking subtests in these tests as static. For > xdp_adjust_frags.c, there is just one subtest, so just export > the subtest directly. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > tools/testing/selftests/bpf/prog_tests/ksyms_module.c | 4 ++-- > tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c | 6 ------ > tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c | 4 ++-- > tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c | 4 ++-- > tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 2 +- > 5 files changed, 7 insertions(+), 13 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c > index ecc58c9e7631..a1ebac70ec29 100644 > --- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c > +++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c > @@ -6,7 +6,7 @@ > #include "test_ksyms_module.lskel.h" > #include "test_ksyms_module.skel.h" > > -void test_ksyms_module_lskel(void) > +static void test_ksyms_module_lskel(void) > { > struct test_ksyms_module_lskel *skel; > int err; > @@ -33,7 +33,7 @@ void test_ksyms_module_lskel(void) > test_ksyms_module_lskel__destroy(skel); > } > > -void test_ksyms_module_libbpf(void) > +static void test_ksyms_module_libbpf(void) > { > struct test_ksyms_module *skel; > int err; > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c > index 134d0ac32f59..fc2d8fa8dac5 100644 > --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c > @@ -102,9 +102,3 @@ void test_xdp_update_frags(void) > out: > bpf_object__close(obj); > } > - > -void test_xdp_adjust_frags(void) > -{ > - if (test__start_subtest("xdp_adjust_frags")) > - test_xdp_update_frags(); > -} I suggest keep test_xdp_adjust_frags and mark test_xdp_update_frags as static function, and this is also good for future extension. It is confusing that test_xdp_update_frags test in file xdp_adjust_frags.c. Typical prog_tests/ test has {test,serial_test}_<TEST> test with file name <TEST>.c file. [...]
Hi, On 2/8/2022 2:17 AM, Yonghong Song wrote: > > > On 2/5/22 8:31 PM, Hou Tao wrote: >> Two subtests in ksyms_module.c are not qualified as static, so these >> subtests are exported as standalone tests in tests.h and lead to >> confusion for the output of "./test_progs -t ksyms_module". >> >> By using the following command: >> >> grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \ >> tools/testing/selftests/bpf/prog_tests/*.c | \ >> awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1' >> >> Find out that other tests also have the similar problem, so fix >> these tests by marking subtests in these tests as static. For >> xdp_adjust_frags.c, there is just one subtest, so just export >> the subtest directly. [...] >> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c >> b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c >> index 134d0ac32f59..fc2d8fa8dac5 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c >> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c >> @@ -102,9 +102,3 @@ void test_xdp_update_frags(void) >> out: >> bpf_object__close(obj); >> } >> - >> -void test_xdp_adjust_frags(void) >> -{ >> - if (test__start_subtest("xdp_adjust_frags")) >> - test_xdp_update_frags(); >> -} > > I suggest keep test_xdp_adjust_frags and mark > test_xdp_update_frags as static function, and > this is also good for future extension. > It is confusing that test_xdp_update_frags > test in file xdp_adjust_frags.c. Typical > prog_tests/ test has {test,serial_test}_<TEST> test > with file name <TEST>.c file. > Will do and thanks for your suggestion. Regards, Tao > [...] > .
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c index ecc58c9e7631..a1ebac70ec29 100644 --- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c +++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c @@ -6,7 +6,7 @@ #include "test_ksyms_module.lskel.h" #include "test_ksyms_module.skel.h" -void test_ksyms_module_lskel(void) +static void test_ksyms_module_lskel(void) { struct test_ksyms_module_lskel *skel; int err; @@ -33,7 +33,7 @@ void test_ksyms_module_lskel(void) test_ksyms_module_lskel__destroy(skel); } -void test_ksyms_module_libbpf(void) +static void test_ksyms_module_libbpf(void) { struct test_ksyms_module *skel; int err; diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c index 134d0ac32f59..fc2d8fa8dac5 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c @@ -102,9 +102,3 @@ void test_xdp_update_frags(void) out: bpf_object__close(obj); } - -void test_xdp_adjust_frags(void) -{ - if (test__start_subtest("xdp_adjust_frags")) - test_xdp_update_frags(); -} 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 528a8c387720..21ceac24e174 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c @@ -133,7 +133,7 @@ static void test_xdp_adjust_tail_grow2(void) bpf_object__close(obj); } -void test_xdp_adjust_frags_tail_shrink(void) +static void test_xdp_adjust_frags_tail_shrink(void) { const char *file = "./test_xdp_adjust_tail_shrink.o"; __u32 exp_size; @@ -200,7 +200,7 @@ void test_xdp_adjust_frags_tail_shrink(void) bpf_object__close(obj); } -void test_xdp_adjust_frags_tail_grow(void) +static void test_xdp_adjust_frags_tail_grow(void) { const char *file = "./test_xdp_adjust_tail_grow.o"; __u32 exp_size; diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c index b353e1f3acb5..f775a1613833 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c @@ -8,7 +8,7 @@ #define IFINDEX_LO 1 -void test_xdp_with_cpumap_helpers(void) +static void test_xdp_with_cpumap_helpers(void) { struct test_xdp_with_cpumap_helpers *skel; struct bpf_prog_info info = {}; @@ -68,7 +68,7 @@ void test_xdp_with_cpumap_helpers(void) test_xdp_with_cpumap_helpers__destroy(skel); } -void test_xdp_with_cpumap_frags_helpers(void) +static void test_xdp_with_cpumap_frags_helpers(void) { struct test_xdp_with_cpumap_frags_helpers *skel; struct bpf_prog_info info = {}; 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 463a72fc3e70..ead40016c324 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c @@ -81,7 +81,7 @@ static void test_neg_xdp_devmap_helpers(void) } } -void test_xdp_with_devmap_frags_helpers(void) +static void test_xdp_with_devmap_frags_helpers(void) { struct test_xdp_with_devmap_frags_helpers *skel; struct bpf_prog_info info = {};
Two subtests in ksyms_module.c are not qualified as static, so these subtests are exported as standalone tests in tests.h and lead to confusion for the output of "./test_progs -t ksyms_module". By using the following command: grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \ tools/testing/selftests/bpf/prog_tests/*.c | \ awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1' Find out that other tests also have the similar problem, so fix these tests by marking subtests in these tests as static. For xdp_adjust_frags.c, there is just one subtest, so just export the subtest directly. Signed-off-by: Hou Tao <houtao1@huawei.com> --- tools/testing/selftests/bpf/prog_tests/ksyms_module.c | 4 ++-- tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c | 6 ------ tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c | 4 ++-- tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c | 4 ++-- tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 2 +- 5 files changed, 7 insertions(+), 13 deletions(-)