bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters
@ 2023-10-07 12:45 Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 1/8] cgroup: Prepare for using css_task_iter_*() in BPF Chuyi Zhou
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

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/20230925105552.817513-1-zhouchuyi@bytedance.com/

* 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/20230912070149.969939-1-zhouchuyi@bytedance.com/

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/20230827072057.1591929-1-zhouchuyi@bytedance.com/):
- 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/20230810081319.65668-1-zhouchuyi@bytedance.com/
link[2]: https://lore.kernel.org/all/20230810183513.684836-1-davemarchevsky@fb.com/

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 1/8] cgroup: Prepare for using css_task_iter_*() in BPF
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs Chuyi Zhou
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

This patch makes some preparations for using css_task_iter_*() in BPF
Program.

1. Flags CSS_TASK_ITER_* are #define-s and it's not easy for bpf prog to
use them. Convert them to enum so bpf prog can take them from vmlinux.h.

2. In the next patch we will add css_task_iter_*() in common kfuncs which
is not safe. Since css_task_iter_*() does spin_unlock_irq() which might
screw up irq flags depending on the context where bpf prog is running.
So we should use irqsave/irqrestore here and the switching is harmless.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 include/linux/cgroup.h | 12 +++++-------
 kernel/cgroup/cgroup.c | 18 ++++++++++++------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index b307013b9c6c..0ef0af66080e 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -40,13 +40,11 @@ struct kernel_clone_args;
 #define CGROUP_WEIGHT_DFL		100
 #define CGROUP_WEIGHT_MAX		10000
 
-/* walk only threadgroup leaders */
-#define CSS_TASK_ITER_PROCS		(1U << 0)
-/* walk all threaded css_sets in the domain */
-#define CSS_TASK_ITER_THREADED		(1U << 1)
-
-/* internal flags */
-#define CSS_TASK_ITER_SKIPPED		(1U << 16)
+enum {
+	CSS_TASK_ITER_PROCS    = (1U << 0),  /* walk only threadgroup leaders */
+	CSS_TASK_ITER_THREADED = (1U << 1),  /* walk all threaded css_sets in the domain */
+	CSS_TASK_ITER_SKIPPED  = (1U << 16), /* internal flags */
+};
 
 /* a css_task_iter should be treated as an opaque object */
 struct css_task_iter {
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1fb7f562289d..b6d64f3b8888 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4917,9 +4917,11 @@ static void css_task_iter_advance(struct css_task_iter *it)
 void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
 			 struct css_task_iter *it)
 {
+	unsigned long irqflags;
+
 	memset(it, 0, sizeof(*it));
 
-	spin_lock_irq(&css_set_lock);
+	spin_lock_irqsave(&css_set_lock, irqflags);
 
 	it->ss = css->ss;
 	it->flags = flags;
@@ -4933,7 +4935,7 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
 
 	css_task_iter_advance(it);
 
-	spin_unlock_irq(&css_set_lock);
+	spin_unlock_irqrestore(&css_set_lock, irqflags);
 }
 
 /**
@@ -4946,12 +4948,14 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
  */
 struct task_struct *css_task_iter_next(struct css_task_iter *it)
 {
+	unsigned long irqflags;
+
 	if (it->cur_task) {
 		put_task_struct(it->cur_task);
 		it->cur_task = NULL;
 	}
 
-	spin_lock_irq(&css_set_lock);
+	spin_lock_irqsave(&css_set_lock, irqflags);
 
 	/* @it may be half-advanced by skips, finish advancing */
 	if (it->flags & CSS_TASK_ITER_SKIPPED)
@@ -4964,7 +4968,7 @@ struct task_struct *css_task_iter_next(struct css_task_iter *it)
 		css_task_iter_advance(it);
 	}
 
-	spin_unlock_irq(&css_set_lock);
+	spin_unlock_irqrestore(&css_set_lock, irqflags);
 
 	return it->cur_task;
 }
@@ -4977,11 +4981,13 @@ struct task_struct *css_task_iter_next(struct css_task_iter *it)
  */
 void css_task_iter_end(struct css_task_iter *it)
 {
+	unsigned long irqflags;
+
 	if (it->cur_cset) {
-		spin_lock_irq(&css_set_lock);
+		spin_lock_irqsave(&css_set_lock, irqflags);
 		list_del(&it->iters_node);
 		put_css_set_locked(it->cur_cset);
-		spin_unlock_irq(&css_set_lock);
+		spin_unlock_irqrestore(&css_set_lock, irqflags);
 	}
 
 	if (it->cur_dcset)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 1/8] cgroup: Prepare for using css_task_iter_*() in BPF Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 14:40   ` kernel test robot
  2023-10-07 12:45 ` [PATCH bpf-next v4 3/8] bpf: Introduce task open coded " Chuyi Zhou
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

This patch adds kfuncs bpf_iter_css_task_{new,next,destroy} which allow
creation and manipulation of struct bpf_iter_css_task in open-coded
iterator style. These kfuncs actually wrapps css_task_iter_{start,next,
end}. BPF programs can use these kfuncs through bpf_for_each macro for
iteration of all tasks under a css.

css_task_iter_*() would try to get the global spin-lock *css_set_lock*, so
the bpf side has to be careful in where it allows to use this iter.
Currently we only allow it in bpf_lsm and bpf iter-s.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 kernel/bpf/helpers.c                          |  3 ++
 kernel/bpf/task_iter.c                        | 53 +++++++++++++++++++
 kernel/bpf/verifier.c                         | 23 ++++++++
 .../testing/selftests/bpf/bpf_experimental.h  |  8 +++
 4 files changed, 87 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index dd1c69ee3375..8699cabb52b2 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2549,6 +2549,9 @@ BTF_ID_FLAGS(func, bpf_dynptr_slice_rdwr, KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_num_new, KF_ITER_NEW)
 BTF_ID_FLAGS(func, bpf_iter_num_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_dynptr_adjust)
 BTF_ID_FLAGS(func, bpf_dynptr_is_null)
 BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 7473068ed313..2cfcb4dd8a37 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -7,6 +7,7 @@
 #include <linux/fs.h>
 #include <linux/fdtable.h>
 #include <linux/filter.h>
+#include <linux/bpf_mem_alloc.h>
 #include <linux/btf_ids.h>
 #include "mmap_unlock_work.h"
 
@@ -803,6 +804,58 @@ const struct bpf_func_proto bpf_find_vma_proto = {
 	.arg5_type	= ARG_ANYTHING,
 };
 
+struct bpf_iter_css_task {
+	__u64 __opaque[1];
+} __attribute__((aligned(8)));
+
+struct bpf_iter_css_task_kern {
+	struct css_task_iter *css_it;
+} __attribute__((aligned(8)));
+
+__bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
+		struct cgroup_subsys_state *css, unsigned int flags)
+{
+	struct bpf_iter_css_task_kern *kit = (void *)it;
+
+	BUILD_BUG_ON(sizeof(struct bpf_iter_css_task_kern) != sizeof(struct bpf_iter_css_task));
+	BUILD_BUG_ON(__alignof__(struct bpf_iter_css_task_kern) !=
+					__alignof__(struct bpf_iter_css_task));
+	kit->css_it = NULL;
+	switch (flags) {
+	case CSS_TASK_ITER_PROCS | CSS_TASK_ITER_THREADED:
+	case CSS_TASK_ITER_PROCS:
+	case 0:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	kit->css_it = bpf_mem_alloc(&bpf_global_ma, sizeof(struct css_task_iter));
+	if (!kit->css_it)
+		return -ENOMEM;
+	css_task_iter_start(css, flags, kit->css_it);
+	return 0;
+}
+
+__bpf_kfunc struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it)
+{
+	struct bpf_iter_css_task_kern *kit = (void *)it;
+
+	if (!kit->css_it)
+		return NULL;
+	return css_task_iter_next(kit->css_it);
+}
+
+__bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
+{
+	struct bpf_iter_css_task_kern *kit = (void *)it;
+
+	if (!kit->css_it)
+		return;
+	css_task_iter_end(kit->css_it);
+	bpf_mem_free(&bpf_global_ma, kit->css_it);
+}
+
 DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
 
 static void do_mmap_read_unlock(struct irq_work *entry)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index eed7350e15f4..528d375c17ee 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10450,6 +10450,7 @@ enum special_kfunc_type {
 	KF_bpf_percpu_obj_new_impl,
 	KF_bpf_percpu_obj_drop_impl,
 	KF_bpf_throw,
+	KF_bpf_iter_css_task_new,
 };
 
 BTF_SET_START(special_kfunc_set)
@@ -10473,6 +10474,7 @@ BTF_ID(func, bpf_dynptr_clone)
 BTF_ID(func, bpf_percpu_obj_new_impl)
 BTF_ID(func, bpf_percpu_obj_drop_impl)
 BTF_ID(func, bpf_throw)
+BTF_ID(func, bpf_iter_css_task_new)
 BTF_SET_END(special_kfunc_set)
 
 BTF_ID_LIST(special_kfunc_list)
@@ -10498,6 +10500,7 @@ BTF_ID(func, bpf_dynptr_clone)
 BTF_ID(func, bpf_percpu_obj_new_impl)
 BTF_ID(func, bpf_percpu_obj_drop_impl)
 BTF_ID(func, bpf_throw)
+BTF_ID(func, bpf_iter_css_task_new)
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
 {
@@ -11028,6 +11031,20 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
 						  &meta->arg_rbtree_root.field);
 }
 
+static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
+{
+	enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+
+	switch (prog_type) {
+	case BPF_PROG_TYPE_LSM:
+		return true;
+	case BPF_TRACE_ITER:
+		return env->prog->aux->sleepable;
+	default:
+		return false;
+	}
+}
+
 static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta,
 			    int insn_idx)
 {
@@ -11278,6 +11295,12 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 			break;
 		}
 		case KF_ARG_PTR_TO_ITER:
