RE: [RFC][PATCH v3 07/15] libbpf: Introduce bpf_obj_get_opts()

[Roberto Sassu <roberto.sassu@huawei.com>]

> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Friday, July 22, 2022 8:02 PM
> On Fri, Jul 22, 2022 at 10:58 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > On Fri, Jul 22, 2022 at 10:20 AM Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> > >
> > > Introduce bpf_obj_get_opts(), to let the caller pass the needed permissions
> > > for the operation. Keep the existing bpf_obj_get() to request read-write
> > > permissions.
> > >
> > > bpf_obj_get() allows the caller to get a file descriptor from a pinned
> > > object with the provided pathname. Specifying permissions has only effect
> > > on maps (for links, the permission must be always read-write).
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > ---
> > >  tools/lib/bpf/bpf.c      | 12 +++++++++++-
> > >  tools/lib/bpf/bpf.h      |  2 ++
> > >  tools/lib/bpf/libbpf.map |  1 +
> > >  3 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > index 5f2785a4c358..0df088890864 100644
> > > --- a/tools/lib/bpf/bpf.c
> > > +++ b/tools/lib/bpf/bpf.c
> > > @@ -577,18 +577,28 @@ int bpf_obj_pin(int fd, const char *pathname)
> > >         return libbpf_err_errno(ret);
> > >  }
> > >
> > > -int bpf_obj_get(const char *pathname)
> > > +int bpf_obj_get_opts(const char *pathname,
> > > +                    const struct bpf_get_fd_opts *opts)
> >
> > I'm still not sure whether it's a good idea to mix get_fd with
> > obj_get/pin operations? [1] seems more clear.
> 
> +1

I think so. Both types of functions are accessing the same object,
just in a different way: one by ID, and another by path.

Consider the case I mentioned, map_parse_fds() in bpftool. It calls
both type of functions. What opts a caller of this function should
provide, if they are different?

> > It just so happens that (differently named) flags in BPF_OBJ_GET and
> > BPF_XXX_GET_FD_BY_ID align, but maybe we shouldn't depend on it?
> >
> > Also, it seems only bpf_map_get_fd_by_id currently accepts flags? So
> > this sharing makes even more sense?

As I mentioned in another email, Andrii requested me in v2 to add
opts to all bpf_*_get_fd_by_id() functions.

> +1
> 
> Roberto, the patch set is broken in many ways.

Could you please explain?

Thanks

Roberto