diff mbox series

[RFC,bpf-next,1/5] selftests/bpf: support custom per-test flags and multiple expected messages

Message ID 20230123145148.2791939-2-eddyz87@gmail.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series test_verifier tests migration to inline assembly | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-8 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_progs_parallel on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-36 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-37 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs on s390x with gcc
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 10 maintainers not CCed: linux-kselftest@vger.kernel.org kpsingh@kernel.org haoluo@google.com song@kernel.org martin.lau@linux.dev sdf@google.com john.fastabend@gmail.com shuah@kernel.org jolsa@kernel.org mykolal@fb.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning CHECK: braces {} should be used on all arms of this statement WARNING: line length of 100 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 94 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Eduard Zingerman Jan. 23, 2023, 2:51 p.m. UTC 
From: Andrii Nakryiko <andrii@kernel.org>

Extend __flag attribute by allowing to specify one of the following:
 * BPF_F_STRICT_ALIGNMENT
 * BPF_F_ANY_ALIGNMENT
 * BPF_F_TEST_RND_HI32
 * BPF_F_TEST_STATE_FREQ
 * BPF_F_SLEEPABLE
 * BPF_F_XDP_HAS_FRAGS
 * Some numeric value

Extend __msg attribute by allowing to specify multiple exepcted messages.
All messages are expected to be present in the verifier log in the
order of application.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
[ Eduard: added commit message, formatting ]
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 tools/testing/selftests/bpf/test_loader.c | 69 ++++++++++++++++++++---
 tools/testing/selftests/bpf/test_progs.h  |  1 +
 2 files changed, 61 insertions(+), 9 deletions(-)

Comments

Andrii Nakryiko Feb. 28, 2023, 6:53 p.m. UTC | #1 
On Mon, Jan 23, 2023 at 6:52 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> From: Andrii Nakryiko <andrii@kernel.org>
>
> Extend __flag attribute by allowing to specify one of the following:
>  * BPF_F_STRICT_ALIGNMENT
>  * BPF_F_ANY_ALIGNMENT
>  * BPF_F_TEST_RND_HI32
>  * BPF_F_TEST_STATE_FREQ
>  * BPF_F_SLEEPABLE
>  * BPF_F_XDP_HAS_FRAGS
>  * Some numeric value
>
> Extend __msg attribute by allowing to specify multiple exepcted messages.
> All messages are expected to be present in the verifier log in the
> order of application.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> [ Eduard: added commit message, formatting ]
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---

hey Eduard,

When you get a chance, can you please send this patch separately from
the rest of the test_verifier rework patch set (it probably makes
sense to also add #define __flags in this patch as well, given you are
parsing its definition in this patch).

This would great help me with my work that uses all this
assembly-level test facilities. Thanks!



>  tools/testing/selftests/bpf/test_loader.c | 69 ++++++++++++++++++++---
>  tools/testing/selftests/bpf/test_progs.h  |  1 +
>  2 files changed, 61 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> index 679efb3aa785..bf41390157bf 100644
> --- a/tools/testing/selftests/bpf/test_loader.c
> +++ b/tools/testing/selftests/bpf/test_loader.c
> @@ -13,12 +13,15 @@
>  #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
>  #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
>  #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
> +#define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
>

[...]
Eduard Zingerman Feb. 28, 2023, 10:30 p.m. UTC | #2 
On Tue, 2023-02-28 at 10:53 -0800, Andrii Nakryiko wrote:
> On Mon, Jan 23, 2023 at 6:52 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > From: Andrii Nakryiko <andrii@kernel.org>
> > 
> > Extend __flag attribute by allowing to specify one of the following:
> >  * BPF_F_STRICT_ALIGNMENT
> >  * BPF_F_ANY_ALIGNMENT
> >  * BPF_F_TEST_RND_HI32
> >  * BPF_F_TEST_STATE_FREQ
> >  * BPF_F_SLEEPABLE
> >  * BPF_F_XDP_HAS_FRAGS
> >  * Some numeric value
> > 
> > Extend __msg attribute by allowing to specify multiple exepcted messages.
> > All messages are expected to be present in the verifier log in the
> > order of application.
> > 
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > [ Eduard: added commit message, formatting ]
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> 
> hey Eduard,
> 
> When you get a chance, can you please send this patch separately from
> the rest of the test_verifier rework patch set (it probably makes
> sense to also add #define __flags in this patch as well, given you are
> parsing its definition in this patch).
> 
> This would great help me with my work that uses all this
> assembly-level test facilities. Thanks!