+			if (meta->func_id == special_kfunc_list[KF_bpf_iter_css_task_new]) {
+				if (!check_css_task_iter_allowlist(env)) {
+					verbose(env, "css_task_iter is only allowed in bpf_lsm and bpf iter-s\n");
+					return -EINVAL;
+				}
+			}
 			ret = process_iter_arg(env, regno, insn_idx, meta);
 			if (ret < 0)
 				return ret;
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 9aa29564bd74..8b53537e0f27 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -450,4 +450,12 @@ extern void bpf_throw(u64 cookie) __ksym;
 		__bpf_assert_op(LHS, <=, END, value, false);		\
 	})
 
+struct bpf_iter_css_task;
+struct cgroup_subsys_state;
+extern int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
+		struct cgroup_subsys_state *css, unsigned int flags) __weak __ksym;
+extern struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it) __weak __ksym;
+extern void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it) __weak __ksym;
+
+
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 3/8] bpf: Introduce task open coded iterator kfuncs
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 1/8] cgroup: Prepare for using css_task_iter_*() in BPF Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 15:22   ` kernel test robot
  2023-10-07 12:45 ` [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded " Chuyi Zhou
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

This patch adds kfuncs bpf_iter_task_{new,next,destroy} which allow
creation and manipulation of struct bpf_iter_task in open-coded iterator
style. BPF programs can use these kfuncs or through bpf_for_each macro to
iterate all processes in the system.

The API design keep consistent with SEC("iter/task"). bpf_iter_task_new()
accepts a specific task and iterating type which allows:

1. iterating all process in the system(BPF_TASK_ITER_ALL_PROCS)

2. iterating all threads in the system(BPF_TASK_ITER_ALL_THREADS)

3. iterating all threads of a specific task(BPF_TASK_ITER_PROC_THREADS)

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 kernel/bpf/helpers.c                          |  3 +
 kernel/bpf/task_iter.c                        | 82 +++++++++++++++++++
 .../testing/selftests/bpf/bpf_experimental.h  |  5 ++
 3 files changed, 90 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 8699cabb52b2..8111437a999e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2552,6 +2552,9 @@ BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_dynptr_adjust)
 BTF_ID_FLAGS(func, bpf_dynptr_is_null)
 BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 2cfcb4dd8a37..c2e1c3cbffea 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -856,6 +856,88 @@ __bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
 	bpf_mem_free(&bpf_global_ma, kit->css_it);
 }
 
+struct bpf_iter_task {
+	__u64 __opaque[3];
+} __attribute__((aligned(8)));
+
+struct bpf_iter_task_kern {
+	struct task_struct *task;
+	struct task_struct *pos;
+	unsigned int flags;
+} __attribute__((aligned(8)));
+
+enum {
+	BPF_TASK_ITER_ALL_PROCS,
+	BPF_TASK_ITER_ALL_THREADS,
+	BPF_TASK_ITER_PROC_THREADS
+};
+
+__bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
+		struct task_struct *task, unsigned int flags)
+{
+	struct bpf_iter_task_kern *kit = (void *)it;
+
+	BUILD_BUG_ON(sizeof(struct bpf_iter_task_kern) != sizeof(struct bpf_iter_task));
+	BUILD_BUG_ON(__alignof__(struct bpf_iter_task_kern) !=
+					__alignof__(struct bpf_iter_task));
+
+	kit->task = kit->pos = NULL;
+	switch (flags) {
+	case BPF_TASK_ITER_ALL_THREADS:
+	case BPF_TASK_ITER_ALL_PROCS:
+	case BPF_TASK_ITER_PROC_THREADS:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (flags == BPF_TASK_ITER_PROC_THREADS)
+		kit->task = task;
+	else
+		kit->task = &init_task;
+	kit->pos = kit->task;
+	kit->flags = flags;
+	return 0;
+}
+
+__bpf_kfunc struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it)
+{
+	struct bpf_iter_task_kern *kit = (void *)it;
+	struct task_struct *pos;
+	unsigned int flags;
+
+	flags = kit->flags;
+	pos = kit->pos;
+
+	if (!pos)
+		goto out;
+
+	if (flags == BPF_TASK_ITER_ALL_PROCS)
+		goto get_next_task;
+
+	kit->pos = next_thread(kit->pos);
+	if (kit->pos == kit->task) {
+		if (flags == BPF_TASK_ITER_PROC_THREADS) {
+			kit->pos = NULL;
+			goto out;
+		}
+	} else
+		goto out;
+
+get_next_task:
+	kit->pos = next_task(kit->pos);
+	kit->task = kit->pos;
+	if (kit->pos == &init_task)
+		kit->pos = NULL;
+
+out:
+	return pos;
+}
+
+__bpf_kfunc void bpf_iter_task_destroy(struct bpf_iter_task *it)
+{
+}
+
 DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
 
 static void do_mmap_read_unlock(struct irq_work *entry)
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 8b53537e0f27..1ec82997cce7 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -457,5 +457,10 @@ extern int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
 extern struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it) __weak __ksym;
 extern void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it) __weak __ksym;
 
