Re: [PATCH bpf-next 4/4] bpftool: use a local bpf_perf_event_value to fix accessing its fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 12, 2023 at 3:34 AM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote:
>
> From: Alexander Lobakin <alobakin@xxxxx>
>
> Fix the following error when building bpftool:
>
>   CLANG   profiler.bpf.o
>   CLANG   pid_iter.bpf.o
> skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
>         __uint(value_size, sizeof(struct bpf_perf_event_value));
>                            ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h:7:8: note: forward declaration of 'struct bpf_perf_event_value'
> struct bpf_perf_event_value;
>        ^
>
> struct bpf_perf_event_value is being used in the kernel only when
> CONFIG_BPF_EVENTS is enabled, so it misses a BTF entry then.
> Define struct bpf_perf_event_value___local with the
> `preserve_access_index` attribute inside the pid_iter BPF prog to
> allow compiling on any configs. It is a full mirror of a UAPI
> structure, so is compatible both with and w/o CO-RE.
> bpf_perf_event_read_value() requires a pointer of the original type,
> so a cast is needed.
>
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Alexander Lobakin <alobakin@xxxxx>
> Signed-off-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> ---

What's the point of using vmlinux.h at all if we redefine every single
type? bpf_perf_event_value is part of BPF UAPI, so if we included
linux/bpf.h header we'd get it.

This feels a bit split-brained. We either drop vmlinux.h completely
and use UAPI headers + CO-RE-relocatable definitions of internal
types, or we make sure that vmlinux.h does work (e.g., by pre-checking
in a very small version of it). Both using vmlinux.h and not relying
on it having necessary types seems like the worst of both worlds?...


Quentin, can you see if you can come up with some simple way to use
vmlinux.h if building from inside kernel repo, but using a minimized
vmlinux.h generated using `bpftool gen min_core_btf` when building
from Github mirror? Sync script could also generate this minimal
vmlinux.h automatically, presumably?


>  tools/bpf/bpftool/skeleton/profiler.bpf.c | 27 ++++++++++++++---------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>

[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux