On Mon, Jun 27, 2022 at 9:09 PM Yonghong Song [off-list ref] wrote:

On 6/10/22 12:44 PM, Yosry Ahmed wrote:

quoted

From: Hao Luo <redacted>

Cgroup_iter is a type of bpf_iter. It walks over cgroups in two modes:

  - walking a cgroup's descendants.
  - walking a cgroup's ancestors.

The implementation has another choice, BPF_ITER_CGROUP_PARENT_UP.
We should add it here as well.

Sorry about the late reply. I meant to write two modes: walking up and
walking down. And two sub-modes when walking down: pre-order and
post-order.

But it seems this is confusing. I'm going to use three modes in the
next version: UP, PRE and POST.

Besides, since this patch modifies the bpf_iter_link_info, and that
breaks the btf_dump selftest, I need to include the fix of the
selftest in this patch.

quoted

When attaching cgroup_iter, one can set a cgroup to the iter_link
created from attaching. This cgroup is passed as a file descriptor and
serves as the starting point of the walk. If no cgroup is specified,
the starting point will be the root cgroup.

For walking descendants, one can specify the order: either pre-order or
post-order. For walking ancestors, the walk starts at the specified
cgroup and ends at the root.

One can also terminate the walk early by returning 1 from the iter
program.

Note that because walking cgroup hierarchy holds cgroup_mutex, the iter
program is called with cgroup_mutex held.

Overall looks good to me with a few nits below.

Acked-by: Yonghong Song <redacted>

quoted

Signed-off-by: Hao Luo <redacted>
Signed-off-by: Yosry Ahmed <redacted>
---

[...]

quoted

+
+/* cgroup_iter provides two modes of traversal to the cgroup hierarchy.
+ *
+ *  1. Walk the descendants of a cgroup.
+ *  2. Walk the ancestors of a cgroup.

three modes here?

Same here. Will use 'three modes' in the next version.

quoted

+ *

[...]

quoted

+
+static int bpf_iter_attach_cgroup(struct bpf_prog *prog,
+                               union bpf_iter_link_info *linfo,
+                               struct bpf_iter_aux_info *aux)
+{
+     int fd = linfo->cgroup.cgroup_fd;
+     struct cgroup *cgrp;
+
+     if (fd)
+             cgrp = cgroup_get_from_fd(fd);
+     else /* walk the entire hierarchy by default. */
+             cgrp = cgroup_get_from_path("/");
+
+     if (IS_ERR(cgrp))
+             return PTR_ERR(cgrp);
+
+     aux->cgroup.start = cgrp;
+     aux->cgroup.order = linfo->cgroup.traversal_order;

The legality of traversal_order should be checked.

Sounds good. Will do.

quoted

+     return 0;
+}
+

[...]

quoted

+static void bpf_iter_cgroup_show_fdinfo(const struct bpf_iter_aux_info *aux,
+                                     struct seq_file *seq)
+{
+     char *buf;
+
+     buf = kzalloc(PATH_MAX, GFP_KERNEL);
+     if (!buf) {
+             seq_puts(seq, "cgroup_path:\n");

This is a really unlikely case. maybe "cgroup_path:<unknown>"?

Sure, no problem. This is a path that is unlikely to hit.

quoted

+             goto show_order;

[...]