[PATCH v1 bpf-next 0/9] bpftool: Add end-to-end testing

[PATCH v1 bpf-next 0/9] bpftool: Add end-to-end testing

[Manu Bretelle]

This series introduce a more ergonomic end-to-end testing for `bpftool`.

While there is already some `bpftool` tests, they are so far shallow
tests, validating features (test_bpftool.py), or validating expectations
by mean of grepping for expected output in payload
(test_bpftool_metadata.sh), which is hard to extend.

`bpftool` being an operational tool, it is important to ensure that it
operates correctly and reliably when needed, e.g when dealing with
operational issues/incidents.
By performing end-to-end validation of `bpftool` functionalities, we can
ensure that no regression is introduced.

To improve testability, and make end-to-end testing easier, this series is
leveraging `libbpf-rs` [0], allowing us to leverage an existing testing
framework [1], and the ability to load and natively interact with bpf
programs, maps, struct_ops... and finally running `bpftool` command and
checking its output, behaviour, against a set of known expectations.

Currently, the series comes with a set of tests that validate basic
operations such as listing maps, and progs, dumping them, and being able
to attribute the pid that loaded/created those. For struct_ops, it tests
that we can list, dump, and also unregister them by id or name.


    test bpftool_tests::run_bpftool ... ok
    test bpftool_tests::run_bpftool_map_dump_id ... ok
    test bpftool_tests::run_bpftool_map_list ... ok
    test bpftool_tests::run_bpftool_map_pids ... ok
    test bpftool_tests::run_bpftool_prog_list ... ok
    test bpftool_tests::run_bpftool_prog_pids ... ok
    test bpftool_tests::run_bpftool_prog_show_id ... ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ... ok
    test bpftool_tests::run_bpftool_struct_ops_dump_name ... ok
    test bpftool_tests::run_bpftool_struct_ops_list ... ok

[0] https://docs.rs/libbpf-rs/latest/libbpf_rs/
[1] https://doc.rust-lang.org/book/ch11-00-testing.html

Manu Bretelle (9):
  bpftool: add testing skeleton
  bpftool: add libbpf-rs dependency and minimal bpf program
  bpftool: open and load bpf object
  bpftool: Add test to verify that pids are associated to maps.
  bpftool: add test for bpftool prog
  bpftool: test that we can dump and read the content of a map
  bpftool: Add struct_ops tests
  bpftool: Add bpftool_tests README.md
  bpftool: Add Makefile to facilitate bpftool_tests usage

 .../selftests/bpf/bpftool_tests/.gitignore    |   3 +
 .../selftests/bpf/bpftool_tests/Cargo.toml    |  14 +
 .../selftests/bpf/bpftool_tests/Makefile      |  22 +
 .../selftests/bpf/bpftool_tests/README.md     |  91 ++++
 .../selftests/bpf/bpftool_tests/build.rs      |  16 +
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  82 ++++
 .../bpf/bpftool_tests/src/bpf/vmlinux.h       |   1 +
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 408 ++++++++++++++++++
 .../selftests/bpf/bpftool_tests/src/main.rs   |   2 +
 9 files changed, 639 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/.gitignore
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Makefile
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/README.md
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/build.rs
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
 create mode 120000 tools/testing/selftests/bpf/bpftool_tests/src/bpf/vmlinux.h
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/main.rs

-- 
2.39.3

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

[Manu Bretelle]

Add minimal cargo project to test bpftool.
An environment variable `BPFTOOL_PATH` can be used to provide the path
to bpftool, defaulting to /usr/sbin/bpftool

    $ cargo check --tests
        Finished dev [unoptimized + debuginfo] target(s) in 0.00s
    $ cargo clippy --tests
        Finished dev [unoptimized + debuginfo] target(s) in 0.05s
    $ BPFTOOL_PATH='../bpftool' cargo test -- --nocapture
        Finished test [unoptimized + debuginfo] target(s) in 0.05s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-172b867215e9364e)

    running 1 test
    Running command "../bpftool" "version"
    test bpftool_tests::run_bpftool ... ok

    test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.00s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../selftests/bpf/bpftool_tests/.gitignore    |  3 +++
 .../selftests/bpf/bpftool_tests/Cargo.toml    |  4 ++++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 20 +++++++++++++++++++
 .../selftests/bpf/bpftool_tests/src/main.rs   |  3 +++
 4 files changed, 30 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/.gitignore
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/main.rs

