On Tue, Jun 21, 2022 at 6:01 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Tue, Jun 21, 2022 at 5:36 AM Kumar Kartikeya Dwivedi > <memxor@xxxxxxxxx> wrote: > > > > On Tue, Jun 21, 2022 at 06:58:09AM IST, KP Singh wrote: > > > In preparation for the addition of bpf_getxattr kfunc. > > > > > > Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx> [...] > > > +++ b/kernel/bpf/btf.c > > > @@ -7264,6 +7264,7 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type) > > > case BPF_PROG_TYPE_STRUCT_OPS: > > > return BTF_KFUNC_HOOK_STRUCT_OPS; > > > case BPF_PROG_TYPE_TRACING: > > > + case BPF_PROG_TYPE_LSM: > > > return BTF_KFUNC_HOOK_TRACING; > > > > Should we define another BTF_KFUNC_HOOK_LSM instead? Otherwise when you register > > for tracing or lsm progs, you write to the same hook instead, so kfunc enabled > > for tracing progs also gets enabled for lsm, I guess that is not what user > > intends when registering kfunc set. > > It's probably ok for this case. > We might combine BTF_KFUNC_HOOK* into fewer hooks and I did this intentionally. We do this similarly for helpers too and we unfortunately, we do realize that LSM hooks are not there in all places (esp. where one is auditing stuff). So in reality one does use a combination of LSM and tracing hooks with the policy decision coming from the LSM hook but the state gets accumulated from different hooks. > scope them by attach_btf_id. > Everything is 'tracing' like at the end. > Upcoming hid-bpf is 'tracing'. lsm is 'tracing'. > but we may need to reduce the scope of kfuncs > based on attach point or based on argument type. > tc vs xdp don't need to be separate sets. > Their types (skb vs xdp_md) are different, so only > right kfuncs can be used, but bpf_ct_release() is needed > in both tc and xdp. > So we could combine tc and xdp into 'btf_kfunc_hook_networking' > without compromising the safety. > acquire vs release ideally would be indicated with btf_tag, > but gcc still doesn't have support for btf_tag and we cannot > require the kernel to be built with clang. > acquire, release, sleepable, ret_null should be flags on a kfunc > instead of a set. It would be easier to manage and less boilerplate +1 This would be awesome, I gave it a shot to use btf_tag but hit this feature gap in GCC. - KP > code. Not clear how to do this cleanly. > export_symbol approach is a bit heavy and requires name being unique > across kernel and modules. > func name mangling or typedef-ing comes to mind. not so clean either.
