Re: [PATCH bpf-next 2/3] selftests/bpf: Integrate vmtest configs

["Daniel Müller" <deso@posteo.net>]

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