On Thu, Jul 28, 2022 at 12:58 AM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > From: Andrii Nakryiko [mailto:andrii.nakryiko@xxxxxxxxx] > > Sent: Thursday, July 28, 2022 1:03 AM > > On Tue, Jul 19, 2022 at 12:40 PM Joe Burton <jevburton.kernel@xxxxxxxxx> > > wrote: > > > > > > From: Joe Burton <jevburton@xxxxxxxxxx> > > > > > > Add an extensible variant of bpf_obj_get() capable of setting the > > > `file_flags` parameter. > > > > > > This parameter is needed to enable unprivileged access to BPF maps. > > > Without a method like this, users must manually make the syscall. > > > > > > Signed-off-by: Joe Burton <jevburton@xxxxxxxxxx> > > > --- > > > tools/lib/bpf/bpf.c | 10 ++++++++++ > > > tools/lib/bpf/bpf.h | 9 +++++++++ > > > tools/lib/bpf/libbpf.map | 1 + > > > 3 files changed, 20 insertions(+) > > > > > > > I agree that bpf_obj_get_opts should be separate from bpf_get_fd_opts. > > Just because both currently have file_flags in them doesn't mean that > > they should/will always stay in sync. So two separate opts for two > > separate APIs makes sense to me. > > > > So I'd accept this patch, but please see a few small things below and > > send v3. Thanks! > > Should map_parse_fds() accept two opts, or just the flags > to be set on locally-defined variables? it's because map_parse_fds() is used with both get_fd_by_id() and bpf_obj_get(), right? I'd pass flags and construct correct set of options internally, based on which BPF command you need to use to get FD > > Thanks > > Roberto > > > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > > > index 5eb0df90eb2b..5acb0e8bd13c 100644 > > > --- a/tools/lib/bpf/bpf.c > > > +++ b/tools/lib/bpf/bpf.c > > > @@ -578,12 +578,22 @@ int bpf_obj_pin(int fd, const char *pathname) > > > } > > > > > > int bpf_obj_get(const char *pathname) > > > +{ > > > + LIBBPF_OPTS(bpf_obj_get_opts, opts); > > > > if you were doing it this way, here should be an empty line. But > > really you can/should just pass NULL instead of opts in this case. > > > > > + return bpf_obj_get_opts(pathname, &opts); > > > +} > > > + > > > +int bpf_obj_get_opts(const char *pathname, const struct bpf_obj_get_opts > > *opts) > > > { > > > union bpf_attr attr; > > > int fd; > > > > > > + if (!OPTS_VALID(opts, bpf_obj_get_opts)) > > > + return libbpf_err(-EINVAL); > > > + > > > memset(&attr, 0, sizeof(attr)); > > > attr.pathname = ptr_to_u64((void *)pathname); > > > + attr.file_flags = OPTS_GET(opts, file_flags, 0); > > > > > > fd = sys_bpf_fd(BPF_OBJ_GET, &attr, sizeof(attr)); > > > return libbpf_err_errno(fd); > > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h > > > index 88a7cc4bd76f..f31b493b5f9a 100644 > > > --- a/tools/lib/bpf/bpf.h > > > +++ b/tools/lib/bpf/bpf.h > > > @@ -270,8 +270,17 @@ LIBBPF_API int bpf_map_update_batch(int fd, const > > void *keys, const void *values > > > __u32 *count, > > > const struct bpf_map_batch_opts *opts); > > > > > > +struct bpf_obj_get_opts { > > > + size_t sz; /* size of this struct for forward/backward compatibility */ > > > + > > > + __u32 file_flags; > > > > please add size_t :0; to avoid non-zero-initialized padding (we do it > > in a lot of other opts structs) > > > > > > > +}; > > > +#define bpf_obj_get_opts__last_field file_flags > > > + > > > LIBBPF_API int bpf_obj_pin(int fd, const char *pathname); > > > LIBBPF_API int bpf_obj_get(const char *pathname); > > > +LIBBPF_API int bpf_obj_get_opts(const char *pathname, > > > + const struct bpf_obj_get_opts *opts); > > > > > > struct bpf_prog_attach_opts { > > > size_t sz; /* size of this struct for forward/backward compatibility */ > > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > > > index 0625adb9e888..119e6e1ea7f1 100644 > > > --- a/tools/lib/bpf/libbpf.map > > > +++ b/tools/lib/bpf/libbpf.map > > > @@ -355,6 +355,7 @@ LIBBPF_0.8.0 { > > > > > > LIBBPF_1.0.0 { > > > global: > > > + bpf_obj_get_opts; > > > bpf_prog_query_opts; > > > bpf_program__attach_ksyscall; > > > btf__add_enum64; > > > -- > > > 2.37.0.170.g444d1eabd0-goog > > >
