2023-10-07 12:47:12

by Chuyi Zhou

[permalink] [raw]
Subject: [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters

Hi,

This is version 4 of task, css_task and css iters support.
Thanks for your review!

--- Changelog ---

v3 -> v4:https://lore.kernel.org/all/[email protected]/

* Address all the comments from Andrii in patch-3 ~ patch-6
* Collect Tejun's ack
* Add a extra patch to rename bpf_iter_task.c to bpf_iter_tasks.c
* Seperate three BPF program files for selftests (iters_task.c iters_css_task.c iters_css.c)

v2 -> v3:https://lore.kernel.org/lkml/[email protected]/

Patch 1 (cgroup: Prepare for using css_task_iter_*() in BPF)
* Add tj's ack and Alexei's suggest-by.
Patch 2 (bpf: Introduce css_task open-coded iterator kfuncs)
* Use bpf_mem_alloc/bpf_mem_free rather than kzalloc()
* Add KF_TRUSTED_ARGS for bpf_iter_css_task_new (Alexei)
* Move bpf_iter_css_task's definition from uapi/linux/bpf.h to
kernel/bpf/task_iter.c and we can use it from vmlinux.h
* Move bpf_iter_css_task_XXX's declaration from bpf_helpers.h to
bpf_experimental.h
Patch 3 (Introduce task open coded iterator kfuncs)
* Change th API design keep consistent with SEC("iter/task"), support
iterating all threads(BPF_TASK_ITERATE_ALL) and threads of a
specific task (BPF_TASK_ITERATE_THREAD).(Andrii)
* Move bpf_iter_task's definition from uapi/linux/bpf.h to
kernel/bpf/task_iter.c and we can use it from vmlinux.h
* Move bpf_iter_task_XXX's declaration from bpf_helpers.h to
bpf_experimental.h
Patch 4 (Introduce css open-coded iterator kfuncs)
* Change th API design keep consistent with cgroup_iters, reuse
BPF_CGROUP_ITER_DESCENDANTS_PRE/BPF_CGROUP_ITER_DESCENDANTS_POST
/BPF_CGROUP_ITER_ANCESTORS_UP(Andrii)
* Add KF_TRUSTED_ARGS for bpf_iter_css_new
* Move bpf_iter_css's definition from uapi/linux/bpf.h to
kernel/bpf/task_iter.c and we can use it from vmlinux.h
* Move bpf_iter_task_XXX's declaration from bpf_helpers.h to
bpf_experimental.h
Patch 4 (Introduce css open-coded iterator kfuncs)
* Change th API design keep consistent with cgroup_iters, reuse
BPF_CGROUP_ITER_DESCENDANTS_PRE/BPF_CGROUP_ITER_DESCENDANTS_POST
/BPF_CGROUP_ITER_ANCESTORS_UP(Andrii)
* Add KF_TRUSTED_ARGS for bpf_iter_css_new
* Move bpf_iter_css's definition from uapi/linux/bpf.h to
kernel/bpf/task_iter.c and we can use it from vmlinux.h
* Move bpf_iter_css_XXX's declaration from bpf_helpers.h to
bpf_experimental.h
Patch 5 (teach the verifier to enforce css_iter and task_iter in RCU CS)
* Add KF flag KF_RCU_PROTECTED to maintain kfuncs which need RCU CS.(Andrii)
* Consider STACK_ITER when using bpf_for_each_spilled_reg.
Patch 6 (Let bpf_iter_task_new accept null task ptr)
* Add this extra patch to let bpf_iter_task_new accept a 'nullable'
* task pointer(Andrii)
Patch 7 (selftests/bpf: Add tests for open-coded task and css iter)
* Add failure testcase(Alexei)


Changes from v1(https://lore.kernel.org/lkml/[email protected]/):
- Add a pre-patch to make some preparations before supporting css_task
iters.(Alexei)
- Add an allowlist for css_task iters(Alexei)
- Let bpf progs do explicit bpf_rcu_read_lock() when using process
iters and css_descendant iters.(Alexei)
---------------------

In some BPF usage scenarios, it will be useful to iterate the process and
css directly in the BPF program. One of the expected scenarios is
customizable OOM victim selection via BPF[1].

Inspired by Dave's task_vma iter[2], this patchset adds three types of
open-coded iterator kfuncs:

1. bpf_task_iters. It can be used to
1) iterate all process in the system, like for_each_forcess() in kernel.
2) iterate all threads in the system.
3) iterate all threads of a specific task

