| Message ID | 20230512103354.48374-3-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpftool: Fix skeletons compilation for older kernels | expand |
On 5/12/23 3:33 AM, Quentin Monnet wrote: > From: Alexander Lobakin <alobakin@pm.me> > > 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 'container_of() is not CO-REd' is incorrect. #define container_of(ptr, type, member) \ ({ \ void *__mptr = (void *)(ptr); \ ((type *)(__mptr - offsetof(type, member))); \ }) offsetof() will do necessary CO-RE relocation if the field is specified with preserve_access_index attribute. So container_of will actually do CO-RE relocation as well. > 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 <andrii@kernel.org> > Signed-off-by: Alexander Lobakin <alobakin@pm.me> > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > 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); > }
2023-05-12 07:47 UTC-0700 ~ Yonghong Song <yhs@meta.com> > > > On 5/12/23 3:33 AM, Quentin Monnet wrote: >> From: Alexander Lobakin <alobakin@pm.me> >> >> 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 > > 'container_of() is not CO-REd' is incorrect. > > #define container_of(ptr, type, member) \ > ({ \ > void *__mptr = (void *)(ptr); \ > ((type *)(__mptr - offsetof(type, member))); \ > }) > > > offsetof() will do necessary CO-RE relocation if the field is specified > with preserve_access_index attribute. So container_of will actually > do CO-RE relocation as well. Thanks! I'll amend the description for the next iteration. Quentin
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); }