Re: [RFC PATCH bpf-next 6/7] sched_ext: Make SCX use BPF capabilities

[Tejun Heo <tj@kernel.org>]

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?

- There are benefits to centralizing all the caps in a single place but it
  can also be kinda cumbersome.

- 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.

Thanks.

-- 
tejun