[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYx=dSXp-TkpjzyhSP+9WY71uR4Xq4Um5YzerbfOtJOfA@mail.gmail.com>
Date: Thu, 20 Apr 2023 16:07:38 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Michal Suchánek <msuchanek@...e.de>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
Alexander Lobakin <alobakin@...lbox.org>,
Alexei Starovoitov <ast@...nel.org>,
Shung-Hsi Yu <shung-hsi.yu@...e.com>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Song Liu <songliubraving@...com>,
Kumar Kartikeya Dwivedi <memxor@...il.com>,
bpf@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 bpf 02/11] bpftool: define a local bpf_perf_link to fix
accessing its fields
On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <msuchanek@...e.de> wrote:
>
> On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > From: Michal Suchánek <msuchanek@...e.de>
> > 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.
Are we talking about bpftool built from kernel sources or from Github?
Kernel source version should have access to latest UAPI headers and so
BPF_LINK_TYPE_PERF_EVENT should be available. Github version, if it
doesn't do that already, can use UAPI headers distributed (and used
for building) with libbpf through submodule.
>
> Thanks
>
> Michal
Powered by blists - more mailing lists