+struct bpf_iter_task;
+extern int bpf_iter_task_new(struct bpf_iter_task *it,
+		struct task_struct *task, unsigned int flags) __weak __ksym;
+extern struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it) __weak __ksym;
+extern void bpf_iter_task_destroy(struct bpf_iter_task *it) __weak __ksym;
 
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded iterator kfuncs
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (2 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 3/8] bpf: Introduce task open coded " Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 16:05   ` kernel test robot
  2023-10-11  4:44   ` Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 5/8] bpf: teach the verifier to enforce css_iter and task_iter in RCU CS Chuyi Zhou
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

This Patch adds kfuncs bpf_iter_css_{new,next,destroy} which allow
creation and manipulation of struct bpf_iter_css in open-coded iterator
style. These kfuncs actually wrapps css_next_descendant_{pre, post}.
css_iter can be used to:

1) iterating a sepcific cgroup tree with pre/post/up order

2) iterating cgroup_subsystem in BPF Prog, like
for_each_mem_cgroup_tree/cpuset_for_each_descendant_pre in kernel.

The API design is consistent with cgroup_iter. bpf_iter_css_new accepts
parameters defining iteration order and starting css. Here we also reuse
BPF_CGROUP_ITER_DESCENDANTS_PRE, BPF_CGROUP_ITER_DESCENDANTS_POST,
BPF_CGROUP_ITER_ANCESTORS_UP enums.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 kernel/bpf/cgroup_iter.c                      | 59 +++++++++++++++++++
 kernel/bpf/helpers.c                          |  3 +
 .../testing/selftests/bpf/bpf_experimental.h  |  6 ++
 3 files changed, 68 insertions(+)

diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index 810378f04fbc..9c6ad892ae82 100644
--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -294,3 +294,62 @@ static int __init bpf_cgroup_iter_init(void)
 }
 
 late_initcall(bpf_cgroup_iter_init);
+
+struct bpf_iter_css {
+	__u64 __opaque[3];
+} __attribute__((aligned(8)));
+
+struct bpf_iter_css_kern {
+	struct cgroup_subsys_state *start;
+	struct cgroup_subsys_state *pos;
+	unsigned int flags;
+} __attribute__((aligned(8)));
+
+__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
+		struct cgroup_subsys_state *start, unsigned int flags)
+{
+	struct bpf_iter_css_kern *kit = (void *)it;
+
+	BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css));
+	BUILD_BUG_ON(__alignof__(struct bpf_iter_css_kern) != __alignof__(struct bpf_iter_css));
+
+	kit->start = NULL;
+	switch (flags) {
+	case BPF_CGROUP_ITER_DESCENDANTS_PRE:
+	case BPF_CGROUP_ITER_DESCENDANTS_POST:
+	case BPF_CGROUP_ITER_ANCESTORS_UP:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	kit->start = start;
+	kit->pos = NULL;
+	kit->flags = flags;
+	return 0;
+}
+
+__bpf_kfunc struct cgroup_subsys_state *bpf_iter_css_next(struct bpf_iter_css *it)
+{
+	struct bpf_iter_css_kern *kit = (void *)it;
+
+	if (!kit->start)
+		return NULL;
+
+	switch (kit->flags) {
+	case BPF_CGROUP_ITER_DESCENDANTS_PRE:
+		kit->pos = css_next_descendant_pre(kit->pos, kit->start);
+		break;
+	case BPF_CGROUP_ITER_DESCENDANTS_POST:
+		kit->pos = css_next_descendant_post(kit->pos, kit->start);
+		break;
+	case BPF_CGROUP_ITER_ANCESTORS_UP:
+		kit->pos = kit->pos ? kit->pos->parent : kit->start;
+	}
+
+	return kit->pos;
+}
+
+__bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
+{
+}
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 8111437a999e..7596e83d6715 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2555,6 +2555,9 @@ BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_dynptr_adjust)
 BTF_ID_FLAGS(func, bpf_dynptr_is_null)
 BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 1ec82997cce7..9aab609f6edd 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -463,4 +463,10 @@ extern int bpf_iter_task_new(struct bpf_iter_task *it,
 extern struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it) __weak __ksym;
 extern void bpf_iter_task_destroy(struct bpf_iter_task *it) __weak __ksym;
 
+struct bpf_iter_css;
+extern int bpf_iter_css_new(struct bpf_iter_css *it,
+				struct cgroup_subsys_state *start, unsigned int flags) __weak __ksym;
+extern struct cgroup_subsys_state *bpf_iter_css_next(struct bpf_iter_css *it) __weak __ksym;
+extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym;
+
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 5/8] bpf: teach the verifier to enforce css_iter and task_iter in RCU CS
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (3 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded " Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 6/8] bpf: Let bpf_iter_task_new accept null task ptr Chuyi Zhou
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

css_iter and task_iter should be used in rcu section. Specifically, in
sleepable progs explicit bpf_rcu_read_lock() is needed before use these
iters. In normal bpf progs that have implicit rcu_read_lock(), it's OK to
use them directly.

This patch adds a new a KF flag KF_RCU_PROTECTED for bpf_iter_task_new and
bpf_iter_css_new. It means the kfunc should be used in RCU CS. We check
whether we are in rcu cs before we want to invoke this kfunc. If the rcu
protection is guaranteed, we would let st->type = PTR_TO_STACK | MEM_RCU.
Once user do rcu_unlock during the iteration, state MEM_RCU of regs would
be cleared. is_iter_reg_valid_init() will reject if reg->type is UNTRUSTED.

It is worth noting that currently, bpf_rcu_read_unlock does not
clear the state of the STACK_ITER reg, since bpf_for_each_spilled_reg
only considers STACK_SPILL. This patch also let bpf_for_each_spilled_reg
search STACK_ITER.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 include/linux/bpf_verifier.h | 19 ++++++++------
 include/linux/btf.h          |  1 +
 kernel/bpf/helpers.c         |  4 +--
 kernel/bpf/verifier.c        | 50 ++++++++++++++++++++++++++++--------
 4 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 94ec766432f5..e67cd45a85be 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -386,19 +386,18 @@ struct bpf_verifier_state {
 	u32 jmp_history_cnt;
 };
 
-#define bpf_get_spilled_reg(slot, frame)				\
+#define bpf_get_spilled_reg(slot, frame, mask)				\
 	(((slot < frame->allocated_stack / BPF_REG_SIZE) &&		\
-	  (frame->stack[slot].slot_type[0] == STACK_SPILL))		\
+	  ((1 << frame->stack[slot].slot_type[0]) & (mask))) \
 	 ? &frame->stack[slot].spilled_ptr : NULL)
 
 /* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
-#define bpf_for_each_spilled_reg(iter, frame, reg)			\
-	for (iter = 0, reg = bpf_get_spilled_reg(iter, frame);		\
+#define bpf_for_each_spilled_reg(iter, frame, reg, mask)			\
+	for (iter = 0, reg = bpf_get_spilled_reg(iter, frame, mask);		\
 	     iter < frame->allocated_stack / BPF_REG_SIZE;		\
-	     iter++, reg = bpf_get_spilled_reg(iter, frame))
+	     iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
 
-/* Invoke __expr over regsiters in __vst, setting __state and __reg */
-#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr)   \
+#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
 	({                                                               \
 		struct bpf_verifier_state *___vstate = __vst;            \
 		int ___i, ___j;                                          \
@@ -410,7 +409,7 @@ struct bpf_verifier_state {
 				__reg = &___regs[___j];                  \
 				(void)(__expr);                          \
 			}                                                \
-			bpf_for_each_spilled_reg(___j, __state, __reg) { \
+			bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
 				if (!__reg)                              \
 					continue;                        \
 				(void)(__expr);                          \
@@ -418,6 +417,10 @@ struct bpf_verifier_state {
 		}                                                        \
 	})
 
+/* Invoke __expr over regsiters in __vst, setting __state and __reg */
+#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
+	bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
+
 /* linked list of verifier states used to prune search */
 struct bpf_verifier_state_list {
 	struct bpf_verifier_state state;
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 928113a80a95..c2231c64d60b 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -74,6 +74,7 @@
 #define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
 #define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
 #define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
+#define KF_RCU_PROTECTED (1 << 11) /* kfunc should be protected by rcu cs when they are invoked */
 
 /*
  * Tag marking a kernel function as a kfunc. This is meant to minimize the
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 7596e83d6715..fb345507b12b 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2552,10 +2552,10 @@ BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
-BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
 BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
-BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
 BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_dynptr_adjust)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 528d375c17ee..3a60cc87520e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1173,7 +1173,12 @@ static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg
 
 static void __mark_reg_known_zero(struct bpf_reg_state *reg);
 
+static bool in_rcu_cs(struct bpf_verifier_env *env);
+
+static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta);
+
 static int mark_stack_slots_iter(struct bpf_verifier_env *env,
+				 struct bpf_kfunc_call_arg_meta *meta,
 				 struct bpf_reg_state *reg, int insn_idx,
 				 struct btf *btf, u32 btf_id, int nr_slots)
 {
@@ -1194,6 +1199,12 @@ static int mark_stack_slots_iter(struct bpf_verifier_env *env,
 
 		__mark_reg_known_zero(st);
 		st->type = PTR_TO_STACK; /* we don't have dedicated reg type */
+		if (is_kfunc_rcu_protected(meta)) {
+			if (in_rcu_cs(env))
+				st->type |= MEM_RCU;
+			else
+				st->type |= PTR_UNTRUSTED;
+		}
 		st->live |= REG_LIVE_WRITTEN;
 		st->ref_obj_id = i == 0 ? id : 0;
 		st->iter.btf = btf;
@@ -1268,7 +1279,7 @@ static bool is_iter_reg_valid_uninit(struct bpf_verifier_env *env,
 	return true;
 }
 
-static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
+static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 				   struct btf *btf, u32 btf_id, int nr_slots)
 {
 	struct bpf_func_state *state = func(env, reg);
@@ -1276,26 +1287,28 @@ static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_
 
 	spi = iter_get_spi(env, reg, nr_slots);
 	if (spi < 0)
-		return false;
+		return -EINVAL;
 
 	for (i = 0; i < nr_slots; i++) {
 		struct bpf_stack_state *slot = &state->stack[spi - i];
 		struct bpf_reg_state *st = &slot->spilled_ptr;
 
+		if (st->type & PTR_UNTRUSTED)
+			return -EPROTO;
 		/* only main (first) slot has ref_obj_id set */
 		if (i == 0 && !st->ref_obj_id)
-			return false;
+			return -EINVAL;
 		if (i != 0 && st->ref_obj_id)
-			return false;
+			return -EINVAL;
 		if (st->iter.btf != btf || st->iter.btf_id != btf_id)
-			return false;
+			return -EINVAL;
 
 		for (j = 0; j < BPF_REG_SIZE; j++)
 			if (slot->slot_type[j] != STACK_ITER)
-				return false;
+				return -EINVAL;
 	}
 
-	return true;
+	return 0;
 }
 
 /* Check if given stack slot is "special":
@@ -7618,15 +7631,24 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
 				return err;
 		}
 
-		err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
+		err = mark_stack_slots_iter(env, meta, reg, insn_idx, meta->btf, btf_id, nr_slots);
 		if (err)
 			return err;
 	} else {
 		/* iter_next() or iter_destroy() expect initialized iter state*/
-		if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
+		err = is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots);
+		switch (err) {
+		case 0:
+			break;
+		case -EINVAL:
 			verbose(env, "expected an initialized iter_%s as arg #%d\n",
 				iter_type_str(meta->btf, btf_id), regno);
-			return -EINVAL;
+			return err;
+		case -EPROTO:
+			verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
+			return err;
+		default:
+			return err;
 		}
 
 		spi = iter_get_spi(env, reg, nr_slots);
@@ -10209,6 +10231,11 @@ static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
 	return meta->kfunc_flags & KF_RCU;
 }
 
+static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
+{
+	return meta->kfunc_flags & KF_RCU_PROTECTED;
+}
+
 static bool __kfunc_param_match_suffix(const struct btf *btf,
 				       const struct btf_param *arg,
 				       const char *suffix)
@@ -11560,6 +11587,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	if (env->cur_state->active_rcu_lock) {
 		struct bpf_func_state *state;
 		struct bpf_reg_state *reg;
+		u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
 
 		if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
 			verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
@@ -11570,7 +11598,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
 			return -EINVAL;
 		} else if (rcu_unlock) {
-			bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
+			bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
 				if (reg->type & MEM_RCU) {
 					reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
 					reg->type |= PTR_UNTRUSTED;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 6/8] bpf: Let bpf_iter_task_new accept null task ptr
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (4 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 5/8] bpf: teach the verifier to enforce css_iter and task_iter in RCU CS Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 7/8] selftests/bpf: rename bpf_iter_task.c to bpf_iter_tasks.c Chuyi Zhou
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

When using task_iter to iterate all threads of a specific task, we enforce
that the user must pass a valid task pointer to ensure safety. However,
when iterating all threads/process in the system, BPF verifier still
require a valid ptr instead of "nullable" pointer, even though it's
pointless, which is a kind of surprising from usability standpoint. It
would be nice if we could let that kfunc accept a explicit null pointer
when we are using BPF_TASK_ITER_ALL_{PROCS, THREADS} and a valid pointer
when using BPF_TASK_ITER_THREAD.

Given a trival kfunc:
	__bpf_kfunc void FN(struct TYPE_A *obj);

BPF Prog would reject a nullptr for obj. The error info is:
"arg#x pointer type xx xx must point to scalar, or struct with scalar"
reported by get_kfunc_ptr_arg_type(). The reg->type is SCALAR_VALUE and
the btf type of ref_t is not scalar or scalar_struct which leads to the
rejection of get_kfunc_ptr_arg_type.

This patch add "__nullable" annotation:
	__bpf_kfunc void FN(struct TYPE_A *obj__nullable);
Here __nullable indicates obj can be optional, user can pass a explicit
nullptr or a normal TYPE_A pointer. In get_kfunc_ptr_arg_type(), we will
detect whether the current arg is optional and register is null, If so,
return a new kfunc_ptr_arg_type KF_ARG_PTR_TO_NULL and skip to the next
arg in check_kfunc_args().

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 kernel/bpf/task_iter.c |  7 +++++--
 kernel/bpf/verifier.c  | 13 ++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index c2e1c3cbffea..773be9a221f5 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -873,7 +873,7 @@ enum {
 };
 
 __bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
-		struct task_struct *task, unsigned int flags)
+		struct task_struct *task__nullable, unsigned int flags)
 {
 	struct bpf_iter_task_kern *kit = (void *)it;
 
@@ -885,14 +885,17 @@ __bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
 	switch (flags) {
 	case BPF_TASK_ITER_ALL_THREADS:
 	case BPF_TASK_ITER_ALL_PROCS:
+		break;
 	case BPF_TASK_ITER_PROC_THREADS:
+		if (!task__nullable)
+			return -EINVAL;
 		break;
 	default:
 		return -EINVAL;
 	}
 
 	if (flags == BPF_TASK_ITER_PROC_THREADS)
-		kit->task = task;
+		kit->task = task__nullable;
 	else
 		kit->task = &init_task;
 	kit->pos = kit->task;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3a60cc87520e..d09697dbfd9c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10310,6 +10310,11 @@ static bool is_kfunc_arg_refcounted_kptr(const struct btf *btf, const struct btf
 	return __kfunc_param_match_suffix(btf, arg, "__refcounted_kptr");
 }
 
+static bool is_kfunc_arg_nullable(const struct btf *btf, const struct btf_param *arg)
+{
+	return __kfunc_param_match_suffix(btf, arg, "__nullable");
+}
+
 static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
 					  const struct btf_param *arg,
 					  const char *name)
@@ -10452,6 +10457,7 @@ enum kfunc_ptr_arg_type {
 	KF_ARG_PTR_TO_CALLBACK,
 	KF_ARG_PTR_TO_RB_ROOT,
 	KF_ARG_PTR_TO_RB_NODE,
+	KF_ARG_PTR_TO_NULL,
 };
 
 enum special_kfunc_type {
@@ -10608,6 +10614,8 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env,
 	if (is_kfunc_arg_callback(env, meta->btf, &args[argno]))
 		return KF_ARG_PTR_TO_CALLBACK;
 
+	if (is_kfunc_arg_nullable(meta->btf, &args[argno]) && register_is_null(reg))
+		return KF_ARG_PTR_TO_NULL;
 
 	if (argno + 1 < nargs &&
 	    (is_kfunc_arg_mem_size(meta->btf, &args[argno + 1], &regs[regno + 1]) ||
@@ -11158,7 +11166,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 		}
 
 		if ((is_kfunc_trusted_args(meta) || is_kfunc_rcu(meta)) &&
-		    (register_is_null(reg) || type_may_be_null(reg->type))) {
+		    (register_is_null(reg) || type_may_be_null(reg->type)) &&
+			!is_kfunc_arg_nullable(meta->btf, &args[i])) {
 			verbose(env, "Possibly NULL pointer passed to trusted arg%d\n", i);
 			return -EACCES;
 		}
@@ -11183,6 +11192,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 			return kf_arg_type;
 
 		switch (kf_arg_type) {
+		case KF_ARG_PTR_TO_NULL:
+			continue;
 		case KF_ARG_PTR_TO_ALLOC_BTF_ID:
 		case KF_ARG_PTR_TO_BTF_ID:
 			if (!is_kfunc_trusted_args(meta) && !is_kfunc_rcu(meta))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 7/8] selftests/bpf: rename bpf_iter_task.c to bpf_iter_tasks.c
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (5 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 6/8] bpf: Let bpf_iter_task_new accept null task ptr Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-07 12:45 ` [PATCH bpf-next v4 8/8] selftests/bpf: Add tests for open-coded task and css iter Chuyi Zhou
  2023-10-10  8:01 ` [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Daniel Borkmann
  8 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

The newly-added struct bpf_iter_task has a name collision with a selftest
for the seq_file task iter's bpf skel, so the selftests/bpf/progs file is
renamed in order to avoid the collision.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 .../selftests/bpf/prog_tests/bpf_iter.c        | 18 +++++++++---------
 .../{bpf_iter_task.c => bpf_iter_tasks.c}      |  0
 2 files changed, 9 insertions(+), 9 deletions(-)
 rename tools/testing/selftests/bpf/progs/{bpf_iter_task.c => bpf_iter_tasks.c} (100%)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 1f02168103dd..dc60e8e125cd 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -7,7 +7,7 @@
 #include "bpf_iter_ipv6_route.skel.h"
 #include "bpf_iter_netlink.skel.h"
 #include "bpf_iter_bpf_map.skel.h"
-#include "bpf_iter_task.skel.h"
+#include "bpf_iter_tasks.skel.h"
 #include "bpf_iter_task_stack.skel.h"
 #include "bpf_iter_task_file.skel.h"
 #include "bpf_iter_task_vma.skel.h"
@@ -215,12 +215,12 @@ static void *do_nothing_wait(void *arg)
 static void test_task_common_nocheck(struct bpf_iter_attach_opts *opts,
 				     int *num_unknown, int *num_known)
 {
-	struct bpf_iter_task *skel;
+	struct bpf_iter_tasks *skel;
 	pthread_t thread_id;
 	void *ret;
 
-	skel = bpf_iter_task__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "bpf_iter_task__open_and_load"))
+	skel = bpf_iter_tasks__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "bpf_iter_tasks__open_and_load"))
 		return;
 
 	ASSERT_OK(pthread_mutex_lock(&do_nothing_mutex), "pthread_mutex_lock");
@@ -239,7 +239,7 @@ static void test_task_common_nocheck(struct bpf_iter_attach_opts *opts,
 	ASSERT_FALSE(pthread_join(thread_id, &ret) || ret != NULL,
 		     "pthread_join");
 
-	bpf_iter_task__destroy(skel);
+	bpf_iter_tasks__destroy(skel);
 }
 
 static void test_task_common(struct bpf_iter_attach_opts *opts, int num_unknown, int num_known)
@@ -307,10 +307,10 @@ static void test_task_pidfd(void)
 
 static void test_task_sleepable(void)
 {
-	struct bpf_iter_task *skel;
+	struct bpf_iter_tasks *skel;
 
-	skel = bpf_iter_task__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "bpf_iter_task__open_and_load"))
+	skel = bpf_iter_tasks__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "bpf_iter_tasks__open_and_load"))
 		return;
 
 	do_dummy_read(skel->progs.dump_task_sleepable);
@@ -320,7 +320,7 @@ static void test_task_sleepable(void)
 	ASSERT_GT(skel->bss->num_success_copy_from_user_task, 0,
 		  "num_success_copy_from_user_task");
 
-	bpf_iter_task__destroy(skel);
+	bpf_iter_tasks__destroy(skel);
 }
 
 static void test_task_stack(void)
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task.c b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
similarity index 100%
rename from tools/testing/selftests/bpf/progs/bpf_iter_task.c
rename to tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH bpf-next v4 8/8] selftests/bpf: Add tests for open-coded task and css iter
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (6 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 7/8] selftests/bpf: rename bpf_iter_task.c to bpf_iter_tasks.c Chuyi Zhou
@ 2023-10-07 12:45 ` Chuyi Zhou
  2023-10-10  8:01 ` [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Daniel Borkmann
  8 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-07 12:45 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel, Chuyi Zhou

This patch adds three subtests to demonstrate these patterns and validating
correctness.

subtest1:

1) We use task_iter to iterate all process in the system and search for the
current process with a given pid.

2) We create some threads in current process context, and use
BPF_TASK_ITER_PROC_THREADS to iterate all threads of current process. As
expected, we would find all the threads of current process.

3) We create some threads and use BPF_TASK_ITER_ALL_THREADS to iterate all
threads in the system. As expected, we would find all the threads which was
created.

subtest2: We create a cgroup and add the current task to the cgroup. In the
BPF program, we would use bpf_for_each(css_task, task, css) to iterate all
tasks under the cgroup. As expected, we would find the current process.

subtest3:

1) We create a cgroup tree. In the BPF program, we use
bpf_for_each(css, pos, root, XXX) to iterate all descendant under the root
with pre and post order. As expected, we would find all descendant and the
last iterating cgroup in post-order is root cgroup, the first iterating
cgroup in pre-order is root cgroup.

2) We wse BPF_CGROUP_ITER_ANCESTORS_UP to traverse the cgroup tree starting
from leaf and root separately, and record the height. The diff of the
hights would be the total tree-high - 1.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 .../testing/selftests/bpf/prog_tests/iters.c  | 161 ++++++++++++++++++
 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 ++++++++++++
 5 files changed, 423 insertions(+)
 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

diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
index 10804ae5ae97..d1d5dc3bedd3 100644
--- a/tools/testing/selftests/bpf/prog_tests/iters.c
+++ b/tools/testing/selftests/bpf/prog_tests/iters.c
@@ -1,13 +1,24 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
 
+#include <sys/syscall.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <malloc.h>
+#include <stdlib.h>
 #include <test_progs.h>
+#include "cgroup_helpers.h"
 
 #include "iters.skel.h"
 #include "iters_state_safety.skel.h"
 #include "iters_looping.skel.h"
 #include "iters_num.skel.h"
 #include "iters_testmod_seq.skel.h"
+#include "iters_task.skel.h"
+#include "iters_css_task.skel.h"
+#include "iters_css.skel.h"
+#include "iters_task_failure.skel.h"
 
 static void subtest_num_iters(void)
 {
@@ -90,6 +101,149 @@ static void subtest_testmod_seq_iters(void)
 	iters_testmod_seq__destroy(skel);
 }
 
+static pthread_mutex_t do_nothing_mutex;
+
+static void *do_nothing_wait(void *arg)
+{
+	pthread_mutex_lock(&do_nothing_mutex);
+	pthread_mutex_unlock(&do_nothing_mutex);
+
+	pthread_exit(arg);
+}
+
+#define thread_num 2
+
+static void subtest_task_iters(void)
+{
+	struct iters_task *skel;
+	pthread_t thread_ids[thread_num];
+	void *ret;
+	int err;
+
+	skel = iters_task__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		goto cleanup;
+	err = iters_task__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+	skel->bss->target_pid = getpid();
+	err = iters_task__attach(skel);
+	if (!ASSERT_OK(err, "iters_task__attach"))
+		goto cleanup;
+	pthread_mutex_lock(&do_nothing_mutex);
+	for (int i = 0; i < thread_num; i++)
+		ASSERT_OK(pthread_create(&thread_ids[i], NULL, &do_nothing_wait, NULL),
+			"pthread_create");
+
+	syscall(SYS_getpgid);
+	iters_task__detach(skel);
+	ASSERT_EQ(skel->bss->process_cnt, 1, "process_cnt");
+	ASSERT_EQ(skel->bss->thread_cnt, thread_num + 1, "thread_cnt");
+	ASSERT_EQ(skel->bss->all_thread_cnt, thread_num + 1, "all_thread_cnt");
+	pthread_mutex_unlock(&do_nothing_mutex);
+	for (int i = 0; i < thread_num; i++)
+		pthread_join(thread_ids[i], &ret);
+cleanup:
+	iters_task__destroy(skel);
+}
+
+extern int stack_mprotect(void);
+
+static void subtest_css_task_iters(void)
+{
+	struct iters_css_task *skel;
+	int err, cg_fd, cg_id;
+	const char *cgrp_path = "/cg1";
+
+	err = setup_cgroup_environment();
+	if (!ASSERT_OK(err, "setup_cgroup_environment"))
+		goto cleanup;
+	cg_fd = create_and_get_cgroup(cgrp_path);
+	if (!ASSERT_GE(cg_fd, 0, "cg_create"))
+		goto cleanup;
+	cg_id = get_cgroup_id(cgrp_path);
+	err = join_cgroup(cgrp_path);
+	if (!ASSERT_OK(err, "setup_cgroup_environment"))
+		goto cleanup;
+
+	skel = iters_css_task__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		goto cleanup;
+
+	err = iters_css_task__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+
+	skel->bss->target_pid = getpid();
+	skel->bss->cg_id = cg_id;
+	err = iters_css_task__attach(skel);
+
+	err = stack_mprotect();
+	if (!ASSERT_OK(err, "iters_task__attach"))
+		goto cleanup;
+
+	iters_css_task__detach(skel);
+	ASSERT_EQ(skel->bss->css_task_cnt, 1, "css_task_cnt");
+
+cleanup:
+	cleanup_cgroup_environment();
+	iters_css_task__destroy(skel);
+}
+
+static void subtest_css_iters(void)
+{
+	struct iters_css *skel;
+	struct {
+		const char *path;
+		int fd;
+	} cgs[] = {
+		{ "/cg1" },
+		{ "/cg1/cg2" },
+		{ "/cg1/cg2/cg3" },
+		{ "/cg1/cg2/cg3/cg4" },
+	};
+	int err, cg_nr = ARRAY_SIZE(cgs);
+	int i;
+
+	err = setup_cgroup_environment();
+	if (!ASSERT_OK(err, "setup_cgroup_environment"))
+		goto cleanup;
+	for (i = 0; i < cg_nr; i++) {
+		cgs[i].fd = create_and_get_cgroup(cgs[i].path);
+		if (!ASSERT_GE(cgs[i].fd, 0, "cg_create"))
+			goto cleanup;
+	}
+
+	skel = iters_css__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		goto cleanup;
+	err = iters_css__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+
+	skel->bss->target_pid = getpid();
+	skel->bss->root_cg_id = get_cgroup_id(cgs[0].path);
+	skel->bss->leaf_cg_id = get_cgroup_id(cgs[cg_nr - 1].path);
+	err = iters_css__attach(skel);
+
+	if (!ASSERT_OK(err, "iters_task__attach"))
+		goto cleanup;
+
+	syscall(SYS_getpgid);
+	ASSERT_EQ(skel->bss->pre_css_dec_cnt, cg_nr, "pre order search dec count");
+	ASSERT_EQ(skel->bss->first_cg_id, get_cgroup_id(cgs[0].path),
+				"pre order search first cgroup id");
+
+	ASSERT_EQ(skel->bss->post_css_dec_cnt, cg_nr, "post order search dec count");
+	ASSERT_EQ(skel->bss->last_cg_id, get_cgroup_id(cgs[0].path),
+				"post order search last cgroup id");
+	ASSERT_EQ(skel->bss->tree_high, cg_nr - 1, "tree high");
+	iters_css__detach(skel);
+cleanup:
+	cleanup_cgroup_environment();
+	iters_css__destroy(skel);
+}
+
 void test_iters(void)
 {
 	RUN_TESTS(iters_state_safety);
@@ -103,4 +257,11 @@ void test_iters(void)
 		subtest_num_iters();
 	if (test__start_subtest("testmod_seq"))
 		subtest_testmod_seq_iters();
+	if (test__start_subtest("task"))
+		subtest_task_iters();
+	if (test__start_subtest("css_task"))
+		subtest_css_task_iters();
+	if (test__start_subtest("css"))
+		subtest_css_iters();
+	RUN_TESTS(iters_task_failure);
 }
diff --git a/tools/testing/selftests/bpf/progs/iters_css.c b/tools/testing/selftests/bpf/progs/iters_css.c
new file mode 100644
index 000000000000..1422a7956c44
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iters_css.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023 Chuyi Zhou <zhouchuyi@bytedance.com> */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+pid_t target_pid = 0;
+u64 root_cg_id;
+u64 leaf_cg_id;
+
+u64 last_cg_id = 0;
+u64 first_cg_id = 0;
+
+int post_css_dec_cnt = 0;
+int pre_css_dec_cnt = 0;
+int tree_high = 0;
+
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
+void bpf_cgroup_release(struct cgroup *p) __ksym;
+void bpf_rcu_read_lock(void) __ksym;
+void bpf_rcu_read_unlock(void) __ksym;
+
+SEC("fentry.s/" SYS_PREFIX "sys_getpgid")
+int iter_css_for_each(const void *ctx)
+{
+	struct task_struct *cur_task = bpf_get_current_task_btf();
+	struct cgroup_subsys_state *root_css, *leaf_css, *pos;
+	struct cgroup *root_cgrp, *leaf_cgrp, *cur_cgrp;
+
+	if (cur_task->pid != target_pid)
+		return 0;
+
+	root_cgrp = bpf_cgroup_from_id(root_cg_id);
+
+	if (!root_cgrp)
+		return 0;
+
+	leaf_cgrp = bpf_cgroup_from_id(leaf_cg_id);
+
+	if (!leaf_cgrp) {
+		bpf_cgroup_release(root_cgrp);
+		return 0;
+	}
+	root_css = &root_cgrp->self;
+	leaf_css = &leaf_cgrp->self;
+	bpf_rcu_read_lock();
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
+		cur_cgrp = pos->cgroup;
+		post_css_dec_cnt += 1;
+		last_cg_id = cur_cgrp->kn->id;
+	}
+
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_PRE) {
+		cur_cgrp = pos->cgroup;
+		pre_css_dec_cnt += 1;
+		if (!first_cg_id)
+			first_cg_id = cur_cgrp->kn->id;
+	}
+
+	bpf_for_each(css, pos, leaf_css, BPF_CGROUP_ITER_ANCESTORS_UP)
+		tree_high += 1;
+
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_ANCESTORS_UP)
+		tree_high -= 1;
+	bpf_rcu_read_unlock();
+	bpf_cgroup_release(root_cgrp);
+	bpf_cgroup_release(leaf_cgrp);
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/iters_css_task.c b/tools/testing/selftests/bpf/progs/iters_css_task.c
new file mode 100644
index 000000000000..506a2755234e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iters_css_task.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023 Chuyi Zhou <zhouchuyi@bytedance.com> */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
+void bpf_cgroup_release(struct cgroup *p) __ksym;
+
+pid_t target_pid = 0;
+int css_task_cnt = 0;
+u64 cg_id;
+
+SEC("lsm/file_mprotect")
+int BPF_PROG(iter_css_task_for_each)
+{
+	struct task_struct *cur_task = bpf_get_current_task_btf();
+	struct cgroup_subsys_state *css;
+	struct task_struct *task;
+	struct cgroup *cgrp;
+
+	if (cur_task->pid != target_pid)
+		return 0;
+
+	cgrp = bpf_cgroup_from_id(cg_id);
+
+	if (cgrp == NULL)
+		return 0;
+	css = &cgrp->self;
+
+	bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS)
+		if (task->pid == target_pid)
+			css_task_cnt += 1;
+
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/iters_task.c b/tools/testing/selftests/bpf/progs/iters_task.c
new file mode 100644
index 000000000000..bd6d4f7b5e59
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iters_task.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023 Chuyi Zhou <zhouchuyi@bytedance.com> */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+pid_t target_pid = 0;
+int process_cnt = 0;
+int thread_cnt = 0;
+int all_thread_cnt = 0;
+
+void bpf_rcu_read_lock(void) __ksym;
+void bpf_rcu_read_unlock(void) __ksym;
+
+SEC("fentry.s/" SYS_PREFIX "sys_getpgid")
+int iter_task_for_each_sleep(void *ctx)
+{
+	struct task_struct *cur_task = bpf_get_current_task_btf();
+	struct task_struct *pos;
+
+	if (cur_task->pid != target_pid)
+		return 0;
+	bpf_rcu_read_lock();
+	bpf_for_each(task, pos, NULL, BPF_TASK_ITER_ALL_PROCS)
+		if (pos->pid == target_pid)
+			process_cnt += 1;
+
+	bpf_for_each(task, pos, cur_task, BPF_TASK_ITER_PROC_THREADS)
+		thread_cnt += 1;
+
+	bpf_for_each(task, pos, NULL, BPF_TASK_ITER_ALL_THREADS)
+		if (pos->tgid == target_pid)
+			all_thread_cnt += 1;
+	bpf_rcu_read_unlock();
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/iters_task_failure.c b/tools/testing/selftests/bpf/progs/iters_task_failure.c
new file mode 100644
index 000000000000..c3bf96a67dba
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iters_task_failure.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023 Chuyi Zhou <zhouchuyi@bytedance.com> */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
+void bpf_cgroup_release(struct cgroup *p) __ksym;
+void bpf_rcu_read_lock(void) __ksym;
+void bpf_rcu_read_unlock(void) __ksym;
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("expected an RCU CS when using bpf_iter_task_next")
+int BPF_PROG(iter_tasks_without_lock)
+{
+	struct task_struct *pos;
+
+	bpf_for_each(task, pos, NULL, BPF_TASK_ITER_ALL_PROCS) {
+
+	}
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("expected an RCU CS when using bpf_iter_css_next")
+int BPF_PROG(iter_css_without_lock)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
+
+	}
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("expected an RCU CS when using bpf_iter_task_next")
+int BPF_PROG(iter_tasks_lock_and_unlock)
+{
+	struct task_struct *pos;
+
+	bpf_rcu_read_lock();
+	bpf_for_each(task, pos, NULL, BPF_TASK_ITER_ALL_PROCS) {
+		bpf_rcu_read_unlock();
+
+		bpf_rcu_read_lock();
+	}
+	bpf_rcu_read_unlock();
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("expected an RCU CS when using bpf_iter_css_next")
+int BPF_PROG(iter_css_lock_and_unlock)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_rcu_read_lock();
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
+		bpf_rcu_read_unlock();
+
+		bpf_rcu_read_lock();
+	}
+	bpf_rcu_read_unlock();
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("css_task_iter is only allowed in bpf_lsm and bpf iter-s")
+int BPF_PROG(iter_css_task_for_each)
+{
+	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;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs
  2023-10-07 12:45 ` [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs Chuyi Zhou
@ 2023-10-07 14:40   ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-10-07 14:40 UTC (permalink / raw)
  To: Chuyi Zhou, bpf
  Cc: oe-kbuild-all, ast, daniel, andrii, martin.lau, tj, linux-kernel,
	Chuyi Zhou

Hi Chuyi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Chuyi-Zhou/cgroup-Prepare-for-using-css_task_iter_-in-BPF/20231007-204750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20231007124522.34834-3-zhouchuyi%40bytedance.com
patch subject: [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310072246.OfAldQpf-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310072246.OfAldQpf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310072246.OfAldQpf-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/bpf/task_iter.c:815:17: warning: no previous prototype for 'bpf_iter_css_task_new' [-Wmissing-prototypes]
     815 | __bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
         |                 ^~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:840:33: warning: no previous prototype for 'bpf_iter_css_task_next' [-Wmissing-prototypes]
     840 | __bpf_kfunc struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it)
         |                                 ^~~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:849:18: warning: no previous prototype for 'bpf_iter_css_task_destroy' [-Wmissing-prototypes]
     849 | __bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
         |                  ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/bpf_iter_css_task_new +815 kernel/bpf/task_iter.c

   814	
 > 815	__bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
   816			struct cgroup_subsys_state *css, unsigned int flags)
   817	{
   818		struct bpf_iter_css_task_kern *kit = (void *)it;
   819	
   820		BUILD_BUG_ON(sizeof(struct bpf_iter_css_task_kern) != sizeof(struct bpf_iter_css_task));
   821		BUILD_BUG_ON(__alignof__(struct bpf_iter_css_task_kern) !=
   822						__alignof__(struct bpf_iter_css_task));
   823		kit->css_it = NULL;
   824		switch (flags) {
   825		case CSS_TASK_ITER_PROCS | CSS_TASK_ITER_THREADED:
   826		case CSS_TASK_ITER_PROCS:
   827		case 0:
   828			break;
   829		default:
   830			return -EINVAL;
   831		}
   832	
   833		kit->css_it = bpf_mem_alloc(&bpf_global_ma, sizeof(struct css_task_iter));
   834		if (!kit->css_it)
   835			return -ENOMEM;
   836		css_task_iter_start(css, flags, kit->css_it);
   837		return 0;
   838	}
   839	
 > 840	__bpf_kfunc struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it)
   841	{
   842		struct bpf_iter_css_task_kern *kit = (void *)it;
   843	
   844		if (!kit->css_it)
   845			return NULL;
   846		return css_task_iter_next(kit->css_it);
   847	}
   848	
 > 849	__bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
   850	{
   851		struct bpf_iter_css_task_kern *kit = (void *)it;
   852	
   853		if (!kit->css_it)
   854			return;
   855		css_task_iter_end(kit->css_it);
   856		bpf_mem_free(&bpf_global_ma, kit->css_it);
   857	}
   858	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 3/8] bpf: Introduce task open coded iterator kfuncs
  2023-10-07 12:45 ` [PATCH bpf-next v4 3/8] bpf: Introduce task open coded " Chuyi Zhou
@ 2023-10-07 15:22   ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-10-07 15:22 UTC (permalink / raw)
  To: Chuyi Zhou, bpf
  Cc: oe-kbuild-all, ast, daniel, andrii, martin.lau, tj, linux-kernel,
	Chuyi Zhou

Hi Chuyi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Chuyi-Zhou/cgroup-Prepare-for-using-css_task_iter_-in-BPF/20231007-204750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20231007124522.34834-4-zhouchuyi%40bytedance.com
patch subject: [PATCH bpf-next v4 3/8] bpf: Introduce task open coded iterator kfuncs
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310072354.0oARP80g-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310072354.0oARP80g-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310072354.0oARP80g-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/bpf/task_iter.c:815:17: warning: no previous prototype for 'bpf_iter_css_task_new' [-Wmissing-prototypes]
     815 | __bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
         |                 ^~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/task_iter.c:840:33: warning: no previous prototype for 'bpf_iter_css_task_next' [-Wmissing-prototypes]
     840 | __bpf_kfunc struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it)
         |                                 ^~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/task_iter.c:849:18: warning: no previous prototype for 'bpf_iter_css_task_destroy' [-Wmissing-prototypes]
     849 | __bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
         |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:875:17: warning: no previous prototype for 'bpf_iter_task_new' [-Wmissing-prototypes]
     875 | __bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
         |                 ^~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:903:33: warning: no previous prototype for 'bpf_iter_task_next' [-Wmissing-prototypes]
     903 | __bpf_kfunc struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it)
         |                                 ^~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:937:18: warning: no previous prototype for 'bpf_iter_task_destroy' [-Wmissing-prototypes]
     937 | __bpf_kfunc void bpf_iter_task_destroy(struct bpf_iter_task *it)
         |                  ^~~~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:
   kernel/bpf/task_iter.c: In function 'bpf_iter_task_new':
   include/linux/compiler_types.h:425:45: error: call to '__compiletime_assert_438' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct bpf_iter_task_kern) != sizeof(struct bpf_iter_task)
     425 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:406:25: note: in definition of macro '__compiletime_assert'
     406 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:425:9: note: in expansion of macro '_compiletime_assert'
     425 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^~~~~~~~~~~~~~~~
   kernel/bpf/task_iter.c:880:9: note: in expansion of macro 'BUILD_BUG_ON'
     880 |         BUILD_BUG_ON(sizeof(struct bpf_iter_task_kern) != sizeof(struct bpf_iter_task));
         |         ^~~~~~~~~~~~


vim +/bpf_iter_task_new +875 kernel/bpf/task_iter.c

   814	
 > 815	__bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
   816			struct cgroup_subsys_state *css, unsigned int flags)
   817	{
   818		struct bpf_iter_css_task_kern *kit = (void *)it;
   819	
   820		BUILD_BUG_ON(sizeof(struct bpf_iter_css_task_kern) != sizeof(struct bpf_iter_css_task));
   821		BUILD_BUG_ON(__alignof__(struct bpf_iter_css_task_kern) !=
   822						__alignof__(struct bpf_iter_css_task));
   823		kit->css_it = NULL;
   824		switch (flags) {
   825		case CSS_TASK_ITER_PROCS | CSS_TASK_ITER_THREADED:
   826		case CSS_TASK_ITER_PROCS:
   827		case 0:
   828			break;
   829		default:
   830			return -EINVAL;
   831		}
   832	
   833		kit->css_it = bpf_mem_alloc(&bpf_global_ma, sizeof(struct css_task_iter));
   834		if (!kit->css_it)
   835			return -ENOMEM;
   836		css_task_iter_start(css, flags, kit->css_it);
   837		return 0;
   838	}
   839	
 > 840	__bpf_kfunc struct task_struct *bpf_iter_css_task_next(struct bpf_iter_css_task *it)
   841	{
   842		struct bpf_iter_css_task_kern *kit = (void *)it;
   843	
   844		if (!kit->css_it)
   845			return NULL;
   846		return css_task_iter_next(kit->css_it);
   847	}
   848	
 > 849	__bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
   850	{
   851		struct bpf_iter_css_task_kern *kit = (void *)it;
   852	
   853		if (!kit->css_it)
   854			return;
   855		css_task_iter_end(kit->css_it);
   856		bpf_mem_free(&bpf_global_ma, kit->css_it);
   857	}
   858	
   859	struct bpf_iter_task {
   860		__u64 __opaque[3];
   861	} __attribute__((aligned(8)));
   862	
   863	struct bpf_iter_task_kern {
   864		struct task_struct *task;
   865		struct task_struct *pos;
   866		unsigned int flags;
   867	} __attribute__((aligned(8)));
   868	
   869	enum {
   870		BPF_TASK_ITER_ALL_PROCS,
   871		BPF_TASK_ITER_ALL_THREADS,
   872		BPF_TASK_ITER_PROC_THREADS
   873	};
   874	
 > 875	__bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
   876			struct task_struct *task, unsigned int flags)
   877	{
   878		struct bpf_iter_task_kern *kit = (void *)it;
   879	
   880		BUILD_BUG_ON(sizeof(struct bpf_iter_task_kern) != sizeof(struct bpf_iter_task));
   881		BUILD_BUG_ON(__alignof__(struct bpf_iter_task_kern) !=
   882						__alignof__(struct bpf_iter_task));
   883	
   884		kit->task = kit->pos = NULL;
   885		switch (flags) {
   886		case BPF_TASK_ITER_ALL_THREADS:
   887		case BPF_TASK_ITER_ALL_PROCS:
   888		case BPF_TASK_ITER_PROC_THREADS:
   889			break;
   890		default:
   891			return -EINVAL;
   892		}
   893	
   894		if (flags == BPF_TASK_ITER_PROC_THREADS)
   895			kit->task = task;
   896		else
   897			kit->task = &init_task;
   898		kit->pos = kit->task;
   899		kit->flags = flags;
   900		return 0;
   901	}
   902	
 > 903	__bpf_kfunc struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it)
   904	{
   905		struct bpf_iter_task_kern *kit = (void *)it;
   906		struct task_struct *pos;
   907		unsigned int flags;
   908	
   909		flags = kit->flags;
   910		pos = kit->pos;
   911	
   912		if (!pos)
   913			goto out;
   914	
   915		if (flags == BPF_TASK_ITER_ALL_PROCS)
   916			goto get_next_task;
   917	
   918		kit->pos = next_thread(kit->pos);
   919		if (kit->pos == kit->task) {
   920			if (flags == BPF_TASK_ITER_PROC_THREADS) {
   921				kit->pos = NULL;
   922				goto out;
   923			}
   924		} else
   925			goto out;
   926	
   927	get_next_task:
   928		kit->pos = next_task(kit->pos);
   929		kit->task = kit->pos;
   930		if (kit->pos == &init_task)
   931			kit->pos = NULL;
   932	
   933	out:
   934		return pos;
   935	}
   936	
 > 937	__bpf_kfunc void bpf_iter_task_destroy(struct bpf_iter_task *it)
   938	{
   939	}
   940	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded iterator kfuncs
  2023-10-07 12:45 ` [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded " Chuyi Zhou
@ 2023-10-07 16:05   ` kernel test robot
  2023-10-11  4:44   ` Chuyi Zhou
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-10-07 16:05 UTC (permalink / raw)
  To: Chuyi Zhou, bpf
  Cc: oe-kbuild-all, ast, daniel, andrii, martin.lau, tj, linux-kernel,
	Chuyi Zhou

Hi Chuyi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Chuyi-Zhou/cgroup-Prepare-for-using-css_task_iter_-in-BPF/20231007-204750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20231007124522.34834-5-zhouchuyi%40bytedance.com
patch subject: [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded iterator kfuncs
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310072337.CzRlbffm-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310072337.CzRlbffm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310072337.CzRlbffm-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/bpf/cgroup_iter.c:308:17: warning: no previous prototype for 'bpf_iter_css_new' [-Wmissing-prototypes]
     308 | __bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
         |                 ^~~~~~~~~~~~~~~~
>> kernel/bpf/cgroup_iter.c:332:41: warning: no previous prototype for 'bpf_iter_css_next' [-Wmissing-prototypes]
     332 | __bpf_kfunc struct cgroup_subsys_state *bpf_iter_css_next(struct bpf_iter_css *it)
         |                                         ^~~~~~~~~~~~~~~~~
>> kernel/bpf/cgroup_iter.c:353:18: warning: no previous prototype for 'bpf_iter_css_destroy' [-Wmissing-prototypes]
     353 | __bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
         |                  ^~~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:
   kernel/bpf/cgroup_iter.c: In function 'bpf_iter_css_new':
   include/linux/compiler_types.h:425:45: error: call to '__compiletime_assert_320' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css)
     425 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:406:25: note: in definition of macro '__compiletime_assert'
     406 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:425:9: note: in expansion of macro '_compiletime_assert'
     425 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^~~~~~~~~~~~~~~~
   kernel/bpf/cgroup_iter.c:313:9: note: in expansion of macro 'BUILD_BUG_ON'
     313 |         BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css));
         |         ^~~~~~~~~~~~


