Re: [PATCH v2 bpf-next] fold test_current_pid_tgid_new_ns into into test_progs
From: Andrii Nakryiko <hidden>
Date: 2020-06-23 18:11:03
Also in:
bpf
On Tue, Jun 23, 2020 at 5:48 AM Carlos Neira [off-list ref] wrote:
quoted hunk ↗ jump to hunk
folds tests from test_current_pid_tgid_new_ns into test_progs. Signed-off-by: Carlos Neira <redacted> --- tools/testing/selftests/bpf/Makefile | 3 +- .../bpf/prog_tests/ns_current_pid_tgid.c | 112 +++++++++++- .../bpf/test_current_pid_tgid_new_ns.c | 159 ------------------ 3 files changed, 112 insertions(+), 162 deletions(-) delete mode 100644 tools/testing/selftests/bpf/test_current_pid_tgid_new_ns.cdiff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 22aaec74ea0a..7b2ea7adccb0 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile@@ -36,8 +36,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \ test_cgroup_storage \ test_netcnt test_tcpnotify_user test_sock_fields test_sysctl \ - test_progs-no_alu32 \ - test_current_pid_tgid_new_ns + test_progs-no_alu32
Please update .gitignore as well.
# Also test bpf-gcc, if present ifneq ($(BPF_GCC),)
[...]
+ + snprintf(nspath, sizeof(nspath) - 1, "/proc/%d/ns/pid", ppid); + pidns_fd = open(nspath, O_RDONLY); + + if (CHECK(unshare(CLONE_NEWPID), + "unshare CLONE_NEWPID", + "error: %s\n", strerror(errno))) + return; + + pid = vfork();
is vfork necessary()? Maybe just stick to fork(), as in original implementation?
+ if (CHECK(pid < 0, "ns_current_pid_tgid_new_ns", "vfork error: %s\n",
+ strerror(errno))) {
+ return;
+ }
+ if (pid > 0) {
+ printf("waiting pid is %u\n", pid);indentation off?
+ usleep(5); + wait(NULL);
waitpid() for specific child would be more reliable, no?
+ return;
+ } else {what if fork failed?
+ const char *probe_name = "raw_tracepoint/sys_enter"; + const char *file = "test_ns_current_pid_tgid.o"; + int err, key = 0, duration = 0; + struct bpf_link *link = NULL; + struct bpf_program *prog; + struct bpf_map *bss_map; + struct bpf_object *obj; + struct bss bss; + struct stat st; + __u64 id; +
[...]
+ err = bpf_map_lookup_elem(bpf_map__fd(bss_map), &key, &bss); + if (CHECK(err, "set_bss", "failed to get bss : %d\n", err)) + goto cleanup; + + if (CHECK(id != bss.pid_tgid, "Compare user pid/tgid vs bpf pid/tgid", + "User pid/tgid %llu BPF pid/tgid %llu\n", id, bss.pid_tgid)) + goto cleanup;
Good half of all this code could be removed if you used BPF skeleton, see other tests utilizing *.skel.h for inspiration.
+cleanup:
+ setns(pidns_fd, CLONE_NEWPID);
+ bpf_link__destroy(link);
+ bpf_object__close(obj);
+ }
+}
+
+void test_ns_current_pid_tgid(void)
+{
+ if (test__start_subtest("ns_current_pid_tgid_global_ns"))
+ test_ns_current_pid_tgid_global_ns();
+ if (test__start_subtest("ns_current_pid_tgid_new_ns"))
+ test_ns_current_pid_tgid_new_ns();
+}[...]