From 26d82640d5ba2c3b32d79597be2dcf820ed78b16 Mon Sep 17 00:00:00 2001 From: Yucong Sun Date: Mon, 16 Aug 2021 21:47:29 -0700 Subject: [PATCH 1/4] selftests/bpf: Skip loading bpf_testmod when using -l to list tests. When using "-l", test_progs often is executed as non-root user, load_bpf_testmod() will fail and output errors. This patch skips loading bpf testmod when "-l" is specified, making output cleaner. Signed-off-by: Yucong Sun Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20210817044732.3263066-2-fallentree@fb.com --- tools/testing/selftests/bpf/test_progs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 6f103106a39b..532af3353edf 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -755,7 +755,7 @@ int main(int argc, char **argv) save_netns(); stdio_hijack(); env.has_testmod = true; - if (load_bpf_testmod()) { + if (!env.list_test_names && load_bpf_testmod()) { fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n"); env.has_testmod = false; } @@ -803,7 +803,7 @@ int main(int argc, char **argv) if (test->need_cgroup_cleanup) cleanup_cgroup_environment(); } - if (env.has_testmod) + if (!env.list_test_names && env.has_testmod) unload_bpf_testmod(); stdio_restore(); From f667d1d66760fcb27aee6c9964eefde39a464afe Mon Sep 17 00:00:00 2001 From: Yucong Sun Date: Mon, 16 Aug 2021 21:47:30 -0700 Subject: [PATCH 2/4] selftests/bpf: Correctly display subtest skip status In skip_account(), test->skip_cnt is set to 0 at the end, this makes next print statement never display SKIP status for the subtest. This patch moves the accounting logic after the print statement, fixing the issue. This patch also added SKIP status display for normal tests. Signed-off-by: Yucong Sun Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20210817044732.3263066-3-fallentree@fb.com --- tools/testing/selftests/bpf/test_progs.c | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 532af3353edf..f0fbead40883 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -148,18 +148,18 @@ void test__end_subtest() struct prog_test_def *test = env.test; int sub_error_cnt = test->error_cnt - test->old_error_cnt; - if (sub_error_cnt) - env.fail_cnt++; - else if (test->skip_cnt == 0) - env.sub_succ_cnt++; - skip_account(); - dump_test_log(test, sub_error_cnt); fprintf(env.stdout, "#%d/%d %s:%s\n", test->test_num, test->subtest_num, test->subtest_name, sub_error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK")); + if (sub_error_cnt) + env.fail_cnt++; + else if (test->skip_cnt == 0) + env.sub_succ_cnt++; + skip_account(); + free(test->subtest_name); test->subtest_name = NULL; } @@ -786,17 +786,18 @@ int main(int argc, char **argv) test__end_subtest(); test->tested = true; - if (test->error_cnt) - env.fail_cnt++; - else - env.succ_cnt++; - skip_account(); dump_test_log(test, test->error_cnt); fprintf(env.stdout, "#%d %s:%s\n", test->test_num, test->test_name, - test->error_cnt ? "FAIL" : "OK"); + test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK")); + + if (test->error_cnt) + env.fail_cnt++; + else + env.succ_cnt++; + skip_account(); reset_affinity(); restore_netns(); From 99c4fd8b92b3dc6db1afa0e252d3054d501a03ca Mon Sep 17 00:00:00 2001 From: Yucong Sun Date: Mon, 16 Aug 2021 21:47:31 -0700 Subject: [PATCH 3/4] selftests/bpf: Also print test name in subtest status message This patch add test name in subtest status message line, making it possible to grep ':OK' in the output to generate a list of passed test+subtest names, which can be processed to generate argument list to be used with "-a", "-d" exact string matching. Example: #1/1 align/mov:OK .. #1/12 align/pointer variable subtraction:OK #1 align:OK Signed-off-by: Yucong Sun Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20210817044732.3263066-4-fallentree@fb.com --- tools/testing/selftests/bpf/test_progs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index f0fbead40883..90539b15b744 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -150,8 +150,8 @@ void test__end_subtest() dump_test_log(test, sub_error_cnt); - fprintf(env.stdout, "#%d/%d %s:%s\n", - test->test_num, test->subtest_num, test->subtest_name, + fprintf(env.stdout, "#%d/%d %s/%s:%s\n", + test->test_num, test->subtest_num, test->test_name, test->subtest_name, sub_error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK")); if (sub_error_cnt) From 74339a8f866cdcca3f701c859b43b538890d905b Mon Sep 17 00:00:00 2001 From: Yucong Sun Date: Mon, 16 Aug 2021 21:47:32 -0700 Subject: [PATCH 4/4] selftests/bpf: Support glob matching for test selector. This patch adds '-a' and '-d' arguments supporting both exact string match as well as using '*' wildcard in test/subtests selection. '-a' and '-t' can co-exists, same as '-d' and '-b', in which case they just add to the list of allowed or denied test selectors. Caveat: Same as the current substring matching mechanism, test and subtest selector applies independently, 'a*/b*' will execute all tests matching "a*", and with subtest name matching "b*", but tests matching "a*" that has no subtests will also be executed. Signed-off-by: Yucong Sun Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20210817044732.3263066-5-fallentree@fb.com --- tools/testing/selftests/bpf/test_progs.c | 78 +++++++++++++++++++----- 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 90539b15b744..cc1cd240445d 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -13,6 +13,28 @@ #include /* backtrace */ #include +/* Adapted from perf/util/string.c */ +static bool glob_match(const char *str, const char *pat) +{ + while (*str && *pat && *pat != '*') { + if (*str != *pat) + return false; + str++; + pat++; + } + /* Check wild card */ + if (*pat == '*') { + while (*pat == '*') + pat++; + if (!*pat) /* Tail wild card matches all */ + return true; + while (*str) + if (glob_match(str++, pat)) + return true; + } + return !*str && !*pat; +} + #define EXIT_NO_TEST 2 #define EXIT_ERR_SETUP_INFRA 3 @@ -55,12 +77,12 @@ static bool should_run(struct test_selector *sel, int num, const char *name) int i; for (i = 0; i < sel->blacklist.cnt; i++) { - if (strstr(name, sel->blacklist.strs[i])) + if (glob_match(name, sel->blacklist.strs[i])) return false; } for (i = 0; i < sel->whitelist.cnt; i++) { - if (strstr(name, sel->whitelist.strs[i])) + if (glob_match(name, sel->whitelist.strs[i])) return true; } @@ -450,6 +472,8 @@ enum ARG_KEYS { ARG_VERBOSE = 'v', ARG_GET_TEST_CNT = 'c', ARG_LIST_TEST_NAMES = 'l', + ARG_TEST_NAME_GLOB_ALLOWLIST = 'a', + ARG_TEST_NAME_GLOB_DENYLIST = 'd', }; static const struct argp_option opts[] = { @@ -467,6 +491,10 @@ static const struct argp_option opts[] = { "Get number of selected top-level tests " }, { "list", ARG_LIST_TEST_NAMES, NULL, 0, "List test names that would run (without running them) " }, + { "allow", ARG_TEST_NAME_GLOB_ALLOWLIST, "NAMES", 0, + "Run tests with name matching the pattern (supports '*' wildcard)." }, + { "deny", ARG_TEST_NAME_GLOB_DENYLIST, "NAMES", 0, + "Don't run tests with name matching the pattern (supports '*' wildcard)." }, {}, }; @@ -491,36 +519,48 @@ static void free_str_set(const struct str_set *set) free(set->strs); } -static int parse_str_list(const char *s, struct str_set *set) +static int parse_str_list(const char *s, struct str_set *set, bool is_glob_pattern) { char *input, *state = NULL, *next, **tmp, **strs = NULL; - int cnt = 0; + int i, cnt = 0; input = strdup(s); if (!input) return -ENOMEM; - set->cnt = 0; - set->strs = NULL; - while ((next = strtok_r(state ? NULL : input, ",", &state))) { tmp = realloc(strs, sizeof(*strs) * (cnt + 1)); if (!tmp) goto err; strs = tmp; - strs[cnt] = strdup(next); - if (!strs[cnt]) - goto err; + if (is_glob_pattern) { + strs[cnt] = strdup(next); + if (!strs[cnt]) + goto err; + } else { + strs[cnt] = malloc(strlen(next) + 2 + 1); + if (!strs[cnt]) + goto err; + sprintf(strs[cnt], "*%s*", next); + } cnt++; } - set->cnt = cnt; - set->strs = (const char **)strs; + tmp = realloc(set->strs, sizeof(*strs) * (cnt + set->cnt)); + if (!tmp) + goto err; + memcpy(tmp + set->cnt, strs, sizeof(*strs) * cnt); + set->strs = (const char **)tmp; + set->cnt += cnt; + free(input); + free(strs); return 0; err: + for (i = 0; i < cnt; i++) + free(strs[i]); free(strs); free(input); return -ENOMEM; @@ -553,29 +593,35 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state) } break; } + case ARG_TEST_NAME_GLOB_ALLOWLIST: case ARG_TEST_NAME: { char *subtest_str = strchr(arg, '/'); if (subtest_str) { *subtest_str = '\0'; if (parse_str_list(subtest_str + 1, - &env->subtest_selector.whitelist)) + &env->subtest_selector.whitelist, + key == ARG_TEST_NAME_GLOB_ALLOWLIST)) return -ENOMEM; } - if (parse_str_list(arg, &env->test_selector.whitelist)) + if (parse_str_list(arg, &env->test_selector.whitelist, + key == ARG_TEST_NAME_GLOB_ALLOWLIST)) return -ENOMEM; break; } + case ARG_TEST_NAME_GLOB_DENYLIST: case ARG_TEST_NAME_BLACKLIST: { char *subtest_str = strchr(arg, '/'); if (subtest_str) { *subtest_str = '\0'; if (parse_str_list(subtest_str + 1, - &env->subtest_selector.blacklist)) + &env->subtest_selector.blacklist, + key == ARG_TEST_NAME_GLOB_DENYLIST)) return -ENOMEM; } - if (parse_str_list(arg, &env->test_selector.blacklist)) + if (parse_str_list(arg, &env->test_selector.blacklist, + key == ARG_TEST_NAME_GLOB_DENYLIST)) return -ENOMEM; break; }