On Mon, Mar 21, 2022 at 4:24 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Fri, Mar 18, 2022 at 12:13 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> 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@xxxxxx>
> > > ---
> > > 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).
> >
> > > +{
> > > + union bpf_attr attr;
> > > + int fd;
> > > +
> > > + memset(&attr, 0, sizeof(attr));
> > > + attr.raw_tracepoint.name = ptr_to_u64(name);
> > > + attr.raw_tracepoint.prog_fd = prog_fd;
> > > + attr.raw_tracepoint.bpf_cookie = bpf_cookie;
> > > +
> > > + fd = sys_bpf_fd(BPF_RAW_TRACEPOINT_OPEN, &attr, sizeof(attr));
> > > + return libbpf_err_errno(fd);
> > > +}
> > > +
> > > int bpf_btf_load(const void *btf_data, size_t btf_size, const struct bpf_btf_load_opts *opts)
> > > {
> > > const size_t attr_sz = offsetofend(union bpf_attr, btf_log_level);
> > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > > index 5253cb4a4c0a..23bebcdaf23b 100644
> > > --- a/tools/lib/bpf/bpf.h
> > > +++ b/tools/lib/bpf/bpf.h
> > > @@ -477,6 +477,7 @@ LIBBPF_API int bpf_prog_query(int target_fd, enum bpf_attach_type type,
> > > __u32 query_flags, __u32 *attach_flags,
> > > __u32 *prog_ids, __u32 *prog_cnt);
> > > LIBBPF_API int bpf_raw_tracepoint_open(const char *name, int prog_fd);
> > > +LIBBPF_API int bpf_raw_tracepoint_cookie_open(const char *name, int prog_fd, __u64 bpf_cookie);
> > > LIBBPF_API int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf,
> > > __u32 *buf_len, __u32 *prog_id, __u32 *fd_type,
> > > __u64 *probe_offset, __u64 *probe_addr);
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index df1b947792c8..20f947a385fa 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -434,6 +434,7 @@ LIBBPF_0.7.0 {
> > > bpf_xdp_detach;
> > > bpf_xdp_query;
> > > bpf_xdp_query_id;
> > > + bpf_raw_tracepoint_cookie_open;
>
> this (if still needed) should go into 0.8.0 section
>
> > > libbpf_probe_bpf_helper;
> > > libbpf_probe_bpf_map_type;
> > > libbpf_probe_bpf_prog_type;
> > > --
> > > 2.30.2
> > >
> >
> > --
