Hello,
On Thu, Mar 28, 2024 at 12:46:03PM -0700, Alexei Starovoitov wrote:
To use kernel_file_open() it would need path, inode, cred.
Yeah, it's more involved and potentially more controversial. That said, just to push the argument a bit further, at least for path, it'd be nice to have a helper anyway which can return cgroup path. I don't know whether we'd need direct inode access. For cred, just share some root cred?
None of that is available now. Allocating all these structures just to wrap a cgroup pointer feels like overkill. Of course, it would solve the need to introduce other cgroup apis that are already available via text based cgroupfs read/write. So there are pros and cons in both approaches. Maybe the 3rd option would be to expose: cgroup_lock() as a special blend of acquire plus lock.
Oh, let's not expose that. That has been a source of problem for some use cases and we may have to change how cgroups are locked.
Then there will be no need for bpf_task_freeze_cgroup() with task argument. Instead cgroup_freeze() will be such kfunc that takes cgroup argument and the verifier will check that cgroup was acquired/locked. Sort-of what we check to access bpf_rb_root.
There's also cgroup.kill which would be useful for similar use cases. We can add interface for both but idk. Let's say we have something like the following (pardon the bad naming):
bpf_cgroup_knob_write(struct cgroup *cgrp, char *filename, char *buf)
Would that work? I'm not necessarily in love with the idea or against adding separate helpers but the duplication still bothers me a bit.
Thanks.