[bpf-next] bpf: Add bpf_cgroup_from_id() kfunc

Commit Message

Tejun Heo Feb. 21, 2023, 10:08 p.m. UTC

cgroup ID is an userspace-visible 64bit value uniquely identifying a given
cgroup. As the IDs are used widely, it's useful to be able to look up the
matching cgroups. Add bpf_cgroup_from_id().

Signed-off-by: Tejun Heo <[e-mail]>
---
 Documentation/bpf/kfuncs.rst                  | 10 +++--
 kernel/bpf/helpers.c                          | 18 ++++++++
 .../selftests/bpf/prog_tests/cgrp_kfunc.c     |  1 +
 .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
 .../selftests/bpf/progs/cgrp_kfunc_success.c  | 42 +++++++++++++++++++
 5 files changed, 69 insertions(+), 3 deletions(-)

Comments

Alexei Starovoitov Feb. 22, 2023, 9:20 p.m. UTC | #1

On Tue, Feb 21, 2023 at 2:09 PM Tejun Heo <[e-mail]> wrote:
>
> cgroup ID is an userspace-visible 64bit value uniquely identifying a given
> cgroup. As the IDs are used widely, it's useful to be able to look up the
> matching cgroups. Add bpf_cgroup_from_id().
>
> Signed-off-by: Tejun Heo <[e-mail]>
> ---
>  Documentation/bpf/kfuncs.rst                  | 10 +++--
>  kernel/bpf/helpers.c                          | 18 ++++++++
>  .../selftests/bpf/prog_tests/cgrp_kfunc.c     |  1 +
>  .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
>  .../selftests/bpf/progs/cgrp_kfunc_success.c  | 42 +++++++++++++++++++
>  5 files changed, 69 insertions(+), 3 deletions(-)

Looks great, but could you please split it into two patches:
One for helpers.c and kfuncs.rst and another for selftests
with [PATCH bpf-next] selftests/bpf: subject ?
Doesn't matter much in this case, but it might ease backporting
and easier to track patches grouped by subj.

Tejun Heo Feb. 23, 2023, 1:22 a.m. UTC | #2

On Wed, Feb 22, 2023 at 01:20:19PM -0800, Alexei Starovoitov wrote:
> On Tue, Feb 21, 2023 at 2:09 PM Tejun Heo <[e-mail]> wrote:
> >
> > cgroup ID is an userspace-visible 64bit value uniquely identifying a given
> > cgroup. As the IDs are used widely, it's useful to be able to look up the
> > matching cgroups. Add bpf_cgroup_from_id().
> >
> > Signed-off-by: Tejun Heo <[e-mail]>
> > ---
> >  Documentation/bpf/kfuncs.rst                  | 10 +++--
> >  kernel/bpf/helpers.c                          | 18 ++++++++
> >  .../selftests/bpf/prog_tests/cgrp_kfunc.c     |  1 +
> >  .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
> >  .../selftests/bpf/progs/cgrp_kfunc_success.c  | 42 +++++++++++++++++++
> >  5 files changed, 69 insertions(+), 3 deletions(-)
> 
> Looks great, but could you please split it into two patches:
> One for helpers.c and kfuncs.rst and another for selftests
> with [PATCH bpf-next] selftests/bpf: subject ?
> Doesn't matter much in this case, but it might ease backporting
> and easier to track patches grouped by subj.

Will do.

Thanks.

Patch

diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
index ca96ef3f6896..226313747be5 100644
--- a/Documentation/bpf/kfuncs.rst
+++ b/Documentation/bpf/kfuncs.rst
@@ -583,13 +583,17 @@  You may also acquire a reference to a ``struct cgroup`` kptr that's already
 
 ----
 
-Another kfunc available for interacting with ``struct cgroup *`` objects is
-bpf_cgroup_ancestor(). This allows callers to access the ancestor of a cgroup,
-and return it as a cgroup kptr.
+Other kfuncs available for interacting with ``struct cgroup *`` objects are
+bpf_cgroup_ancestor() and bpf_cgroup_from_id(), allowing callers to access
+the ancestor of a cgroup and find a cgroup by its ID, respectively. Both
+return a cgroup kptr.
 
 .. kernel-doc:: kernel/bpf/helpers.c
    :identifiers: bpf_cgroup_ancestor
 
