On Mon, 2022-03-21 at 21:32 -0700, Andrii Nakryiko wrote: > On Mon, Mar 21, 2022 at 6:15 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Mon, Mar 21, 2022 at 4:24 PM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > I remember I brought this up earlier, but I forgot the outcome. > > > What > > > if don't touch BPF_RAW_TRACEPOINT_OPEN and instead allow to > > > create all > > > the same links through more universal BPF_LINK_CREATE command. > > > And > > > only there we add bpf_cookie? There are few advantages: > > > > > > 1. We can separate raw_tracepoint and trampoline-based programs > > > more > > > cleanly in UAPI (it will be two separate structs: > > > link_create.raw_tp > > > with raw tracepoint name vs link_create.trampoline, or whatever > > > the > > > name, with cookie and stuff). Remember that raw_tp won't support > > > bpf_cookie for now, so it would be another advantage not to > > > promise > > > cookie in UAPI. > > > > What would it look like? > > Technically link_create has prog_fd and perf_event.bpf_cookie > > already. > > > > case BPF_PROG_TYPE_TRACING: > > ret = tracing_bpf_link_attach(attr, uattr, prog); > > would just gain a few more checks for prog->expected_attach_type ? > > > > Then link_create cmd will be equivalent to raw_tp_open. > > With and without bpf_cookie. > > ? > > Yes, except I'd leave perf_event for perf_event-based attachments > (kprobe, uprobe, tracepoint) and would define a separate substruct > for > trampoline-based programs. Something like this (I only compile-tested > it, of course). I've also simplified prog_type/expected_attach_type > logic a bit because it felt like a total maze to me and I was getting > lost all the time. Gmail will probably corrupt all the whitespaces, > sorry about that in advance. > > Seems like we could already attach BPF_PROG_TYPE_EXT both through > RAW_TRACEPOINT_OPEN and LINK_CREATE, I didn't realize that. The > "patch" below leaves raw_tp handling > (BPF_PROG_TYPE_TRACING+BPF_TRACE_RAW_TP, > BPF_PROG_TYPE_RAW_TRACEPOINT, > and BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE) in RAW_TRACEPOINT_OPEN. If > we want to completely unify all the bpf_link creations under > LINK_CREATE, see extra small "patch" at the very bottom. I just implemented and tested a patch of tracing links with bpf_link_create, so it can be done with both raw_tp_open and bpf_link_create. Do we want to remove raw_tp_open() eventually? Should I remove raw_tp_open() supports of cookies?