Re: [PATCH bpf-next v8 5/9] bpftool: Generate skeleton for global percpu data

[Andrii Nakryiko]

On Wed, Jul 1, 2026 at 9:49 AM Quentin Monnet <qmo@xxxxxxxxxx> wrote:
>
> 2026-06-29 23:24 UTC+0800 ~ Leon Hwang <leon.hwang@xxxxxxxxx>
> > Enhance bpftool to generate skeletons that properly handle global percpu
> > variables. The generated skeleton now includes a dedicated structure for
> > percpu data, allowing users to initialize and access percpu variables more
> > efficiently.
> >
> > For global percpu variables, the skeleton now includes a nested
> > structure, e.g.:
> >
> > struct test_global_percpu_data {
> >       struct bpf_object_skeleton *skeleton;
> >       struct bpf_object *obj;
> >       struct {
> >               struct bpf_map *percpu;
> >       } maps;
> >       // ...
> >       struct test_global_percpu_data__percpu {
> >               int data;
> >               char run;
> >               struct {
> >                       char set;
> >                       int i;
> >                       int nums[7];
> >               } struct_data;
> >               int nums[7];
> >       } *percpu;
> >
> >       // ...
> > };
> >
> >   * The "struct test_global_percpu_data__percpu *percpu" points to
> >     initialized data, which is actually "maps.percpu->mmaped".
> >   * Before loading the skeleton, updating the
> >     "struct test_global_percpu_data__percpu *percpu" modifies the initial
> >     value of the corresponding global percpu variables.
> >   * After loading the skeleton, "maps.percpu->mmaped" has been marked as
> >     read-only in libbpf. If users want to update the global percpu
> >     variables, they have to update the "maps.percpu" map instead.
> >   * For lightweight skeleton, "lskel->percpu" will be protected by
> >     "mprotect(p, sz, PROT_READ)".
> >   * For subskeleton, those variables of global percpu data will be
> >     skipped.
> >
> > Assisted-by: Codex:gpt-5.5-xhigh
> > Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
> > ---
> >  tools/bpf/bpftool/gen.c       | 49 +++++++++++++++++++++++++++++++----
> >  tools/lib/bpf/skel_internal.h | 24 +++++++++++++++--
> >  2 files changed, 66 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index 6ae7262ebe0c..2e60296358db 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -92,7 +92,7 @@ static void get_header_guard(char *guard, const char *obj_name, const char *suff
> >
> >  static bool get_map_ident(const struct bpf_map *map, char *buf, size_t buf_sz)
> >  {
> > -     static const char *sfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
> > +     static const char *sfxs[] = { ".data", ".rodata", ".bss", ".percpu", ".kconfig" };
> >       const char *name = bpf_map__name(map);
> >       int i, n;
> >
> > @@ -117,7 +117,7 @@ static bool get_map_ident(const struct bpf_map *map, char *buf, size_t buf_sz)
> >
> >  static bool get_datasec_ident(const char *sec_name, char *buf, size_t buf_sz)
> >  {
> > -     static const char *pfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
> > +     static const char *pfxs[] = { ".data", ".rodata", ".bss", ".percpu", ".kconfig" };
> >       int i, n;
> >
> >       /* recognize hard coded LLVM section name */
> > @@ -254,6 +254,20 @@ static const struct btf_type *find_type_for_map(struct btf *btf, const char *map
> >       return NULL;
> >  }
> >
> > +static bool bpf_map_is_skel_data(const struct bpf_map *map)
> > +{
> > +     if (!bpf_map__is_internal(map))
> > +             return false;
> > +
> > +     if (bpf_map__map_flags(map) & BPF_F_MMAPABLE)
> > +             return true;
> > +
> > +     if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
> > +             return true;
> > +
> > +     return false;
> > +}
> > +
> >  static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
> >  {
> >       size_t tmp_sz;
> > @@ -263,7 +277,7 @@ static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
> >               return true;
> >       }
> >
> > -     if (!bpf_map__is_internal(map) || !(bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> > +     if (!bpf_map_is_skel_data(map))
> >               return false;
> >
> >       if (!get_map_ident(map, buf, sz))
>
>
> Thanks! The bpftool patch looks good, with one reservation: after this
> patch, I believe "is_mmapable_map(map, ...)" will return true if map is
> a percpu map, although percpu maps aren't mmap-able, so we should
> probably update the name of that function to avoid any confusion?
>

Great observation, Quentin!

bpf_map_is_skel_data() I think was supposed to be exactly that generic
name. But it seems like Leon went half-way through with unification.
Unless there are some subtle situations where per-cpu array shouldn't
be handled where is_mmapable_map() is handled, we should rename
is_mmapable_map() and add BPF_MAP_TYPE_PERCPU_ARRAY check (assuming
it's internal map, of course) there.

Leon, can you please check?

pw-bot: cr


> Quentin