+.. kernel-doc:: kernel/bpf/helpers.c
+   :identifiers: bpf_cgroup_from_id
+
 Eventually, BPF should be updated to allow this to happen with a normal memory
 load in the program itself. This is currently not possible without more work in
 the verifier. bpf_cgroup_ancestor() can be used as follows:
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 5b278a38ae58..a784be6f8bac 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2101,6 +2101,23 @@  __bpf_kfunc struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level)
 	cgroup_get(ancestor);
 	return ancestor;
 }
+
+/**
+ * bpf_cgroup_from_id - Find a cgroup from its ID. A cgroup returned by this
+ * kfunc which is not subsequently stored in a map, must be released by calling
+ * bpf_cgroup_release().
+ * @cgrp: The cgroup for which we're performing a lookup.
+ * @level: The level of ancestor to look up.
+ */
+__bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
+{
+	struct cgroup *cgrp;
+
+	cgrp = cgroup_get_from_id(cgid);
+	if (IS_ERR(cgrp))
+		return NULL;
+	return cgrp;
+}
 #endif /* CONFIG_CGROUPS */
 
 /**
@@ -2167,6 +2184,7 @@  BTF_ID_FLAGS(func, bpf_cgroup_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_cgroup_kptr_get, KF_ACQUIRE | KF_KPTR_GET | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_cgroup_release, KF_RELEASE)
 BTF_ID_FLAGS(func, bpf_cgroup_ancestor, KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_cgroup_from_id, KF_ACQUIRE | KF_RET_NULL)
 #endif
 BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
 BTF_SET8_END(generic_btf_ids)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgrp_kfunc.c b/tools/testing/selftests/bpf/prog_tests/cgrp_kfunc.c
index b3f7985c8504..adda85f97058 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgrp_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgrp_kfunc.c
@@ -84,6 +84,7 @@  static const char * const success_tests[] = {
 	"test_cgrp_xchg_release",
 	"test_cgrp_get_release",
 	"test_cgrp_get_ancestors",
+	"test_cgrp_from_id",
 };
 
 void test_cgrp_kfunc(void)
diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
index 7d30855bfe78..2f8de933b957 100644
--- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
+++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
@@ -24,6 +24,7 @@  struct cgroup *bpf_cgroup_acquire(struct cgroup *p) __ksym;
 struct cgroup *bpf_cgroup_kptr_get(struct cgroup **pp) __ksym;
 void bpf_cgroup_release(struct cgroup *p) __ksym;
 struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level) __ksym;
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
 
 static inline struct __cgrps_kfunc_map_value *cgrps_kfunc_map_value_lookup(struct cgroup *cgrp)
 {
diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_success.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_success.c
index 0c23ea32df9f..42e13aebdd62 100644
--- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_success.c
+++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_success.c
@@ -168,3 +168,45 @@  int BPF_PROG(test_cgrp_get_ancestors, struct cgroup *cgrp, const char *path)
 
 	return 0;
 }
+
+SEC("tp_btf/cgroup_mkdir")
+int BPF_PROG(test_cgrp_from_id, struct cgroup *cgrp, const char *path)
+{
+	struct cgroup *parent, *res;
+	u64 parent_cgid;
+
+	if (!is_test_kfunc_task())
+		return 0;
+
+	/* @cgrp's ID is not visible yet, let's test with the parent */
+	parent = bpf_cgroup_ancestor(cgrp, cgrp->level - 1);
+	if (!parent) {
+		err = 1;
+		return 0;
+	}
+
+	parent_cgid = parent->kn->id;
+	bpf_cgroup_release(parent);
+
+	res = bpf_cgroup_from_id(parent_cgid);
+	if (!res) {
+		err = 2;
+		return 0;
+	}
+
+	bpf_cgroup_release(res);
+
+	if (res != parent) {
+		err = 3;
+		return 0;
+	}
+
+	res = bpf_cgroup_from_id((u64)-1);
+	if (res) {
+		bpf_cgroup_release(res);
+		err = 4;
+		return 0;
+	}
+
+	return 0;
+}