On Mon, Dec 28, 2020 at 7:48 AM Carlos Neira <cneirabustos@xxxxxxxxx> wrote:
>
> Currently tests for bpf_get_ns_current_pid_tgid() are outside test_progs.
> This change folds test cases into test_progs.
>
> Changes from v10:
>
> - Code style fixes.
> - Remove redundant code.
>
> Signed-off-by: Carlos Neira <cneirabustos@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/.gitignore | 1 -
> tools/testing/selftests/bpf/Makefile | 3 +-
> .../bpf/prog_tests/ns_current_pid_tgid.c | 118 ++++++-------
> .../bpf/progs/test_ns_current_pid_tgid.c | 28 +--
> .../bpf/test_current_pid_tgid_new_ns.c | 160 ------------------
> 5 files changed, 69 insertions(+), 241 deletions(-)
> delete mode 100644 tools/testing/selftests/bpf/test_current_pid_tgid_new_ns.c
>
[...]
> -void test_ns_current_pid_tgid(void)
> +static int test_current_pid_tgid(void *args)
> {
> - 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 test_ns_current_pid_tgid__bss *bss;
> + struct test_ns_current_pid_tgid *skel;
> + int err = 0, duration = 0;
> + pid_t tgid, pid;
> struct stat st;
> - __u64 id;
> -
> - obj = bpf_object__open_file(file, NULL);
> - if (CHECK(IS_ERR(obj), "obj_open", "err %ld\n", PTR_ERR(obj)))
> - return;
>
> - err = bpf_object__load(obj);
> - if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + skel = test_ns_current_pid_tgid__open_and_load();
> + if (CHECK(!skel, "skel_open_load", "failed to load skeleton\n"))
> goto cleanup;
err will be 0 here, and test failure won't be detected (if it happens
in a child process), right? please check all the other cases. It might
be better to assign err = -1 at the beginning and just assign err = 0
after all tests succeeded, right before the clean up.
>
> - bss_map = bpf_object__find_map_by_name(obj, "test_ns_.bss");
> - if (CHECK(!bss_map, "find_bss_map", "failed\n"))
> - goto cleanup;
> + pid = syscall(SYS_gettid);
> + tgid = getpid();
>
[...]
> -cleanup:
> - bpf_link__destroy(link);
> - bpf_object__close(obj);
> + if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", strerror(errno)))
> + exit(EXIT_FAILURE);
> +
> + if (CHECK(WEXITSTATUS(wstatus) != 0, "newns_pidtgid", "failed"))
> + exit(EXIT_FAILURE);
this exit will happen in the parent process, which is test_progs test
runner itself, right? Which will prematurely stop all of the remaining
tests to run. I.e., you can't just exit(), you need to just return and
let test_progs keep running.
> +}
> +
> +void test_ns_current_pid_tgid(void)
> +{
> + if (test__start_subtest("ns_current_pid_tgid_root_ns"))
> + test_current_pid_tgid(NULL);
> + if (test__start_subtest("ns_current_pid_tgid_new_ns"))
> + test_ns_current_pid_tgid_new_ns();
> }
[...]
