On 2025/1/17 16:58, Tejun Heo wrote:
Hello,
On Thu, Jan 16, 2025 at 07:41:11PM +0000, Juntong Deng wrote:
...
+static int bpf_scx_bpf_capabilities_adjust(unsigned long *bpf_capabilities,
+ u32 context_info, bool enter)
+{
+ if (enter) {
+ switch (context_info) {
+ case offsetof(struct sched_ext_ops, select_cpu):
+ ENABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_SELECT_CPU);
+ ENABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_ENQUEUE);
+ break;
...
+ }
+ } else {
+ switch (context_info) {
+ case offsetof(struct sched_ext_ops, select_cpu):
+ DISABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_SELECT_CPU);
+ DISABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_ENQUEUE);
+ break;
...
+ }
+ return 0;
+}
From sched_ext's POV, this, or whatever works is fine but I have some basic
comments:
- The caps are u32. If all kfuncs use this facility for access control, it's
plausible or even likely that 32 is not going to be enough. I suppose it
can be turned into u64 and then a proper bitmap later? Maybe good idea to
start out with a proper bitmap in the first place?
Thanks for your reply.
I considered this, and 32 capabilities is definitely not enough.
So although in BTF_ID_FLAGS the capability is 32 bits, this is used as
an index and a bitmap is used in struct bpf_verifier_env.
We can have UINT_MAX quantity capability.
- There are benefits to centralizing all the caps in a single place but it
can also be kinda cumbersome.
Yes, I agree, maybe centralized declarations are cumbersome.
But the purpose of centralized declaration is to give each capability a
unique number for distinction.
I am not sure there is a non-centralized declarative way to do this.
Do you have any suggested approach?
- Even with global defs, the cap adjustments are procedural, not
declarative. If it needs to be procedural anyway, I wonder whether the
global defs are necessary in the first place. What prevents implementing
it the other way around - pass in the calling context and provide helpers
and macros to respond yay or nay procedurally.
You are right!
Thanks for pointing this out!
If it is procedural, then global definitions are really not necessary.
I will rethink this.
Thanks.
