On Thu, Jul 14, 2022 at 7:04 AM Daniel Müller <deso@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> > > > --- > > > 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
