[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 5 May 2023 11:46:34 -0700
From: Yonghong Song <yhs@...a.com>
To: Feng Zhou <zhoufeng.zf@...edance.com>, Hao Luo <haoluo@...gle.com>
Cc: martin.lau@...ux.dev, ast@...nel.org, daniel@...earbox.net,
andrii@...nel.org, song@...nel.org, yhs@...com,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
jolsa@...nel.org, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, mykolal@...com,
shuah@...nel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, yangzhenze@...edance.com,
wangdongdong.6@...edance.com
Subject: Re: [PATCH bpf-next v6 2/2] selftests/bpf: Add testcase for
bpf_task_under_cgroup
On 5/5/23 12:24 AM, Feng Zhou wrote:
> 在 2023/5/5 15:13, Hao Luo 写道:
>> On Thu, May 4, 2023 at 11:08 PM Feng zhou <zhoufeng.zf@...edance.com>
>> wrote:
>>>
>>> From: Feng Zhou <zhoufeng.zf@...edance.com>
>>>
>>> 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@...edance.com>
>>> Acked-by: Yonghong Song <yhs@...com>
>>> ---
>>
>> 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
>
> Yes, your method is also very good. The reason why I did this is because
> of Song's suggestion before, hope that the parameter of the hook point
> will have a task, so I chose this to test.
The motivation of this patch is:
Trace sched related functions, such as enqueue_task_fair, it is
necessary to
specify a task instead of the current task which within a given cgroup.
So I think it is okay to have a test related to sched.
Powered by blists - more mailing lists