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

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Wed Jul 15, 2026 at 11:32 AM EDT, Leon Hwang wrote:
> 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.
>
> Acked-by: Quentin Monnet <qmo@kernel.org>
> Assisted-by: Codex:gpt-5.5-xhigh
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  tools/bpf/bpftool/gen.c       | 76 +++++++++++++++++++++++++++--------
>  tools/lib/bpf/skel_internal.h | 24 ++++++++++-
>  2 files changed, 81 insertions(+), 19 deletions(-)
>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index a01d06d22d1a..b60db1108149 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -101,6 +101,12 @@ static bool get_map_ident(const struct bpf_map *map, char *buf, size_t buf_sz)
>  		return true;
>  	}
>  
> +	if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY) {
> +		snprintf(buf, buf_sz, "%s", name + 1);
> +		sanitize_identifier(buf);
> +		return true;
> +	}
> +
>  	for  (i = 0, n = ARRAY_SIZE(sfxs); i < n; i++) {
>  		const char *sfx = sfxs[i], *p;
>  
> @@ -117,7 +123,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,7 +260,7 @@ static const struct btf_type *find_type_for_map(struct btf *btf, const char *map
>  	return NULL;
>  }
>  
> -static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
> +static bool is_skel_data(const struct bpf_map *map, char *buf, size_t sz)

Here we change the function to take PERCPU_ARRAY into account, but then 
immediately turn around and add checks of the form is_skel_data && type
!= BPF_MAP_TYPE_PERCPU_ARRAY. If we keep both the existing mmapable_map
and add a separate is_skel_data map (possibly using is_mappable_map),
we can remove those additional checks.

>  {
>  	size_t tmp_sz;
>  
> @@ -263,13 +269,19 @@ 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_internal(map))
>  		return false;
>  
>  	if (!get_map_ident(map, buf, sz))
>  		return false;
>  
> -	return true;
> +	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 int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
> @@ -286,8 +298,11 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
>  		return -errno;
>  
>  	bpf_object__for_each_map(map, obj) {
> -		/* only generate definitions for memory-mapped internal maps */
> -		if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
> +		/*
> +		 * Only generate definitions for internal maps that have
> +		 * mmapped data.
> +		 */
> +		if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>  			continue;
>  
>  		sec = find_type_for_map(btf, map_ident);
> @@ -339,8 +354,14 @@ static int codegen_subskel_datasecs(struct bpf_object *obj, const char *obj_name
>  		return -errno;
>  
>  	bpf_object__for_each_map(map, obj) {
> -		/* only generate definitions for memory-mapped internal maps */
> -		if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
> +		/*
> +		 * Only generate definitions for internal maps that have
> +		 * mmapped data.
> +		 */
> +		if (!is_skel_data(map, map_ident, sizeof(map_ident)))
> +			continue;
> +
> +		if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>  			continue;

Case in point, the old is_mmapable_map would work fine here.

>  
>  		sec = find_type_for_map(btf, map_ident);
> @@ -493,7 +514,10 @@ static size_t bpf_map_mmap_sz(const struct bpf_map *map)
>  	return map_sz;
>  }
>  
> -/* Emit type size asserts for all top-level fields in memory-mapped internal maps. */
> +/*
> + * Emit type size asserts for all top-level fields in internal maps that
> + * have mmaped data.

I do not think the new comment makes it any clearer, the difference is
"memory-mapped internal maps" and "maps that are memory-mapped initially
but actually inaccessible after loading because there's no way to 
represent them as a mapping of the skeleton" AFAICT which is not easy
to infer currently.

> + */
>  static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
>  {
>  	struct btf *btf = bpf_object__btf(obj);
> @@ -517,7 +541,7 @@ static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
>  		", obj_name);
>  
>  	bpf_object__for_each_map(map, obj) {
> -		if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
> +		if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>  			continue;
>  
>  		sec = find_type_for_map(btf, map_ident);
> @@ -669,7 +693,8 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
>  		if (!get_map_ident(map, ident, sizeof(ident)))
>  			continue;
>  		if (bpf_map__is_internal(map) &&
> -		    (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> +		    ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
> +		     bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY))

I think this gives us a hint on how we could structure is_skel_data
above, we now have a special calss that is "mappable maps + PERCPU_ARRAY
whose initial representation in userspace is mappable, but the loaded
map isn't".

>  			printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zu);\n",
>  			       ident, bpf_map_mmap_sz(map));
>  		codegen("\
> @@ -741,7 +766,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
>  		const void *mmap_data = NULL;
>  		size_t mmap_size = 0;
>  
> -		if (!is_mmapable_map(map, ident, sizeof(ident)))
> +		if (!is_skel_data(map, ident, sizeof(ident)))
>  			continue;
>  
>  		codegen("\
> @@ -849,8 +874,22 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
>  	bpf_object__for_each_map(map, obj) {
>  		const char *mmap_flags;
>  
> -		if (!is_mmapable_map(map, ident, sizeof(ident)))
> +		if (!is_skel_data(map, ident, sizeof(ident)))
> +			continue;
> +
> +		if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY) {
> +			codegen("\
> +		\n\
> +			err = skel_protect_map_data(skel->%1$s, &skel->maps.%1$s.initial_value, %2$zd);\n\
> +			if (err)					    \n\
> +				return err;				    \n\
> +		#ifdef __KERNEL__					    \n\
> +			skel->%1$s = NULL;				    \n\
> +		#endif							    \n\
> +			",
> +			ident, bpf_map_mmap_sz(map));
>  			continue;
> +		}
>  
>  		if (bpf_map__map_flags(map) & BPF_F_RDONLY_PROG)
>  			mmap_flags = "PROT_READ";
> @@ -955,8 +994,7 @@ codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool
>  				map->map = &obj->maps.%s;	    \n\
>  			",
>  			i, bpf_map__name(map), ident);
> -		/* memory-mapped internal maps */
> -		if (mmaped && is_mmapable_map(map, ident, sizeof(ident))) {
> +		if (mmaped && is_skel_data(map, ident, sizeof(ident))) {
>  			printf("\tmap->mmaped = (void **)&obj->%s;\n", ident);
>  		}
>  
> @@ -1740,7 +1778,9 @@ static int do_subskeleton(int argc, char **argv)
>  		/* Also count all maps that have a name */
>  		map_cnt++;
>  
> -		if (!is_mmapable_map(map, ident, sizeof(ident)))
> +		if (!is_skel_data(map, ident, sizeof(ident)))
> +			continue;
> +		if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>  			continue;

Another instance of what I was talking about.

>  
>  		map_type_id = bpf_map__btf_value_type_id(map);
> @@ -1863,7 +1903,9 @@ static int do_subskeleton(int argc, char **argv)
>  
>  	/* walk through each symbol and emit the runtime representation */
>  	bpf_object__for_each_map(map, obj) {
> -		if (!is_mmapable_map(map, ident, sizeof(ident)))
> +		if (!is_skel_data(map, ident, sizeof(ident)))
> +			continue;
> +		if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>  			continue;
>  
>  		map_type_id = bpf_map__btf_value_type_id(map);
> diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
> index 53fee53d36d5..1f3f332dffbe 100644
> --- a/tools/lib/bpf/skel_internal.h
> +++ b/tools/lib/bpf/skel_internal.h
> @@ -131,8 +131,10 @@ static inline void skel_free_map_data(void *p, __u64 addr, size_t sz)
>  {
>  	if (addr != ~0ULL)
>  		kvfree(p);
> -	/* When addr == ~0ULL the 'p' points to
> -	 * ((struct bpf_array *)map)->value. See skel_finalize_map_data.
> +	/*
> +	 * When addr == ~0ULL the init buffer has already been released.
> +	 * For skel_finalize_map_data(), 'p' points to
> +	 * ((struct bpf_array *)map)->value.
>  	 */
>  }
>  
> @@ -170,6 +172,15 @@ static inline void *skel_finalize_map_data(__u64 *init_val, size_t mmap_sz, int
>  	return addr;
>  }
>  
> +static inline int skel_protect_map_data(void *p, __u64 *init_val, size_t sz)
> +{
> +	(void)sz;
> +
> +	kvfree(p);
> +	*init_val = ~0ULL;
> +	return 0;
> +}
> +
>  #else
>  
>  static inline void *skel_alloc(size_t size)
> @@ -208,6 +219,15 @@ static inline void *skel_finalize_map_data(__u64 *init_val, size_t mmap_sz, int
>  		return NULL;
>  	return addr;
>  }
> +
> +static inline int skel_protect_map_data(void *p, __u64 *init_val, size_t sz)
> +{
> +	(void)init_val;
> +
> +	if (mprotect(p, sz, PROT_READ))
> +		return -errno;
> +	return 0;
> +}
>  #endif
>  
>  static inline int skel_closenz(int fd)