Hi Andrii,

Rebase didn't change anything in the patch, I added __flags macro,
some some comments, and started the CI job: [1].

Feels weird to post it, tbh, because it's 100% your code w/o added
value from my side.

Thanks,
Eduard

[1] https://github.com/kernel-patches/bpf/pull/4688
> 
> 
> 
> >  tools/testing/selftests/bpf/test_loader.c | 69 ++++++++++++++++++++---
> >  tools/testing/selftests/bpf/test_progs.h  |  1 +
> >  2 files changed, 61 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> > index 679efb3aa785..bf41390157bf 100644
> > --- a/tools/testing/selftests/bpf/test_loader.c
> > +++ b/tools/testing/selftests/bpf/test_loader.c
> > @@ -13,12 +13,15 @@
> >  #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
> >  #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
> >  #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
> > +#define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
> > 
> 
> [...]
Andrii Nakryiko March 1, 2023, 5:12 p.m. UTC | #3 
On Tue, Feb 28, 2023 at 2:30 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2023-02-28 at 10:53 -0800, Andrii Nakryiko wrote:
> > On Mon, Jan 23, 2023 at 6:52 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > From: Andrii Nakryiko <andrii@kernel.org>
> > >
> > > Extend __flag attribute by allowing to specify one of the following:
> > >  * BPF_F_STRICT_ALIGNMENT
> > >  * BPF_F_ANY_ALIGNMENT
> > >  * BPF_F_TEST_RND_HI32
> > >  * BPF_F_TEST_STATE_FREQ
> > >  * BPF_F_SLEEPABLE
> > >  * BPF_F_XDP_HAS_FRAGS
> > >  * Some numeric value
> > >
> > > Extend __msg attribute by allowing to specify multiple exepcted messages.
> > > All messages are expected to be present in the verifier log in the
> > > order of application.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > [ Eduard: added commit message, formatting ]
> > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > > ---
> >
> > hey Eduard,
> >
> > When you get a chance, can you please send this patch separately from
> > the rest of the test_verifier rework patch set (it probably makes
> > sense to also add #define __flags in this patch as well, given you are
> > parsing its definition in this patch).
> >
> > This would great help me with my work that uses all this
> > assembly-level test facilities. Thanks!
>
> Hi Andrii,
>
> Rebase didn't change anything in the patch, I added __flags macro,
> some some comments, and started the CI job: [1].
>
> Feels weird to post it, tbh, because it's 100% your code w/o added
> value from my side.

you took the effort to prepare it for submission, testing, and
integrating into your work, so feels well deserved


>
> Thanks,
> Eduard
>
> [1] https://github.com/kernel-patches/bpf/pull/4688

apart from test flakiness, looks good, please send a patch "officially"

