Re: [PATCH bpf-next v6 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup

[Hao Luo]

On Thu, May 4, 2023 at 11:08 PM Feng zhou <zhoufeng.zf@xxxxxxxxxxxxx> wrote:
>
> From: Feng Zhou <zhoufeng.zf@xxxxxxxxxxxxx>
>
> test_progs:
> Tests new kfunc bpf_task_under_cgroup().
>
> The bpf program saves the new task's pid within a given cgroup to
> the remote_pid, which is convenient for the user-mode program to
> verify the test correctness.
>
> The user-mode program creates its own mount namespace, and mounts the
> cgroupsv2 hierarchy in there, call the fork syscall, then check if
> remote_pid and local_pid are unequal.
>
> Signed-off-by: Feng Zhou <zhoufeng.zf@xxxxxxxxxxxxx>
> Acked-by: Yonghong Song <yhs@xxxxxx>
> ---

Hi Feng,

I have a comment about the methodology of the test, but the patch
looks ok to me. Why do we have to test via a tracing program? I think
what we need is just a task and a cgroup. Since we have the kfunc
bpf_task_from_pid() and bpf_cgroup_from_id(), we can write a syscall
program which takes a pid and a cgroup id as input and get the task
and cgroup objects directly in the program.

I like testing via a syscall program because it doesn't depend on the
newtask tracepoint and it should be simpler. But I'm ok with the
current version of the patch, just have some thoughts.

Hao