vim +/bpf_iter_css_new +308 kernel/bpf/cgroup_iter.c

   307	
 > 308	__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
   309			struct cgroup_subsys_state *start, unsigned int flags)
   310	{
   311		struct bpf_iter_css_kern *kit = (void *)it;
   312	
   313		BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css));
   314		BUILD_BUG_ON(__alignof__(struct bpf_iter_css_kern) != __alignof__(struct bpf_iter_css));
   315	
   316		kit->start = NULL;
   317		switch (flags) {
   318		case BPF_CGROUP_ITER_DESCENDANTS_PRE:
   319		case BPF_CGROUP_ITER_DESCENDANTS_POST:
   320		case BPF_CGROUP_ITER_ANCESTORS_UP:
   321			break;
   322		default:
   323			return -EINVAL;
   324		}
   325	
   326		kit->start = start;
   327		kit->pos = NULL;
   328		kit->flags = flags;
   329		return 0;
   330	}
   331	
 > 332	__bpf_kfunc struct cgroup_subsys_state *bpf_iter_css_next(struct bpf_iter_css *it)
   333	{
   334		struct bpf_iter_css_kern *kit = (void *)it;
   335	
   336		if (!kit->start)
   337			return NULL;
   338	
   339		switch (kit->flags) {
   340		case BPF_CGROUP_ITER_DESCENDANTS_PRE:
   341			kit->pos = css_next_descendant_pre(kit->pos, kit->start);
   342			break;
   343		case BPF_CGROUP_ITER_DESCENDANTS_POST:
   344			kit->pos = css_next_descendant_post(kit->pos, kit->start);
   345			break;
   346		case BPF_CGROUP_ITER_ANCESTORS_UP:
   347			kit->pos = kit->pos ? kit->pos->parent : kit->start;
   348		}
   349	
   350		return kit->pos;
   351	}
   352	
 > 353	__bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters
  2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
                   ` (7 preceding siblings ...)
  2023-10-07 12:45 ` [PATCH bpf-next v4 8/8] selftests/bpf: Add tests for open-coded task and css iter Chuyi Zhou