diff --git a/tools/testing/selftests/bpf/bpftool_tests/.gitignore b/tools/testing/selftests/bpf/bpftool_tests/.gitignore
new file mode 100644
index 000000000000..cf8177c6aabd
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/target/**
+/Cargo.lock
diff --git a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
new file mode 100644
index 000000000000..34df3002003f
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
@@ -0,0 +1,4 @@
+[package]
+name = "bpftool_tests"
+version = "0.1.0"
+edition = "2021"
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
new file mode 100644
index 000000000000..251dbf3861fe
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+use std::process::Command;
+
+const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
+const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";
+
+/// Run a bpftool command and returns the output
+fn run_bpftool_command(args: &[&str]) -> std::process::Output {
+    let mut cmd = Command::new(std::env::var(BPFTOOL_PATH_ENV).unwrap_or(BPFTOOL_PATH.to_string()));
+    cmd.args(args);
+    println!("Running command {:?}", cmd);
+    cmd.output().expect("failed to execute process")
+}
+
+/// Simple test to make sure we can run bpftool
+#[test]
+fn run_bpftool() {
+    let output = run_bpftool_command(&["version"]);
+    assert!(output.status.success());
+}
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/main.rs b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
new file mode 100644
index 000000000000..6b4ffcde7406
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+#[cfg(test)]
+mod bpftool_tests;
-- 
2.39.3

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

[Alexei Starovoitov]

On Thu, Nov 16, 2023 at 11:43 AM Manu Bretelle <chantr4@gmail.com> wrote:
>
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +use std::process::Command;
> +
> +const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
> +const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";
> +
> +/// Run a bpftool command and returns the output
> +fn run_bpftool_command(args: &[&str]) -> std::process::Output {
> +    let mut cmd = Command::new(std::env::var(BPFTOOL_PATH_ENV).unwrap_or(BPFTOOL_PATH.to_string()));
> +    cmd.args(args);
> +    println!("Running command {:?}", cmd);
> +    cmd.output().expect("failed to execute process")
> +}
> +
> +/// Simple test to make sure we can run bpftool
> +#[test]
> +fn run_bpftool() {
> +    let output = run_bpftool_command(&["version"]);
> +    assert!(output.status.success());
> +}
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/main.rs b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
> new file mode 100644
> index 000000000000..6b4ffcde7406
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +#[cfg(test)]
> +mod bpftool_tests;

There is rust in the kernel tree already.
Most of it is #![no_std] and the rest depend on
rust_is_available.sh and the kernel build system.
This rust usage doesn't fit into two existing rust categories afaics.

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?

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

[Quentin Monnet]

2023-11-21 01:38 UTC+0000 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Thu, Nov 16, 2023 at 11:43 AM Manu Bretelle <chantr4@gmail.com> wrote:
>>
>> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +use std::process::Command;
>> +
>> +const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
>> +const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";
>> +
>> +/// Run a bpftool command and returns the output
>> +fn run_bpftool_command(args: &[&str]) -> std::process::Output {
>> +    let mut cmd = Command::new(std::env::var(BPFTOOL_PATH_ENV).unwrap_or(BPFTOOL_PATH.to_string()));
>> +    cmd.args(args);
>> +    println!("Running command {:?}", cmd);
>> +    cmd.output().expect("failed to execute process")
>> +}
>> +
>> +/// Simple test to make sure we can run bpftool
>> +#[test]
>> +fn run_bpftool() {
>> +    let output = run_bpftool_command(&["version"]);
>> +    assert!(output.status.success());
>> +}
>> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/main.rs b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
>> new file mode 100644
>> index 000000000000..6b4ffcde7406
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/main.rs
>> @@ -0,0 +1,3 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +#[cfg(test)]
>> +mod bpftool_tests;
> 
> There is rust in the kernel tree already.
> Most of it is #![no_std] and the rest depend on
> rust_is_available.sh and the kernel build system.
> This rust usage doesn't fit into two existing rust categories afaics.

I haven't followed closely the introduction of Rust in the kernel
repository, so apologies if I'm incorrect. From what I understand,
#![no_std] is to avoid linking against the std-crate, which is necessary
for Rust code that needs to be compiled as part of the kernel or
modules, but is maybe not relevant for something external like a test
suite? As for rust_is_available.sh, we would need a way to determine
whether Rust is available indeed, before plugging these tests into the
Makefile for the BPF selftests.

As far as I'm aware, these would be the first selftests written in Rust
in the repo (other than for the code under rust/). I'm fine having tests
in Rust for bpftool, for what it matters. Whether we want selftests in
Rust in the kernel repo is another thing.

> 
> 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.

Quentin

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

[Alexei Starovoitov]

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.

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.

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

[Andrii Nakryiko]

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.
>
> 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?

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

[Quentin Monnet]

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.
>>
>> 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.

Manu, did you have other reasons in mind?

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

[Andrii Nakryiko]

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.
> >>
> >> 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.

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.

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

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

[Manu Bretelle]

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?

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

[Quentin Monnet]

2023-11-21 16:42 UTC+0000 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> 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.
> 
> 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.

True. I've been hoping the tests would look easy enough that devs could
update them without being particularly versed in Rust, but this is
probably wishful thinking, and prone to getting bugs in the tests.

I don't have a good proposal to address this, so I agree, this is
probably not a reasonable requirement.

> If tester and bpftool are not sync then they can be in separate repos.

Makes sense. I'd like to have the tests in the same repo, but for this
time, let's focus on getting these Rust tests added to the BPF CI infra
instead, if there's no easy way to switch to a more consensual language.
Manu, thoughts?

Quentin

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

[Manu Bretelle]

On Mon, Nov 27, 2023 at 05:07:15PM +0000, Quentin Monnet wrote:
> 2023-11-21 16:42 UTC+0000 ~ Alexei Starovoitov
> <alexei.starovoitov@gmail.com>
> > 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.
> > 
> > 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.
> 
> True. I've been hoping the tests would look easy enough that devs could
> update them without being particularly versed in Rust, but this is
> probably wishful thinking, and prone to getting bugs in the tests.
> 
> I don't have a good proposal to address this, so I agree, this is
> probably not a reasonable requirement.
> 
> > If tester and bpftool are not sync then they can be in separate repos.
> 
> Makes sense. I'd like to have the tests in the same repo, but for this
> time, let's focus on getting these Rust tests added to the BPF CI infra
> instead, if there's no easy way to switch to a more consensual language.
> Manu, thoughts?

I am fine with that, the work I have done cleaning my original code for this
series is (or at least with minimal changes) self-contained.
Having them hosted outside the tree and used is likely better than nothing.
People can still build upon, and experience will help informing if we should
eventually try to merge this back in.


Manu
> 
> Quentin

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

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> Add minimal cargo project to test bpftool.
> An environment variable `BPFTOOL_PATH` can be used to provide the path
> to bpftool, defaulting to /usr/sbin/bpftool
> 
>     $ cargo check --tests
>         Finished dev [unoptimized + debuginfo] target(s) in 0.00s
>     $ cargo clippy --tests
>         Finished dev [unoptimized + debuginfo] target(s) in 0.05s
>     $ BPFTOOL_PATH='../bpftool' cargo test -- --nocapture
>         Finished test [unoptimized + debuginfo] target(s) in 0.05s
>          Running unittests src/main.rs
>     (target/debug/deps/bpftool_tests-172b867215e9364e)
> 
>     running 1 test
>     Running command "../bpftool" "version"
>     test bpftool_tests::run_bpftool ... ok
> 
>     test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
>     out; finished in 0.00s
> 
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
> ---
>  .../selftests/bpf/bpftool_tests/.gitignore    |  3 +++
>  .../selftests/bpf/bpftool_tests/Cargo.toml    |  4 ++++
>  .../bpf/bpftool_tests/src/bpftool_tests.rs    | 20 +++++++++++++++++++
>  .../selftests/bpf/bpftool_tests/src/main.rs   |  3 +++
>  4 files changed, 30 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/.gitignore
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/main.rs
> 
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/.gitignore b/tools/testing/selftests/bpf/bpftool_tests/.gitignore
> new file mode 100644
> index 000000000000..cf8177c6aabd
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/target/**
> +/Cargo.lock
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
> new file mode 100644
> index 000000000000..34df3002003f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
> @@ -0,0 +1,4 @@
> +[package]
> +name = "bpftool_tests"
> +version = "0.1.0"
> +edition = "2021"
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> new file mode 100644
> index 000000000000..251dbf3861fe
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause

Any particular reason for this particular choice of license? If not,
could you please reconsider? All sources for bpftool use (GPL-2.0-only
OR BSD-2-Clause), as you use in the .gitignore above, so it would make
sense to have the related tests with the same licenses. It would
certainly make things easier if someone need to ship the tests along
with the sources in the future.

(Same comment for the other Rust files you add in this commit and the next.)

> +use std::process::Command;
> +
> +const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
> +const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";

This is a decent choice given that it's where the binary will likely end
up for most distributions, but I'd maybe use "/usr/local/sbin/bpftool"
instead to remain consistent with the prefix in bpftool's Makefile, and
default to where we install bpftool when we "make install" it.

[PATCH v1 bpf-next 2/9] bpftool: add libbpf-rs dependency and minimal bpf program

[Manu Bretelle]

Setting up the basic requirements to build a bpf program using
libbpf-rs.

    $ cargo test
       Compiling bpftool_tests v0.1.0
    (/data/users/chantra/bpf-next/tools/bpf/bpftool/testing)
        Finished test [unoptimized + debuginfo] target(s) in 0.64s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-b5112057d979eb52)

    running 1 test
    test bpftool_tests::run_bpftool ... ok

    test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.00s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../selftests/bpf/bpftool_tests/Cargo.toml    |  7 +++++++
 .../selftests/bpf/bpftool_tests/build.rs      | 17 ++++++++++++++++
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c | 20 +++++++++++++++++++
 .../bpf/bpftool_tests/src/bpf/vmlinux.h       |  1 +
 .../bpf/bpftool_tests/src/bpftool_tests.rs    |  4 ++++
 5 files changed, 49 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/build.rs
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
 create mode 120000 tools/testing/selftests/bpf/bpftool_tests/src/bpf/vmlinux.h

diff --git a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
index 34df3002003f..35c834082351 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
+++ b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
@@ -2,3 +2,10 @@
 name = "bpftool_tests"
 version = "0.1.0"
 edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+[dependencies]
+libbpf-rs = "0.21"
+
+[build-dependencies]
+libbpf-cargo = "0.21"
diff --git a/tools/testing/selftests/bpf/bpftool_tests/build.rs b/tools/testing/selftests/bpf/bpftool_tests/build.rs
new file mode 100644
index 000000000000..ce01824fcd1d
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/build.rs
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+use libbpf_cargo::SkeletonBuilder;
+use std::env;
+use std::path::PathBuf;
+
+const SRC: &str = "src/bpf/bpftool_tests.bpf.c";
+
+fn main() {
+    let mut out =
+        PathBuf::from(env::var_os("OUT_DIR").expect("OUT_DIR must be set in build script"));
+    out.push("bpftool_tests.skel.rs");
+    SkeletonBuilder::new()
+        .source(SRC)
+        .build_and_generate(&out)
+        .unwrap();
+    println!("cargo:rerun-if-changed={SRC}");
+}
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
new file mode 100644
index 000000000000..8b92171145de
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+char LICENSE[] SEC("license") = "Dual BSD/GPL";
+
+int my_pid = 0;
+
+SEC("tp/syscalls/sys_enter_write")
+int handle_tp(void *ctx)
+{
+	int pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != my_pid)
+		return 0;
+
+	bpf_printk("BPF triggered from PID %d.\n", pid);
+
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/vmlinux.h b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/vmlinux.h
new file mode 120000
index 000000000000..f9515b260426
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/vmlinux.h
@@ -0,0 +1 @@
+../../../tools/include/vmlinux.h
\ No newline at end of file
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index 251dbf3861fe..35eb35831dce 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -1,4 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+mod bpftool_tests_skel {
+    include!(concat!(env!("OUT_DIR"), "/bpftool_tests.skel.rs"));
+}
+
 use std::process::Command;
 
 const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
-- 
2.39.3

[PATCH v1 bpf-next 3/9] bpftool: open and load bpf object

[Manu Bretelle]

Add an initial test that excercises opening and loading a bpf object.

    BPFTOOL_PATH=../bpftool
    CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
    --nocapture
        Finished test [unoptimized + debuginfo] target(s) in 0.04s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)

    running 2 tests
    Running command "../bpftool" "version"
    Running command "../bpftool" "map" "list" "--json"
    test bpftool_tests::run_bpftool ... ok
    test bpftool_tests::run_bpftool_map_list ... ok

    test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.19s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../selftests/bpf/bpftool_tests/Cargo.toml    |  3 ++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 50 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
index 35c834082351..cc1fa65189f2 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
+++ b/tools/testing/selftests/bpf/bpftool_tests/Cargo.toml
@@ -5,7 +5,10 @@ edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 [dependencies]
+anyhow = "1"
 libbpf-rs = "0.21"
+serde = { version = "1", features = ["derive"] }
+serde_json = "1"
 
 [build-dependencies]
 libbpf-cargo = "0.21"
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index 35eb35831dce..fb58898a4a1a 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -3,11 +3,49 @@ mod bpftool_tests_skel {
     include!(concat!(env!("OUT_DIR"), "/bpftool_tests.skel.rs"));
 }
 
+use anyhow::Result;
+use bpftool_tests_skel::BpftoolTestsSkel;
+use bpftool_tests_skel::BpftoolTestsSkelBuilder;
+use libbpf_rs::skel::OpenSkel;
+use libbpf_rs::skel::SkelBuilder;
+use serde::Deserialize;
+use serde::Serialize;
+
 use std::process::Command;
 
 const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
 const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";
 
+/// A struct representing a pid entry from map/prog dump
+#[derive(Serialize, Deserialize, Debug)]
+struct Pid {
+    comm: String,
+    pid: u64,
+}
+
+/// A struct representing a map entry from `bpftool map list -j`
+#[derive(Serialize, Deserialize, Debug)]
+struct Map {
+    name: Option<String>,
+    id: u64,
+    r#type: String,
+    #[serde(default)]
+    pids: Vec<Pid>,
+}
+
+/// Setup our bpftool_tests.bpf.c program.
+/// Open and load and return an opened object.
+fn setup() -> Result<BpftoolTestsSkel<'static>> {
+    let mut skel_builder = BpftoolTestsSkelBuilder::default();
+    skel_builder.obj_builder.debug(false);
+
+    let open_skel = skel_builder.open()?;
+
+    let skel = open_skel.load()?;
+
+    Ok(skel)
+}
+
 /// Run a bpftool command and returns the output
 fn run_bpftool_command(args: &[&str]) -> std::process::Output {
     let mut cmd = Command::new(std::env::var(BPFTOOL_PATH_ENV).unwrap_or(BPFTOOL_PATH.to_string()));
@@ -22,3 +60,15 @@ fn run_bpftool() {
     let output = run_bpftool_command(&["version"]);
     assert!(output.status.success());
 }
+
+/// A test to validate that we can list maps using bpftool
+#[test]
+fn run_bpftool_map_list() {
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&["map", "list", "--json"]);
+
+    let maps = serde_json::from_slice::<Vec<Map>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(output.status.success(), "bpftool returned an error.");
+    assert!(!maps.is_empty(), "No maps were listed");
+}
-- 
2.39.3

[PATCH v1 bpf-next 4/9] bpftool: Add test to verify that pids are associated to maps.

[Manu Bretelle]

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  7 +++++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 29 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
index 8b92171145de..dbd4e2aad277 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
@@ -4,6 +4,13 @@
 
 char LICENSE[] SEC("license") = "Dual BSD/GPL";
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 10240);
+	__type(key, u32);
+	__type(value, u64);
+} pid_write_calls SEC(".maps");
+
 int my_pid = 0;
 
 SEC("tp/syscalls/sys_enter_write")
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index fb58898a4a1a..a832b255e988 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -72,3 +72,32 @@ fn run_bpftool_map_list() {
     assert!(output.status.success(), "bpftool returned an error.");
     assert!(!maps.is_empty(), "No maps were listed");
 }
+
+/// A test to validate that we can find PIDs associated with a map
+#[test]
+fn run_bpftool_map_pids() {
+    let map_name = "pid_write_calls";
+
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&["map", "list", "--json"]);
+
+    let maps = serde_json::from_slice::<Vec<Map>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    // `pid_write_calls` is a map our bpftool_tests.bpf.c uses. It should have at least
+    // one entry for our current process.
+    let map = maps
+        .iter()
+        .find(|m| m.name.is_some() && m.name.as_ref().unwrap() == map_name)
+        .unwrap_or_else(|| panic!("Did not find {} map", map_name));
+
+    let mypid = std::process::id() as u64;
+    assert!(
+        map.pids.iter().any(|p| p.pid == mypid),
+        "Did not find test runner pid ({}) in pids list associated with map *{}*: {:?}",
+        mypid,
+        map_name,
+        map.pids
+    );
+}
-- 
2.39.3

Re: [PATCH v1 bpf-next 4/9] bpftool: Add test to verify that pids are associated to maps.

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>

I'd keep at least the command to invoke the test in the commit
description, in case someone only looks at this commit log and wonders
how to launch the test.

Also this test is not supposed to succeed with all versions of bpftool
(we need support for skeletons in bpftool for dumping the PIDs). So I'd
like to have either 1) a check on "bpftool version" to make sure that
the required feature is present, and skip the test otherwise, or 2) an
option in the Cargo.toml to enforce or skip this test.

From what I understand, you're using this test to make sure that PIDs
are present, to avoid relying on the output from "bpftool version" only;
so maybe option 2 makes more sense, a parameter to tell cargo to run the
test and expect the PIDs to be present, whatever "bpftool version" says?

Quentin

[PATCH v1 bpf-next 5/9] bpftool: add test for bpftool prog

[Manu Bretelle]

Add tests that validate `bpftool prog list` functions, that we can
associate pids with them, and that we can show a prog by id.

    $ BPFTOOL_PATH=../bpftool
    CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
    --nocapture
        Finished test [unoptimized + debuginfo] target(s) in 0.05s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)

    running 6 tests
    Running command "../bpftool" "version"
    test bpftool_tests::run_bpftool ... ok
    Running command "../bpftool" "map" "list" "--json"
    Running command "../bpftool" "prog" "list" "--json"
    Running command "../bpftool" "prog" "list" "--json"
    Running command "../bpftool" "map" "list" "--json"
    Running command "../bpftool" "prog" "show" "id" "3160417" "--json"
    test bpftool_tests::run_bpftool_prog_show_id ... ok
    test bpftool_tests::run_bpftool_map_pids ... ok
    test bpftool_tests::run_bpftool_map_list ... ok
    test bpftool_tests::run_bpftool_prog_pids ... ok
    test bpftool_tests::run_bpftool_prog_list ... ok

    test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.20s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  2 +-
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 79 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
index dbd4e2aad277..e2b18dd36207 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
@@ -14,7 +14,7 @@ struct {
 int my_pid = 0;
 
 SEC("tp/syscalls/sys_enter_write")
-int handle_tp(void *ctx)
+int handle_tp_sys_enter_write(void *ctx)
 {
 	int pid = bpf_get_current_pid_tgid() >> 32;
 
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index a832b255e988..695a1cbc5be8 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -8,9 +8,10 @@ mod bpftool_tests_skel {
 use bpftool_tests_skel::BpftoolTestsSkelBuilder;
 use libbpf_rs::skel::OpenSkel;
 use libbpf_rs::skel::SkelBuilder;
+use libbpf_rs::Program;
 use serde::Deserialize;
 use serde::Serialize;
-
+use std::os::fd::AsFd;
 use std::process::Command;
 
 const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
@@ -23,6 +24,17 @@ struct Pid {
     pid: u64,
 }
 
+/// A struct representing a prog entry from `bpftool prog list -j`
+#[derive(Serialize, Deserialize, Debug)]
+struct Prog {
+    name: Option<String>,
+    id: u32,
+    r#type: String,
+    tag: String,
+    #[serde(default)]
+    pids: Vec<Pid>,
+}
+
 /// A struct representing a map entry from `bpftool map list -j`
 #[derive(Serialize, Deserialize, Debug)]
 struct Map {
@@ -101,3 +113,68 @@ fn run_bpftool_map_pids() {
         map.pids
     );
 }
+
+/// A test to validate that we can list programs using bpftool
+#[test]
+fn run_bpftool_prog_list() {
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&["prog", "list", "--json"]);
+
+    let progs = serde_json::from_slice::<Vec<Prog>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(output.status.success());
+    assert!(!progs.is_empty(), "No programs were listed");
+}
+
+/// A test to validate that we can find PIDs associated with a program
+#[test]
+fn run_bpftool_prog_pids() {
+    let hook_name = "handle_tp_sys_enter_write";
+
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&["prog", "list", "--json"]);
+
+    let progs = serde_json::from_slice::<Vec<Prog>>(&output.stdout).expect("Failed to parse JSON");
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    // `handle_tp_sys_enter_write` is a hook our bpftool_tests.bpf.c attaches
+    // to (tp/syscalls/sys_enter_write).
+    // It should have at least one entry for our current process.
+    let prog = progs
+        .iter()
+        .find(|m| m.name.is_some() && m.name.as_ref().unwrap() == hook_name)
+        .unwrap_or_else(|| panic!("Did not find {} prog", hook_name));
+
+    let mypid = std::process::id() as u64;
+    assert!(
+        prog.pids.iter().any(|p| p.pid == mypid),
+        "Did not find test runner pid ({}) in pids list associated with prog *{}*: {:?}",
+        mypid,
+        hook_name,
+        prog.pids
+    );
+}
+
+/// A test to validate that we can run `bpftool prog show <id>`
+/// an extract the expected information.
+#[test]
+fn run_bpftool_prog_show_id() {
+    let skel = setup().expect("Failed to set up BPF program");
+    let binding = skel.progs();
+    let handle_tp_sys_enter_write = binding.handle_tp_sys_enter_write();
+    let prog_id =
+        Program::get_id_by_fd(handle_tp_sys_enter_write.as_fd()).expect("Failed to get prog ID");
+
+    let output = run_bpftool_command(&["prog", "show", "id", &prog_id.to_string(), "--json"]);
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    let prog = serde_json::from_slice::<Prog>(&output.stdout).expect("Failed to parse JSON");
+
+    assert_eq!(prog_id, prog.id);
+    assert_eq!(
+        handle_tp_sys_enter_write.name(),
+        prog.name
+            .expect("Program handle_tp_sys_enter_write has no name")
+    );
+    assert_eq!("tracepoint", prog.r#type);
+}
-- 
2.39.3

Re: [PATCH v1 bpf-next 5/9] bpftool: add test for bpftool prog

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> Add tests that validate `bpftool prog list` functions, that we can
> associate pids with them, and that we can show a prog by id.
> 
>     $ BPFTOOL_PATH=../bpftool
>     CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
>     --nocapture
>         Finished test [unoptimized + debuginfo] target(s) in 0.05s
>          Running unittests src/main.rs
>     (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
> 
>     running 6 tests
>     Running command "../bpftool" "version"
>     test bpftool_tests::run_bpftool ... ok
>     Running command "../bpftool" "map" "list" "--json"
>     Running command "../bpftool" "prog" "list" "--json"
>     Running command "../bpftool" "prog" "list" "--json"
>     Running command "../bpftool" "map" "list" "--json"
>     Running command "../bpftool" "prog" "show" "id" "3160417" "--json"
>     test bpftool_tests::run_bpftool_prog_show_id ... ok
>     test bpftool_tests::run_bpftool_map_pids ... ok
>     test bpftool_tests::run_bpftool_map_list ... ok
>     test bpftool_tests::run_bpftool_prog_pids ... ok
>     test bpftool_tests::run_bpftool_prog_list ... ok
> 
>     test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
>     out; finished in 0.20s
> 
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
> ---
>  .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  2 +-
>  .../bpf/bpftool_tests/src/bpftool_tests.rs    | 79 ++++++++++++++++++-
>  2 files changed, 79 insertions(+), 2 deletions(-)
> 

> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> index a832b255e988..695a1cbc5be8 100644
> --- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs

> @@ -101,3 +113,68 @@ fn run_bpftool_map_pids() {
>          map.pids
>      );
>  }
> +
> +/// A test to validate that we can list programs using bpftool
> +#[test]
> +fn run_bpftool_prog_list() {
> +    let _skel = setup().expect("Failed to set up BPF program");
> +    let output = run_bpftool_command(&["prog", "list", "--json"]);
> +
> +    let progs = serde_json::from_slice::<Vec<Prog>>(&output.stdout).expect("Failed to parse JSON");
> +
> +    assert!(output.status.success());
> +    assert!(!progs.is_empty(), "No programs were listed");
> +}
> +
> +/// A test to validate that we can find PIDs associated with a program
> +#[test]
> +fn run_bpftool_prog_pids() {
> +    let hook_name = "handle_tp_sys_enter_write";
> +
> +    let _skel = setup().expect("Failed to set up BPF program");
> +    let output = run_bpftool_command(&["prog", "list", "--json"]);
> +
> +    let progs = serde_json::from_slice::<Vec<Prog>>(&output.stdout).expect("Failed to parse JSON");
> +    assert!(output.status.success(), "bpftool returned an error.");
> +
> +    // `handle_tp_sys_enter_write` is a hook our bpftool_tests.bpf.c attaches
> +    // to (tp/syscalls/sys_enter_write).
> +    // It should have at least one entry for our current process.
> +    let prog = progs
> +        .iter()
> +        .find(|m| m.name.is_some() && m.name.as_ref().unwrap() == hook_name)
> +        .unwrap_or_else(|| panic!("Did not find {} prog", hook_name));
> +
> +    let mypid = std::process::id() as u64;
> +    assert!(
> +        prog.pids.iter().any(|p| p.pid == mypid),
> +        "Did not find test runner pid ({}) in pids list associated with prog *{}*: {:?}",
> +        mypid,
> +        hook_name,
> +        prog.pids
> +    );
> +}
> +
> +/// A test to validate that we can run `bpftool prog show <id>`
> +/// an extract the expected information.

s/an/and/

> +#[test]
> +fn run_bpftool_prog_show_id() {
> +    let skel = setup().expect("Failed to set up BPF program");
> +    let binding = skel.progs();
> +    let handle_tp_sys_enter_write = binding.handle_tp_sys_enter_write();
> +    let prog_id =
> +        Program::get_id_by_fd(handle_tp_sys_enter_write.as_fd()).expect("Failed to get prog ID");
> +
> +    let output = run_bpftool_command(&["prog", "show", "id", &prog_id.to_string(), "--json"]);
> +    assert!(output.status.success(), "bpftool returned an error.");
> +
> +    let prog = serde_json::from_slice::<Prog>(&output.stdout).expect("Failed to parse JSON");
> +
> +    assert_eq!(prog_id, prog.id);
> +    assert_eq!(
> +        handle_tp_sys_enter_write.name(),
> +        prog.name
> +            .expect("Program handle_tp_sys_enter_write has no name")
> +    );
> +    assert_eq!("tracepoint", prog.r#type);
> +}

[PATCH v1 bpf-next 6/9] bpftool: test that we can dump and read the content of a map

[Manu Bretelle]

This test adds a hardcoded key/value pair to a test map and verify
that bpftool is able to dump its content and read/format it.

    $ BPFTOOL_PATH=../bpftool
    CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
    --nocapture
        Finished test [unoptimized + debuginfo] target(s) in 0.05s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)

    running 7 tests
    Running command "../bpftool" "version"
    test bpftool_tests::run_bpftool ... ok
    Running command "../bpftool" "map" "dump" "id" "1848713" "--json"
    Running command "../bpftool" "prog" "list" "--json"
    Running command "../bpftool" "map" "list" "--json"
    Running command "../bpftool" "prog" "list" "--json"
    Running command "../bpftool" "prog" "show" "id" "3160759" "--json"
    Running command "../bpftool" "map" "list" "--json"
    test bpftool_tests::run_bpftool_map_dump_id ... ok
    test bpftool_tests::run_bpftool_prog_show_id ... ok
    test bpftool_tests::run_bpftool_map_pids ... ok
    test bpftool_tests::run_bpftool_prog_pids ... ok
    test bpftool_tests::run_bpftool_prog_list ... ok
    test bpftool_tests::run_bpftool_map_list ... ok

    test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.22s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  7 ++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 86 +++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
index e2b18dd36207..a90c8921b4ee 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
@@ -11,6 +11,13 @@ struct {
 	__type(value, u64);
 } pid_write_calls SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 10);
+	__type(key, char[6]);
+	__type(value, char[6]);
+} bpftool_test_map SEC(".maps");
+
 int my_pid = 0;
 
 SEC("tp/syscalls/sys_enter_write")
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index 695a1cbc5be8..90187152c1d1 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -45,6 +45,37 @@ struct Map {
     pids: Vec<Pid>,
 }
 
+/// A struct representing a formatted map entry from `bpftool map dump -j`
+#[derive(Serialize, Deserialize, Debug)]
+struct FormattedMapItem {
+    key: String,
+    value: String,
+}
+
+type MapVecString = Vec<String>;
+
+/// A struct representing a map entry from `bpftool map dump -j`
+#[derive(Serialize, Deserialize, Debug)]
+struct MapItem {
+    key: MapVecString,
+    value: MapVecString,
+    formatted: Option<FormattedMapItem>,
+}
+
+/// A helper function to convert a vector of strings as returned by bpftool
+/// into a vector of bytes.
+/// bpftool returns key/value in the form of a sequence of strings
+/// hexadecimal numbers. We need to convert them back to bytes.
+/// for instance, the value of the key "key" is represented as ["0x6b","0x65","0x79"]
+fn to_vec_u8(m: &MapVecString) -> Vec<u8> {
+    m.iter()
+        .map(|s| {
+            u8::from_str_radix(s.trim_start_matches("0x"), 16)
+                .unwrap_or_else(|_| panic!("Failed to parse {:?}", s))
+        })
+        .collect()
+}
+
 /// Setup our bpftool_tests.bpf.c program.
 /// Open and load and return an opened object.
 fn setup() -> Result<BpftoolTestsSkel<'static>> {
@@ -114,6 +145,61 @@ fn run_bpftool_map_pids() {
     );
 }
 
+/// A test to validate that we can run `bpftool map dump <id>`
+/// and extract the expected information.
+/// The test adds a key, value pair to the map and then dumps it.
+/// The test validates that the dumped data matches what was added.
+/// It also validate that bpftool was able to format the key/value pairs.
+#[test]
+fn run_bpftool_map_dump_id() {
+    // By having key/value null terminated, we can check that bpftool also returns the
+    // formatted content.
+    let key = b"key\0\0\0";
+    let value = b"value\0";
+    let skel = setup().expect("Failed to set up BPF program");
+    let binding = skel.maps();
+    let bpftool_test_map_map = binding.bpftool_test_map();
+    bpftool_test_map_map
+        .update(key, value, libbpf_rs::MapFlags::NO_EXIST)
+        .expect("Failed to update map");
+    let map_id = bpftool_test_map_map
+        .info()
+        .expect("Failed to get map info")
+        .info
+        .id;
+
+    let output = run_bpftool_command(&["map", "dump", "id", &map_id.to_string(), "--json"]);
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    let items =
+        serde_json::from_slice::<Vec<MapItem>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert_eq!(items.len(), 1);
+
+    let item = items.first().expect("Expected a map item");
+    assert_eq!(to_vec_u8(&item.key), key);
+    assert_eq!(to_vec_u8(&item.value), value);
+
+    // Validate "formatted" values.
+    // The keys and values are null terminated so we need to trim them before comparing.
+    let formatted = item
+        .formatted
+        .as_ref()
+        .expect("Formatted values are missing");
+    assert_eq!(
+        formatted.key,
+        std::str::from_utf8(key)
+            .expect("Invalid UTF-8")
+            .trim_end_matches('\0'),
+    );
+    assert_eq!(
+        formatted.value,
+        std::str::from_utf8(value)
+            .expect("Invalid UTF-8")
+            .trim_end_matches('\0'),
+    );
+}
+
 /// A test to validate that we can list programs using bpftool
 #[test]
 fn run_bpftool_prog_list() {
-- 
2.39.3

Re: [PATCH v1 bpf-next 6/9] bpftool: test that we can dump and read the content of a map

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> This test adds a hardcoded key/value pair to a test map and verify
> that bpftool is able to dump its content and read/format it.
> 
>     $ BPFTOOL_PATH=../bpftool
>     CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
>     --nocapture
>         Finished test [unoptimized + debuginfo] target(s) in 0.05s
>          Running unittests src/main.rs
>     (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
> 
>     running 7 tests
>     Running command "../bpftool" "version"
>     test bpftool_tests::run_bpftool ... ok
>     Running command "../bpftool" "map" "dump" "id" "1848713" "--json"
>     Running command "../bpftool" "prog" "list" "--json"
>     Running command "../bpftool" "map" "list" "--json"
>     Running command "../bpftool" "prog" "list" "--json"
>     Running command "../bpftool" "prog" "show" "id" "3160759" "--json"
>     Running command "../bpftool" "map" "list" "--json"
>     test bpftool_tests::run_bpftool_map_dump_id ... ok
>     test bpftool_tests::run_bpftool_prog_show_id ... ok
>     test bpftool_tests::run_bpftool_map_pids ... ok
>     test bpftool_tests::run_bpftool_prog_pids ... ok
>     test bpftool_tests::run_bpftool_prog_list ... ok
>     test bpftool_tests::run_bpftool_map_list ... ok
> 
>     test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
>     out; finished in 0.22s
> 
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
> ---
>  .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  7 ++
>  .../bpf/bpftool_tests/src/bpftool_tests.rs    | 86 +++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
> index e2b18dd36207..a90c8921b4ee 100644
> --- a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
> @@ -11,6 +11,13 @@ struct {
>  	__type(value, u64);
>  } pid_write_calls SEC(".maps");
>  
> +struct {
> +	__uint(type, BPF_MAP_TYPE_HASH);
> +	__uint(max_entries, 10);
> +	__type(key, char[6]);
> +	__type(value, char[6]);
> +} bpftool_test_map SEC(".maps");
> +
>  int my_pid = 0;
>  
>  SEC("tp/syscalls/sys_enter_write")
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> index 695a1cbc5be8..90187152c1d1 100644
> --- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> +++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
> @@ -45,6 +45,37 @@ struct Map {
>      pids: Vec<Pid>,
>  }
>  
> +/// A struct representing a formatted map entry from `bpftool map dump -j`
> +#[derive(Serialize, Deserialize, Debug)]
> +struct FormattedMapItem {
> +    key: String,
> +    value: String,
> +}
> +
> +type MapVecString = Vec<String>;
> +
> +/// A struct representing a map entry from `bpftool map dump -j`
> +#[derive(Serialize, Deserialize, Debug)]
> +struct MapItem {
> +    key: MapVecString,
> +    value: MapVecString,
> +    formatted: Option<FormattedMapItem>,
> +}
> +
> +/// A helper function to convert a vector of strings as returned by bpftool
> +/// into a vector of bytes.
> +/// bpftool returns key/value in the form of a sequence of strings
> +/// hexadecimal numbers. We need to convert them back to bytes.

So sorry about that.

> +/// for instance, the value of the key "key" is represented as ["0x6b","0x65","0x79"]

s/for/For/

> +fn to_vec_u8(m: &MapVecString) -> Vec<u8> {
> +    m.iter()
> +        .map(|s| {
> +            u8::from_str_radix(s.trim_start_matches("0x"), 16)
> +                .unwrap_or_else(|_| panic!("Failed to parse {:?}", s))
> +        })
> +        .collect()
> +}
> +
>  /// Setup our bpftool_tests.bpf.c program.
>  /// Open and load and return an opened object.
>  fn setup() -> Result<BpftoolTestsSkel<'static>> {
> @@ -114,6 +145,61 @@ fn run_bpftool_map_pids() {
>      );
>  }
>  
> +/// A test to validate that we can run `bpftool map dump <id>`
> +/// and extract the expected information.
> +/// The test adds a key, value pair to the map and then dumps it.
> +/// The test validates that the dumped data matches what was added.
> +/// It also validate that bpftool was able to format the key/value pairs.

s/validate/validates/
s/pairs/pair/

> +#[test]
> +fn run_bpftool_map_dump_id() {
> +    // By having key/value null terminated, we can check that bpftool also returns the
> +    // formatted content.
> +    let key = b"key\0\0\0";
> +    let value = b"value\0";
> +    let skel = setup().expect("Failed to set up BPF program");
> +    let binding = skel.maps();
> +    let bpftool_test_map_map = binding.bpftool_test_map();
> +    bpftool_test_map_map
> +        .update(key, value, libbpf_rs::MapFlags::NO_EXIST)
> +        .expect("Failed to update map");
> +    let map_id = bpftool_test_map_map
> +        .info()
> +        .expect("Failed to get map info")
> +        .info
> +        .id;
> +
> +    let output = run_bpftool_command(&["map", "dump", "id", &map_id.to_string(), "--json"]);
> +    assert!(output.status.success(), "bpftool returned an error.");
> +
> +    let items =
> +        serde_json::from_slice::<Vec<MapItem>>(&output.stdout).expect("Failed to parse JSON");
> +
> +    assert_eq!(items.len(), 1);
> +
> +    let item = items.first().expect("Expected a map item");
> +    assert_eq!(to_vec_u8(&item.key), key);
> +    assert_eq!(to_vec_u8(&item.value), value);
> +
> +    // Validate "formatted" values.
> +    // The keys and values are null terminated so we need to trim them before comparing.

key and value, singular

> +    let formatted = item
> +        .formatted
> +        .as_ref()
> +        .expect("Formatted values are missing");
> +    assert_eq!(
> +        formatted.key,
> +        std::str::from_utf8(key)
> +            .expect("Invalid UTF-8")

Do we need to check the UTF-8 encoding for the strings we provide in the
test?

> +            .trim_end_matches('\0'),
> +    );
> +    assert_eq!(
> +        formatted.value,
> +        std::str::from_utf8(value)
> +            .expect("Invalid UTF-8")
> +            .trim_end_matches('\0'),
> +    );
> +}
> +
>  /// A test to validate that we can list programs using bpftool
>  #[test]
>  fn run_bpftool_prog_list() {

[PATCH v1 bpf-next 7/9] bpftool: Add struct_ops tests

[Manu Bretelle]

Add test to verify that we can:
- unregister a struct_ops by id and name
- list struct_ops
- dump a struct_ops by name

    $ RUST_TEST_THREADS=1 BPFTOOL_PATH=../tools/build/bpftool/bpftool
    CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" cargo test --
    --nocapture
        Finished test [unoptimized + debuginfo] target(s) in 0.05s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)

    running 11 tests
    test bpftool_tests::run_bpftool ... Running command
    "../tools/build/bpftool/bpftool" "version"
    ok
    test bpftool_tests::run_bpftool_map_dump_id ... Running command
    "../tools/build/bpftool/bpftool" "map" "dump" "id" "1851884" "--json"
    ok
    test bpftool_tests::run_bpftool_map_list ... Running command
    "../tools/build/bpftool/bpftool" "map" "list" "--json"
    ok
    test bpftool_tests::run_bpftool_map_pids ... Running command
    "../tools/build/bpftool/bpftool" "map" "list" "--json"
    ok
    test bpftool_tests::run_bpftool_prog_list ... Running command
    "../tools/build/bpftool/bpftool" "prog" "list" "--json"
    ok
    test bpftool_tests::run_bpftool_prog_pids ... Running command
    "../tools/build/bpftool/bpftool" "prog" "list" "--json"
    ok
    test bpftool_tests::run_bpftool_prog_show_id ... Running command
    "../tools/build/bpftool/bpftool" "prog" "show" "id" "3166535" "--json"
    ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... Running
    command "../tools/build/bpftool/bpftool" "struct_ops" "dump" "name"
    "bt_e2e_tco" "--pretty"
    Running command "../tools/build/bpftool/bpftool" "struct_ops"
    "unregister" "id" "1851939"
    ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ...
    Running command "../tools/build/bpftool/bpftool" "struct_ops" "dump"
    "name" "bt_e2e_tco" "--pretty"
    Running command "../tools/build/bpftool/bpftool" "struct_ops"
    "unregister" "name" "bt_e2e_tco"
    ok
    test bpftool_tests::run_bpftool_struct_ops_dump_name ... Running command
    "../tools/build/bpftool/bpftool" "struct_ops" "dump" "name" "bt_e2e_tco"
    "--pretty"
    ok
    test bpftool_tests::run_bpftool_struct_ops_list ... Running command
    "../tools/build/bpftool/bpftool" "struct_ops" "list" "--json"
    ok

    test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 1.84s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  55 +++++++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 145 ++++++++++++++++++
 2 files changed, 200 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
index a90c8921b4ee..c59df9d947ed 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpf/bpftool_tests.bpf.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
 
 char LICENSE[] SEC("license") = "Dual BSD/GPL";
 
@@ -32,3 +34,56 @@ int handle_tp_sys_enter_write(void *ctx)
 
 	return 0;
 }
+
+// struct_ops example
+
+#define BPF_STRUCT_OPS(name, args...)	\
+	SEC("struct_ops/" #name)			\
+	BPF_PROG(name, args)
+
+void BPF_STRUCT_OPS(tcp_init, struct sock *sk)
+{
+	return;
+}
+
+void BPF_STRUCT_OPS(in_ack_event, struct sock *sk, __u32 flags)
+{
+	return;
+}
+
+__u32 BPF_STRUCT_OPS(ssthresh, struct sock *sk)
+{
+	return 0;
+}
+
+void BPF_STRUCT_OPS(set_state, struct sock *sk, __u8 new_state)
+{
+	return;
+}
+
+void BPF_STRUCT_OPS(cwnd_event, struct sock *sk, enum tcp_ca_event ev)
+{
+	return;
+}
+
+__u32 BPF_STRUCT_OPS(cwnd_undo, struct sock *sk)
+{
+	return 0;
+}
+
+void BPF_STRUCT_OPS(cong_avoid, struct sock *sk, __u32 ack, __u32 acked)
+{
+	return;
+}
+
+SEC(".struct_ops")
+struct tcp_congestion_ops bt_e2e_tco = {
+	.init = (void *)tcp_init,
+	.in_ack_event = (void *)in_ack_event,
+	.cwnd_event = (void *)cwnd_event,
+	.ssthresh = (void *)ssthresh,
+	.cong_avoid = (void *)cong_avoid,
+	.undo_cwnd = (void *)cwnd_undo,
+	.set_state = (void *)set_state,
+	.name = "bt_e2e_tco",
+};
diff --git a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
index 90187152c1d1..ecb9e92d7bac 100644
--- a/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
+++ b/tools/testing/selftests/bpf/bpftool_tests/src/bpftool_tests.rs
@@ -7,6 +7,7 @@ mod bpftool_tests_skel {
 use bpftool_tests_skel::BpftoolTestsSkel;
 use bpftool_tests_skel::BpftoolTestsSkelBuilder;
 use libbpf_rs::skel::OpenSkel;
+use libbpf_rs::skel::Skel;
 use libbpf_rs::skel::SkelBuilder;
 use libbpf_rs::Program;
 use serde::Deserialize;
@@ -16,6 +17,7 @@ mod bpftool_tests_skel {
 
 const BPFTOOL_PATH_ENV: &str = "BPFTOOL_PATH";
 const BPFTOOL_PATH: &str = "/usr/sbin/bpftool";
+const STRUCT_OPS_MAP_NAME: &str = "bt_e2e_tco";
 
 /// A struct representing a pid entry from map/prog dump
 #[derive(Serialize, Deserialize, Debug)]
@@ -62,6 +64,34 @@ struct MapItem {
     formatted: Option<FormattedMapItem>,
 }
 
+/// A struct representing the map info from `bpftool struct_ops dump id <id>`
+#[derive(Serialize, Deserialize, Debug)]
+struct MapInfo {
+    name: String,
+    id: u64,
+}
+
+/// A struct representing a struct_ops entry from `bpftool struct_ops list -j`
+#[derive(Serialize, Deserialize, Debug)]
+struct StructOps {
+    name: String,
+    id: u64,
+    kernel_struct_ops: String,
+}
+
+/// A struct representing a struct_ops entry from `bpftool struct_ops dump id <id>`
+#[derive(Serialize, Deserialize, Debug)]
+struct StructOpsCongestion {}
+
+/// A struct representing a struct_ops entry from `bpftool struct_ops dump id <id>`
+/// The returned json seems to be a tuple of two elements.
+/// the first one being a MapInfo, the second one being a StructOpsCongestion.
+#[derive(Serialize, Deserialize, Debug)]
+struct StructOpsDump {
+    bpf_map_info: Option<MapInfo>,
+    bpf_struct_ops_tcp_congestion_ops: Option<StructOpsCongestion>,
+}
+
 /// A helper function to convert a vector of strings as returned by bpftool
 /// into a vector of bytes.
 /// bpftool returns key/value in the form of a sequence of strings
@@ -264,3 +294,118 @@ fn run_bpftool_prog_show_id() {
     );
     assert_eq!("tracepoint", prog.r#type);
 }
+
+/// A test to validate that we can list struct_ops using bpftool
+#[test]
+fn run_bpftool_struct_ops_list() {
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&["struct_ops", "list", "--json"]);
+
+    let maps =
+        serde_json::from_slice::<Vec<StructOps>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(output.status.success(), "bpftool returned an error.");
+    assert!(!maps.is_empty(), "No maps were listed");
+}
+
+/// A test to validate that we can dump a struct_ops program using its name
+#[test]
+fn run_bpftool_struct_ops_dump_name() {
+    let _skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&[
+        "struct_ops",
+        "dump",
+        "name",
+        STRUCT_OPS_MAP_NAME,
+        "--pretty",
+    ]);
+
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    let struct_ops =
+        serde_json::from_slice::<Vec<StructOpsDump>>(&output.stdout).expect("Failed to parse JSON");
+    assert!(!struct_ops.is_empty(), "No struct_ops were found");
+
+    assert_eq!(
+        struct_ops[0]
+            .bpf_map_info
+            .as_ref()
+            .expect("Missing bpf_map_info field")
+            .name,
+        STRUCT_OPS_MAP_NAME
+    );
+}
+
+/// A test to validate that we can unregister a struct_ops using its id
+#[test]
+fn run_bpftool_struct_ops_can_unregister_id() {
+    let skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&[
+        "struct_ops",
+        "dump",
+        "name",
+        STRUCT_OPS_MAP_NAME,
+        "--pretty",
+    ]);
+
+    let _link = skel
+        .object()
+        .map(STRUCT_OPS_MAP_NAME)
+        .unwrap_or_else(|| panic!("Could not find map {}", STRUCT_OPS_MAP_NAME))
+        .attach_struct_ops()
+        .expect("Could not attach to struct_ops");
+
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    let struct_ops =
+        serde_json::from_slice::<Vec<StructOpsDump>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(!struct_ops.is_empty(), "No struct_ops were found");
+
+    let id = struct_ops[0]
+        .bpf_map_info
+        .as_ref()
+        .expect("Missing bpf_map_info field")
+        .id;
+
+    let output = run_bpftool_command(&["struct_ops", "unregister", "id", &id.to_string()]);
+    assert!(
+        output.status.success(),
+        "Failed to unregister struct_ops program: {:?}",
+        output
+    );
+}
+
+/// A test to validate that we can unregister a struct_ops using its name
+#[test]
+fn run_bpftool_struct_ops_can_unregister_name() {
+    let skel = setup().expect("Failed to set up BPF program");
+    let output = run_bpftool_command(&[
+        "struct_ops",
+        "dump",
+        "name",
+        STRUCT_OPS_MAP_NAME,
+        "--pretty",
+    ]);
+
+    let _link = skel
+        .object()
+        .map(STRUCT_OPS_MAP_NAME)
+        .unwrap_or_else(|| panic!("Could not find map {}", STRUCT_OPS_MAP_NAME))
+        .attach_struct_ops()
+        .expect("Could not attach to struct_ops");
+
+    assert!(output.status.success(), "bpftool returned an error.");
+
+    let struct_ops =
+        serde_json::from_slice::<Vec<StructOpsDump>>(&output.stdout).expect("Failed to parse JSON");
+
+    assert!(!struct_ops.is_empty(), "No struct_ops were found");
+
+    let output = run_bpftool_command(&["struct_ops", "unregister", "name", STRUCT_OPS_MAP_NAME]);
+    assert!(
+        output.status.success(),
+        "Failed to unregister struct_ops program: {:?}",
+        output
+    );
+}
-- 
2.39.3

[PATCH v1 bpf-next 8/9] bpftool: Add bpftool_tests README.md

[Manu Bretelle]

A README.md explaining how to run bpftool tests.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../selftests/bpf/bpftool_tests/README.md     | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/README.md

diff --git a/tools/testing/selftests/bpf/bpftool_tests/README.md b/tools/testing/selftests/bpf/bpftool_tests/README.md
new file mode 100644
index 000000000000..8ee5d656f6f8
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/README.md
@@ -0,0 +1,91 @@
+## About the testing Framework
+
+The testing framework uses [RUST's testing framework](https://doc.rust-lang.org/rustc/tests/index.html)
+and [libbpf-rs](https://docs.rs/libbpf-rs/latest/libbpf_rs/).
+
+The former takes care of scheduling tests and reporting their successes/failures.
+The latter is used to load bpf programs, maps, and possibly interact with them
+programatically through libbpf API.
+This allows us to set the environment we want to test and check that `bpftool`
+does what we expect.
+
+This document assumes you have [`cargo` and `rust` installed](https://doc.rust-lang.org/cargo/getting-started/installation.html).
+
+## Testing bpftool
+
+This should be no different than typical [`cargo test`](https://doc.rust-lang.org/cargo/commands/cargo-test.html)
+but there is a few subtleties to consider when running `bpftool` tests:
+
+1. bpftool needs to run with root privileges for the most part. So the runner needs to run as root.
+1. each tests load a program, possibly modify it, and check expectations. In order to be deterministic, tests need to run serially.
+
+### Environment variable
+
+A few environment variable can be used to control the behaviour of the tests:
+- `RUST_TEST_THREADS`: This should be set to 1 to run one test at a time and avoid tests to step onto each others.
+- `BPFTOOL_PATH`: Allow passing an alternate location for `bpftool`. Default: `/usr/sbin/bpftool`
+
+### Running the test suite
+
+Here are a few options to make this happen:
+
+```
+# build the test binary, extract the test executable location
+# and run it with sudo, 1 test at a time.
+eval sudo BPFTOOL_PATH=$(pwd)/../bpftool RUST_TEST_THREADS=1 \
+    $(cargo test --no-run \
+        --message-format=json | jq '. | select(.executable != null ).executable' \
+    )
+```
+
+or alternatively, one can use the [`CARGO_TARGET_<triple>_RUNNER` environment variable](https://doc.rust-lang.org/cargo/reference/environment-variables.html#:~:text=CARGO_TARGET_%3Ctriple%3E_RUNNER).
+
+The benefit of that approach is that compilation errors will show directly in the terminal.
+
+```
+CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" \
+    BPFTOOL_PATH=$(pwd)/../bpftool \
+    RUST_TEST_THREADS=1 \
+    cargo test
+```
+
+### Running tests against built kernel/bpftool
+
+Using [vmtest](https://github.com/danobi/vmtest):
+
+```
+$ KERNEL_REPO=~/devel/bpf-next/
+$ vmtest -k $KERNEL_REPO/arch/x86_64/boot/bzImage "BPFTOOL_PATH=$KERNEL_REPO/tools/bpf/bpftool/bpftool RUST_TEST_THREADS=1 cargo test"
+=> bzImage
+===> Booting
+===> Setting up VM
+===> Running command
+    Finished test [unoptimized + debuginfo] target(s) in 2.06s
+     Running unittests src/main.rs (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
+
+running 11 tests
+test bpftool_tests::run_bpftool ... ok
+test bpftool_tests::run_bpftool_map_dump_id ... ok
+test bpftool_tests::run_bpftool_map_list ... ok
+test bpftool_tests::run_bpftool_map_pids ... ok
+test bpftool_tests::run_bpftool_prog_list ... ok
+test bpftool_tests::run_bpftool_prog_pids ... ok
+test bpftool_tests::run_bpftool_prog_show_id ... ok
+test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... ok
+test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ... ok
+test bpftool_tests::run_bpftool_struct_ops_dump_name ... ok
+test bpftool_tests::run_bpftool_struct_ops_list ... ok
+
+test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.88s
+```
+
+the return code will be 0 on success, non-zero otherwise.
+
+
+## Caveat
+
+Currently, libbpf-sys crate either uses a vendored libbpf, or the system one.
+This could possibly limit tests against features that are being introduced.
+
+That being said, this is not a blocker now, and can be fixed upstream.
+https://github.com/libbpf/libbpf-sys/issues/70 tracks this on libbpf-sys side.
-- 
2.39.3

Re: [PATCH v1 bpf-next 8/9] bpftool: Add bpftool_tests README.md

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> A README.md explaining how to run bpftool tests.
> 
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
> ---
>  .../selftests/bpf/bpftool_tests/README.md     | 91 +++++++++++++++++++
>  1 file changed, 91 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/README.md
> 
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/README.md b/tools/testing/selftests/bpf/bpftool_tests/README.md
> new file mode 100644
> index 000000000000..8ee5d656f6f8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/README.md
> @@ -0,0 +1,91 @@
> +## About the testing Framework
> +
> +The testing framework uses [RUST's testing framework](https://doc.rust-lang.org/rustc/tests/index.html)
> +and [libbpf-rs](https://docs.rs/libbpf-rs/latest/libbpf_rs/).
> +
> +The former takes care of scheduling tests and reporting their successes/failures.
> +The latter is used to load bpf programs, maps, and possibly interact with them

nit: s/bpf/BPF/

> +programatically through libbpf API.

Typo (programmatically)

> +This allows us to set the environment we want to test and check that `bpftool`
> +does what we expect.
> +
> +This document assumes you have [`cargo` and `rust` installed](https://doc.rust-lang.org/cargo/getting-started/installation.html).
> +
> +## Testing bpftool
> +
> +This should be no different than typical [`cargo test`](https://doc.rust-lang.org/cargo/commands/cargo-test.html)
> +but there is a few subtleties to consider when running `bpftool` tests:

"there are"

> +
> +1. bpftool needs to run with root privileges for the most part. So the runner needs to run as root.
> +1. each tests load a program, possibly modify it, and check expectations. In order to be deterministic, tests need to run serially.

2. Each test loads..., modifies, checks

> +
> +### Environment variable
> +
> +A few environment variable can be used to control the behaviour of the tests:

s/variable/variables/
(s/behaviour/behavior/? Not sure of the guidance on US/British English
in docs.)

> +- `RUST_TEST_THREADS`: This should be set to 1 to run one test at a time and avoid tests to step onto each others.

"each other"

> +- `BPFTOOL_PATH`: Allow passing an alternate location for `bpftool`. Default: `/usr/sbin/bpftool`
> +
> +### Running the test suite
> +
> +Here are a few options to make this happen:
> +
> +```
> +# build the test binary, extract the test executable location
> +# and run it with sudo, 1 test at a time.
> +eval sudo BPFTOOL_PATH=$(pwd)/../bpftool RUST_TEST_THREADS=1 \
> +    $(cargo test --no-run \
> +        --message-format=json | jq '. | select(.executable != null ).executable' \
> +    )
> +```
> +
> +or alternatively, one can use the [`CARGO_TARGET_<triple>_RUNNER` environment variable](https://doc.rust-lang.org/cargo/reference/environment-variables.html#:~:text=CARGO_TARGET_%3Ctriple%3E_RUNNER).
> +
> +The benefit of that approach is that compilation errors will show directly in the terminal.
> +
> +```
> +CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E" \
> +    BPFTOOL_PATH=$(pwd)/../bpftool \
> +    RUST_TEST_THREADS=1 \
> +    cargo test
> +```
> +
> +### Running tests against built kernel/bpftool
> +
> +Using [vmtest](https://github.com/danobi/vmtest):
> +
> +```
> +$ KERNEL_REPO=~/devel/bpf-next/
> +$ vmtest -k $KERNEL_REPO/arch/x86_64/boot/bzImage "BPFTOOL_PATH=$KERNEL_REPO/tools/bpf/bpftool/bpftool RUST_TEST_THREADS=1 cargo test"
> +=> bzImage
> +===> Booting
> +===> Setting up VM
> +===> Running command
> +    Finished test [unoptimized + debuginfo] target(s) in 2.06s
> +     Running unittests src/main.rs (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
> +
> +running 11 tests
> +test bpftool_tests::run_bpftool ... ok
> +test bpftool_tests::run_bpftool_map_dump_id ... ok
> +test bpftool_tests::run_bpftool_map_list ... ok
> +test bpftool_tests::run_bpftool_map_pids ... ok
> +test bpftool_tests::run_bpftool_prog_list ... ok
> +test bpftool_tests::run_bpftool_prog_pids ... ok
> +test bpftool_tests::run_bpftool_prog_show_id ... ok
> +test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... ok
> +test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ... ok
> +test bpftool_tests::run_bpftool_struct_ops_dump_name ... ok
> +test bpftool_tests::run_bpftool_struct_ops_list ... ok
> +
> +test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.88s
> +```
> +
> +the return code will be 0 on success, non-zero otherwise.
> +
> +
> +## Caveat
> +
> +Currently, libbpf-sys crate either uses a vendored libbpf, or the system one.
> +This could possibly limit tests against features that are being introduced.
> +
> +That being said, this is not a blocker now, and can be fixed upstream.
> +https://github.com/libbpf/libbpf-sys/issues/70 tracks this on libbpf-sys side.

Kernel docs usually use rST rather than Markdown for formatting.
Although there's rust/alloc/README.md, so I'm not entirely sure what's
best here. Unless there's a good reason for Markdown, we probably want
to stick to rST? This also depends on whether or not this patchset makes
it into the kernel repo, obviously.

Quentin

[PATCH v1 bpf-next 9/9] bpftool: Add Makefile to facilitate bpftool_tests usage

[Manu Bretelle]

Provide some Makefile targets to run the linter, build the tests, and
run them on the host or in a VM.

    $ make vmtest
    cargo test --no-run --offline
        Finished test [unoptimized + debuginfo] target(s) in 0.05s
      Executable unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
    vmtest -k /data/users/chantra/bpf-next/arch/x86/boot/bzImage
    "RUST_TEST_THREADS=1 BPFTOOL_PATH=../tools/build/bpftool/bpftool cargo
    test"
    => bzImage
    ===> Booting
    ===> Setting up VM
    ===> Running command
        Finished test [unoptimized + debuginfo] target(s) in 2.05s
         Running unittests src/main.rs
    (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)

    running 11 tests
    test bpftool_tests::run_bpftool ... ok
    test bpftool_tests::run_bpftool_map_dump_id ... ok
    test bpftool_tests::run_bpftool_map_list ... ok
    test bpftool_tests::run_bpftool_map_pids ... ok
    test bpftool_tests::run_bpftool_prog_list ... ok
    test bpftool_tests::run_bpftool_prog_pids ... ok
    test bpftool_tests::run_bpftool_prog_show_id ... ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... ok
    test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ... ok
    test bpftool_tests::run_bpftool_struct_ops_dump_name ... ok
    test bpftool_tests::run_bpftool_struct_ops_list ... ok

    test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 2.88s

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
---
 .../selftests/bpf/bpftool_tests/Makefile      | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Makefile

diff --git a/tools/testing/selftests/bpf/bpftool_tests/Makefile b/tools/testing/selftests/bpf/bpftool_tests/Makefile
new file mode 100644
index 000000000000..0b5633cda8b4
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpftool_tests/Makefile
@@ -0,0 +1,22 @@
+BPFTOOL := ../tools/build/bpftool/bpftool
+
+lint:
+	cargo clippy --tests
+	cargo check --tests
+
+# Build the tests but do not run them.
+build-test:
+	cargo test --no-run --offline
+
+# Run the tests on the host using sudo
+test: build-test
+	RUST_TEST_THREADS=1 BPFTOOL_PATH=$(BPFTOOL) sudo -E $(shell which cargo) test --offline
+
+# Run the tests in a vm. Requires danobi/vmtest.
+vmtest: build-test
+	$(eval repo_root := $(shell git rev-parse --show-toplevel))
+	$(eval kernel_img := $(shell make -s -C $(repo_root) image_name))
+	vmtest -k $(repo_root)/$(kernel_img) "RUST_TEST_THREADS=1 BPFTOOL_PATH=$(BPFTOOL) cargo test"
+
+clean:
+	cargo clean
-- 
2.39.3

Re: [PATCH v1 bpf-next 9/9] bpftool: Add Makefile to facilitate bpftool_tests usage

[Quentin Monnet]

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <chantr4@gmail.com>
> Provide some Makefile targets to run the linter, build the tests, and
> run them on the host or in a VM.
> 
>     $ make vmtest
>     cargo test --no-run --offline
>         Finished test [unoptimized + debuginfo] target(s) in 0.05s
>       Executable unittests src/main.rs
>     (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
>     vmtest -k /data/users/chantra/bpf-next/arch/x86/boot/bzImage
>     "RUST_TEST_THREADS=1 BPFTOOL_PATH=../tools/build/bpftool/bpftool cargo
>     test"
>     => bzImage
>     ===> Booting
>     ===> Setting up VM
>     ===> Running command
>         Finished test [unoptimized + debuginfo] target(s) in 2.05s
>          Running unittests src/main.rs
>     (target/debug/deps/bpftool_tests-afa5a7eef3cdeafb)
> 
>     running 11 tests
>     test bpftool_tests::run_bpftool ... ok
>     test bpftool_tests::run_bpftool_map_dump_id ... ok
>     test bpftool_tests::run_bpftool_map_list ... ok
>     test bpftool_tests::run_bpftool_map_pids ... ok
>     test bpftool_tests::run_bpftool_prog_list ... ok
>     test bpftool_tests::run_bpftool_prog_pids ... ok
>     test bpftool_tests::run_bpftool_prog_show_id ... ok
>     test bpftool_tests::run_bpftool_struct_ops_can_unregister_id ... ok
>     test bpftool_tests::run_bpftool_struct_ops_can_unregister_name ... ok
>     test bpftool_tests::run_bpftool_struct_ops_dump_name ... ok
>     test bpftool_tests::run_bpftool_struct_ops_list ... ok
> 
>     test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
>     out; finished in 2.88s
> 
> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
> ---
>  .../selftests/bpf/bpftool_tests/Makefile      | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/bpftool_tests/Makefile
> 
> diff --git a/tools/testing/selftests/bpf/bpftool_tests/Makefile b/tools/testing/selftests/bpf/bpftool_tests/Makefile
> new file mode 100644
> index 000000000000..0b5633cda8b4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpftool_tests/Makefile
> @@ -0,0 +1,22 @@

I'd recommend adding a SPDX tag. I don't know how relevant this is for a
Makefile, but this would prevent to fall it under the default GPL-only
license, and keep it consistent with the rest of the test framework
you're introducing.

> +BPFTOOL := ../tools/build/bpftool/bpftool
> +
> +lint:
> +	cargo clippy --tests
> +	cargo check --tests
> +
> +# Build the tests but do not run them.
> +build-test:
> +	cargo test --no-run --offline
> +
> +# Run the tests on the host using sudo
> +test: build-test
> +	RUST_TEST_THREADS=1 BPFTOOL_PATH=$(BPFTOOL) sudo -E $(shell which cargo) test --offline
> +
> +# Run the tests in a vm. Requires danobi/vmtest.

Please provide the full URL to the repository. I'd also add a comment to
make it explicit that this is distinct from the vmtest.sh from the
selftests.

> +vmtest: build-test
> +	$(eval repo_root := $(shell git rev-parse --show-toplevel))
> +	$(eval kernel_img := $(shell make -s -C $(repo_root) image_name))
> +	vmtest -k $(repo_root)/$(kernel_img) "RUST_TEST_THREADS=1 BPFTOOL_PATH=$(BPFTOOL) cargo test"
> +
> +clean:
> +	cargo clean