Re: [PATCH bpf-next v7 4/5] libbpf: Assign cookies to links in libbpf.

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

On Mon, 2022-05-09 at 12:05 -0700, Andrii Nakryiko wrote:
> On Sat, May 7, 2022 at 8:21 PM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > 
> > Add a cookie field to the attributes of bpf_link_create().
> > Add bpf_program__attach_trace_opts() to attach a cookie to a link.
> > 
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
> >  tools/lib/bpf/bpf.c      |  8 ++++++++
> >  tools/lib/bpf/bpf.h      |  3 +++
> >  tools/lib/bpf/libbpf.c   | 32 ++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   | 12 ++++++++++++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  5 files changed, 56 insertions(+)
> > 
> 
> I have a gripe with better code reuse, but that's internal change so
> we can do it in a follow up.
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 73a5192defb3..df9be47d67bc 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -11440,6 +11440,38 @@ struct bpf_link
> > *bpf_program__attach_trace(const struct bpf_program *prog)
> >         return bpf_program__attach_btf_id(prog);
> >  }
> > 
> > +struct bpf_link *bpf_program__attach_trace_opts(const struct
> > bpf_program *prog,
> > +                                               const struct
> > bpf_trace_opts *opts)
> 
> there is bpf_program__attach_btf_id() that does all of this except
> for
> the cookie. It would be nicer to extend bpf_program__attach_btf_id(),
> which won't break any API because it's an internal helper, add
> optional bpf_trace_opts to it and then just redirect
> bpf_program__attach_trace_opts() to bpf_program__attach_btf_id and
> update all the existing callers with just passing NULL for opts.

Fixed!  I copied the code from bpf_program__attach_btf_id() for
testing, and forgot to refactor it.

> 
> We can do that as a follow up, given your patch set seems to be
> pretty
> much ready to be landed.
> 
> > +{
> > +       char errmsg[STRERR_BUFSIZE];
> > +       struct bpf_link *link;
> > +       int prog_fd, pfd;
> > +       LIBBPF_OPTS(bpf_link_create_opts, link_opts);
> > +
> 
> [...]