@ 2023-10-10  8:01 ` Daniel Borkmann
  2023-10-10  8:14   ` Chuyi Zhou
  8 siblings, 1 reply; 16+ messages in thread
From: Daniel Borkmann @ 2023-10-10  8:01 UTC (permalink / raw)
  To: Chuyi Zhou, bpf; +Cc: ast, andrii, martin.lau, tj, linux-kernel

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/20230925105552.817513-1-zhouchuyi@bytedance.com/
> 
> * 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters
  2023-10-10  8:01 ` [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Daniel Borkmann
@ 2023-10-10  8:14   ` Chuyi Zhou
  0 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-10  8:14 UTC (permalink / raw)
  To: Daniel Borkmann, bpf; +Cc: ast, andrii, martin.lau, tj, linux-kernel

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/20230925105552.817513-1-zhouchuyi@bytedance.com/
>>
>> * 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded iterator kfuncs
  2023-10-07 12:45 ` [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded " Chuyi Zhou
  2023-10-07 16:05   ` kernel test robot
@ 2023-10-11  4:44   ` Chuyi Zhou
  2023-10-11  5:32     ` Chuyi Zhou
  1 sibling, 1 reply; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-11  4:44 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel



在 2023/10/7 20:45, Chuyi Zhou 写道:
> This Patch adds kfuncs bpf_iter_css_{new,next,destroy} which allow
> creation and manipulation of struct bpf_iter_css in open-coded iterator
> style. These kfuncs actually wrapps css_next_descendant_{pre, post}.
> css_iter can be used to:
> 
> 1) iterating a sepcific cgroup tree with pre/post/up order
> 
> 2) iterating cgroup_subsystem in BPF Prog, like
> for_each_mem_cgroup_tree/cpuset_for_each_descendant_pre in kernel.
> 
> The API design is consistent with cgroup_iter. bpf_iter_css_new accepts
> parameters defining iteration order and starting css. Here we also reuse
> BPF_CGROUP_ITER_DESCENDANTS_PRE, BPF_CGROUP_ITER_DESCENDANTS_POST,
> BPF_CGROUP_ITER_ANCESTORS_UP enums.
> 
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
> Acked-by: Tejun Heo <tj@kernel.org>
> ---
>   kernel/bpf/cgroup_iter.c                      | 59 +++++++++++++++++++
>   kernel/bpf/helpers.c                          |  3 +
>   .../testing/selftests/bpf/bpf_experimental.h  |  6 ++
>   3 files changed, 68 insertions(+)
> 
> diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
> index 810378f04fbc..9c6ad892ae82 100644
> --- a/kernel/bpf/cgroup_iter.c
> +++ b/kernel/bpf/cgroup_iter.c
> @@ -294,3 +294,62 @@ static int __init bpf_cgroup_iter_init(void)
>   }
>   
>   late_initcall(bpf_cgroup_iter_init);
> +
> +struct bpf_iter_css {
> +	__u64 __opaque[3];
> +} __attribute__((aligned(8)));
> +
> +struct bpf_iter_css_kern {
> +	struct cgroup_subsys_state *start;
> +	struct cgroup_subsys_state *pos;
> +	unsigned int flags;
> +} __attribute__((aligned(8)));
> +
> +__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
> +		struct cgroup_subsys_state *start, unsigned int flags)
> +{
> +	struct bpf_iter_css_kern *kit = (void *)it;
> +
> +	BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css));
> +	BUILD_BUG_ON(__alignof__(struct bpf_iter_css_kern) != __alignof__(struct bpf_iter_css));
> +

