On Tue, Feb 1, 2022 at 12:17 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Mon, Jan 31, 2022 at 10:47 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> > > struct bpf_array_aux {
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 16a7574292a5..3fa27346ab4b 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -1425,6 +1425,7 @@ union bpf_attr {
> > > struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
> > > __u64 name;
> > > __u32 prog_fd;
> > > + __u64 bpf_cookie;
> > > } raw_tracepoint;
> > >
> >
> > As an aside, Alexei, should we bite a bullet and allow attaching
> > raw_tp, fentry/fexit, and other tracing prog attachment through the
> > LINK_CREATE command? BPF_RAW_TRACEPOINT_OPEN makes little sense for
> > anything but raw_tp programs.
>
> raw_tp_open is used for raw_tp, tp_btf, lsm, fentry.
> iirc it's creating a normal bpf_link underneath.
> link_create doesn't exist for raw_tp and friends,
> so this is the best place to add a cookie.
> We can add an alias cmd (instead of raw_tp_open)
> to make it a bit cleaner from uapi naming pov.
> We can allow link_create to do the attach in all those cases as well,
> but it's a different discussion.
I was actually proposing exactly the latter: to allow LINK_CREATE to
create all the programs that RAW_TRACEPOINT_OPEN allows. It's already
confusing because bpf_iter programs are created using LINK_CREATE
(realized that when I saw your recent patches). Also extension
programs are attached through LINK_CREATE. So while we can't get rid
of BPF_RAW_TRACEPOINT_OPEN, I hoped we can add lsm and fentry support
as well (I don't mind raw_tp/tp_btf there as well for completeness),
so at least in the future it would be we all just a universal
LINK_CREATE command.
> link_create.perf_event.bpf_cookie isn't the best name.
> That name was a cause of confusion for me.
> I thought it applies to perf_event only,
> but it's for kuprobe too.
Yeah, not great, but given it was "attach to perf_event FD" command,
it seemed like the most accurate name at the time :) I still don't
know what I'd call it today, apart from having separate (and
duplicate) link_create.kprobe.bpf_cookie,
link_create.uprobe.bpf_cookie, etc. At least libbpf is hiding it
behind bpf_program__attach_kprobe and bpf_program__attach_uprobe,
though.
> So plenty of bikeshedding to do if we decide to do
> link_create for raw_tp. Hence, for now, I'd add a cookie to
> raw_tp/tp_btf/lsm/fentry like this patch is doing.
Sure, that's fine, it was a long shot anyway. But I'd like to get back
to this discussion when we are going to multi-attach fentry/fexit :)
