On Mon, Mar 18, 2024 at 11:03:01PM -0600, Jose Fernandez wrote:
This patch enhances the BPF helpers by adding a kfunc to retrieve the cgroup v2 of a task, addressing a previous limitation where only bpf_task_get_cgroup1 was available for cgroup v1. The new kfunc is particularly useful for scenarios where obtaining the cgroup ID of a task other than the "current" one is necessary, which the existing bpf_get_current_cgroup_id helper cannot accommodate. A specific use case at Netflix involved the sched_switch tracepoint, where we had to get the cgroup IDs of both the prev and next tasks.
The bpf_task_get_cgroup kfunc acquires and returns a reference to a task's default cgroup, ensuring thread-safe access by correctly implementing RCU read locking and unlocking. It leverages the existing cgroup.h helper, and cgroup_tryget to safely acquire a reference to it.
Signed-off-by: Jose Fernandez josef@netflix.com Reviewed-by: Tycho Andersen tycho@tycho.pizza Acked-by: Yonghong Song yonghong.song@linux.dev Acked-by: Stanislav Fomichev sdf@google.com
Acked-by: Tejun Heo tj@kernel.org
but some questions below
+__bpf_kfunc struct cgroup *bpf_task_get_cgroup(struct task_struct *task) +{
- struct cgroup *cgrp;
- rcu_read_lock();
- cgrp = task_dfl_cgroup(task);
- if (!cgroup_tryget(cgrp))
cgrp = NULL;- rcu_read_unlock();
- return cgrp;
+}
So, as this is a lot easier in cgroup2, the above can probably be written directly in BPF (untested and not sure the necessary annotations are in place, so please take it with a big grain of salt):
bpf_rcu_read_lock(); cgrp = task->cgroups->dfl_cgrp; cgrp = bpf_cgroup_from_id(cgrp->kn.id); bpf_rcu_read_unlock();
If all you need is ID, it's even simpler:
bpf_rcu_read_lock(); cgrp_id = task->cgroups->dfl_cgrp->kn.id; bpf_rcu_read_unlock();
In the first example, it's not great that we go from task pointer to cgroup pointer to ID and then back to acquired cgroup pointer. I wonder whether what we really want is to support something like the following:
bpf_rcu_read_lock(); cgrp = bpf_cgroup_tryget(task->cgroups->dfl_cgrp); bpf_rcu_read_unlock();
What do you think?
Thanks.