Test for bpf_perf_link #88
Conversation
|
Thanks! But such contributions shouldn't be submitted against this repo which is just a mirror, this is the kind of fixes that need to be sent to the BPF mailing list. As for the fix itself, we shouldn't need another feature detection item. We should be able to deal with the absence of |
|
Thanks for pointing out that patchset. Apparently this is not limited to old kernels, the feature can be disabled by a config option so this should be handled more gracefully. |
When CONFIG_PERF_EVENTS is not set, struct perf_event remains empty.
However, the structure is being used by bpftool indirectly via BTF.
This leads to:
skeleton/pid_iter.bpf.c:49:30: error: no member named 'bpf_cookie' in 'struct perf_event'
return BPF_CORE_READ(event, bpf_cookie);
~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
...
skeleton/pid_iter.bpf.c:49:9: error: returning 'void' from a function with incompatible result type '__u64' (aka 'unsigned long long')
return BPF_CORE_READ(event, bpf_cookie);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Tools and samples can't use any CONFIG_ definitions, so the fields
used there should always be present.
Define struct perf_event___local with the `preserve_access_index`
attribute inside the pid_iter BPF prog to allow compiling on any
configs. CO-RE will substitute it with the real struct perf_event
accesses later on.
Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Suggested-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
When building bpftool with !CONFIG_PERF_EVENTS:
skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
perf_link = container_of(link, struct bpf_perf_link, link);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
((type *)(__mptr - offsetof(type, member))); \
^~~~~~~~~~~~~~~~~~~~~~
src/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
#define offsetof(TYPE, MEMBER) ((unsigned long)&((TYPE *)0)->MEMBER)
~~~~~~~~~~~^
skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
struct bpf_perf_link *perf_link;
^
&bpf_perf_link is being defined and used only under the ifdef.
Define struct bpf_perf_link___local with the `preserve_access_index`
attribute inside the pid_iter BPF prog to allow compiling on any
configs. CO-RE will substitute it with the real struct bpf_perf_link
accesses later on.
container_of() is not CO-REd, but it is a noop for
bpf_perf_link <-> bpf_link and the local copy is a full mirror of
the original structure.
Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
Suggested-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
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));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/bootstrap/libbpf/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
src/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 <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
Kernels before version 5.15 do not define BPF_LINK_TYPE_PERF_EVENT which causes bpftool build to fail when the running kernel is older than 5.15. Add local definition of the value. Based on code provided by Quentin Monnet <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]/ Signed-off-by: Michal Suchanek <[email protected]>
|
This is now dump of the relevant patches form the mentioned mail thread. With this the package can be built but YMMV. Hopefully this will eventually make it into the official repository. |
|
Thanks! Yes it will, these patches are long overdue. I'll try to post them this weekend if I can, next week otherwise. |
Out of curiosity, what kernel versions did you check? 5.14, I guess, but did you manage to build on 5.3 as well? |
|
I am using distribution kernels so the results are not transferable to other environments. Interestingly it did build even with the 5.3 kernel version that is not patched to support module BTF and as far as BTF goes it should be pretty close to upstream. |
|
@hramrach I've got a slightly different patchset, with the CO-RE checks for existence of the enum value ( So just checking before sending the series, what clang/LLVM versions are available on your env? |
|
Thanks for updating the patches. It builds with llvm 12/13/16 depending on distribution release. Not sure there is much point going further back in llvm archaeology, somebody with a different llvm collection would have to answer that. I don't have any older llvm prebuilt that is also usable for bpf. The exact usability is also questionable, any functional tests will have to come once the package is buildable, and the scope depends very much on how people happen to use it. |
LLVM 12+ is all I needed to know. Perfect, thanks! |
vmlinux.h is generated from the running kernel, not system headers.
When the running kernel is older than 5.15 it lacks bpf_perf_link and build fails.
Detect presence of the structure and skip building the part of code that requires it when it is not present.
Fixes: #17