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

[Alexander Lobakin]

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.
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).

> 
> Thanks
> 
> Michal
> 
Thanks,
Olek