From: Quentin Monnet <quentin@isovalent.com>
To: Ilya Leoshkevich <iii@linux.ibm.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>
Cc: bpf@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>
Subject: Re: [PATCH bpf-next v2 10/16] bpftool: Use bpf_{btf,link,map,prog}_get_info_by_fd()
Date: Fri, 10 Feb 2023 14:41:46 +0000 [thread overview]
Message-ID: <d5168e6e-701f-a0f6-e504-ff45d7657832@isovalent.com> (raw)
In-Reply-To: <20230210001210.395194-11-iii@linux.ibm.com>
2023-02-10 01:12 UTC+0100 ~ Ilya Leoshkevich <iii@linux.ibm.com>
> 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@linux.ibm.com>
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;
> }
>
[...]
next prev parent reply other threads:[~2023-02-10 14:41 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-10 0:11 [PATCH bpf-next v2 00/16] selftests/bpf: Add Memory Sanitizer support Ilya Leoshkevich
2023-02-10 0:11 ` [PATCH bpf-next v2 01/16] selftests/bpf: Quote host tools Ilya Leoshkevich
2023-02-10 0:11 ` [PATCH bpf-next v2 02/16] tools: runqslower: Add EXTRA_CFLAGS and EXTRA_LDFLAGS support Ilya Leoshkevich
2023-02-10 0:11 ` [PATCH bpf-next v2 03/16] selftests/bpf: Split SAN_CFLAGS and SAN_LDFLAGS Ilya Leoshkevich
2023-02-10 0:11 ` [PATCH bpf-next v2 04/16] selftests/bpf: Forward SAN_CFLAGS and SAN_LDFLAGS to runqslower and libbpf Ilya Leoshkevich
2023-02-10 0:11 ` [PATCH bpf-next v2 05/16] selftests/bpf: Attach to fopen()/fclose() in uprobe_autoattach Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 06/16] selftests/bpf: Attach to fopen()/fclose() in attach_probe Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 07/16] libbpf: Fix alen calculation in libbpf_nla_dump_errormsg() Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 08/16] libbpf: Introduce bpf_{btf,link,map,prog}_get_info_by_fd() Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 09/16] libbpf: Use bpf_{btf,link,map,prog}_get_info_by_fd() Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 10/16] bpftool: " Ilya Leoshkevich
2023-02-10 10:36 ` Ilya Leoshkevich
2023-02-10 14:41 ` Quentin Monnet [this message]
2023-02-10 0:12 ` [PATCH bpf-next v2 11/16] perf: " Ilya Leoshkevich
2023-02-10 23:26 ` Andrii Nakryiko
2023-02-10 0:12 ` [PATCH bpf-next v2 12/16] samples/bpf: " Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 13/16] selftests/bpf: " Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 14/16] libbpf: Factor out is_percpu_bpf_map_type() Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 15/16] libbpf: Add MSan annotations Ilya Leoshkevich
2023-02-10 0:12 ` [PATCH bpf-next v2 16/16] selftests/bpf: " Ilya Leoshkevich
2023-02-10 23:38 ` [PATCH bpf-next v2 00/16] selftests/bpf: Add Memory Sanitizer support Andrii Nakryiko
2023-02-10 23:40 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d5168e6e-701f-a0f6-e504-ff45d7657832@isovalent.com \
--to=quentin@isovalent.com \
--cc=agordeev@linux.ibm.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=iii@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox