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

Commit Message

Manu Bretelle Nov. 16, 2023, 7:42 p.m. UTC

Signed-off-by: Manu Bretelle <[e-mail]>
---
 .../bpftool_tests/src/bpf/bpftool_tests.bpf.c |  7 +++++
 .../bpf/bpftool_tests/src/bpftool_tests.rs    | 29 +++++++++++++++++++
 2 files changed, 36 insertions(+)

Comments

Quentin Monnet Nov. 21, 2023, 4:26 p.m. UTC | #1

2023-11-16 19:43 UTC+0000 ~ Manu Bretelle <[e-mail]>
> Signed-off-by: Manu Bretelle <[e-mail]>

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

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
+    );
+}