On Wed, 2024-07-03 at 17:15 -0700, Andrii Nakryiko wrote:
> Old versions of libbpf don't handle varying sizes of bpf_map_skeleton
> struct correctly. As such, BPF skeleton generated by newest bpftool
> might not be compatible with older libbpf (though only when libbpf is
> used as a shared library), even though it, by design, should.
>
> Going forward libbpf will be fixed, plus we'll release bug fixed
> versions of relevant old libbpfs, but meanwhile try to mitigate from
> bpftool side by conservatively assuming older and smaller definition of
> bpf_map_skeleton, if possible. Meaning, if there are no struct_ops maps.
>
> If there are struct_ops, then presumably user would like to have
> auto-attaching logic and struct_ops map link placeholders, so use the
> full bpf_map_skeleton definition in that case.
>
> Co-developed-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
Silly question, here is a fragment of the profiler.skel.h generated
with bpftool build (tools/bpf/bpftool/profiler.skel.h):
static inline int
profiler_bpf__create_skeleton(struct profiler_bpf *obj)
{
/* ... */
map = (struct bpf_map_skeleton *)((char *)s->maps + 0 * s->map_skel_sz);
map->name = "events";
map->map = &obj->maps.events;
/* ... 4 more like this ... */
/* ... */
s->progs[0].name = "fentry_XXX";
s->progs[0].prog = &obj->progs.fentry_XXX;
s->progs[0].link = &obj->links.fentry_XXX;
s->progs[1].name = "fexit_XXX";
s->progs[1].prog = &obj->progs.fexit_XXX;
s->progs[1].link = &obj->links.fexit_XXX;
/* ... */
}
Do we need to handle 'progs' array access in a same way as maps?
[...]
> @@ -878,23 +895,22 @@ codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool
>
> codegen("\
> \n\
> - \n\
> - s->maps[%zu].name = \"%s\"; \n\
> - s->maps[%zu].map = &obj->maps.%s; \n\
> + \n\
> + map = (struct bpf_map_skeleton *)((char *)s->maps + %zu * s->map_skel_sz);\n\
> + map->name = \"%s\"; \n\
> + map->map = &obj->maps.%s; \n\
> ",
> - i, bpf_map__name(map), i, ident);
> + i, bpf_map__name(map), ident);
> /* memory-mapped internal maps */
> if (mmaped && is_mmapable_map(map, ident, sizeof(ident))) {
> - printf("\ts->maps[%zu].mmaped = (void **)&obj->%s;\n",
> - i, ident);
> + printf("\tmap->mmaped = (void **)&obj->%s; \n", ident);
^^
nit: this generates extra white space
> }
>
> if (populate_links && bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS) {
> codegen("\
> \n\
> - s->maps[%zu].link = &obj->links.%s;\n\
> - ",
> - i, ident);
> + map->link = &obj->links.%s; \n\
> + ", ident);
> }
> i++;
> }
[...]
