On Tue, Dec 22, 2020 at 12:26:04PM -0800, Andrii Nakryiko wrote: > On Mon, Dec 21, 2020 at 2:25 PM 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 v9: > > > > - Added test in root namespace. > > - Fixed changed tracepoint from sys_enter to sys_usleep. > > - Fixed pid, tgid values were inverted. > > - Used CLONE(2) for namespaced test, the new process pid will be 1. > > - Used ASSERTEQ on pid/tgid validation. > > - Added comment on CLONE(2) call > > > > Signed-off-by: Carlos Neira <cneirabustos@xxxxxxxxx> > > --- Andrii, Thanks for taking the time to check this out. For the code style issues I found out that I was not using the --strict flag for checkpatch.pl so I missed those warnings, now I'm using--strict as default. I should look into using cindent or clang-format to avoid this. > > See checkpatch.pl errors ([0]), ignore the "do not initialize globals > with zero" ones. Next time, please don't wait for me to point out > every single instance where you didn't align wrapped around > parameters. > > [0] https://patchwork.hopto.org/static/nipa/405025/11985347/checkpatch/stdout > > > tools/testing/selftests/bpf/.gitignore | 1 - > > tools/testing/selftests/bpf/Makefile | 3 +- > > .../bpf/prog_tests/ns_current_pid_tgid.c | 149 ++++++++++------ > > .../bpf/progs/test_ns_current_pid_tgid.c | 29 ++-- > > .../bpf/test_current_pid_tgid_new_ns.c | 160 ------------------ > > 5 files changed, 106 insertions(+), 236 deletions(-) > > delete mode 100644 tools/testing/selftests/bpf/test_current_pid_tgid_new_ns.c > > You are right, the code is redundant I'll reuse it. > newns_pidtgid and test_ns_current_pid_tgid_global_ns look identical to > me, am I missing something on why you didn't reuse the testing logic > between the two? > > > +{ > > + struct test_ns_current_pid_tgid__bss *bss; > > + int err = 0, duration = 0; > > + struct test_ns_current_pid_tgid *skel; > > + pid_t pid, tgid; > > + struct stat st; > > > > [...]