On Tue, Feb 08, 2022 at 04:25:15PM -0800, Yonghong Song wrote:
>
>
> On 2/8/22 11:13 AM, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > Generealize light skeleton by hiding mmap details in skel_internal.h
> > In this form generated lskel.h is usable both by user space and by the kernel.
> >
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > ---
> > tools/bpf/bpftool/gen.c | 45 ++++++++++++++++++++++++-----------------
> > 1 file changed, 27 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index eacfc6a2060d..903abbf077ce 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -472,7 +472,7 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
> > continue;
> > if (bpf_map__is_internal(map) &&
> > (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> > - printf("\tmunmap(skel->%1$s, %2$zd);\n",
> > + printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zd);\n",
> > ident, bpf_map_mmap_sz(map));
> > codegen("\
> > \n\
> > @@ -481,7 +481,7 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
> > }
> > codegen("\
> > \n\
> > - free(skel); \n\
> > + skel_free(skel); \n\
> > } \n\
> > ",
> > obj_name);
> > @@ -525,7 +525,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> > { \n\
> > struct %1$s *skel; \n\
> > \n\
> > - skel = calloc(sizeof(*skel), 1); \n\
> > + skel = skel_alloc(sizeof(*skel)); \n\
> > if (!skel) \n\
> > goto cleanup; \n\
> > skel->ctx.sz = (void *)&skel->links - (void *)skel; \n\
> > @@ -544,18 +544,12 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> > codegen("\
> > \n\
> > - skel->%1$s = \n\
> > - mmap(NULL, %2$zd, PROT_READ | PROT_WRITE,\n\
> > - MAP_SHARED | MAP_ANONYMOUS, -1, 0); \n\
> > - if (skel->%1$s == (void *) -1) \n\
> > - goto cleanup; \n\
> > - memcpy(skel->%1$s, (void *)\"\\ \n\
> > - ", ident, bpf_map_mmap_sz(map));
> > + skel->%1$s = skel_prep_map_data((void *)\"\\ \n\
> > + ", ident);
> > mmap_data = bpf_map__initial_value(map, &mmap_size);
> > print_hex(mmap_data, mmap_size);
> > - printf("\", %2$zd);\n"
> > - "\tskel->maps.%1$s.initial_value = (__u64)(long)skel->%1$s;\n",
> > - ident, mmap_size);
> > + printf("\", %1$zd, %2$zd);\n",
> > + bpf_map_mmap_sz(map), mmap_size);
> > }
> > codegen("\
> > \n\
> > @@ -592,6 +586,24 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> > codegen("\
> > \n\
> > \"; \n\
> > + ");
> > + bpf_object__for_each_map(map, obj) {
> > + size_t mmap_size = 0;
> > +
> > + if (!get_map_ident(map, ident, sizeof(ident)))
> > + continue;
> > +
> > + if (!bpf_map__is_internal(map) ||
> > + !(bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> > + continue;
> > +
> > + bpf_map__initial_value(map, &mmap_size);
> > + printf("\tskel->maps.%1$s.initial_value ="
> > + " skel_prep_init_value((void **)&skel->%1$s, %2$zd, %3$zd);\n",
> > + ident, bpf_map_mmap_sz(map), mmap_size);
> > + }
> > + codegen("\
> > + \n\
> > err = bpf_load_and_run(&opts); \n\
> > if (err < 0) \n\
> > return err; \n\
> > @@ -611,9 +623,8 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> > else
> > mmap_flags = "PROT_READ | PROT_WRITE";
> > - printf("\tskel->%1$s =\n"
> > - "\t\tmmap(skel->%1$s, %2$zd, %3$s, MAP_SHARED | MAP_FIXED,\n"
> > - "\t\t\tskel->maps.%1$s.map_fd, 0);\n",
> > + printf("\tskel->%1$s = skel_finalize_map_data(&skel->maps.%1$s.initial_value,\n"
> > + "\t\t\t%2$zd, %3$s, skel->maps.%1$s.map_fd);\n",
> > ident, bpf_map_mmap_sz(map), mmap_flags);
> > }
> > codegen("\
> > @@ -751,8 +762,6 @@ static int do_skeleton(int argc, char **argv)
> > #ifndef %2$s \n\
> > #define %2$s \n\
> > \n\
> > - #include <stdlib.h> \n\
> > - #include <bpf/bpf.h> \n\
>
> I noticed that in patch2, the "bpf.h" is used instead of <bpf/bpf.h>.
> Any particular reason for this or it is a bug fix?
skel_internal.h didn't include bpf.h directly.
gen_loader.c needs it. It does:
#include "skel_internal.h"
because gen_loader.c is part of libbpf.
libbpf sources cannot do #include <bpf/...>
If skel_internal.h did
#include <bpf/bpf.h>
there would be a build error:
In file included from gen_loader.c:15:
skel_internal.h:17:10: fatal error: bpf/bpf.h: No such file or directory
#include <bpf/bpf.h>
Hence #include "bpf.h" in skel_internal.h
So it works for libbpf's gen_loader.c and for generated lskel.h too.
