Re: [PATCH v1 bpf-next 1/9] bpftool: add testing skeleton

[Manu Bretelle <chantr4@gmail.com>]

I am going to start by apologizing for dropping the ball for so long... I
originally planned to get back to this after thanksgiving holidays... but weeks
snowballed one after the other.

On Mon, Nov 27, 2023 at 10:39:34AM -0800, Andrii Nakryiko wrote:
> On Mon, Nov 27, 2023 at 9:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2023-11-21 19:50 UTC+0000 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > > On Tue, Nov 21, 2023 at 8:42 AM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > >>
> > >> On Tue, Nov 21, 2023 at 8:26 AM Quentin Monnet <quentin@isovalent.com> wrote:
> > >>>
> > >>>>
> > >>>> Does it have to leave in the kernel tree?
> > >>>> We have bpftool on github, maybe it can be there?
> > >>>> Do you want to run bpftool tester as part of BPF CI and that's why
> > >>>> you want it to be in the kernel tree?
> > >>>
> > >>> It doesn't _have_ to be in the kernel tree, although it's a nice place
> > >>> where to have it. We have bpftool on GitHub, but the CI that runs there
> > >>> is triggered only when syncing the mirror to check that mirroring is not
> > >>> broken, so after new patches are applied to bpf-next. If we want this on
> > >>> GitHub, we would rather target the BPF CI infra.
> > >>>
> > >>> A nice point of having it in the repo would be the ability to add tests
> > >>> at the same time as we add features in bpftool, of course.

Indeed, it does not have to live in the tree, while it could be more convenient
as Quentin highlighted, as much as running it on BPF CI we could be just fine
by having it hosted in a separate repo.
People can always have a clone of the repo and use it to validate the behaviour
has not changed, or changed in expected ways, and have a separate PR if tests
are added. Definitely not as convenient, but likely better than nothing.


> > >>
> > >> Sounds nice in theory, but in practice that would mean that
> > >> every bpftool developer adding a new feature would need to learn rust
> > >> to add a corresponding test?
> > >> I suspect devs might object to such a requirement.
> > >> If tester and bpftool are not sync then they can be in separate repos.
> > >
> > > I'm also wondering what Rust and libbpf-rs dependency adds here? It
> > > feels like bash+jq or Python script should be able to "drive" bpftool
> > > CLI and validate output, no?
> >
> > As I understand, one advantage is to get an easy way to tap into
> > libbpf's functions to load the objects we need in order to test the
> > different bpftool features. There are a number of program/map types that
> > we just cannot load with bpftool at this time, so we need to set up the
> > objects we need with another loader. Libbpf-rs allows to do that, and
> > the "cargo test" infra seems like a convenient way to focus on the tests
> > only. Bash+jq wouldn't allow to load objects unsupported by bpftool, for
> > example.

There were a couple of reasons that you correctly enumerated:
- having a built-in test runner (that could have been other languages)
- libbpf-{cargo,rs} was taking care of the machinery with skeleton, lifecycle of
  a BPF program.
- "native access" to access/manipulate BPF objects from the testing language and
  use bpftool as a blackbox.
- caring about writing the test, not a framework to run them.
- convenience of the rust toolchain to manage depedencies.
- the bells and whistles that come with the language that make formatting/linting
  a no-brainer.
- bash+jq would have probably either limited, or getting overly complex/brittle
  beyond basic checks, and hard to maintain as more tests get added.
- python would have been filling this gap, but without native interaction.

aside from that, another motivation that helped with the choice is that I
originally wrote this as a way to validate bpftool was meeting our requirements
internally as we sync and deploy it, and rust is one of the languages that is
supported to run in our internal vm testing framework.

> 
> Can we use veristat to load BPF object files? we might need some
> option to auto-pin programs in some directory or something to keep
> them live long enough, I suppose, but it's totally in our control.
> 

This probably solves the loading part, but we should also be able to do this
with bpftool too.

> >
> > Manu, did you have other reasons in mind?