On Thu, Feb 16, 2023 at 3:08 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Tue, Feb 14, 2023 at 3:12 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
> >
> > These are type-safe wrappers around bpf_obj_get_info_by_fd(). They
> > found one problem in selftests, and are also useful for adding
> > Memory Sanitizer annotations.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > ---
> > tools/lib/bpf/bpf.c | 24 ++++++++++++++++++++++++
> > tools/lib/bpf/bpf.h | 13 +++++++++++++
> > tools/lib/bpf/libbpf.map | 5 +++++
> > 3 files changed, 42 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 9aff98f42a3d..b562019271fe 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -1044,6 +1044,30 @@ int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len)
> > return libbpf_err_errno(err);
> > }
> >
> > +int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
> > + __u32 *info_len)
> > +{
> > + return bpf_obj_get_info_by_fd(prog_fd, info, info_len);
> > +}
> > +
> > +int bpf_map_get_info_by_fd(int map_fd, struct bpf_map_info *info,
> > + __u32 *info_len)
> > +{
> > + return bpf_obj_get_info_by_fd(map_fd, info, info_len);
> > +}
> > +
> > +int bpf_btf_get_info_by_fd(int btf_fd, struct bpf_btf_info *info,
> > + __u32 *info_len)
> > +{
> > + return bpf_obj_get_info_by_fd(btf_fd, info, info_len);
> > +}
> > +
> > +int bpf_link_get_info_by_fd(int link_fd, struct bpf_link_info *info,
> > + __u32 *info_len)
>
> fits under 100 characters, please keep on single line
>
> > +{
> > + return bpf_obj_get_info_by_fd(link_fd, info, info_len);
> > +}
> > +
> > int bpf_raw_tracepoint_open(const char *name, int prog_fd)
> > {
> > const size_t attr_sz = offsetofend(union bpf_attr, raw_tracepoint);
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 7468978d3c27..9f698088c9bc 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -386,6 +386,19 @@ LIBBPF_API int bpf_link_get_fd_by_id(__u32 id);
> > LIBBPF_API int bpf_link_get_fd_by_id_opts(__u32 id,
> > const struct bpf_get_fd_by_id_opts *opts);
> > LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
> > +/* Type-safe variants of bpf_obj_get_info_by_fd(). The callers still needs to
> > + * pass info_len, which should normally be
> > + * sizeof(struct bpf_{prog,map,btf,link}_info), in order to be compatible with
> > + * different libbpf and kernel versions.
> > + */
>
> let's add proper doc comments for new APIs, see bpf_map_update_batch
> for an example
>
It was sad to require you to respin first 5 patches because of this
doc comment issue, so I unwrapped lines and landed first 5 patches as
is. Please do follow up with doc comments, and let's figure out what
we do about MSan annotations separately. Thanks for clean ups and
improvements!
> > +LIBBPF_API int bpf_prog_get_info_by_fd(int prog_fd, struct bpf_prog_info *info,
> > + __u32 *info_len);
> > +LIBBPF_API int bpf_map_get_info_by_fd(int map_fd, struct bpf_map_info *info,
> > + __u32 *info_len);
> > +LIBBPF_API int bpf_btf_get_info_by_fd(int btf_fd, struct bpf_btf_info *info,
> > + __u32 *info_len);
> > +LIBBPF_API int bpf_link_get_info_by_fd(int link_fd, struct bpf_link_info *info,
> > + __u32 *info_len);
> >
>
> ditto, single lines are the best
>
> > struct bpf_prog_query_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 11c36a3c1a9f..50dde1f6521e 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -384,4 +384,9 @@ LIBBPF_1.1.0 {
> > } LIBBPF_1.0.0;
> >
> > LIBBPF_1.2.0 {
> > + global:
> > + bpf_btf_get_info_by_fd;
> > + bpf_link_get_info_by_fd;
> > + bpf_map_get_info_by_fd;
> > + bpf_prog_get_info_by_fd;
> > } LIBBPF_1.1.0;
> > --
> > 2.39.1
> >