> >
> >
> >
> > >  tools/testing/selftests/bpf/test_loader.c | 69 ++++++++++++++++++++---
> > >  tools/testing/selftests/bpf/test_progs.h  |  1 +
> > >  2 files changed, 61 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> > > index 679efb3aa785..bf41390157bf 100644
> > > --- a/tools/testing/selftests/bpf/test_loader.c
> > > +++ b/tools/testing/selftests/bpf/test_loader.c
> > > @@ -13,12 +13,15 @@
> > >  #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
> > >  #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
> > >  #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
> > > +#define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
> > >
> >
> > [...]
>
Eduard Zingerman March 1, 2023, 5:58 p.m. UTC | #4 
On Wed, 2023-03-01 at 09:12 -0800, Andrii Nakryiko wrote:
[...]
> > Rebase didn't change anything in the patch, I added __flags macro,
> > some some comments, and started the CI job: [1].
> > 
> > Feels weird to post it, tbh, because it's 100% your code w/o added
> > value from my side.
> 
> you took the effort to prepare it for submission, testing, and
> integrating into your work, so feels well deserved
> 
> 
> > 
> > Thanks,
> > Eduard
> > 
> > [1] https://github.com/kernel-patches/bpf/pull/4688
> 
> apart from test flakiness, looks good, please send a patch "officially"

Done.
Local test run does not show any issues.

> 
> > > 
> > > 
> > > 
> > > >  tools/testing/selftests/bpf/test_loader.c | 69 ++++++++++++++++++++---
> > > >  tools/testing/selftests/bpf/test_progs.h  |  1 +
> > > >  2 files changed, 61 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> > > > index 679efb3aa785..bf41390157bf 100644
> > > > --- a/tools/testing/selftests/bpf/test_loader.c
> > > > +++ b/tools/testing/selftests/bpf/test_loader.c
> > > > @@ -13,12 +13,15 @@
> > > >  #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
> > > >  #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
> > > >  #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
> > > > +#define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
> > > > 
> > > 
> > > [...]
> >
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index 679efb3aa785..bf41390157bf 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -13,12 +13,15 @@ 
 #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
 #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
 #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
+#define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
 
 struct test_spec {
 	const char *name;
 	bool expect_failure;
-	const char *expect_msg;
+	const char **expect_msgs;
+	size_t expect_msg_cnt;
 	int log_level;
+	int prog_flags;
 };
 
 static int tester_init(struct test_loader *tester)
