[PATCH v4 bpf-next 0/4] selftests/bpf: Improve the usability of

[PATCH v4 bpf-next 0/4] selftests/bpf: Improve the usability of

[Yucong Sun]

This short series adds two new "-a", "-d" switch to test_progs,
supporting exact string match, as well as '*' wildchar. It also cleans
up the output to make it possible to generate allowlist/denylist using
grep.

Yucong Sun (4):
  selftests/bpf: skip loading bpf_testmod when using -l to list tests.
  selftests/bpf: correctly display subtest skip status
  selftests/bpf: also print test name in subtest status message
  selftests/bpf: Support glob matching for test selector.

 tools/testing/selftests/bpf/test_progs.c | 93 ++++++++++++++++++------
 tools/testing/selftests/bpf/test_progs.h |  1 +
 2 files changed, 72 insertions(+), 22 deletions(-)

-- 
2.30.2

[PATCH v4 bpf-next 1/4] selftests/bpf: skip loading bpf_testmod when using -l to list tests.

[Yucong Sun]

When using "-l", test_progs often is executed as non-root user,
load_bpf_testmod() will fail and output errors. This patch skip loading bpf
testmod when "-l" is specified, making output cleaner.

Signed-off-by: Yucong Sun <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();
 
-- 
2.30.2

[PATCH v4 bpf-next 2/4] selftests/bpf: correctly display subtest skip status

[Yucong Sun]

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 <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();
-- 
2.30.2

[PATCH v4 bpf-next 3/4] selftests/bpf: also print test name in subtest status message

[Yucong Sun]

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 <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)
-- 
2.30.2

[PATCH v4 bpf-next 4/4] selftests/bpf: Support glob matching for test selector.

[Yucong Sun]

This patch adds '-a' and '-d' arguments, supporting 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.

Caveat: As same as the current substring matching mechanism, test and subtest
selector applies independently, 'a*/b*' will execute all tests matches "a*",
and with subtest name matches "b*", but tests matches "a*" but has no subtests
will also be executed.

Signed-off-by: Yucong Sun <fallentree@fb.com>
---
 tools/testing/selftests/bpf/test_progs.c | 64 +++++++++++++++++++++---
 tools/testing/selftests/bpf/test_progs.h |  1 +
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 90539b15b744..f5dbaa29d370 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;
@@ -508,8 +536,14 @@ static int parse_str_list(const char *s, struct str_set *set)
 		if (!tmp)
 			goto err;
 		strs = tmp;
+		if (is_glob_pattern)
+			strs[cnt] = strdup(next);
+		else {
+			char* pat = malloc(strlen(next) + 2 + 1);
+			sprintf(pat, "*%s*", next);
+			strs[cnt] = pat;
+		}
 
-		strs[cnt] = strdup(next);
 		if (!strs[cnt])
 			goto err;
 
@@ -553,29 +587,43 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 		}
 		break;
 	}