This would cause the fail of netdev/build_32bit CI 
(https://netdev.bots.linux.dev/static/nipa/790929/13412333/build_32bit/stderr):

tools/testing/selftests/kvm/settings: warning: ignored by one of the 
.gitignore files
../kernel/bpf/cgroup_iter.c:308:17: warning: no previous prototype for 
‘bpf_iter_css_new’ [-Wmissing-prototypes]
   308 | __bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
       |                 ^~~~~~~~~~~~~~~~
../kernel/bpf/cgroup_iter.c:332:41: warning: no previous prototype for 
‘bpf_iter_css_next’ [-Wmissing-prototypes]
   332 | __bpf_kfunc struct cgroup_subsys_state 
*bpf_iter_css_next(struct bpf_iter_css *it)
       |                                         ^~~~~~~~~~~~~~~~~
../kernel/bpf/cgroup_iter.c:353:18: warning: no previous prototype for 
‘bpf_iter_css_destroy’ [-Wmissing-prototypes]
   353 | __bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
       |                  ^~~~~~~~~~~~~~~~~~~~
In file included from <command-line>:
../kernel/bpf/cgroup_iter.c: In function ‘bpf_iter_css_new’:
./../include/linux/compiler_types.h:425:45: error: call to 
‘__compiletime_assert_322’ declared with attribute error: BUILD_BUG_ON 
failed: sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css)
   425 |         _compiletime_assert(condition, msg, 
__compiletime_assert_, __COUNTER__)
       |                                             ^