2. bpf_css_iters. It works like css_task_iter_{start, next, end} and would
be used to iterating tasks/threads under a css.

3. css_iters. It works like css_next_descendant_{pre, post} to iterating all
descendant css.

BPF programs can use these kfuncs directly or through bpf_for_each macro.

link[1]: https://lore.kernel.org/lkml/[email protected]/
link[2]: https://lore.kernel.org/all/[email protected]/

Chuyi Zhou (8):
cgroup: Prepare for using css_task_iter_*() in BPF
bpf: Introduce css_task open-coded iterator kfuncs
bpf: Introduce task open coded iterator kfuncs
bpf: Introduce css open-coded iterator kfuncs
bpf: teach the verifier to enforce css_iter and task_iter in RCU CS
bpf: Let bpf_iter_task_new accept null task ptr
selftests/bpf: rename bpf_iter_task.c to bpf_iter_tasks.c
selftests/bpf: Add tests for open-coded task and css iter

include/linux/bpf_verifier.h | 19 ++-
include/linux/btf.h | 1 +
include/linux/cgroup.h | 12 +-
kernel/bpf/cgroup_iter.c | 59 +++++++
kernel/bpf/helpers.c | 9 +
kernel/bpf/task_iter.c | 138 +++++++++++++++
kernel/bpf/verifier.c | 86 ++++++++--
kernel/cgroup/cgroup.c | 18 +-
.../testing/selftests/bpf/bpf_experimental.h | 19 +++
.../selftests/bpf/prog_tests/bpf_iter.c | 18 +-
.../testing/selftests/bpf/prog_tests/iters.c | 161 ++++++++++++++++++
.../{bpf_iter_task.c => bpf_iter_tasks.c} | 0
tools/testing/selftests/bpf/progs/iters_css.c | 74 ++++++++
.../selftests/bpf/progs/iters_css_task.c | 42 +++++
.../testing/selftests/bpf/progs/iters_task.c | 41 +++++
.../selftests/bpf/progs/iters_task_failure.c | 105 ++++++++++++
16 files changed, 760 insertions(+), 42 deletions(-)
rename tools/testing/selftests/bpf/progs/{bpf_iter_task.c => bpf_iter_tasks.c} (100%)
create mode 100644 tools/testing/selftests/bpf/progs/iters_css.c
create mode 100644 tools/testing/selftests/bpf/progs/iters_css_task.c
create mode 100644 tools/testing/selftests/bpf/progs/iters_task.c
create mode 100644 tools/testing/selftests/bpf/progs/iters_task_failure.c

--
2.20.1


2023-10-10 08:02:31

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters

On 10/7/23 2:45 PM, Chuyi Zhou wrote:
> Hi,
>
> This is version 4 of task, css_task and css iters support.
> Thanks for your review!
>
> --- Changelog ---
>
> v3 -> v4:https://lore.kernel.org/all/[email protected]/
>
> * Address all the comments from Andrii in patch-3 ~ patch-6
> * Collect Tejun's ack
> * Add a extra patch to rename bpf_iter_task.c to bpf_iter_tasks.c
> * Seperate three BPF program files for selftests (iters_task.c iters_css_task.c iters_css.c)

This fails to build BPF selftests:

