* [PATCH 0/2] Improve error handling of verifier tests
@ 2020-11-28 19:25 Florian Lehner
2020-11-28 19:25 ` [PATCH 1/2] selftests/bpf: Avoid errno clobbering Florian Lehner
2020-11-28 19:25 ` [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program Florian Lehner
0 siblings, 2 replies; 5+ messages in thread
From: Florian Lehner @ 2020-11-28 19:25 UTC (permalink / raw)
To: bpf
Cc: netdev, ast, daniel, andrii, john.fastabend, Florian Lehner,
Krzesimir Nowak
These patches improve the error handling for verifier tests. With "Test
the 32bit narrow read" Krzesimir Nowak provided these patches first, but
they were never merged.
The improved error handling helps to implement and test BPF program types
that are not supported yet.
Florian Lehner (2):
selftests/bpf: Avoid errno clobbering
selftests/bpf: Print reason when a tester could not run a program
tools/testing/selftests/bpf/test_verifier.c | 28 ++++++++++++++++-----
1 file changed, 22 insertions(+), 6 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] selftests/bpf: Avoid errno clobbering
2020-11-28 19:25 [PATCH 0/2] Improve error handling of verifier tests Florian Lehner
@ 2020-11-28 19:25 ` Florian Lehner
2020-12-02 1:24 ` Andrii Nakryiko
2020-11-28 19:25 ` [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program Florian Lehner
1 sibling, 1 reply; 5+ messages in thread
From: Florian Lehner @ 2020-11-28 19:25 UTC (permalink / raw)
To: bpf
Cc: netdev, ast, daniel, andrii, john.fastabend, Florian Lehner,
Stanislav Fomichev, Krzesimir Nowak
Commit 8184d44c9a57 ("selftests/bpf: skip verifier tests for unsupported
program types") added a check to skip unsupported program types. As
bpf_probe_prog_type can change errno, do_single_test should save it before
printing a reason why a supported BPF program type failed to load.
Fixes: 8184d44c9a57 ("selftests/bpf: skip verifier tests for unsupported program types")
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
Signed-off-by: Florian Lehner <dev@der-flo.net>
---
tools/testing/selftests/bpf/test_verifier.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 4bfe3aa2cfc4..ceea9409639e 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -936,6 +936,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int run_errs, run_successes;
int map_fds[MAX_NR_MAPS];
const char *expected_err;
+ int saved_errno;
int fixup_skips;
__u32 pflags;
int i, err;
@@ -997,6 +998,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
}
fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
+ saved_errno = errno;
/* BPF_PROG_TYPE_TRACING requires more setup and
* bpf_probe_prog_type won't give correct answer
@@ -1013,7 +1015,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
if (expected_ret == ACCEPT || expected_ret == VERBOSE_ACCEPT) {
if (fd_prog < 0) {
printf("FAIL\nFailed to load prog '%s'!\n",
- strerror(errno));
+ strerror(saved_errno));
goto fail_log;
}
#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program
2020-11-28 19:25 [PATCH 0/2] Improve error handling of verifier tests Florian Lehner
2020-11-28 19:25 ` [PATCH 1/2] selftests/bpf: Avoid errno clobbering Florian Lehner
@ 2020-11-28 19:25 ` Florian Lehner
2020-12-02 1:23 ` Andrii Nakryiko
1 sibling, 1 reply; 5+ messages in thread
From: Florian Lehner @ 2020-11-28 19:25 UTC (permalink / raw)
To: bpf
Cc: netdev, ast, daniel, andrii, john.fastabend, Florian Lehner,
Krzesimir Nowak
Print a message when the returned error is about a program type being
not supported or because of permission problems.
These messages are expected if the program to test was actually
executed.
Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
Signed-off-by: Florian Lehner <dev@der-flo.net>
---
tools/testing/selftests/bpf/test_verifier.c | 24 ++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index ceea9409639e..bd95894b7ea0 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -875,19 +875,33 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
__u8 tmp[TEST_DATA_LEN << 2];
__u32 size_tmp = sizeof(tmp);
uint32_t retval;
- int err;
+ int err, saved_errno;
if (unpriv)
set_admin(true);
err = bpf_prog_test_run(fd_prog, 1, data, size_data,
tmp, &size_tmp, &retval, NULL);
+ saved_errno = errno;
+
if (unpriv)
set_admin(false);
- if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
- printf("Unexpected bpf_prog_test_run error ");
- return err;
+
+ if (err) {
+ switch (errno) {
+ case 524/*ENOTSUPP*/:
+ printf("Did not run the program (not supported) ");
+ return 0;
+ case EPERM:
+ printf("Did not run the program (no permission) ");
+ return 0;
+ default:
+ printf("FAIL: Unexpected bpf_prog_test_run error (%s) ",
+ strerror(saved_errno));
+ return err;
+ }
}
- if (!err && retval != expected_val &&
+
+ if (retval != expected_val &&
expected_val != POINTER_VALUE) {
printf("FAIL retval %d != %d ", retval, expected_val);
return 1;
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program
2020-11-28 19:25 ` [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program Florian Lehner
@ 2020-12-02 1:23 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2020-12-02 1:23 UTC (permalink / raw)
To: Florian Lehner
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, john fastabend, Krzesimir Nowak
On Sat, Nov 28, 2020 at 11:29 AM Florian Lehner <dev@der-flo.net> wrote:
>
> Print a message when the returned error is about a program type being
> not supported or because of permission problems.
> These messages are expected if the program to test was actually
> executed.
>
> Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
> Signed-off-by: Florian Lehner <dev@der-flo.net>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 24 ++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index ceea9409639e..bd95894b7ea0 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -875,19 +875,33 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
> __u8 tmp[TEST_DATA_LEN << 2];
> __u32 size_tmp = sizeof(tmp);
> uint32_t retval;
> - int err;
> + int err, saved_errno;
>
> if (unpriv)
> set_admin(true);
> err = bpf_prog_test_run(fd_prog, 1, data, size_data,
> tmp, &size_tmp, &retval, NULL);
> + saved_errno = errno;
> +
> if (unpriv)
> set_admin(false);
> - if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
> - printf("Unexpected bpf_prog_test_run error ");
> - return err;
> +
> + if (err) {
> + switch (errno) {
nit: stick to using saved_errno consistently, set_admin() does a lot
of things that can change errno
> + case 524/*ENOTSUPP*/:
> + printf("Did not run the program (not supported) ");
> + return 0;
> + case EPERM:
> + printf("Did not run the program (no permission) ");
> + return 0;
This should be ok to ignore *only* in unpriv mode, no?
> + default:
> + printf("FAIL: Unexpected bpf_prog_test_run error (%s) ",
> + strerror(saved_errno));
> + return err;
> + }
> }
> - if (!err && retval != expected_val &&
> +
> + if (retval != expected_val &&
> expected_val != POINTER_VALUE) {
> printf("FAIL retval %d != %d ", retval, expected_val);
> return 1;
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] selftests/bpf: Avoid errno clobbering
2020-11-28 19:25 ` [PATCH 1/2] selftests/bpf: Avoid errno clobbering Florian Lehner
@ 2020-12-02 1:24 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2020-12-02 1:24 UTC (permalink / raw)
To: Florian Lehner
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, john fastabend, Stanislav Fomichev,
Krzesimir Nowak
On Sat, Nov 28, 2020 at 11:28 AM Florian Lehner <dev@der-flo.net> wrote:
>
> Commit 8184d44c9a57 ("selftests/bpf: skip verifier tests for unsupported
> program types") added a check to skip unsupported program types. As
> bpf_probe_prog_type can change errno, do_single_test should save it before
> printing a reason why a supported BPF program type failed to load.
>
> Fixes: 8184d44c9a57 ("selftests/bpf: skip verifier tests for unsupported program types")
> Cc: Stanislav Fomichev <sdf@google.com>
> Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
> Signed-off-by: Florian Lehner <dev@der-flo.net>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/testing/selftests/bpf/test_verifier.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 4bfe3aa2cfc4..ceea9409639e 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -936,6 +936,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> int run_errs, run_successes;
> int map_fds[MAX_NR_MAPS];
> const char *expected_err;
> + int saved_errno;
> int fixup_skips;
> __u32 pflags;
> int i, err;
> @@ -997,6 +998,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> }
>
> fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
> + saved_errno = errno;
>
> /* BPF_PROG_TYPE_TRACING requires more setup and
> * bpf_probe_prog_type won't give correct answer
> @@ -1013,7 +1015,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> if (expected_ret == ACCEPT || expected_ret == VERBOSE_ACCEPT) {
> if (fd_prog < 0) {
> printf("FAIL\nFailed to load prog '%s'!\n",
> - strerror(errno));
> + strerror(saved_errno));
> goto fail_log;
> }
> #ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-02 1:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-28 19:25 [PATCH 0/2] Improve error handling of verifier tests Florian Lehner
2020-11-28 19:25 ` [PATCH 1/2] selftests/bpf: Avoid errno clobbering Florian Lehner
2020-12-02 1:24 ` Andrii Nakryiko
2020-11-28 19:25 ` [PATCH 2/2] selftests/bpf: Print reason when a tester could not run a program Florian Lehner
2020-12-02 1:23 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).