* [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
* 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
* [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
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).