./../include/linux/compiler_types.h:406:25: note: in definition of macro 
‘__compiletime_assert’
   406 |                         prefix ## suffix(); 
         \
       |                         ^~~~~~
./../include/linux/compiler_types.h:425:9: note: in expansion of macro 
‘_compiletime_assert’
   425 |         _compiletime_assert(condition, msg, 
__compiletime_assert_, __COUNTER__)
       |         ^~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:39:37: note: in expansion of macro 
‘compiletime_assert’
    39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), 
msg)
       |                                     ^~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:50:9: note: in expansion of macro 
‘BUILD_BUG_ON_MSG’
    50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " 
#condition)
       |         ^~~~~~~~~~~~~~~~
../kernel/bpf/cgroup_iter.c:313:9: note: in expansion of macro 
‘BUILD_BUG_ON’
   313 |         BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != 
sizeof(struct bpf_iter_css));


The reason seems on 32-bit machine, sizeof(struct bpf_iter_css) is 24 
and sizeof(struct bpf_iter_css_kern) is 16.

I was wondering whether the BUILD_BUG_ON check is necessary. Looking at 
the struct bpf_list_node and struct bpf_list_node_kern wich are very 
similay to bpf_iter_css, I didn't see the BUILD_BUG_ON check when 
convert from (struct bpf_list_node *) to (struct bpf_list_node_kern *)