@@ -67,7 +70,8 @@  static int parse_test_spec(struct test_loader *tester,
 
 	for (i = 1; i < btf__type_cnt(btf); i++) {
 		const struct btf_type *t;
-		const char *s;
+		const char *s, *val;
+		char *e;
 
 		t = btf__type_by_id(btf, i);
 		if (!btf_is_decl_tag(t))
@@ -82,14 +86,48 @@  static int parse_test_spec(struct test_loader *tester,
 		} else if (strcmp(s, TEST_TAG_EXPECT_SUCCESS) == 0) {
 			spec->expect_failure = false;
 		} else if (str_has_pfx(s, TEST_TAG_EXPECT_MSG_PFX)) {
-			spec->expect_msg = s + sizeof(TEST_TAG_EXPECT_MSG_PFX) - 1;
+			void *tmp;
+			const char **msg;
+
+			tmp = realloc(spec->expect_msgs,
+				      (1 + spec->expect_msg_cnt) * sizeof(void *));
+			if (!tmp) {
+				ASSERT_FAIL("failed to realloc memory for messages\n");
+				return -ENOMEM;
+			}
+			spec->expect_msgs = tmp;
+			msg = &spec->expect_msgs[spec->expect_msg_cnt++];
+			*msg = s + sizeof(TEST_TAG_EXPECT_MSG_PFX) - 1;
 		} else if (str_has_pfx(s, TEST_TAG_LOG_LEVEL_PFX)) {
+			val = s + sizeof(TEST_TAG_LOG_LEVEL_PFX) - 1;
 			errno = 0;
-			spec->log_level = strtol(s + sizeof(TEST_TAG_LOG_LEVEL_PFX) - 1, NULL, 0);
-			if (errno) {
+			spec->log_level = strtol(val, &e, 0);
+			if (errno || e[0] != '\0') {
 				ASSERT_FAIL("failed to parse test log level from '%s'", s);
 				return -EINVAL;
 			}
+		} else if (str_has_pfx(s, TEST_TAG_PROG_FLAGS_PFX)) {
+			val = s + sizeof(TEST_TAG_PROG_FLAGS_PFX) - 1;
+			if (strcmp(val, "BPF_F_STRICT_ALIGNMENT") == 0) {
+				spec->prog_flags |= BPF_F_STRICT_ALIGNMENT;
+			} else if (strcmp(val, "BPF_F_ANY_ALIGNMENT") == 0) {
+				spec->prog_flags |= BPF_F_ANY_ALIGNMENT;
+			} else if (strcmp(val, "BPF_F_TEST_RND_HI32") == 0) {
+				spec->prog_flags |= BPF_F_TEST_RND_HI32;
+			} else if (strcmp(val, "BPF_F_TEST_STATE_FREQ") == 0) {
+				spec->prog_flags |= BPF_F_TEST_STATE_FREQ;
+			} else if (strcmp(val, "BPF_F_SLEEPABLE") == 0) {
+				spec->prog_flags |= BPF_F_SLEEPABLE;
+			} else if (strcmp(val, "BPF_F_XDP_HAS_FRAGS") == 0) {
+				spec->prog_flags |= BPF_F_XDP_HAS_FRAGS;
+			} else /* assume numeric value */ {
+				errno = 0;
+				spec->prog_flags |= strtol(val, &e, 0);
+				if (errno || e[0] != '\0') {
+					ASSERT_FAIL("failed to parse test prog flags from '%s'", s);
+					return -EINVAL;
+				}
+			}
 		}
 	}
 
@@ -101,7 +139,7 @@  static void prepare_case(struct test_loader *tester,
 			 struct bpf_object *obj,
 			 struct bpf_program *prog)
 {
-	int min_log_level = 0;
+	int min_log_level = 0, prog_flags;
 
 	if (env.verbosity > VERBOSE_NONE)
 		min_log_level = 1;
@@ -119,7 +157,11 @@  static void prepare_case(struct test_loader *tester,
 	else
 		bpf_program__set_log_level(prog, spec->log_level);
 
+	prog_flags = bpf_program__flags(prog);
+	bpf_program__set_flags(prog, prog_flags | spec->prog_flags);
+
 	tester->log_buf[0] = '\0';
+	tester->next_match_pos = 0;
 }
 
 static void emit_verifier_log(const char *log_buf, bool force)
@@ -135,17 +177,26 @@  static void validate_case(struct test_loader *tester,
 			  struct bpf_program *prog,
 			  int load_err)
 {
-	if (spec->expect_msg) {
+	int i, j;
+
+	for (i = 0; i < spec->expect_msg_cnt; i++) {
 		char *match;
+		const char *expect_msg;
+
+		expect_msg = spec->expect_msgs[i];
 
-		match = strstr(tester->log_buf, spec->expect_msg);
+		match = strstr(tester->log_buf + tester->next_match_pos, expect_msg);
 		if (!ASSERT_OK_PTR(match, "expect_msg")) {
 			/* if we are in verbose mode, we've already emitted log */
 			if (env.verbosity == VERBOSE_NONE)
 				emit_verifier_log(tester->log_buf, true /*force*/);
-			fprintf(stderr, "EXPECTED MSG: '%s'\n", spec->expect_msg);
+			for (j = 0; j < i; j++)
+				fprintf(stderr, "MATCHED  MSG: '%s'\n", spec->expect_msgs[j]);
+			fprintf(stderr, "EXPECTED MSG: '%s'\n", expect_msg);
 			return;
 		}
+
+		tester->next_match_pos = match - tester->log_buf + strlen(expect_msg);
 	}
 }
 
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 3f058dfadbaf..9af80704f20a 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -410,6 +410,7 @@  int write_sysctl(const char *sysctl, const char *value);
 struct test_loader {
 	char *log_buf;
 	size_t log_buf_sz;
+	size_t next_match_pos;
 
 	struct bpf_object *obj;
 };