2023-02-10 01:12 UTC+0100 ~ Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> Use the new type-safe wrappers around bpf_obj_get_info_by_fd().
> lease enter the commit message for your changes. Lines starting
>
> Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
Hi, here are a few comments from my side inline below. Other than these,
the patch looks good to me, thanks.
> ---
> tools/bpf/bpftool/btf.c | 13 ++++++++-----
> tools/bpf/bpftool/btf_dumper.c | 4 ++--
> tools/bpf/bpftool/cgroup.c | 4 ++--
> tools/bpf/bpftool/common.c | 13 +++++++------
> tools/bpf/bpftool/link.c | 4 ++--
> tools/bpf/bpftool/main.h | 3 ++-
> tools/bpf/bpftool/map.c | 8 ++++----
> tools/bpf/bpftool/prog.c | 24 +++++++++++++-----------
> tools/bpf/bpftool/struct_ops.c | 6 +++---
> 9 files changed, 43 insertions(+), 36 deletions(-)
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 352290ba7b29..91fcb75babe3 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
[...]
> @@ -789,7 +789,10 @@ build_btf_type_table(struct hashmap *tab, enum bpf_obj_type type,
> }
>
> memset(info, 0, *len);
> - err = bpf_obj_get_info_by_fd(fd, info, len);
> + if (type == BPF_OBJ_PROG)
> + err = bpf_prog_get_info_by_fd(fd, info, len);
> + else
> + err = bpf_map_get_info_by_fd(fd, info, len);
Not obvious to me why we should change this one, I suppose knowing the
type helps with the Memory Sanitizer?
[...]
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 620032042576..5a73ccf14332 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
[...]
> @@ -1026,7 +1026,8 @@ int map_parse_fd(int *argc, char ***argv)
> return fd;
> }
>
> -int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len)
> +int map_parse_fd_and_info(int *argc, char ***argv, struct bpf_map_info *info,
> + __u32 *info_len)
This is not strictly related to the other changes, please don't forget
to mention it when you fix the commit description.
> {
> int err;
> int fd;
[...]
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index e87738dbffc1..1944d000038c 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
[...]
> @@ -2170,9 +2170,10 @@ static char *profile_target_name(int tgt_fd)
> char *name = NULL;
> int err;
>
> - err = bpf_obj_get_info_by_fd(tgt_fd, &info, &info_len);
> + err = bpf_prog_get_info_by_fd(tgt_fd, &info, &info_len);
> if (err) {
> - p_err("failed to bpf_obj_get_info_by_fd for prog FD %d", tgt_fd);
> + p_err("failed to bpf_prog_get_info_by_fd for prog FD %d",
> + tgt_fd);
Nit: Maybe just drop the function name here, and keep "failed to get
info for prog FD [...]"? Same below.
> goto out;
> }
>
> @@ -2183,7 +2184,8 @@ static char *profile_target_name(int tgt_fd)
>
> func_info_rec_size = info.func_info_rec_size;
> if (info.nr_func_info == 0) {
> - p_err("bpf_obj_get_info_by_fd for prog FD %d found 0 func_info", tgt_fd);
> + p_err("bpf_prog_get_info_by_fd for prog FD %d found 0 func_info",
> + tgt_fd);
> goto out;
> }
>
[...]