[...]
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:166:6: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26: note: uninitialized use occurs here
iters_css_task__destroy(skel);
^~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:166:2: note: remove the 'if' if its condition is always false
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:162:6: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_GE(cg_fd, 0, "cg_create"))
TEST-OBJ [test_progs] xdp.test.o
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26: note: uninitialized use occurs here
iters_css_task__destroy(skel);
^~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:162:2: note: remove the 'if' if its condition is always false
if (!ASSERT_GE(cg_fd, 0, "cg_create"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:159:6: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26: note: uninitialized use occurs here
iters_css_task__destroy(skel);
^~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:159:2: note: remove the 'if' if its condition is always false
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:154:29: note: initialize the variable 'skel' to silence this warning
struct iters_css_task *skel;
^
= NULL
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:213:7: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_GE(cgs[i].fd, 0, "cg_create"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:244:21: note: uninitialized use occurs here
iters_css__destroy(skel);
^~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:213:3: note: remove the 'if' if its condition is always false
if (!ASSERT_GE(cgs[i].fd, 0, "cg_create"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:209:6: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:244:21: note: uninitialized use occurs here
iters_css__destroy(skel);
^~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:209:2: note: remove the 'if' if its condition is always false
if (!ASSERT_OK(err, "setup_cgroup_environment"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:195:24: note: initialize the variable 'skel' to silence this warning
struct iters_css *skel;
^
= NULL
5 errors generated.
make: *** [Makefile:605: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/iters.test.o] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
Error: Process completed with exit code 2.

2023-10-10 08:15:34

by Chuyi Zhou

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters

Hello,

在 2023/10/10 16:01, Daniel Borkmann 写道:
> On 10/7/23 2:45 PM, Chuyi Zhou wrote:
>> Hi,
>>
>> This is version 4 of task, css_task and css iters support.
>> Thanks for your review!
>>
>> --- Changelog ---
>>
>> v3 ->
>> v4:https://lore.kernel.org/all/[email protected]/
>>
>> * Address all the comments from Andrii in patch-3 ~ patch-6
>> * Collect Tejun's ack
>> * Add a extra patch to rename bpf_iter_task.c to bpf_iter_tasks.c
>> * Seperate three BPF program files for selftests (iters_task.c
>> iters_css_task.c iters_css.c)
>
> This fails to build BPF selftests:
>

Yes, thanks for the remind!

I didn't notice this error since it may only occurs when using llvm-16
to compile the selftest, and when we using gcc, it works OK.
(https://github.com/kernel-patches/bpf/actions/runs/6462875618/job/17545170863)

I can reproduce this error in my environment. Before sending next
version, I would use LLVM-16 to double check.

Thanks.


> [...]
>  /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:166:6: error: variable 'skel' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26:
> note: uninitialized use occurs here
>           iters_css_task__destroy(skel);
>                                   ^~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:166:2:
> note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:162:6:
> error: variable 'skel' is used uninitialized whenever 'if' condition is
> true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_GE(cg_fd, 0, "cg_create"))
>     TEST-OBJ [test_progs] xdp.test.o
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26:
> note: uninitialized use occurs here
>           iters_css_task__destroy(skel);
>                                   ^~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:162:2:
> note: remove the 'if' if its condition is always false
>           if (!ASSERT_GE(cg_fd, 0, "cg_create"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:159:6:
> error: variable 'skel' is used uninitialized whenever 'if' condition is
> true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:190:26:
> note: uninitialized use occurs here
>           iters_css_task__destroy(skel);
>                                   ^~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:159:2:
> note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:154:29:
> note: initialize the variable 'skel' to silence this warning
>           struct iters_css_task *skel;
>                                      ^
>                                       = NULL
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:213:7:
> error: variable 'skel' is used uninitialized whenever 'if' condition is
> true [-Werror,-Wsometimes-uninitialized]
>                   if (!ASSERT_GE(cgs[i].fd, 0, "cg_create"))
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:244:21:
> note: uninitialized use occurs here
>           iters_css__destroy(skel);
>                              ^~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:213:3:
> note: remove the 'if' if its condition is always false
>                   if (!ASSERT_GE(cgs[i].fd, 0, "cg_create"))
>                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:209:6:
> error: variable 'skel' is used uninitialized whenever 'if' condition is
> true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:244:21:
> note: uninitialized use occurs here
>           iters_css__destroy(skel);
>                              ^~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:209:2:
> note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK(err, "setup_cgroup_environment"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/iters.c:195:24:
> note: initialize the variable 'skel' to silence this warning
>           struct iters_css *skel;
>                                 ^
>                                  = NULL
>   5 errors generated.
>   make: *** [Makefile:605:
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/iters.test.o] Error 1
>   make: *** Waiting for unfinished jobs....
>   make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
>   Error: Process completed with exit code 2.