/* Non-opaque version of bpf_list_node in uapi/linux/bpf.h */
struct bpf_list_node_kern {
	struct list_head list_head;
	void *owner;
} __attribute__((aligned(8)));

struct bpf_list_node {
	__u64 :64;
	__u64 :64;
	__u64 :64;
} __attribute__((aligned(8)));

__bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
					struct bpf_list_node *node,
					void *meta__ign, u64 off)
{
	struct bpf_list_node_kern *n = (void *)node;

}




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded iterator kfuncs
  2023-10-11  4:44   ` Chuyi Zhou
@ 2023-10-11  5:32     ` Chuyi Zhou
  0 siblings, 0 replies; 16+ messages in thread
From: Chuyi Zhou @ 2023-10-11  5:32 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, martin.lau, tj, linux-kernel



在 2023/10/11 12:44, Chuyi Zhou 写道:
> 
> 
> 在 2023/10/7 20:45, Chuyi Zhou 写道:
>> This Patch adds kfuncs bpf_iter_css_{new,next,destroy} which allow
>> creation and manipulation of struct bpf_iter_css in open-coded iterator
>> style. These kfuncs actually wrapps css_next_descendant_{pre, post}.
>> css_iter can be used to:
>>
>> 1) iterating a sepcific cgroup tree with pre/post/up order
>>
>> 2) iterating cgroup_subsystem in BPF Prog, like
>> for_each_mem_cgroup_tree/cpuset_for_each_descendant_pre in kernel.
>>
>> The API design is consistent with cgroup_iter. bpf_iter_css_new accepts
>> parameters defining iteration order and starting css. Here we also reuse
>> BPF_CGROUP_ITER_DESCENDANTS_PRE, BPF_CGROUP_ITER_DESCENDANTS_POST,
>> BPF_CGROUP_ITER_ANCESTORS_UP enums.
>>
>> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
>> Acked-by: Tejun Heo <tj@kernel.org>
>> ---
>>   kernel/bpf/cgroup_iter.c                      | 59 +++++++++++++++++++
>>   kernel/bpf/helpers.c                          |  3 +
>>   .../testing/selftests/bpf/bpf_experimental.h  |  6 ++
>>   3 files changed, 68 insertions(+)
>>
>> diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
>> index 810378f04fbc..9c6ad892ae82 100644
>> --- a/kernel/bpf/cgroup_iter.c
>> +++ b/kernel/bpf/cgroup_iter.c
>> @@ -294,3 +294,62 @@ static int __init bpf_cgroup_iter_init(void)
>>   }
>>   late_initcall(bpf_cgroup_iter_init);
>> +
>> +struct bpf_iter_css {
>> +    __u64 __opaque[3];
>> +} __attribute__((aligned(8)));
>> +
>> +struct bpf_iter_css_kern {
>> +    struct cgroup_subsys_state *start;
>> +    struct cgroup_subsys_state *pos;
>> +    unsigned int flags;
>> +} __attribute__((aligned(8)));
>> +
>> +__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
>> +        struct cgroup_subsys_state *start, unsigned int flags)
>> +{
>> +    struct bpf_iter_css_kern *kit = (void *)it;
>> +
>> +    BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct 
>> bpf_iter_css));
>> +    BUILD_BUG_ON(__alignof__(struct bpf_iter_css_kern) != 
>> __alignof__(struct bpf_iter_css));
>> +
> 
> This would cause the fail of netdev/build_32bit CI 
> (https://netdev.bots.linux.dev/static/nipa/790929/13412333/build_32bit/stderr):
> 
> tools/testing/selftests/kvm/settings: warning: ignored by one of the 
> .gitignore files
> ../kernel/bpf/cgroup_iter.c:308:17: warning: no previous prototype for 
> ‘bpf_iter_css_new’ [-Wmissing-prototypes]
>    308 | __bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
>        |                 ^~~~~~~~~~~~~~~~
> ../kernel/bpf/cgroup_iter.c:332:41: warning: no previous prototype for 
> ‘bpf_iter_css_next’ [-Wmissing-prototypes]
>    332 | __bpf_kfunc struct cgroup_subsys_state 
> *bpf_iter_css_next(struct bpf_iter_css *it)
>        |                                         ^~~~~~~~~~~~~~~~~
> ../kernel/bpf/cgroup_iter.c:353:18: warning: no previous prototype for 
> ‘bpf_iter_css_destroy’ [-Wmissing-prototypes]
>    353 | __bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
>        |                  ^~~~~~~~~~~~~~~~~~~~
> In file included from <command-line>:
> ../kernel/bpf/cgroup_iter.c: In function ‘bpf_iter_css_new’:
> ./../include/linux/compiler_types.h:425:45: error: call to 
> ‘__compiletime_assert_322’ declared with attribute error: BUILD_BUG_ON 
> failed: sizeof(struct bpf_iter_css_kern) != sizeof(struct bpf_iter_css)
>    425 |         _compiletime_assert(condition, msg, 
> __compiletime_assert_, __COUNTER__)
>        |                                             ^
> ./../include/linux/compiler_types.h:406:25: note: in definition of macro 
> ‘__compiletime_assert’
>    406 |                         prefix ## suffix();         \
>        |                         ^~~~~~
> ./../include/linux/compiler_types.h:425:9: note: in expansion of macro 
> ‘_compiletime_assert’
>    425 |         _compiletime_assert(condition, msg, 
> __compiletime_assert_, __COUNTER__)
>        |         ^~~~~~~~~~~~~~~~~~~
> ../include/linux/build_bug.h:39:37: note: in expansion of macro 
> ‘compiletime_assert’
>     39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), 
> msg)
>        |                                     ^~~~~~~~~~~~~~~~~~
> ../include/linux/build_bug.h:50:9: note: in expansion of macro 
> ‘BUILD_BUG_ON_MSG’
>     50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " 
> #condition)
>        |         ^~~~~~~~~~~~~~~~
> ../kernel/bpf/cgroup_iter.c:313:9: note: in expansion of macro 
> ‘BUILD_BUG_ON’
>    313 |         BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != 
> sizeof(struct bpf_iter_css));
> 
> 
> The reason seems on 32-bit machine, sizeof(struct bpf_iter_css) is 24 
> and sizeof(struct bpf_iter_css_kern) is 16.
> 
> I was wondering whether the BUILD_BUG_ON check is necessary. Looking at 
> the struct bpf_list_node and struct bpf_list_node_kern wich are very 
> similay to bpf_iter_css, I didn't see the BUILD_BUG_ON check when 
> convert from (struct bpf_list_node *) to (struct bpf_list_node_kern *)
> 
> /* Non-opaque version of bpf_list_node in uapi/linux/bpf.h */
> struct bpf_list_node_kern {
>      struct list_head list_head;
>      void *owner;
> } __attribute__((aligned(8)));
> 
> struct bpf_list_node {
>      __u64 :64;
>      __u64 :64;
>      __u64 :64;
> } __attribute__((aligned(8)));
> 
> __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>                      struct bpf_list_node *node,
>                      void *meta__ign, u64 off)
> {
>      struct bpf_list_node_kern *n = (void *)node;
> 
> }
> 

or we can change the BUILD_BUG_ON check, like bpf_timer_kern in 
bpf_timer_init:

--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -310,7 +310,7 @@ __bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css 
*it,
  {
         struct bpf_iter_css_kern *kit = (void *)it;

-       BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) != sizeof(struct 
bpf_iter_css));
+       BUILD_BUG_ON(sizeof(struct bpf_iter_css_kern) > sizeof(struct 
bpf_iter_css));
         BUILD_BUG_ON(__alignof__(struct bpf_iter_css_kern) != 
__alignof__(struct bpf_iter_css));

         kit->start = NULL;
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 773be9a221f5..0772545568f1 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -877,7 +877,7 @@ __bpf_kfunc int bpf_iter_task_new(struct 
bpf_iter_task *it,
  {
         struct bpf_iter_task_kern *kit = (void *)it;

-       BUILD_BUG_ON(sizeof(struct bpf_iter_task_kern) != sizeof(struct 
bpf_iter_task));
+       BUILD_BUG_ON(sizeof(struct bpf_iter_task_kern) > sizeof(struct 
bpf_iter_task));
         BUILD_BUG_ON(__alignof__(struct bpf_iter_task_kern) !=
                                         __alignof__(struct bpf_iter_task));


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-10-11  5:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-07 12:45 [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 1/8] cgroup: Prepare for using css_task_iter_*() in BPF Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 2/8] bpf: Introduce css_task open-coded iterator kfuncs Chuyi Zhou
2023-10-07 14:40   ` kernel test robot
2023-10-07 12:45 ` [PATCH bpf-next v4 3/8] bpf: Introduce task open coded " Chuyi Zhou
2023-10-07 15:22   ` kernel test robot
2023-10-07 12:45 ` [PATCH bpf-next v4 4/8] bpf: Introduce css open-coded " Chuyi Zhou
2023-10-07 16:05   ` kernel test robot
2023-10-11  4:44   ` Chuyi Zhou
2023-10-11  5:32     ` Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 5/8] bpf: teach the verifier to enforce css_iter and task_iter in RCU CS Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 6/8] bpf: Let bpf_iter_task_new accept null task ptr Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 7/8] selftests/bpf: rename bpf_iter_task.c to bpf_iter_tasks.c Chuyi Zhou
2023-10-07 12:45 ` [PATCH bpf-next v4 8/8] selftests/bpf: Add tests for open-coded task and css iter Chuyi Zhou
2023-10-10  8:01 ` [PATCH bpf-next v4 0/8] Add Open-coded task, css_task and css iters Daniel Borkmann
2023-10-10  8:14   ` Chuyi Zhou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).