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.
>
> > +{
> > + 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
> >
>
> --
