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")
Suggested-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
index e2af8e5fb29e..3a4c4f7d83d8 100644
--- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
+++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
@@ -15,6 +15,11 @@ enum bpf_obj_type {
BPF_OBJ_BTF,
};
+struct bpf_perf_link___local {
+ struct bpf_link link;
+ struct file *perf_file;
+} __attribute__((preserve_access_index));
+
struct perf_event___local {
u64 bpf_cookie;
} __attribute__((preserve_access_index));
@@ -45,10 +50,10 @@ static __always_inline __u32 get_obj_id(void *ent, enum bpf_obj_type type)
/* could be used only with BPF_LINK_TYPE_PERF_EVENT links */
static __u64 get_bpf_cookie(struct bpf_link *link)
{
+ struct bpf_perf_link___local *perf_link;
struct perf_event___local *event;
- struct bpf_perf_link *perf_link;
- perf_link = container_of(link, struct bpf_perf_link, link);
+ perf_link = container_of(link, struct bpf_perf_link___local, link);
event = BPF_CORE_READ(perf_link, perf_file, private_data);
return BPF_CORE_READ(event, bpf_cookie);
}
--
2.36.0
Hello,
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.
Thanks
Michal
From: Michal Suchánek <[email protected]>
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
On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> From: Michal Such?nek <[email protected]>
> 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
On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
>
> On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > From: Michal Suchánek <[email protected]>
> > 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
On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
> On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
> >
> > On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > > From: Michal Suchánek <[email protected]>
> > > 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.
It does have a copy of the uapi headers but apparently does not use
them. Using them directly might cause conflict with vmlinux.h, though.
Thanks
Michal
On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <[email protected]> wrote:
>
> On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
> > On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
> > >
> > > On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > > > From: Michal Suchánek <[email protected]>
> > > > 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.
>
> It does have a copy of the uapi headers but apparently does not use
> them. Using them directly might cause conflict with vmlinux.h, though.
Indeed, using the UAPI header here conflicts with vmlinux.h.
Looking again at some code I started last year but never finalised, I
used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with
CO-RE:
enum bpf_link_type___local {
BPF_LINK_TYPE_PERF_EVENT___local = 7,
};
Then guarding accordingly in iter():
[...]
if (obj_type == BPF_OBJ_LINK &&
bpf_core_enum_value_exists(enum bpf_link_type___local,
BPF_LINK_TYPE_PERF_EVENT___local)) {
struct bpf_link *link = (struct bpf_link *) file->private_data;
if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
BPF_LINK_TYPE_PERF_EVENT___local)) {
e.has_bpf_cookie = true;
e.bpf_cookie = get_bpf_cookie(link);
}
}
[...]
Would that approach make sense? I had a VM around with kernel 5.8, and
bpftool compiles there with that change. If I remember correctly, some
older kernel versions required yet more CO-RE work in pid_iter.bpf.c,
and at some point I was struggling, which is why I never submitted
this set.
If this approach looks correct to you Andrii, I can resubmit these
patches with my addition so we can at least fix the build on 5.8
onwards.
Thanks,
Quentin
On Wed, May 3, 2023 at 4:44 PM Quentin Monnet <[email protected]> wrote:
>
> On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <[email protected]> wrote:
> >
> > On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
> > > On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
> > > >
> > > > On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > > > > From: Michal Suchánek <[email protected]>
> > > > > 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.
> >
> > It does have a copy of the uapi headers but apparently does not use
> > them. Using them directly might cause conflict with vmlinux.h, though.
>
> Indeed, using the UAPI header here conflicts with vmlinux.h.
>
> Looking again at some code I started last year but never finalised, I
> used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with
> CO-RE:
>
> enum bpf_link_type___local {
> BPF_LINK_TYPE_PERF_EVENT___local = 7,
> };
>
> Then guarding accordingly in iter():
>
> [...]
> if (obj_type == BPF_OBJ_LINK &&
> bpf_core_enum_value_exists(enum bpf_link_type___local,
> BPF_LINK_TYPE_PERF_EVENT___local)) {
> struct bpf_link *link = (struct bpf_link *) file->private_data;
>
> if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
> BPF_LINK_TYPE_PERF_EVENT___local)) {
> e.has_bpf_cookie = true;
> e.bpf_cookie = get_bpf_cookie(link);
> }
> }
> [...]
>
> Would that approach make sense? I had a VM around with kernel 5.8, and
> bpftool compiles there with that change. If I remember correctly, some
> older kernel versions required yet more CO-RE work in pid_iter.bpf.c,
> and at some point I was struggling, which is why I never submitted
> this set.
>
> If this approach looks correct to you Andrii, I can resubmit these
> patches with my addition so we can at least fix the build on 5.8
> onwards.
Yep, why not? In general, if using vmlinux.h makes life harder and
there is just a small set of types and enums BPF program needs, for
the sake of support of old kernels/distros it might be cleaner just to
define relevant structs, enums, etc explicitly and add
__attribute__((preserve_access_index)) to them to make the
CO-RE-relocatable
>
> Thanks,
> Quentin
Hello,
On Thu, May 04, 2023 at 12:43:52AM +0100, Quentin Monnet wrote:
> On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <[email protected]> wrote:
> >
> > On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
> > > On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
> > > >
> > > > On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
> > > > > From: Michal Suchánek <[email protected]>
> > > > > 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.
> >
> > It does have a copy of the uapi headers but apparently does not use
> > them. Using them directly might cause conflict with vmlinux.h, though.
>
> Indeed, using the UAPI header here conflicts with vmlinux.h.
>
> Looking again at some code I started last year but never finalised, I
> used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with
> CO-RE:
>
> enum bpf_link_type___local {
> BPF_LINK_TYPE_PERF_EVENT___local = 7,
> };
That's the same as I did except I used simple define instead of this
fake enum.
The enum only has value when it is complete and the compiler can check
that a switch uses only known values, and can confuse things when values
are missing.
Thanks
Michal
On 5/4/23 1:18 AM, Michal Suchánek wrote:
> Hello,
>
> On Thu, May 04, 2023 at 12:43:52AM +0100, Quentin Monnet wrote:
>> On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <[email protected]> wrote:
>>>
>>> On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
>>>> On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <[email protected]> wrote:
>>>>>
>>>>> On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
>>>>>> From: Michal Suchánek <[email protected]>
>>>>>> 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.
>>>
>>> It does have a copy of the uapi headers but apparently does not use
>>> them. Using them directly might cause conflict with vmlinux.h, though.
>>
>> Indeed, using the UAPI header here conflicts with vmlinux.h.
>>
>> Looking again at some code I started last year but never finalised, I
>> used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with
>> CO-RE:
>>
>> enum bpf_link_type___local {
>> BPF_LINK_TYPE_PERF_EVENT___local = 7,
>> };
>
> That's the same as I did except I used simple define instead of this
> fake enum.
>
> The enum only has value when it is complete and the compiler can check
> that a switch uses only known values, and can confuse things when values
> are missing.
Currently, enum value CORE is done though a llvm builtin function. So
if the enum value is used in switch cases like
switch(...)
case BPF_LINK_TYPE_PERF_EVENT:
...
CORE relocation will not work in that case since the compiler
expects BPF_LINK_TYPE_PERF_EVENT to be a constant.
> Thanks
>
> Michal