| Message ID | 20210810001625.1140255-3-fallentree@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Improve the usability of test_progs | expand |
On Mon, Aug 9, 2021 at 5:17 PM Yucong Sun <fallentree@fb.com> wrote: > > This patch adds glob matching to test selector, it accepts > simple glob pattern "*", "?", "[]" to match the test names to run. do we really need ? and []? I've been pretty happy so far in retsnoop with supporting just these patterns: 1. exact match ('abc') 2. prefix match ('abc*') 3. suffix match ('*abc') 4. substring match ('*abc*') See [0] for a naive but simple implementation for that logic. So far with test_progs I've only needed two cases from the above: exact and substring matches. So I'm leaning towards keeping it simple, actually. But there is also an issue of backwards compatibility. People using `test_progs -t substr` are used to substring matching logic, so with this change you are breaking this, which will cause frustration, most probably. So maybe let's add a new parameter to specify these globs. E.g., maybe `-a <glob>` for whitelisting (allowlisting), and `-d <glob>` for blacklisting (denylisting)? Also, instead of parsing comma-separated lists as I did initially with -t, we should probably just allow multiple occurences of -a and -d: ./test_progs -a '*core*' -a '*linux' -d 'core_autosize' will allow all CO-RE tests except autosize one, plus will admit vmlinux selftest. Also, keep in mind that there are subtests within some tests, and those should be matches with the same logic as well: ./test_progs -a 'core_reloc/size*' should run: #32/58 size:OK #32/59 size___diff_sz:OK #32/60 size___err_ambiguous:OK It gets a bit trickier with globs for both test and subtest, e.g. '*core*/size*' -- should it match just core_reloc/size* tests as above? Or also core_retro, core_extern, etc tests even though they don't have subtests starting with 'size'? I'd say the latter is more desirable, but I haven't checked how hard that would be to support. [0] https://github.com/anakryiko/retsnoop/blob/2e6217f7a82f421fcf3481cc401390605066ab26/src/mass_attacher.c#L982-L1015 > > The glob matching function is copied from perf/util/string.c > > Signed-off-by: Yucong Sun <fallentree@fb.com> > --- > tools/testing/selftests/bpf/test_progs.c | 94 +++++++++++++++++++++++- > 1 file changed, 92 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 74dde0af1592..c5bffd2e78ae 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -13,6 +13,96 @@ > #include <execinfo.h> /* backtrace */ > #include <linux/membarrier.h> > > +// Copied from perf/util/string.c > + > +/* Character class matching */ > +static bool __match_charclass(const char *pat, char c, const char **npat) > +{ > + bool complement = false, ret = true; > + > + if (*pat == '!') { > + complement = true; > + pat++; > + } > + if (*pat++ == c) /* First character is special */ > + goto end; > + > + while (*pat && *pat != ']') { /* Matching */ > + if (*pat == '-' && *(pat + 1) != ']') { /* Range */ > + if (*(pat - 1) <= c && c <= *(pat + 1)) > + goto end; > + if (*(pat - 1) > *(pat + 1)) > + goto error; > + pat += 2; > + } else if (*pat++ == c) > + goto end; > + } > + if (!*pat) > + goto error; > + ret = false; > + > +end: > + while (*pat && *pat != ']') /* Searching closing */ > + pat++; > + if (!*pat) > + goto error; > + *npat = pat + 1; > + return complement ? !ret : ret; > + > +error: > + return false; > +} > + > +// Copied from perf/util/string.c > +/* Glob/lazy pattern matching */ > +static bool __match_glob(const char *str, const char *pat, bool ignore_space, > + bool case_ins) > +{ > + while (*str && *pat && *pat != '*') { > + if (ignore_space) { > + /* Ignore spaces for lazy matching */ > + if (isspace(*str)) { > + str++; > + continue; > + } > + if (isspace(*pat)) { > + pat++; > + continue; > + } > + } > + if (*pat == '?') { /* Matches any single character */ > + str++; > + pat++; > + continue; > + } else if (*pat == '[') /* Character classes/Ranges */ > + if (__match_charclass(pat + 1, *str, &pat)) { > + str++; > + continue; > + } else > + return false; > + else if (*pat == '\\') /* Escaped char match as normal char */ > + pat++; > + if (case_ins) { > + if (tolower(*str) != tolower(*pat)) > + return false; > + } else 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 (__match_glob(str++, pat, ignore_space, case_ins)) > + return true; > + } > + return !*str && !*pat; > +} > + > #define EXIT_NO_TEST 2 > #define EXIT_ERR_SETUP_INFRA 3 > > @@ -55,12 +145,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 (__match_glob(name, sel->blacklist.strs[i], false, false)) > return false; > } > > for (i = 0; i < sel->whitelist.cnt; i++) { > - if (strstr(name, sel->whitelist.strs[i])) > + if (__match_glob(name, sel->whitelist.strs[i], false, false)) > return true; > } > > -- > 2.30.2 >
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 74dde0af1592..c5bffd2e78ae 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -13,6 +13,96 @@ #include <execinfo.h> /* backtrace */ #include <linux/membarrier.h> +// Copied from perf/util/string.c + +/* Character class matching */ +static bool __match_charclass(const char *pat, char c, const char **npat) +{ + bool complement = false, ret = true; + + if (*pat == '!') { + complement = true; + pat++; + } + if (*pat++ == c) /* First character is special */ + goto end; + + while (*pat && *pat != ']') { /* Matching */ + if (*pat == '-' && *(pat + 1) != ']') { /* Range */ + if (*(pat - 1) <= c && c <= *(pat + 1)) + goto end; + if (*(pat - 1) > *(pat + 1)) + goto error; + pat += 2; + } else if (*pat++ == c) + goto end; + } + if (!*pat) + goto error; + ret = false; + +end: + while (*pat && *pat != ']') /* Searching closing */ + pat++; + if (!*pat) + goto error; + *npat = pat + 1; + return complement ? !ret : ret; + +error: + return false; +} + +// Copied from perf/util/string.c +/* Glob/lazy pattern matching */ +static bool __match_glob(const char *str, const char *pat, bool ignore_space, + bool case_ins) +{ + while (*str && *pat && *pat != '*') { + if (ignore_space) { + /* Ignore spaces for lazy matching */ + if (isspace(*str)) { + str++; + continue; + } + if (isspace(*pat)) { + pat++; + continue; + } + } + if (*pat == '?') { /* Matches any single character */ + str++; + pat++; + continue; + } else if (*pat == '[') /* Character classes/Ranges */ + if (__match_charclass(pat + 1, *str, &pat)) { + str++; + continue; + } else + return false; + else if (*pat == '\\') /* Escaped char match as normal char */ + pat++; + if (case_ins) { + if (tolower(*str) != tolower(*pat)) + return false; + } else 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 (__match_glob(str++, pat, ignore_space, case_ins)) + return true; + } + return !*str && !*pat; +} + #define EXIT_NO_TEST 2 #define EXIT_ERR_SETUP_INFRA 3 @@ -55,12 +145,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 (__match_glob(name, sel->blacklist.strs[i], false, false)) return false; } for (i = 0; i < sel->whitelist.cnt; i++) { - if (strstr(name, sel->whitelist.strs[i])) + if (__match_glob(name, sel->whitelist.strs[i], false, false)) return true; }
This patch adds glob matching to test selector, it accepts simple glob pattern "*", "?", "[]" to match the test names to run. The glob matching function is copied from perf/util/string.c Signed-off-by: Yucong Sun <fallentree@fb.com> --- tools/testing/selftests/bpf/test_progs.c | 94 +++++++++++++++++++++++- 1 file changed, 92 insertions(+), 2 deletions(-)