On Tue, Apr 5, 2022 at 10:35 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote: > > 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. > Nice, please send it as part of your cookie patch set in a separate patch. > Do we want to remove raw_tp_open() eventually? Should I remove > raw_tp_open() supports of cookies? We can't remove existing Linux UAPI, but we can stop extending them. So I'd say let's add cookie only through CREATE_LINK and leave RAW_TRACEPOINT_OPEN as is. > >
