| Message ID | 20240813132831.184362-2-technoboy85@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: enable some functions in cgroup programs | expand |
On Tue, Aug 13, 2024 at 6:28 AM Matteo Croce <technoboy85@gmail.com> wrote: > > From: Matteo Croce <teknoraver@meta.com> > > These kfuncs are enabled even in BPF_PROG_TYPE_TRACING, so they > should be safe also in BPF_CGROUP_* programs. > > In enum btf_kfunc_hook, rename BTF_KFUNC_HOOK_CGROUP_SKB to a more > generic BTF_KFUNC_HOOK_CGROUP, since it's used for all the cgroup > related program types. > > Signed-off-by: Matteo Croce <teknoraver@meta.com> > --- > kernel/bpf/btf.c | 8 ++++++-- > kernel/bpf/helpers.c | 6 ++++++ > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 95426d5b634e..08d094875f00 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -212,7 +212,7 @@ enum btf_kfunc_hook { > BTF_KFUNC_HOOK_TRACING, > BTF_KFUNC_HOOK_SYSCALL, > BTF_KFUNC_HOOK_FMODRET, > - BTF_KFUNC_HOOK_CGROUP_SKB, > + BTF_KFUNC_HOOK_CGROUP, > BTF_KFUNC_HOOK_SCHED_ACT, > BTF_KFUNC_HOOK_SK_SKB, > BTF_KFUNC_HOOK_SOCKET_FILTER, > @@ -8312,8 +8312,12 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type) > case BPF_PROG_TYPE_SYSCALL: > return BTF_KFUNC_HOOK_SYSCALL; > case BPF_PROG_TYPE_CGROUP_SKB: > + case BPF_PROG_TYPE_CGROUP_SOCK: > + case BPF_PROG_TYPE_CGROUP_DEVICE: > case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: > - return BTF_KFUNC_HOOK_CGROUP_SKB; > + case BPF_PROG_TYPE_CGROUP_SOCKOPT: > + case BPF_PROG_TYPE_CGROUP_SYSCTL: > + return BTF_KFUNC_HOOK_CGROUP; > case BPF_PROG_TYPE_SCHED_ACT: > return BTF_KFUNC_HOOK_SCHED_ACT; > case BPF_PROG_TYPE_SK_SKB: > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index d02ae323996b..0d1d97d968b0 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -3052,6 +3052,12 @@ static int __init kfunc_init(void) > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &generic_kfunc_set); > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &generic_kfunc_set); > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_DEVICE, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SYSCTL, &generic_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT, &generic_kfunc_set); So given all those CGROUP_xxx program types map to the same cgroup-generic BTF_KFUNC_HOOK_CGROUP hook, why do we need 6 repetitions of the same thing? I'd say let's keep just one (pick any, CGROUP_SKB, for example) registration. And then we should follow up with cleaning up register_btf_kfunc_id_set() to accept hook type, not program type. And then it will be clean and will make most sense. But other than that looks good to me. pw-bot: cr > ret = ret ?: register_btf_id_dtor_kfuncs(generic_dtors, > ARRAY_SIZE(generic_dtors), > THIS_MODULE); > -- > 2.46.0 >
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 95426d5b634e..08d094875f00 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -212,7 +212,7 @@ enum btf_kfunc_hook { BTF_KFUNC_HOOK_TRACING, BTF_KFUNC_HOOK_SYSCALL, BTF_KFUNC_HOOK_FMODRET, - BTF_KFUNC_HOOK_CGROUP_SKB, + BTF_KFUNC_HOOK_CGROUP, BTF_KFUNC_HOOK_SCHED_ACT, BTF_KFUNC_HOOK_SK_SKB, BTF_KFUNC_HOOK_SOCKET_FILTER, @@ -8312,8 +8312,12 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type) case BPF_PROG_TYPE_SYSCALL: return BTF_KFUNC_HOOK_SYSCALL; case BPF_PROG_TYPE_CGROUP_SKB: + case BPF_PROG_TYPE_CGROUP_SOCK: + case BPF_PROG_TYPE_CGROUP_DEVICE: case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: - return BTF_KFUNC_HOOK_CGROUP_SKB; + case BPF_PROG_TYPE_CGROUP_SOCKOPT: + case BPF_PROG_TYPE_CGROUP_SYSCTL: + return BTF_KFUNC_HOOK_CGROUP; case BPF_PROG_TYPE_SCHED_ACT: return BTF_KFUNC_HOOK_SCHED_ACT; case BPF_PROG_TYPE_SK_SKB: diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index d02ae323996b..0d1d97d968b0 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -3052,6 +3052,12 @@ static int __init kfunc_init(void) ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &generic_kfunc_set); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &generic_kfunc_set); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_DEVICE, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SYSCTL, &generic_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCKOPT, &generic_kfunc_set); ret = ret ?: register_btf_id_dtor_kfuncs(generic_dtors, ARRAY_SIZE(generic_dtors), THIS_MODULE);