| Message ID | 20210817044732.3263066-5-fallentree@fb.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | selftests/bpf: Improve the usability of test_progs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 10 maintainers not CCed: songliubraving@fb.com shuah@kernel.org daniel@iogearbox.net kafai@fb.com netdev@vger.kernel.org ast@kernel.org john.fastabend@gmail.com linux-kselftest@vger.kernel.org kpsingh@kernel.org yhs@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 83 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Aug 16, 2021 at 9:47 PM Yucong Sun <fallentree@fb.com> wrote: > > This patch adds '-a' and '-d' arguments, support exact string match, as well as > using '*' wildcard in test/subtests selection. The old '-t' '-b' arguments > still supports partial string match, but they can't be used together yet. > > This patach also adds support for mulitple '-a' '-d' '-t' '-b' arguments. > > 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 <fallentree@fb.com> > --- > tools/testing/selftests/bpf/test_progs.c | 72 +++++++++++++++++++----- > 1 file changed, 58 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 90539b15b744..c34eb818f115 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -13,6 +13,28 @@ > #include <execinfo.h> /* backtrace */ > #include <linux/membarrier.h> > > +/* 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 (support *)." }, > + { "deny", ARG_TEST_NAME_GLOB_DENYLIST, "NAMES", 0, > + "Don't run tests with name matching the pattern (support *)." }, > {}, > }; > > @@ -491,7 +519,7 @@ 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; > @@ -500,28 +528,38 @@ static int parse_str_list(const char *s, struct str_set *set) > 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; > + if (is_glob_pattern) { > + strs[cnt] = strdup(next); > + } else { > + strs[cnt] = malloc(strlen(next) + 2 + 1); > + if (!strs[cnt]) > + goto err; > + sprintf(strs[cnt], "*%s*", next); > + } > > - strs[cnt] = strdup(next); > if (!strs[cnt]) > goto err; > > 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: > - free(strs); > + if (strs) > + free(strs); free(NULL) is noop, so no need to check if(strs) You also missed the need to free all those strdup()'ed strings, so I added for (i = 0; i < cnt; i++) free(strs[i]); We didn't need it originally, because eventually we call free_str_set(), but in this case those strings will be leaked, because we don't assign strs to set->strs. BTW, a bit cleaner approach would be to just work on set->strs directly, and realloc()ing it as necessary, and only at the end updating set->cnt. You wouldn't need to memcpy and realloc one extra time at the end, and no need for that for + free loop. But it's fine the way it is as well. Applied to bpf-next, thanks a lot for the improvements. > free(input); > return -ENOMEM; > } > @@ -553,29 +591,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; > } > -- > 2.30.2 >
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 90539b15b744..c34eb818f115 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -13,6 +13,28 @@ #include <execinfo.h> /* backtrace */ #include <linux/membarrier.h> +/* 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 (support *)." }, + { "deny", ARG_TEST_NAME_GLOB_DENYLIST, "NAMES", 0, + "Don't run tests with name matching the pattern (support *)." }, {}, }; @@ -491,7 +519,7 @@ 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; @@ -500,28 +528,38 @@ static int parse_str_list(const char *s, struct str_set *set) 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; + if (is_glob_pattern) { + strs[cnt] = strdup(next); + } else { + strs[cnt] = malloc(strlen(next) + 2 + 1); + if (!strs[cnt]) + goto err; + sprintf(strs[cnt], "*%s*", next); + } - strs[cnt] = strdup(next); if (!strs[cnt]) goto err; 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: - free(strs); + if (strs) + free(strs); free(input); return -ENOMEM; } @@ -553,29 +591,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; }
This patch adds '-a' and '-d' arguments, support exact string match, as well as using '*' wildcard in test/subtests selection. The old '-t' '-b' arguments still supports partial string match, but they can't be used together yet. This patach also adds support for mulitple '-a' '-d' '-t' '-b' arguments. 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 <fallentree@fb.com> --- tools/testing/selftests/bpf/test_progs.c | 72 +++++++++++++++++++----- 1 file changed, 58 insertions(+), 14 deletions(-)