| Message ID | 20220712212124.3180314-3-deso@posteo.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Maintain selftest configuration in-tree | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On Tue, Jul 12, 2022 at 2:21 PM Daniel Müller <deso@posteo.net> wrote: > > This change integrates the configuration from the vmtest repository [0], > where it is currently used for testing kernel patches into the existing > configuration pulled in with an earlier patch. The result is a super set > of the configs from the two repositories. > > [0]: https://github.com/kernel-patches/vmtest/tree/831ee8eb72ddb7e03babb8f7e050d52a451237aa/travis-ci/vmtest/configs > > Signed-off-by: Daniel Müller <deso@posteo.net> > --- > tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest | 5 +++++ > .../selftests/bpf/configs/denylist/DENYLIST-latest.s390x | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > index 939de574..ddf8a0c5 100644 > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > @@ -4,3 +4,8 @@ stacktrace_build_id_nmi > stacktrace_build_id > task_fd_query_rawtp > varlen > +btf_dump/btf_dump: syntax > +kprobe_multi_test/bench_attach > +core_reloc/enum64val > +core_reloc/size___diff_sz > +core_reloc/type_based___diff_sz I don't think any of these are necessary anymore. Some of them were due to nightly Clang was stale. > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > index e33cab..36574b0 100644 > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > @@ -63,5 +63,6 @@ bpf_cookie # failed to open_and_load program: -524 > xdp_do_redirect # prog_run_max_size unexpected error: -22 (errno 22) > send_signal # intermittently fails to receive signal > select_reuseport # intermittently fails on new s390x setup > +tc_redirect/tc_redirect_dtime # very flaky same for this, yes it's flaky, but this shouldn't be in this list (I'd rather people actually fix the flakiness, of course). These configs should be "known not working" test cases (e.g., like BPF trampoline-based for s390x, that feature is just not implemented). But flaky tests should go here, they should be ideally fixed and not be blessed officially to be ignored. > xdp_synproxy # JIT does not support calling kernel function (kfunc) > unpriv_bpf_disabled # fentry > -- > 2.30.2 >
On Wed, Jul 13, 2022 at 10:07:02PM -0700, Andrii Nakryiko wrote: > On Tue, Jul 12, 2022 at 2:21 PM Daniel Müller <deso@posteo.net> wrote: > > > > This change integrates the configuration from the vmtest repository [0], > > where it is currently used for testing kernel patches into the existing > > configuration pulled in with an earlier patch. The result is a super set > > of the configs from the two repositories. > > > > [0]: https://github.com/kernel-patches/vmtest/tree/831ee8eb72ddb7e03babb8f7e050d52a451237aa/travis-ci/vmtest/configs > > > > Signed-off-by: Daniel Müller <deso@posteo.net> > > --- > > tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest | 5 +++++ > > .../selftests/bpf/configs/denylist/DENYLIST-latest.s390x | 1 + > > 2 files changed, 6 insertions(+) > > > > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > index 939de574..ddf8a0c5 100644 > > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > @@ -4,3 +4,8 @@ stacktrace_build_id_nmi > > stacktrace_build_id > > task_fd_query_rawtp > > varlen > > +btf_dump/btf_dump: syntax > > +kprobe_multi_test/bench_attach > > +core_reloc/enum64val > > +core_reloc/size___diff_sz > > +core_reloc/type_based___diff_sz > > I don't think any of these are necessary anymore. Some of them were > due to nightly Clang was stale. > > > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > index e33cab..36574b0 100644 > > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > @@ -63,5 +63,6 @@ bpf_cookie # failed to open_and_load program: -524 > > xdp_do_redirect # prog_run_max_size unexpected error: -22 (errno 22) > > send_signal # intermittently fails to receive signal > > select_reuseport # intermittently fails on new s390x setup > > +tc_redirect/tc_redirect_dtime # very flaky > > same for this, yes it's flaky, but this shouldn't be in this list (I'd > rather people actually fix the flakiness, of course). These configs > should be "known not working" test cases (e.g., like BPF > trampoline-based for s390x, that feature is just not implemented). But > flaky tests should go here, they should be ideally fixed and not be > blessed officially to be ignored. I can remove this change from the set. But really from my perspective the entire patch set's concern is not with cleaning up any of the lists -- it is about merging and integrating existing configuration from two others repositories into this one, while preserving what has been done and why in a way that can be followed when looking back at repository histories. My observation has been that at least on x86_64, none of the denied tests caused actual failures when run. And yet, that is best cleaned up subsequently if it were for me. Thanks, Daniel
On Thu, Jul 14, 2022 at 7:04 AM Daniel Müller <deso@posteo.net> wrote: > > On Wed, Jul 13, 2022 at 10:07:02PM -0700, Andrii Nakryiko wrote: > > On Tue, Jul 12, 2022 at 2:21 PM Daniel Müller <deso@posteo.net> wrote: > > > > > > This change integrates the configuration from the vmtest repository [0], > > > where it is currently used for testing kernel patches into the existing > > > configuration pulled in with an earlier patch. The result is a super set > > > of the configs from the two repositories. > > > > > > [0]: https://github.com/kernel-patches/vmtest/tree/831ee8eb72ddb7e03babb8f7e050d52a451237aa/travis-ci/vmtest/configs > > > > > > Signed-off-by: Daniel Müller <deso@posteo.net> > > > --- > > > tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest | 5 +++++ > > > .../selftests/bpf/configs/denylist/DENYLIST-latest.s390x | 1 + > > > 2 files changed, 6 insertions(+) > > > > > > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > > index 939de574..ddf8a0c5 100644 > > > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest > > > @@ -4,3 +4,8 @@ stacktrace_build_id_nmi > > > stacktrace_build_id > > > task_fd_query_rawtp > > > varlen > > > +btf_dump/btf_dump: syntax > > > +kprobe_multi_test/bench_attach > > > +core_reloc/enum64val > > > +core_reloc/size___diff_sz > > > +core_reloc/type_based___diff_sz > > > > I don't think any of these are necessary anymore. Some of them were > > due to nightly Clang was stale. > > > > > diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > > index e33cab..36574b0 100644 > > > --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > > +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x > > > @@ -63,5 +63,6 @@ bpf_cookie # failed to open_and_load program: -524 > > > xdp_do_redirect # prog_run_max_size unexpected error: -22 (errno 22) > > > send_signal # intermittently fails to receive signal > > > select_reuseport # intermittently fails on new s390x setup > > > +tc_redirect/tc_redirect_dtime # very flaky > > > > same for this, yes it's flaky, but this shouldn't be in this list (I'd > > rather people actually fix the flakiness, of course). These configs > > should be "known not working" test cases (e.g., like BPF > > trampoline-based for s390x, that feature is just not implemented). But > > flaky tests should go here, they should be ideally fixed and not be > > blessed officially to be ignored. > > I can remove this change from the set. But really from my perspective > the entire patch set's concern is not with cleaning up any of the lists > -- it is about merging and integrating existing configuration from two > others repositories into this one, while preserving what has been done > and why in a way that can be followed when looking back at repository > histories. > My observation has been that at least on x86_64, none of the denied > tests caused actual failures when run. And yet, that is best cleaned up > subsequently if it were for me. My point is that we shouldn't add them to selftests/bpf first just to clean up later. We can leave those custom additions as is in CI repos (either way we need to allow repos to augment "default" configs/lists) and clean that up there. Generally, allow/deny lists in selftests/bpf should be "authoritative" in the sense that we know that those tests are not supposed to work (right now or at all), we can even teach test_progs to ignore those by default (now that denylist is collocated with test_progs). Anything that's flaky shouldn't be added there, flakiness should be eliminated. With those flaky tests I added in libbpf CI I was the only one suffering from them, so sometimes I opted to just blacklist them for my own sanity. But now we should all share this pain and work together on improving tests! ;) > > Thanks, > Daniel
diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest index 939de574..ddf8a0c5 100644 --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest @@ -4,3 +4,8 @@ stacktrace_build_id_nmi stacktrace_build_id task_fd_query_rawtp varlen +btf_dump/btf_dump: syntax +kprobe_multi_test/bench_attach +core_reloc/enum64val +core_reloc/size___diff_sz +core_reloc/type_based___diff_sz diff --git a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x index e33cab..36574b0 100644 --- a/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x +++ b/tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest.s390x @@ -63,5 +63,6 @@ bpf_cookie # failed to open_and_load program: -524 xdp_do_redirect # prog_run_max_size unexpected error: -22 (errno 22) send_signal # intermittently fails to receive signal select_reuseport # intermittently fails on new s390x setup +tc_redirect/tc_redirect_dtime # very flaky xdp_synproxy # JIT does not support calling kernel function (kfunc) unpriv_bpf_disabled # fentry
This change integrates the configuration from the vmtest repository [0], where it is currently used for testing kernel patches into the existing configuration pulled in with an earlier patch. The result is a super set of the configs from the two repositories. [0]: https://github.com/kernel-patches/vmtest/tree/831ee8eb72ddb7e03babb8f7e050d52a451237aa/travis-ci/vmtest/configs Signed-off-by: Daniel Müller <deso@posteo.net> --- tools/testing/selftests/bpf/configs/denylist/DENYLIST-latest | 5 +++++ .../selftests/bpf/configs/denylist/DENYLIST-latest.s390x | 1 + 2 files changed, 6 insertions(+)