Re: [PATCH v2 bpf 02/11] bpftool: define a local bpf_perf_link to fix accessing its fields

[Michal Suchánek]

On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> From: Michal Suchánek <msuchanek@xxxxxxx>
> Date: Fri, 14 Apr 2023 11:54:57 +0200
> 
> > Hello,
> 
> Hey-hey,
> 
> > 
> > On Thu, Apr 21, 2022 at 12:38:58AM +0000, Alexander Lobakin wrote:
> >> 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);
> >>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
> >>                 ((type *)(__mptr - offsetof(type, member)));    \
> >>                                    ^~~~~~~~~~~~~~~~~~~~~~
> >> tools/bpf/bpftool/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")
> > 
> > This does not solve the problem completely. Kernels that don't have
> > CONFIG_PERF_EVENTS in the first place are also missing the enum value
> > BPF_LINK_TYPE_PERF_EVENT which is used as the condition for handling the
> > cookie.
> 
> Sorry, I haven't been working with my home/private stuff for more than a
> year already. I may get back to it some day when I'm tired of Lua (curse
> words, sorry :D), but for now the series is "a bit" abandoned.

This part still appllies and works for me with the caveat that
BPF_LINK_TYPE_PERF_EVENT also needs to be defined.

> I think there was alternative solution proposed there, which promised to
> be more flexible. But IIRC it also doesn't touch the enum (was it added
> recently? Because it was building just fine a year ago on config without
> perf events).

It was added in 5.15. Not sure there is a kernel.org LTS kernel usable
for CO-RE that does not have it, technically 5.4 would work if it was
built monolithic, it does not have module BTF, only kernel IIRC.

Nonetheless, the approach to handling features completely missing in the
running kernel should be figured out one way or another. I would be
surprised if this was the last feature to be added that bpftool needs to
know about.

Thanks

Michal