[PATCH bpf-next 1/5] Skip loading bpf_testmod when using -l to list tests.

[PATCH bpf-next 1/5] Skip loading bpf_testmod when using -l to list tests.

[Yucong Sun]

Signed-off-by: Yucong Sun <fallentree@fb.com>
---
 tools/testing/selftests/bpf/test_progs.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 6f103106a39b..74dde0af1592 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -754,10 +754,12 @@ int main(int argc, char **argv)
 
 	save_netns();
 	stdio_hijack();
-	env.has_testmod = true;
-	if (load_bpf_testmod()) {
-		fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
-		env.has_testmod = false;
+	if (!env.list_test_names) {
+		env.has_testmod = true;
+		if (load_bpf_testmod()) {
+			fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
+			env.has_testmod = false;
+		}
 	}
 	for (i = 0; i < prog_test_cnt; i++) {
 		struct prog_test_def *test = &prog_test_defs[i];
-- 
2.30.2

[PATCH bpf-next 2/5] Support glob matching for test selector.

[Yucong Sun]

This patch adds glob matching to test selector, it allows user to use "*", "?", "[]" to match test name 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(-)

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

[PATCH bpf-next 3/5] Correctly display subtest skip status

[Yucong Sun]

In skip_account(), test->skip_cnt is set to 0 at the end, this makes next print statment never display SKIP status for the subtest. This patch moves the accounting logic after the print statement, fixing the issue.
---
 tools/testing/selftests/bpf/test_progs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c5bffd2e78ae..82d012671552 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -238,18 +238,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;
 }
-- 
2.30.2

[PATCH bpf-next 4/5] Display test number when listing test names

[Yucong Sun]

---
 tools/testing/selftests/bpf/test_progs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 82d012671552..5cc808992b00 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -867,7 +867,8 @@ int main(int argc, char **argv)
 		}
 
 		if (env.list_test_names) {
-			fprintf(env.stdout, "%s\n", test->test_name);
+			fprintf(env.stdout, "# %d %s\n",
+				test->test_num, test->test_name);
 			env.succ_cnt++;
 			continue;
 		}
-- 
2.30.2

Re: [PATCH bpf-next 4/5] Display test number when listing test names

[Daniel Borkmann]

Hi Yucong,

thanks for your patches!

On 8/10/21 1:36 AM, Yucong Sun wrote:
> ---

Please make sure all of your patches have proper Signed-off-by and at least a
minimal commit message (instead of empty one).

Thanks,
Daniel

>   tools/testing/selftests/bpf/test_progs.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 82d012671552..5cc808992b00 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -867,7 +867,8 @@ int main(int argc, char **argv)
>   		}
>   
>   		if (env.list_test_names) {
> -			fprintf(env.stdout, "%s\n", test->test_name);
> +			fprintf(env.stdout, "# %d %s\n",
> +				test->test_num, test->test_name);
>   			env.succ_cnt++;
>   			continue;
>   		}
> 

Re: [PATCH bpf-next 4/5] Display test number when listing test names

[Yucong Sun]

Thanks, I re-generated the patch series with a cover letter, also added more description to each patch, PTAL.

Cheers.

On 8/9/21, 4:55 PM, "Daniel Borkmann" <daniel@iogearbox.net> wrote:

    Hi Yucong,

    thanks for your patches!

    On 8/10/21 1:36 AM, Yucong Sun wrote:
    > ---

    Please make sure all of your patches have proper Signed-off-by and at least a
    minimal commit message (instead of empty one).

    Thanks,
    Daniel

    >   tools/testing/selftests/bpf/test_progs.c | 3 ++-
    >   1 file changed, 2 insertions(+), 1 deletion(-)
    > 
    > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
    > index 82d012671552..5cc808992b00 100644
    > --- a/tools/testing/selftests/bpf/test_progs.c
    > +++ b/tools/testing/selftests/bpf/test_progs.c
    > @@ -867,7 +867,8 @@ int main(int argc, char **argv)
    >   		}
    >   
    >   		if (env.list_test_names) {
    > -			fprintf(env.stdout, "%s\n", test->test_name);
    > +			fprintf(env.stdout, "# %d %s\n",
    > +				test->test_num, test->test_name);
    >   			env.succ_cnt++;
    >   			continue;
    >   		}
    > 

[PATCH bpf-next 5/5] Record all failed tests and output after the summary line.

[Yucong Sun]

This patch records all failed tests and subtests during the run, output them after the summary line, making it easier to identify failed tests in the long output.

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

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 5cc808992b00..51a70031f07e 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -244,6 +244,11 @@ void test__end_subtest()
 	       test->test_num, test->subtest_num, test->subtest_name,
 	       sub_error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
 
+	if (sub_error_cnt) {
+		fprintf(env.summary_errors, "#%d/%d %s: FAIL\n",
+			test->test_num, test->subtest_num, test->subtest_name);
+	}
+
 	if (sub_error_cnt)
 		env.fail_cnt++;
 	else if (test->skip_cnt == 0)
@@ -816,6 +821,10 @@ int main(int argc, char **argv)
 		.sa_flags = SA_RESETHAND,
 	};
 	int err, i;
+	/* record errors to print after summary line */
+	char *summary_errors_buf;
+	size_t summary_errors_cnt;
+
 
 	sigaction(SIGSEGV, &sigact, NULL);
 
@@ -823,6 +832,9 @@ int main(int argc, char **argv)
 	if (err)
 		return err;
 
+	env.summary_errors = open_memstream(
+		&summary_errors_buf, &summary_errors_cnt);
+
 	err = cd_flavor_subdir(argv[0]);
 	if (err)
 		return err;
@@ -891,6 +903,11 @@ int main(int argc, char **argv)
 			test->test_num, test->test_name,
 			test->error_cnt ? "FAIL" : "OK");
 
+		if(test->error_cnt) {
+			fprintf(env.summary_errors, "#%d %s: FAIL\n",
+				test->test_num, test->test_name);
+		}
+
 		reset_affinity();
 		restore_netns();
 		if (test->need_cgroup_cleanup)
@@ -908,9 +925,14 @@ int main(int argc, char **argv)
 	if (env.list_test_names)
 		goto out;
 
-	fprintf(stdout, "Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
+	fprintf(stdout, "\nSummary: %d/%d PASSED, %d SKIPPED, %d FAILED\n\n",
 		env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt);
 
+	fclose(env.summary_errors);
+	if(env.fail_cnt) {
+		fprintf(stdout, "%s", summary_errors_buf);
+	}
+
 out:
 	free_str_set(&env.test_selector.blacklist);
 	free_str_set(&env.test_selector.whitelist);
@@ -919,6 +941,7 @@ int main(int argc, char **argv)
 	free_str_set(&env.subtest_selector.whitelist);
 	free(env.subtest_selector.num_set);
 	close(env.saved_netns_fd);
+	free(summary_errors_buf);
 
 	if (env.succ_cnt + env.fail_cnt + env.skip_cnt == 0)
 		return EXIT_NO_TEST;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index c8c2bf878f67..63f4e534c6e5 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -82,6 +82,8 @@ struct test_env {
 	int skip_cnt; /* skipped tests */
 
 	int saved_netns_fd;
+
+	FILE* summary_errors;
 };
 
 extern struct test_env env;
-- 
2.30.2