Re: [PATCH bpf-next v3 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs

[Chuyi Zhou <zhouchuyi@xxxxxxxxxxxxx>]

Hello,

在 2023/10/31 08:20, Yonghong Song 写道:
> On 10/25/23 12:59 AM, Chuyi Zhou wrote:
> > This Patch add a test to prove css_task iter can be used in normal
> > sleepable progs.
> >
> > Signed-off-by: Chuyi Zhou <zhouchuyi@xxxxxxxxxxxxx>
> > ---
> >   .../selftests/bpf/progs/iters_task_failure.c  | 19 +++++++++++++++++++
> >   1 file changed, 19 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/iters_task_failure.c 
> > b/tools/testing/selftests/bpf/progs/iters_task_failure.c
> > index 6b1588d70652..fe0b19e545d0 100644
> > --- a/tools/testing/selftests/bpf/progs/iters_task_failure.c
> > +++ b/tools/testing/selftests/bpf/progs/iters_task_failure.c
> > @@ -103,3 +103,22 @@ int BPF_PROG(iter_css_task_for_each)
> >       bpf_cgroup_release(cgrp);
> >       return 0;
> >   }
> > +
> > +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> > +int BPF_PROG(iter_css_task_for_each_sleep)
> > +{
> > +    u64 cg_id = bpf_get_current_cgroup_id();
> > +    struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
> > +    struct cgroup_subsys_state *css;
> > +    struct task_struct *task;
> > +
> > +    if (cgrp == NULL)
> > +        return 0;
> > +    css = &cgrp->self;
> > +
> > +    bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
> > +
> > +    }
> > +    bpf_cgroup_release(cgrp);
> > +    return 0;
> > +}
>
> Could you move this prog toiters_css_task.c and add a subtest in 
> cgroup_iter.c? The file iters_task_failure.c intends for negative tests. 
> This prog succeeds with loading.

Thanks for the review. I will change in next version.

But do we need a extra subtest like subtest_css_task_iters() in iters.c 
or just use RUN_TESTS(iters_css_task) to prove it can be loaded?

If we do need a extra subtest, maybe we can reuse 
subtest_css_task_iters() in iters.c? cgroup_iter.c is used to test 
SEC("iter/cgroup") and iters.c is used to test open-coded iters.

We can let this prog outo-loaded, and use 'syscall(SYS_getpgid)' after 
'stack_mprotect()' to trigger the prog.

static void subtest_css_task_iters(void)
{
	...
	err = stack_mprotect();
	syscall(SYS_getpgid);
	if (!ASSERT_EQ(err, -1, "stack_mprotect") ||
	    !ASSERT_EQ(errno, EPERM, "stack_mprotect"))
		goto cleanup;
	iters_css_task__detach(skel);
	ASSERT_EQ(skel->bss->css_task_cnt_in_lsm, 1, "css_task_cnt_in_lsm");
	ASSERT_EQ(skel->bss->css_task_cnt_in_sleep, 1, "css_task_cnt_in_sleep");
	...
}

What do you think?

Thanks.