Hello,
在 2023/9/28 07:24, Andrii Nakryiko 写道:
On Mon, Sep 25, 2023 at 3:56 AM Chuyi Zhou <zhouchuyi@xxxxxxxxxxxxx> wrote:
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@xxxxxxxxxxxxx>
---
kernel/bpf/cgroup_iter.c | 57 +++++++++++++++++++
kernel/bpf/helpers.c | 3 +
.../testing/selftests/bpf/bpf_experimental.h | 6 ++
3 files changed, 66 insertions(+)
diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index 810378f04fbc..ebc3d9471f52 100644
--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -294,3 +294,60 @@ static int __init bpf_cgroup_iter_init(void)
}
late_initcall(bpf_cgroup_iter_init);
+
+struct bpf_iter_css {
+ __u64 __opaque[2];
+ __u32 __opaque_int[1];
+} __attribute__((aligned(8)));
+
same as before, __opaque[3] only
+struct bpf_iter_css_kern {
+ struct cgroup_subsys_state *start;
+ struct cgroup_subsys_state *pos;
+ int order;
+} __attribute__((aligned(8)));
+
+__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
+ struct cgroup_subsys_state *start, enum bpf_cgroup_iter_order order)
Similarly, I wonder if we should go for a more generic "flags" argument?
+{
+ struct bpf_iter_css_kern *kit = (void *)it;
empty line
+ kit->start = NULL;
+ 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));
please move this up before kit->start assignment, and separate by empty lines
+ switch (order) {
+ 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->order = order;
+ 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;
empty line
+ if (!kit->start)
+ return NULL;
+
+ switch (kit->order) {
+ 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;
+ default:
we know it's BPF_CGROUP_ITER_ANCESTORS_UP, so why not have that here explicitly?
+ kit->pos = kit->pos ? kit->pos->parent : kit->start;
+ }
+
+ return kit->pos;
wouldn't this implementation never return the "start" css? is that intentional?
Thanks for the review.
This implementation actually would return the "start" css.
1. BPF_CGROUP_ITER_DESCENDANTS_PRE:
1.1 when we first call next(), css_next_descendant_pre(NULL, kit->start)
will return kit->start.
1.2 second call next(), css_next_descendant_pre(kit->start, kit->start)
would return a first valid child under kit->start with pre-order
1.3 third call next, css_next_descendant_pre(last_valid_child,
kit->start) would return the next valid child
...
util css_next_descendant_pre return a NULL pointer, which means we have
visited all valid child including "start" css itself.
The above logic is equal to macro 'css_for_each_descendant_pre' in kernel.
Same, BPF_CGROUP_ITER_DESCENDANTS_POST is equal to macro
'css_for_each_descendant_post' which would return 'start' css when we
have visited all valid child.
2. BPF_CGROUP_ITER_ANCESTORS_UP
2.1 when we fisrt call next(), kit->pos is NULL, and we would return
kit->start.
The selftest in patch7 whould check:
1. when we use BPF_CGROUP_ITER_DESCENDANTS_PRE to iterate a cgroup tree,
the first cgroup we visted should be root('start') cgroup.
2. when we use BPF_CGROUP_ITER_DESCENDANTS_POST to iterate a cgroup
tree, the last cgroup we visited should be root('start') cgroup.
Am I miss something important?
Thanks.