+	case ARG_TEST_NAME_GLOB_ALLOWLIST:
 	case ARG_TEST_NAME: {
+		if (env->test_selector.whitelist.cnt || env->subtest_selector.whitelist.cnt) {
+			fprintf(stderr, "-a and -t are mutually exclusive, you can only specify one.\n");
+			return -EINVAL;
+		}
 		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: {
+		if (env->test_selector.blacklist.cnt || env->subtest_selector.blacklist.cnt) {
+			fprintf(stderr, "-d and -b are mutually exclusive, you can only specify one.\n");
+			return -EINVAL;
+		}
 		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;
 	}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index c8c2bf878f67..c475d65dce4f 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -49,6 +49,7 @@ enum verbosity {
 struct str_set {
 	const char **strs;
 	int cnt;
+	bool is_glob_pattern;
 };
 
 struct test_selector {
-- 
2.30.2

Re: [PATCH v4 bpf-next 4/4] selftests/bpf: Support glob matching for test selector.

[Andrii Nakryiko]

On Mon, Aug 16, 2021 at 6:03 PM Yucong Sun <fallentree@fb.com> wrote:
>
> This patch adds '-a' and '-d' arguments, supporting 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.

typo: support

>
> Caveat: As same as the current substring matching mechanism, test and subtest

"Same as"

> selector applies independently, 'a*/b*' will execute all tests matches "a*",

s/matches/matching/ same below

> and with subtest name matches "b*", but tests matches "a*" but has no subtests
> will also be executed.
>
> Signed-off-by: Yucong Sun <fallentree@fb.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 64 +++++++++++++++++++++---
>  tools/testing/selftests/bpf/test_progs.h |  1 +
>  2 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 90539b15b744..f5dbaa29d370 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;
> @@ -508,8 +536,14 @@ static int parse_str_list(const char *s, struct str_set *set)
>                 if (!tmp)
>                         goto err;
>                 strs = tmp;
> +               if (is_glob_pattern)
> +                       strs[cnt] = strdup(next);

coding style: if one branch of if/else uses {}, the other has to use it as well

> +               else {
> +                       char* pat = malloc(strlen(next) + 2 + 1);

coding style: empty line after variable declaration (but I'd just use
strs[cnt] twice without extra var)

also `char *pat`, please use checkpatch.pl to double check your
changes until kernel code style becomes second nature (and even then
it's probably useful to run it from time to time). We do have few
errors reported in test_progs.c, but the script would reported 2 out
of 3 of these problems. At some point we should see if it makes sense
to run checkpatch.pl as one of BPF CI steps.

> +                       sprintf(pat, "*%s*", next);
> +                       strs[cnt] = pat;
> +               }
>
> -               strs[cnt] = strdup(next);
>                 if (!strs[cnt])
>                         goto err;
>
> @@ -553,29 +587,43 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
>                 }
>                 break;
>         }
> +       case ARG_TEST_NAME_GLOB_ALLOWLIST:
>         case ARG_TEST_NAME: {
> +               if (env->test_selector.whitelist.cnt || env->subtest_selector.whitelist.cnt) {
> +                       fprintf(stderr, "-a and -t are mutually exclusive, you can only specify one.\n");
> +                       return -EINVAL;
> +               }

Why are they still exclusive? They are now just alternative ways to
specify the same set of filters, one as substring match, another as
explicit globs. Just like you can use '-t a,b', you can also have '-t
a -a "*b*"', right? They can totally co-exist and it might be useful
to use both sometimes.

>                 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: {
> +               if (env->test_selector.blacklist.cnt || env->subtest_selector.blacklist.cnt) {
> +                       fprintf(stderr, "-d and -b are mutually exclusive, you can only specify one.\n");
> +                       return -EINVAL;
> +               }

same, they can organically co-exist

>                 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;
>         }
> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index c8c2bf878f67..c475d65dce4f 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -49,6 +49,7 @@ enum verbosity {
>  struct str_set {
>         const char **strs;
>         int cnt;
> +       bool is_glob_pattern;


leftovers? doesn't seem to be used

>  };
>
>  struct test_selector {
> --
> 2.30.2
>

Re: [PATCH v4 bpf-next 0/4] selftests/bpf: Improve the usability of

[Andrii Nakryiko]

On Mon, Aug 16, 2021 at 6:03 PM Yucong Sun <fallentree@fb.com> wrote:
>
> This short series adds two new "-a", "-d" switch to test_progs,
> supporting exact string match, as well as '*' wildchar. It also cleans
> up the output to make it possible to generate allowlist/denylist using
> grep.
>

You seem to have lost part of the cover letter subject line?

s/wildchar/wildcard/

> Yucong Sun (4):
>   selftests/bpf: skip loading bpf_testmod when using -l to list tests.
>   selftests/bpf: correctly display subtest skip status
>   selftests/bpf: also print test name in subtest status message
>   selftests/bpf: Support glob matching for test selector.
>
>  tools/testing/selftests/bpf/test_progs.c | 93 ++++++++++++++++++------
>  tools/testing/selftests/bpf/test_progs.h |  1 +
>  2 files changed, 72 insertions(+), 22 deletions(-)
>
> --
> 2.30.2
>

Re: [PATCH v4 bpf-next 4/4] selftests/bpf: Support glob matching for test selector.

[sunyucong]

On Mon, Aug 16, 2021 at 7:45 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Aug 16, 2021 at 6:03 PM Yucong Sun <fallentree@fb.com> wrote:
> >
> > This patch adds '-a' and '-d' arguments, supporting 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.
>
> typo: support
>
> >
> > Caveat: As same as the current substring matching mechanism, test and subtest
>
> "Same as"
>
> > selector applies independently, 'a*/b*' will execute all tests matches "a*",
>
> s/matches/matching/ same below
>
> > and with subtest name matches "b*", but tests matches "a*" but has no subtests
> > will also be executed.
> >
> > Signed-off-by: Yucong Sun <fallentree@fb.com>
> > ---
> >  tools/testing/selftests/bpf/test_progs.c | 64 +++++++++++++++++++++---
> >  tools/testing/selftests/bpf/test_progs.h |  1 +
> >  2 files changed, 57 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index 90539b15b744..f5dbaa29d370 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;
> > @@ -508,8 +536,14 @@ static int parse_str_list(const char *s, struct str_set *set)
> >                 if (!tmp)
> >                         goto err;
> >                 strs = tmp;
> > +               if (is_glob_pattern)
> > +                       strs[cnt] = strdup(next);
>
> coding style: if one branch of if/else uses {}, the other has to use it as well
>
> > +               else {
> > +                       char* pat = malloc(strlen(next) + 2 + 1);
>
> coding style: empty line after variable declaration (but I'd just use
> strs[cnt] twice without extra var)
>
> also `char *pat`, please use checkpatch.pl to double check your
> changes until kernel code style becomes second nature (and even then
> it's probably useful to run it from time to time). We do have few
> errors reported in test_progs.c, but the script would reported 2 out
> of 3 of these problems. At some point we should see if it makes sense
> to run checkpatch.pl as one of BPF CI steps.

Sorry, done!

>
> > +                       sprintf(pat, "*%s*", next);
> > +                       strs[cnt] = pat;
> > +               }
> >
> > -               strs[cnt] = strdup(next);
> >                 if (!strs[cnt])
> >                         goto err;
> >
> > @@ -553,29 +587,43 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> >                 }
> >                 break;
> >         }
> > +       case ARG_TEST_NAME_GLOB_ALLOWLIST:
> >         case ARG_TEST_NAME: {
> > +               if (env->test_selector.whitelist.cnt || env->subtest_selector.whitelist.cnt) {
> > +                       fprintf(stderr, "-a and -t are mutually exclusive, you can only specify one.\n");
> > +                       return -EINVAL;
> > +               }
>
> Why are they still exclusive? They are now just alternative ways to
> specify the same set of filters, one as substring match, another as
> explicit globs. Just like you can use '-t a,b', you can also have '-t
> a -a "*b*"', right? They can totally co-exist and it might be useful
> to use both sometimes.

I added the logic to accept multiple -a -d -t -d switches as requested.

>
> >                 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: {
> > +               if (env->test_selector.blacklist.cnt || env->subtest_selector.blacklist.cnt) {
> > +                       fprintf(stderr, "-d and -b are mutually exclusive, you can only specify one.\n");
> > +                       return -EINVAL;
> > +               }
>
> same, they can organically co-exist
>
> >                 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;
> >         }
> > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> > index c8c2bf878f67..c475d65dce4f 100644
> > --- a/tools/testing/selftests/bpf/test_progs.h
> > +++ b/tools/testing/selftests/bpf/test_progs.h
> > @@ -49,6 +49,7 @@ enum verbosity {
> >  struct str_set {
> >         const char **strs;
> >         int cnt;
> > +       bool is_glob_pattern;
>
>
> leftovers? doesn't seem to be used

Deleted.

>
> >  };
> >
> >  struct test_selector {
> > --
> > 2.30.2
> >