| Message ID | 20220428110839.111042-1-larysa.zaremba@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpftool: Use sysfs vmlinux when dumping BTF by ID | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | pending | PR summary |
| netdev/tree_selection | success | Not a local patch |
On Thu, Apr 28, 2022 at 4:17 AM Larysa Zaremba <larysa.zaremba@intel.com> wrote: > > Currently, dumping almost all BTFs specified by id requires It should and will work only for kernel modules. It won't and shouldn't work for BTFs coming from BPF programs. We shouldn't blindly guess and substitute vmlinux BTF as base BTF, let's fetch bpf_btf_info, check that BTF is from kernel and is not vmlinux, and only in such case substitute vmlinux BTF as base BTF. > using the -B option to pass the base BTF. For most cases > the vmlinux BTF sysfs path should work. > > This patch simplifies dumping by ID usage by attempting to > use vmlinux BTF from sysfs, if the first try of loading BTF by ID > fails with certain conditions. > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com> > --- > tools/bpf/bpftool/btf.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > [...]
On Thu, 28 Apr 2022 21:58:58 -0700 Andrii Nakryiko <andrii@kernel.org> wrote: > On Thu, Apr 28, 2022 at 4:17 AM Larysa Zaremba <larysa.zaremba@intel.com> wrote: > > > > Currently, dumping almost all BTFs specified by id requires > > It should and will work only for kernel modules. It won't and > shouldn't work for BTFs coming from BPF programs. We shouldn't blindly > guess and substitute vmlinux BTF as base BTF, let's fetch > bpf_btf_info, check that BTF is from kernel and is not vmlinux, and > only in such case substitute vmlinux BTF as base BTF. I agree, this is taken into account in v2 > > using the -B option to pass the base BTF. For most cases > > the vmlinux BTF sysfs path should work. > > > > This patch simplifies dumping by ID usage by attempting to > > use vmlinux BTF from sysfs, if the first try of loading BTF by ID > > fails with certain conditions. > > > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > > Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com> > > --- > > tools/bpf/bpftool/btf.c | 35 ++++++++++++++++++++++++++--------- > > 1 file changed, 26 insertions(+), 9 deletions(-) > > Best Regards, Larysa Zaremba
Hello! Just ran into this. I think we also need to pass in errno here to strerror instead of err: On Thu, Apr 28, 2022 at 01:08:40PM +0200, Larysa Zaremba wrote: > if (err) { > p_err("get btf by id (%u): %s", btf_id, strerror(err)); > goto done; > Currently, the error output without a base btf reads: $ bpftool btf dump id 816 Error: get btf by id (816): Unknown error -22 When it should (or at least intends to) read: $ bpftool btf dump id 816 Error: get btf by id (816): Invalid argument I was going to send this patch but if a v2 is going to be sent, figured I mention it. Thanks!
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index a2c665beda87..557f65e2de5c 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -459,6 +459,22 @@ static int dump_btf_c(const struct btf *btf, return err; } +static const char sysfs_vmlinux[] = "/sys/kernel/btf/vmlinux"; + +static struct btf *get_vmlinux_btf_from_sysfs(void) +{ + struct btf *base; + + base = btf__parse(sysfs_vmlinux, NULL); + if (libbpf_get_error(base)) { + p_err("failed to parse vmlinux BTF at '%s': %ld\n", + sysfs_vmlinux, libbpf_get_error(base)); + base = NULL; + } + + return base; +} + static int do_dump(int argc, char **argv) { struct btf *btf = NULL, *base = NULL; @@ -536,18 +552,11 @@ static int do_dump(int argc, char **argv) NEXT_ARG(); } else if (is_prefix(src, "file")) { const char sysfs_prefix[] = "/sys/kernel/btf/"; - const char sysfs_vmlinux[] = "/sys/kernel/btf/vmlinux"; if (!base_btf && strncmp(*argv, sysfs_prefix, sizeof(sysfs_prefix) - 1) == 0 && - strcmp(*argv, sysfs_vmlinux) != 0) { - base = btf__parse(sysfs_vmlinux, NULL); - if (libbpf_get_error(base)) { - p_err("failed to parse vmlinux BTF at '%s': %ld\n", - sysfs_vmlinux, libbpf_get_error(base)); - base = NULL; - } - } + strcmp(*argv, sysfs_vmlinux)) + base = get_vmlinux_btf_from_sysfs(); btf = btf__parse_split(*argv, base ?: base_btf); err = libbpf_get_error(btf); @@ -593,6 +602,14 @@ static int do_dump(int argc, char **argv) if (!btf) { btf = btf__load_from_kernel_by_id_split(btf_id, base_btf); err = libbpf_get_error(btf); + if (err == -EINVAL && !base_btf) { + btf__free(base); + base = get_vmlinux_btf_from_sysfs(); + p_info("Warning: valid base BTF was not specified with -B option, falling back on standard base BTF (sysfs vmlinux)"); + btf = btf__load_from_kernel_by_id_split(btf_id, base); + err = libbpf_get_error(btf); + } + if (err) { p_err("get btf by id (%u): %s", btf_id, strerror(err)); goto done;