Re: [PATCH bpf-next v2 3/4] bpf, x86: Support BPF cookie for fentry/fexit/fmod_ret.

[Kui-Feng Lee <kuifeng@fb.com>]

On Mon, 2022-03-21 at 16:37 -0700, Andrii Nakryiko wrote:
> On Mon, Mar 21, 2022 at 4:24 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> > 
> > On Fri, Mar 18, 2022 at 12:13 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > 
> > > On Tue, Mar 15, 2022 at 05:42:30PM -0700, Kui-Feng Lee wrote:
> > > > Add a bpf_cookie field to attach a cookie to an instance of
> > > > struct
> > > > bpf_link.  The cookie of a bpf_link will be installed when
> > > > calling the
> > > > associated program to make it available to the program.
> > > > 
> > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > ---
> > > >  arch/x86/net/bpf_jit_comp.c    |  4 ++--
> > > >  include/linux/bpf.h            |  1 +
> > > >  include/uapi/linux/bpf.h       |  1 +
> > > >  kernel/bpf/syscall.c           | 11 +++++++----
> > > >  kernel/trace/bpf_trace.c       | 17 +++++++++++++++++
> > > >  tools/include/uapi/linux/bpf.h |  1 +
> > > >  tools/lib/bpf/bpf.c            | 14 ++++++++++++++
> > > >  tools/lib/bpf/bpf.h            |  1 +
> > > >  tools/lib/bpf/libbpf.map       |  1 +
> > > >  9 files changed, 45 insertions(+), 6 deletions(-)
> > > 
> > > please split kernel and libbpf changes into two different
> > > patches.
> > > 
> > 
> > +1
> > 
> > > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > > index f69ce3a01385..dbbf09c84c21 100644
> > > > --- a/tools/lib/bpf/bpf.c
> > > > +++ b/tools/lib/bpf/bpf.c
> > > > @@ -1133,6 +1133,20 @@ int bpf_raw_tracepoint_open(const char
> > > > *name, int prog_fd)
> > > >       return libbpf_err_errno(fd);
> > > >  }
> > > > 
> > > > +int bpf_raw_tracepoint_cookie_open(const char *name, int
> > > > prog_fd, __u64 bpf_cookie)
> > > 
> > > lets introduce opts style to raw_tp_open instead.
> > 
> > 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.
> > 
> > 2. libbpf can be smart enough to pick either RAW_TRACEPOINT_OPEN
> > (and
> > reject it if bpf_cookie is non-zero) or BPF_LINK_CREATE, depending
> > on
> > kernel support. So users would need to only use bpf_link_create()
> > moving forward with all the backwards compatibility preserved.
> > 
> > 
> 
> Oh, and we need bpf_program__attach_trace_opts() as well (regardless
> of whether it is BPF_RAW_TRACEPOINT_OPEN or BPF_LINK_CREATE).
> 
> > 
I removed raw_tp_open_opts() since raw_tp won't support bpf_cookie.
Implemented only bpf_program__attach_trace_opts().