[PATCH bpf-next 0/2] libbpf: capability for resizing datasec maps

[PATCH bpf-next 0/2] libbpf: capability for resizing datasec maps

[JP Kobryn]

In a bpf_object, the datasec maps like .data, .bss, .rodata cannot be
resized with bpf_map__set_value_size() like normal maps can. This patch
series offers a way to allow the resizing of datasec maps.

The thought behind this is to allow for use cases where a given datasec
needs to scale to for example the number of CPU's present. A bpf program
can have a global array in a custom data section with an initial length
and before loading the bpf program, the array length could be extended to
match the CPU count. The selftests included in this series perform this
scaling to an arbitrary value to demonstrate how it can work.

JP Kobryn (2):
  add capability for resizing datasec maps
  selftests for resizing datasec maps

 tools/lib/bpf/libbpf.c                        | 138 +++++++++++++
 .../bpf/prog_tests/global_map_resize.c        | 187 ++++++++++++++++++
 .../bpf/progs/test_global_map_resize.c        |  33 ++++
 3 files changed, 358 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/global_map_resize.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_global_map_resize.c

-- 
2.40.0

[PATCH bpf-next 1/2] libbpf: add capability for resizing datasec maps

[JP Kobryn]

This patch updates bpf_map__set_value_size() so that if the given map is a
datasec, it will attempt to resize it. If the following criteria is met,
the resizing can be performed:
 - BTF info is present
 - the map is a datasec
 - the datasec contains a single variable
 - the single variable is an array

The new map_datasec_resize() function is used to perform the resizing
of the associated memory mapped region and adjust BTF so that the original
array variable points to a new BTF array that is sized to cover the
requested size. The new array size will be rounded up to a multiple of
the element size.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1cbacf9e71f3..991649cacc10 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9412,12 +9412,150 @@ __u32 bpf_map__value_size(const struct bpf_map *map)
 	return map->def.value_size;
 }
 
+static bool map_is_datasec(struct bpf_map *map)
+{
+	struct btf *btf;
+	struct btf_type *map_type;
+
+	btf = bpf_object__btf(map->obj);
+	if (!btf)
+		return false;
+
+	map_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
+
+	return btf_is_datasec(map_type);
+}
+
+static int map_datasec_resize(struct bpf_map *map, __u32 size)
+{
+	int err;
+	struct btf *btf;
+	struct btf_type *datasec_type, *var_type, *resolved_type, *array_element_type;
+	struct btf_var_secinfo *var;
+	struct btf_array *array;
+	__u32 resolved_id, new_array_id;
+	__u32 rounded_sz;
+	__u32 nr_elements;
+	__u32 old_value_sz = map->def.value_size;
+	size_t old_mmap_sz, new_mmap_sz;
+
+	/* btf is required and datasec map must be memory mapped */
+	btf = bpf_object__btf(map->obj);
+	if (!btf) {
+		pr_warn("cannot resize datasec map '%s' while btf info is not present\n",
+				bpf_map__name(map));
+
+		return -EINVAL;
+	}
+
+	datasec_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
+	if (!btf_is_datasec(datasec_type)) {
+		pr_warn("attempted to resize datasec map '%s' but map is not a datasec\n",
+				bpf_map__name(map));
+
+		return -EINVAL;
+	}
+
+	if (!map->mmaped) {
+		pr_warn("cannot resize datasec map '%s' while map is unexpectedly not memory mapped\n",
+				bpf_map__name(map));
+
+		return -EINVAL;
+	}
+
+	/* datasec must only have a single variable */
+	if (btf_vlen(datasec_type) != 1) {
+		pr_warn("cannot resize datasec map '%s' that does not consist of a single var\n",
+				bpf_map__name(map));
+
+		return -EINVAL;
+	}
+
+	/* the single variable has to be an array */
+	var = btf_var_secinfos(datasec_type);
+	resolved_id = btf__resolve_type(btf, var->type);
+	resolved_type = btf_type_by_id(btf, resolved_id);
+	if (!btf_is_array(resolved_type)) {
+		pr_warn("cannot resize datasec map '%s' whose single var is not an array\n",
+				bpf_map__name(map));
+
+		return -EINVAL;
+	}
+
+	/* create a new array based on the existing array but with new length,
+	 * rounding up the requested size for alignment
+	 */
+	array = btf_array(resolved_type);
+	array_element_type = btf_type_by_id(btf, array->type);
+	rounded_sz = roundup(size, array_element_type->size);
+	nr_elements = rounded_sz / array_element_type->size;
+	new_array_id = btf__add_array(btf, array->index_type, array->type,
+			nr_elements);
+	if (new_array_id < 0) {
+		pr_warn("failed to resize datasec map '%s' due to failure in creating new array\n",
+				bpf_map__name(map));
+		err = new_array_id;
+
+		goto fail_array;
+	}
+
+	/* adding a new btf type invalidates existing pointers to btf objects.
+	 * refresh pointers before proceeding
+	 */
+	datasec_type = btf_type_by_id(btf, map->btf_value_type_id);
+	var = btf_var_secinfos(datasec_type);
+	var_type = btf_type_by_id(btf, var->type);
+
+	/* remap the associated memory */
+	old_value_sz = map->def.value_size;
+	old_mmap_sz = bpf_map_mmap_sz(map);
+	map->def.value_size = rounded_sz;
+	new_mmap_sz = bpf_map_mmap_sz(map);
+
+	if (munmap(map->mmaped, old_mmap_sz)) {
+		err = -errno;
+		pr_warn("failed to resize datasec map '%s' due to failure in munmap(), err:%d\n",
+			 bpf_map__name(map), err);
+
+		goto fail_mmap;
+	}
+
+	map->mmaped = mmap(NULL, new_mmap_sz, PROT_READ | PROT_WRITE,
+		   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	if (map->mmaped == MAP_FAILED) {
+		err = -errno;
+		map->mmaped = NULL;
+		pr_warn("failed to resize datasec map '%s' due to failure in mmap(), err:%d\n",
+			 bpf_map__name(map), err);
+
+		goto fail_mmap;
+	}
+
+	/* finally update btf info */
+	datasec_type->size = var->size = rounded_sz;
+	var_type->type = new_array_id;
+
+	return 0;
+
+fail_mmap:
+	map->def.value_size = old_value_sz;
+
+fail_array:
+	return err;
+}
+
 int bpf_map__set_value_size(struct bpf_map *map, __u32 size)
 {
 	if (map->fd >= 0)
 		return libbpf_err(-EBUSY);
+
+	if (map_is_datasec(map))
+		return map_datasec_resize(map, size);
+
 	map->def.value_size = size;
+
 	return 0;
+
 }
 
 __u32 bpf_map__btf_key_type_id(const struct bpf_map *map)
-- 
2.40.0

Re: [PATCH bpf-next 1/2] libbpf: add capability for resizing datasec maps

[Stanislav Fomichev]

On 04/28, JP Kobryn wrote:
> This patch updates bpf_map__set_value_size() so that if the given map is a
> datasec, it will attempt to resize it. If the following criteria is met,
> the resizing can be performed:
>  - BTF info is present
>  - the map is a datasec
>  - the datasec contains a single variable
>  - the single variable is an array
> 
> The new map_datasec_resize() function is used to perform the resizing
> of the associated memory mapped region and adjust BTF so that the original
> array variable points to a new BTF array that is sized to cover the
> requested size. The new array size will be rounded up to a multiple of
> the element size.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1cbacf9e71f3..991649cacc10 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9412,12 +9412,150 @@ __u32 bpf_map__value_size(const struct bpf_map *map)
>  	return map->def.value_size;
>  }
>  
> +static bool map_is_datasec(struct bpf_map *map)
> +{
> +	struct btf *btf;
> +	struct btf_type *map_type;
> +
> +	btf = bpf_object__btf(map->obj);
> +	if (!btf)
> +		return false;
> +
> +	map_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> +
> +	return btf_is_datasec(map_type);
> +}
> +
> +static int map_datasec_resize(struct bpf_map *map, __u32 size)
> +{
> +	int err;
> +	struct btf *btf;
> +	struct btf_type *datasec_type, *var_type, *resolved_type, *array_element_type;
> +	struct btf_var_secinfo *var;
> +	struct btf_array *array;
> +	__u32 resolved_id, new_array_id;
> +	__u32 rounded_sz;
> +	__u32 nr_elements;
> +	__u32 old_value_sz = map->def.value_size;
> +	size_t old_mmap_sz, new_mmap_sz;
> +
> +	/* btf is required and datasec map must be memory mapped */
> +	btf = bpf_object__btf(map->obj);
> +	if (!btf) {
> +		pr_warn("cannot resize datasec map '%s' while btf info is not present\n",
> +				bpf_map__name(map));
> +
> +		return -EINVAL;
> +	}
> +
> +	datasec_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> +	if (!btf_is_datasec(datasec_type)) {
> +		pr_warn("attempted to resize datasec map '%s' but map is not a datasec\n",
> +				bpf_map__name(map));
> +
> +		return -EINVAL;
> +	}
> +
> +	if (!map->mmaped) {
> +		pr_warn("cannot resize datasec map '%s' while map is unexpectedly not memory mapped\n",
> +				bpf_map__name(map));
> +
> +		return -EINVAL;
> +	}
> +
> +	/* datasec must only have a single variable */
> +	if (btf_vlen(datasec_type) != 1) {
> +		pr_warn("cannot resize datasec map '%s' that does not consist of a single var\n",
> +				bpf_map__name(map));
> +
> +		return -EINVAL;
> +	}
> +
> +	/* the single variable has to be an array */
> +	var = btf_var_secinfos(datasec_type);
> +	resolved_id = btf__resolve_type(btf, var->type);
> +	resolved_type = btf_type_by_id(btf, resolved_id);
> +	if (!btf_is_array(resolved_type)) {
> +		pr_warn("cannot resize datasec map '%s' whose single var is not an array\n",
> +				bpf_map__name(map));
> +
> +		return -EINVAL;
> +	}
> +
> +	/* create a new array based on the existing array but with new length,
> +	 * rounding up the requested size for alignment
> +	 */
> +	array = btf_array(resolved_type);
> +	array_element_type = btf_type_by_id(btf, array->type);
> +	rounded_sz = roundup(size, array_element_type->size);
> +	nr_elements = rounded_sz / array_element_type->size;
> +	new_array_id = btf__add_array(btf, array->index_type, array->type,
> +			nr_elements);
> +	if (new_array_id < 0) {
> +		pr_warn("failed to resize datasec map '%s' due to failure in creating new array\n",
> +				bpf_map__name(map));
> +		err = new_array_id;
> +
> +		goto fail_array;
> +	}
> +
> +	/* adding a new btf type invalidates existing pointers to btf objects.
> +	 * refresh pointers before proceeding
> +	 */
> +	datasec_type = btf_type_by_id(btf, map->btf_value_type_id);
> +	var = btf_var_secinfos(datasec_type);
> +	var_type = btf_type_by_id(btf, var->type);
> +
> +	/* remap the associated memory */
> +	old_value_sz = map->def.value_size;
> +	old_mmap_sz = bpf_map_mmap_sz(map);
> +	map->def.value_size = rounded_sz;
> +	new_mmap_sz = bpf_map_mmap_sz(map);
> +
> +	if (munmap(map->mmaped, old_mmap_sz)) {
> +		err = -errno;
> +		pr_warn("failed to resize datasec map '%s' due to failure in munmap(), err:%d\n",
> +			 bpf_map__name(map), err);
> +
> +		goto fail_mmap;
> +	}
> +
> +	map->mmaped = mmap(NULL, new_mmap_sz, PROT_READ | PROT_WRITE,
> +		   MAP_SHARED | MAP_ANONYMOUS, -1, 0);

I'm probably missing something, but how does it work? This just mmaps
new memory which the user-space side will see. What about the BPF side?

I'm also assuming (maybe incorrectly?) that if the map is mmaped, it's
already created in the kernel, so what's the point of the resizing?

Re: [PATCH bpf-next 1/2] libbpf: add capability for resizing datasec maps

[JP Kobryn]

On Fri, Apr 28, 2023 at 04:58:40PM -0700, Stanislav Fomichev wrote:
> On 04/28, JP Kobryn wrote:
> > This patch updates bpf_map__set_value_size() so that if the given map is a
> > datasec, it will attempt to resize it. If the following criteria is met,
> > the resizing can be performed:
> >  - BTF info is present
> >  - the map is a datasec
> >  - the datasec contains a single variable
> >  - the single variable is an array
> > 
> > The new map_datasec_resize() function is used to perform the resizing
> > of the associated memory mapped region and adjust BTF so that the original
> > array variable points to a new BTF array that is sized to cover the
> > requested size. The new array size will be rounded up to a multiple of
> > the element size.
> > 
> > Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 138 insertions(+)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 1cbacf9e71f3..991649cacc10 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -9412,12 +9412,150 @@ __u32 bpf_map__value_size(const struct bpf_map *map)
> >  	return map->def.value_size;
> >  }
> >  
> > +static bool map_is_datasec(struct bpf_map *map)
> > +{
> > +	struct btf *btf;
> > +	struct btf_type *map_type;
> > +
> > +	btf = bpf_object__btf(map->obj);
> > +	if (!btf)
> > +		return false;
> > +
> > +	map_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> > +
> > +	return btf_is_datasec(map_type);
> > +}
> > +
> > +static int map_datasec_resize(struct bpf_map *map, __u32 size)
> > +{
> > +	int err;
> > +	struct btf *btf;
> > +	struct btf_type *datasec_type, *var_type, *resolved_type, *array_element_type;
> > +	struct btf_var_secinfo *var;
> > +	struct btf_array *array;
> > +	__u32 resolved_id, new_array_id;
> > +	__u32 rounded_sz;
> > +	__u32 nr_elements;
> > +	__u32 old_value_sz = map->def.value_size;
> > +	size_t old_mmap_sz, new_mmap_sz;
> > +
> > +	/* btf is required and datasec map must be memory mapped */
> > +	btf = bpf_object__btf(map->obj);
> > +	if (!btf) {
> > +		pr_warn("cannot resize datasec map '%s' while btf info is not present\n",
> > +				bpf_map__name(map));
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	datasec_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> > +	if (!btf_is_datasec(datasec_type)) {
> > +		pr_warn("attempted to resize datasec map '%s' but map is not a datasec\n",
> > +				bpf_map__name(map));
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!map->mmaped) {
> > +		pr_warn("cannot resize datasec map '%s' while map is unexpectedly not memory mapped\n",
> > +				bpf_map__name(map));
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* datasec must only have a single variable */
> > +	if (btf_vlen(datasec_type) != 1) {
> > +		pr_warn("cannot resize datasec map '%s' that does not consist of a single var\n",
> > +				bpf_map__name(map));
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* the single variable has to be an array */
> > +	var = btf_var_secinfos(datasec_type);
> > +	resolved_id = btf__resolve_type(btf, var->type);
> > +	resolved_type = btf_type_by_id(btf, resolved_id);
> > +	if (!btf_is_array(resolved_type)) {
> > +		pr_warn("cannot resize datasec map '%s' whose single var is not an array\n",
> > +				bpf_map__name(map));
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* create a new array based on the existing array but with new length,
> > +	 * rounding up the requested size for alignment
> > +	 */
> > +	array = btf_array(resolved_type);
> > +	array_element_type = btf_type_by_id(btf, array->type);
> > +	rounded_sz = roundup(size, array_element_type->size);
> > +	nr_elements = rounded_sz / array_element_type->size;
> > +	new_array_id = btf__add_array(btf, array->index_type, array->type,
> > +			nr_elements);
> > +	if (new_array_id < 0) {
> > +		pr_warn("failed to resize datasec map '%s' due to failure in creating new array\n",
> > +				bpf_map__name(map));
> > +		err = new_array_id;
> > +
> > +		goto fail_array;
> > +	}
> > +
> > +	/* adding a new btf type invalidates existing pointers to btf objects.
> > +	 * refresh pointers before proceeding
> > +	 */
> > +	datasec_type = btf_type_by_id(btf, map->btf_value_type_id);
> > +	var = btf_var_secinfos(datasec_type);
> > +	var_type = btf_type_by_id(btf, var->type);
> > +
> > +	/* remap the associated memory */
> > +	old_value_sz = map->def.value_size;
> > +	old_mmap_sz = bpf_map_mmap_sz(map);
> > +	map->def.value_size = rounded_sz;
> > +	new_mmap_sz = bpf_map_mmap_sz(map);
> > +
> > +	if (munmap(map->mmaped, old_mmap_sz)) {
> > +		err = -errno;
> > +		pr_warn("failed to resize datasec map '%s' due to failure in munmap(), err:%d\n",
> > +			 bpf_map__name(map), err);
> > +
> > +		goto fail_mmap;
> > +	}
> > +
> > +	map->mmaped = mmap(NULL, new_mmap_sz, PROT_READ | PROT_WRITE,
> > +		   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> 
> I'm probably missing something, but how does it work? This just mmaps
> new memory which the user-space side will see. What about the BPF side?
> 
In general (not specific to this patch), all datasec maps are
memory mapped with an initialization image. See
bpf_object__load_skeleton() to see how this initial mapping is later
associated with the actual bpf maps (file descriptors) kernel side.

> I'm also assuming (maybe incorrectly?) that if the map is mmaped, it's
> already created in the kernel, so what's the point of the resizing?

This is still the initialization image being resized. This resizing
happens before the map is associated kernel side. If the map has already
been created on the bpf side, attempting to resize returns -EBUSY (not
new in this patch).
-- 

Re: [PATCH bpf-next 1/2] libbpf: add capability for resizing datasec maps

[Stanislav Fomichev]

On 05/01, JP Kobryn wrote:
> On Fri, Apr 28, 2023 at 04:58:40PM -0700, Stanislav Fomichev wrote:
> > On 04/28, JP Kobryn wrote:
> > > This patch updates bpf_map__set_value_size() so that if the given map is a
> > > datasec, it will attempt to resize it. If the following criteria is met,
> > > the resizing can be performed:
> > >  - BTF info is present
> > >  - the map is a datasec
> > >  - the datasec contains a single variable
> > >  - the single variable is an array
> > > 
> > > The new map_datasec_resize() function is used to perform the resizing
> > > of the associated memory mapped region and adjust BTF so that the original
> > > array variable points to a new BTF array that is sized to cover the
> > > requested size. The new array size will be rounded up to a multiple of
> > > the element size.
> > > 
> > > Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> > > ---
> > >  tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 138 insertions(+)
> > > 
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 1cbacf9e71f3..991649cacc10 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -9412,12 +9412,150 @@ __u32 bpf_map__value_size(const struct bpf_map *map)
> > >  	return map->def.value_size;
> > >  }
> > >  
> > > +static bool map_is_datasec(struct bpf_map *map)
> > > +{
> > > +	struct btf *btf;
> > > +	struct btf_type *map_type;
> > > +
> > > +	btf = bpf_object__btf(map->obj);
> > > +	if (!btf)
> > > +		return false;
> > > +
> > > +	map_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> > > +
> > > +	return btf_is_datasec(map_type);
> > > +}
> > > +
> > > +static int map_datasec_resize(struct bpf_map *map, __u32 size)
> > > +{
> > > +	int err;
> > > +	struct btf *btf;
> > > +	struct btf_type *datasec_type, *var_type, *resolved_type, *array_element_type;
> > > +	struct btf_var_secinfo *var;
> > > +	struct btf_array *array;
> > > +	__u32 resolved_id, new_array_id;
> > > +	__u32 rounded_sz;
> > > +	__u32 nr_elements;
> > > +	__u32 old_value_sz = map->def.value_size;
> > > +	size_t old_mmap_sz, new_mmap_sz;
> > > +
> > > +	/* btf is required and datasec map must be memory mapped */
> > > +	btf = bpf_object__btf(map->obj);
> > > +	if (!btf) {
> > > +		pr_warn("cannot resize datasec map '%s' while btf info is not present\n",
> > > +				bpf_map__name(map));
> > > +
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	datasec_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> > > +	if (!btf_is_datasec(datasec_type)) {
> > > +		pr_warn("attempted to resize datasec map '%s' but map is not a datasec\n",
> > > +				bpf_map__name(map));
> > > +
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (!map->mmaped) {
> > > +		pr_warn("cannot resize datasec map '%s' while map is unexpectedly not memory mapped\n",
> > > +				bpf_map__name(map));
> > > +
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	/* datasec must only have a single variable */
> > > +	if (btf_vlen(datasec_type) != 1) {
> > > +		pr_warn("cannot resize datasec map '%s' that does not consist of a single var\n",
> > > +				bpf_map__name(map));
> > > +
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	/* the single variable has to be an array */
> > > +	var = btf_var_secinfos(datasec_type);
> > > +	resolved_id = btf__resolve_type(btf, var->type);
> > > +	resolved_type = btf_type_by_id(btf, resolved_id);
> > > +	if (!btf_is_array(resolved_type)) {
> > > +		pr_warn("cannot resize datasec map '%s' whose single var is not an array\n",
> > > +				bpf_map__name(map));
> > > +
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	/* create a new array based on the existing array but with new length,
> > > +	 * rounding up the requested size for alignment
> > > +	 */
> > > +	array = btf_array(resolved_type);
> > > +	array_element_type = btf_type_by_id(btf, array->type);
> > > +	rounded_sz = roundup(size, array_element_type->size);
> > > +	nr_elements = rounded_sz / array_element_type->size;
> > > +	new_array_id = btf__add_array(btf, array->index_type, array->type,
> > > +			nr_elements);
> > > +	if (new_array_id < 0) {
> > > +		pr_warn("failed to resize datasec map '%s' due to failure in creating new array\n",
> > > +				bpf_map__name(map));
> > > +		err = new_array_id;
> > > +
> > > +		goto fail_array;
> > > +	}
> > > +
> > > +	/* adding a new btf type invalidates existing pointers to btf objects.
> > > +	 * refresh pointers before proceeding
> > > +	 */
> > > +	datasec_type = btf_type_by_id(btf, map->btf_value_type_id);
> > > +	var = btf_var_secinfos(datasec_type);
> > > +	var_type = btf_type_by_id(btf, var->type);
> > > +
> > > +	/* remap the associated memory */
> > > +	old_value_sz = map->def.value_size;
> > > +	old_mmap_sz = bpf_map_mmap_sz(map);
> > > +	map->def.value_size = rounded_sz;
> > > +	new_mmap_sz = bpf_map_mmap_sz(map);
> > > +
> > > +	if (munmap(map->mmaped, old_mmap_sz)) {
> > > +		err = -errno;
> > > +		pr_warn("failed to resize datasec map '%s' due to failure in munmap(), err:%d\n",
> > > +			 bpf_map__name(map), err);
> > > +
> > > +		goto fail_mmap;
> > > +	}
> > > +
> > > +	map->mmaped = mmap(NULL, new_mmap_sz, PROT_READ | PROT_WRITE,
> > > +		   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > 
> > I'm probably missing something, but how does it work? This just mmaps
> > new memory which the user-space side will see. What about the BPF side?
> > 
> In general (not specific to this patch), all datasec maps are
> memory mapped with an initialization image. See
> bpf_object__load_skeleton() to see how this initial mapping is later
> associated with the actual bpf maps (file descriptors) kernel side.
> 
> > I'm also assuming (maybe incorrectly?) that if the map is mmaped, it's
> > already created in the kernel, so what's the point of the resizing?
> 
> This is still the initialization image being resized. This resizing
> happens before the map is associated kernel side. If the map has already
> been created on the bpf side, attempting to resize returns -EBUSY (not
> new in this patch).

I see, makes sense now, thanks!

Acked-by: Stanislav Fomichev <sdf@google.com>

Re: [PATCH bpf-next 1/2] libbpf: add capability for resizing datasec maps

[Andrii Nakryiko]

On Fri, Apr 28, 2023 at 3:28 PM JP Kobryn <inwardvessel@gmail.com> wrote:
>
> This patch updates bpf_map__set_value_size() so that if the given map is a
> datasec, it will attempt to resize it. If the following criteria is met,
> the resizing can be performed:
>  - BTF info is present
>  - the map is a datasec
>  - the datasec contains a single variable
>  - the single variable is an array
>

I think it's too restrictive to require all these conditions just to
be able to resize the map. I'd prefer if libbpf allowed user to resize
a map in any case, but if there is BTF information, libbpf will
helpfully adjust it as necessary.

> The new map_datasec_resize() function is used to perform the resizing
> of the associated memory mapped region and adjust BTF so that the original
> array variable points to a new BTF array that is sized to cover the
> requested size. The new array size will be rounded up to a multiple of
> the element size.
>
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1cbacf9e71f3..991649cacc10 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9412,12 +9412,150 @@ __u32 bpf_map__value_size(const struct bpf_map *map)
>         return map->def.value_size;
>  }
>
> +static bool map_is_datasec(struct bpf_map *map)
> +{
> +       struct btf *btf;
> +       struct btf_type *map_type;
> +
> +       btf = bpf_object__btf(map->obj);
> +       if (!btf)
> +               return false;
> +
> +       map_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> +
> +       return btf_is_datasec(map_type);
> +}
> +
> +static int map_datasec_resize(struct bpf_map *map, __u32 size)
> +{
> +       int err;
> +       struct btf *btf;
> +       struct btf_type *datasec_type, *var_type, *resolved_type, *array_element_type;
> +       struct btf_var_secinfo *var;
> +       struct btf_array *array;
> +       __u32 resolved_id, new_array_id;
> +       __u32 rounded_sz;
> +       __u32 nr_elements;
> +       __u32 old_value_sz = map->def.value_size;
> +       size_t old_mmap_sz, new_mmap_sz;
> +
> +       /* btf is required and datasec map must be memory mapped */

as I mentioned above, I'd structure logic to take advantage and adjust
BTF, but not necessarily block the operation

> +       btf = bpf_object__btf(map->obj);
> +       if (!btf) {
> +               pr_warn("cannot resize datasec map '%s' while btf info is not present\n",
> +                               bpf_map__name(map));
> +

nit: let's not have unnecessary empty lines (here and below in many places)

another nit: see other pr_warn()s when something happens with map, we
have relatively consistent "map '%s': <message>" pattern, so let's
follow it here for error messages

> +               return -EINVAL;
> +       }
> +
> +       datasec_type = btf_type_by_id(btf, bpf_map__btf_value_type_id(map));
> +       if (!btf_is_datasec(datasec_type)) {
> +               pr_warn("attempted to resize datasec map '%s' but map is not a datasec\n",
> +                               bpf_map__name(map));
> +
> +               return -EINVAL;
> +       }
> +
> +       if (!map->mmaped) {
> +               pr_warn("cannot resize datasec map '%s' while map is unexpectedly not memory mapped\n",
> +                               bpf_map__name(map));
> +
> +               return -EINVAL;
> +       }
> +
> +       /* datasec must only have a single variable */
> +       if (btf_vlen(datasec_type) != 1) {

so I've been thinking about case like this:

int my_var;
int my_arr[1]; /* should scale to number of CPUs */

I don't see why we wouldn't allow to resize this to 4 + cpu_cnt * 4
size. I don't think it will complicate anything, we just take last
member of DATASEC, it's offset + N * sizeof(array element)


> +               pr_warn("cannot resize datasec map '%s' that does not consist of a single var\n",
> +                               bpf_map__name(map));
> +
> +               return -EINVAL;
> +       }
> +
> +       /* the single variable has to be an array */
> +       var = btf_var_secinfos(datasec_type);
> +       resolved_id = btf__resolve_type(btf, var->type);

use skip_mods_and_typedefs to skip mods and typedefs?
btf__resolve_type() does more than what you want here

> +       resolved_type = btf_type_by_id(btf, resolved_id);
> +       if (!btf_is_array(resolved_type)) {
> +               pr_warn("cannot resize datasec map '%s' whose single var is not an array\n",
> +                               bpf_map__name(map));
> +
> +               return -EINVAL;
> +       }
> +
> +       /* create a new array based on the existing array but with new length,
> +        * rounding up the requested size for alignment
> +        */
> +       array = btf_array(resolved_type);
> +       array_element_type = btf_type_by_id(btf, array->type);
> +       rounded_sz = roundup(size, array_element_type->size);

let's not do auto-rounding, user should know what they are doing, and
if they get calculation wrong, better return explicit error than try
to guess what user actually wanted

> +       nr_elements = rounded_sz / array_element_type->size;
> +       new_array_id = btf__add_array(btf, array->index_type, array->type,
> +                       nr_elements);
> +       if (new_array_id < 0) {
> +               pr_warn("failed to resize datasec map '%s' due to failure in creating new array\n",
> +                               bpf_map__name(map));
> +               err = new_array_id;
> +
> +               goto fail_array;
> +       }
> +
> +       /* adding a new btf type invalidates existing pointers to btf objects.
> +        * refresh pointers before proceeding
> +        */
> +       datasec_type = btf_type_by_id(btf, map->btf_value_type_id);
> +       var = btf_var_secinfos(datasec_type);
> +       var_type = btf_type_by_id(btf, var->type);
> +
> +       /* remap the associated memory */
> +       old_value_sz = map->def.value_size;
> +       old_mmap_sz = bpf_map_mmap_sz(map);
> +       map->def.value_size = rounded_sz;
> +       new_mmap_sz = bpf_map_mmap_sz(map);
> +
> +       if (munmap(map->mmaped, old_mmap_sz)) {
> +               err = -errno;
> +               pr_warn("failed to resize datasec map '%s' due to failure in munmap(), err:%d\n",
> +                        bpf_map__name(map), err);
> +
> +               goto fail_mmap;
> +       }
> +
> +       map->mmaped = mmap(NULL, new_mmap_sz, PROT_READ | PROT_WRITE,
> +                  MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +       if (map->mmaped == MAP_FAILED) {

let's mmap new memory first, and only if that succeeds unmap old one?
Plus, we need to preserve initial values that might have been set
through BPF skeleton or bpf_map__set_initial_value()

and please adjust selftest to validate that modified/non-zero initial
values are preserved during resize; similar to how realloc() behaves


> +               err = -errno;
> +               map->mmaped = NULL;
> +               pr_warn("failed to resize datasec map '%s' due to failure in mmap(), err:%d\n",
> +                        bpf_map__name(map), err);
> +
> +               goto fail_mmap;
> +       }
> +
> +       /* finally update btf info */
> +       datasec_type->size = var->size = rounded_sz;
> +       var_type->type = new_array_id;
> +
> +       return 0;
> +
> +fail_mmap:
> +       map->def.value_size = old_value_sz;
> +
> +fail_array:
> +       return err;
> +}
> +
>  int bpf_map__set_value_size(struct bpf_map *map, __u32 size)
>  {
>         if (map->fd >= 0)
>                 return libbpf_err(-EBUSY);
> +
> +       if (map_is_datasec(map))

so, this is not the best way to check this, see LIBBPF_MAP_BSS,
LIBBPF_MAP_DATA, LIBBPF_MAP_RODATA and libbpf_type field in bpf_map

but as I mentioned above, let's structure it such that
map->def.value_size can always be updated, but libbpf tries to adjust
BTF (we can emit warning if all the logical constraints are not
satisfied; and then clearing btf_value_type_id and btf_value_key_id)?

> +               return map_datasec_resize(map, size);
> +
>         map->def.value_size = size;
> +
>         return 0;
> +
>  }
>
>  __u32 bpf_map__btf_key_type_id(const struct bpf_map *map)
> --
> 2.40.0
>

[PATCH bpf-next 2/2] libbpf: selftests for resizing datasec maps

[JP Kobryn]

This patch adds test coverage for resizing datasec maps. There are two
tests which run a bpf program that sums the elements in the datasec array.
After the datasec array is resized, each elements is assigned a value of 1
so that the sum will be equal to the length of the array. Assertions are
done to verify this. The first test attempts to resize to an aligned
length while the second attempts to resize to a mis-aligned length where
rounding up is expected to occur. The third test attempts to resize maps
that do not meet the necessary criteria and assertions are done to confirm
error codes are returned.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 .../bpf/prog_tests/global_map_resize.c        | 187 ++++++++++++++++++
 .../bpf/progs/test_global_map_resize.c        |  33 ++++
 2 files changed, 220 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/global_map_resize.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_global_map_resize.c

diff --git a/tools/testing/selftests/bpf/prog_tests/global_map_resize.c b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
new file mode 100644
index 000000000000..f38df37664a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <errno.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "test_global_map_resize.skel.h"
+#include "test_progs.h"
+
+static void run_program(void)
+{
+	(void)syscall(__NR_getpid);
+}
+
+static int setup(struct test_global_map_resize **skel)
+{
+	if (!skel)
+		return -1;
+
+	*skel = test_global_map_resize__open();
+	if (!ASSERT_OK_PTR(skel, "test_global_map_resize__open"))
+		return -1;
+
+	(*skel)->rodata->pid = getpid();
+
+	return 0;
+}
+
+static void teardown(struct test_global_map_resize **skel)
+{
+	if (skel && *skel)
+		test_global_map_resize__destroy(*skel);
+}
+
+static int resize_test(struct test_global_map_resize *skel,
+		__u32 element_sz, __u32 desired_sz)
+{
+	int ret = 0;
+	struct bpf_map *map;
+	__u32 initial_sz, actual_sz;
+	size_t nr_elements;
+	int *initial_val;
+	size_t initial_val_sz;
+
+	map = skel->maps.data_my_array;
+
+	initial_sz = bpf_map__value_size(map);
+	ASSERT_EQ(initial_sz, element_sz, "initial size");
+
+	/* round up desired size to align with element size */
+	desired_sz = roundup(desired_sz, element_sz);
+	ret = bpf_map__set_value_size(map, desired_sz);
+	if (!ASSERT_OK(ret, "bpf_map__set_value_size"))
+		return ret;
+
+	/* refresh map pointer to avoid invalidation issues */
+	map = skel->maps.data_my_array;
+
+	actual_sz = bpf_map__value_size(map);
+	ASSERT_EQ(actual_sz, desired_sz, "resize");
+
+	/* set the expected number of elements based on the resized array */
+	nr_elements = roundup(actual_sz, element_sz) / element_sz;
+	skel->rodata->n = nr_elements;
+
+	/* create array for initial map value */
+	initial_val_sz = element_sz * nr_elements;
+	initial_val = malloc(initial_val_sz);
+	if (!ASSERT_OK_PTR(initial_val, "malloc initial_val")) {
+		ret = -ENOMEM;
+
+		goto cleanup;
+	}
+
+	/* fill array with ones */
+	for (int i = 0; i < nr_elements; ++i)
+		initial_val[i] = 1;
+
+	/* set initial value */
+	ASSERT_EQ(initial_val_sz, actual_sz, "initial value size");
+
+	ret = bpf_map__set_initial_value(map, initial_val, initial_val_sz);
+	if (!ASSERT_OK(ret, "bpf_map__set_initial_val"))
+		goto cleanup;
+
+	ret = test_global_map_resize__load(skel);
+	if (!ASSERT_OK(ret, "test_global_map_resize__load"))
+		goto cleanup;
+
+	ret = test_global_map_resize__attach(skel);
+	if (!ASSERT_OK(ret, "test_global_map_resize__attach"))
+		goto cleanup;
+
+	/* run the bpf program which will sum the contents of the array */
+	run_program();
+
+	if (!ASSERT_EQ(skel->bss->sum, nr_elements, "sum"))
+		goto cleanup;
+
+cleanup:
+	if (initial_val)
+		free(initial_val);
+
+	return ret;
+}
+
+static void global_map_resize_aligned_subtest(void)
+{
+	struct test_global_map_resize *skel;
+	const __u32 element_sz = (__u32)sizeof(int);
+	const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2;
+
+	/* preliminary check that desired_sz aligns with element_sz */
+	if (!ASSERT_EQ(desired_sz % element_sz, 0, "alignment"))
+		return;
+
+	if (setup(&skel))
+		goto teardown;
+
+	if (resize_test(skel, element_sz, desired_sz))
+		goto teardown;
+
+teardown:
+	teardown(&skel);
+}
+
+static void global_map_resize_roundup_subtest(void)
+{
+	struct test_global_map_resize *skel;
+	const __u32 element_sz = (__u32)sizeof(int);
+	/* set desired size a fraction of element size beyond an aligned size */
+	const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2 + element_sz / 2;
+
+	/* preliminary check that desired_sz does NOT align with element_sz */
+	if (!ASSERT_NEQ(desired_sz % element_sz, 0, "alignment"))
+		return;
+
+	if (setup(&skel))
+		goto teardown;
+
+	if (resize_test(skel, element_sz, desired_sz))
+		goto teardown;
+
+teardown:
+	teardown(&skel);
+}
+
+static void global_map_resize_invalid_subtest(void)
+{
+	int err;
+	struct test_global_map_resize *skel;
+	struct bpf_map *map;
+	const __u32 desired_sz = 8192;
+
+	if (setup(&skel))
+		goto teardown;
+
+	/* attempt to resize a global datasec map which is an array
+	 * BUT is with a var in same datasec
+	 */
+	map = skel->maps.data_my_array_and_var;
+	err = bpf_map__set_value_size(map, desired_sz);
+	if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
+		goto teardown;
+
+	/* attempt to resize a global datasec map which is NOT an array */
+	map = skel->maps.data_my_non_array;
+	err = bpf_map__set_value_size(map, desired_sz);
+	if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
+		goto teardown;
+
+teardown:
+	teardown(&skel);
+}
+
+void test_global_map_resize(void)
+{
+	if (test__start_subtest("global_map_resize_aligned"))
+		global_map_resize_aligned_subtest();
+
+	if (test__start_subtest("global_map_resize_roundup"))
+		global_map_resize_roundup_subtest();
+
+	if (test__start_subtest("global_map_resize_invalid"))
+		global_map_resize_invalid_subtest();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
new file mode 100644
index 000000000000..cffbba1b6020
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+const volatile pid_t pid;
+const volatile size_t n;
+
+int my_array[1] SEC(".data.my_array");
+
+int my_array_with_neighbor[1] SEC(".data.my_array_and_var");
+int my_var_with_neighbor SEC(".data.my_array_and_var");
+
+int my_non_array SEC(".data.my_non_array");
+
+int sum = 0;
+
+SEC("tp/syscalls/sys_enter_getpid")
+int array_sum(void *ctx)
+{
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	sum = 0;
+
+	for (size_t i = 0; i < n; ++i)
+		sum += my_array[i];
+
+	return 0;
+}
-- 
2.40.0

Re: [PATCH bpf-next 2/2] libbpf: selftests for resizing datasec maps

[Stanislav Fomichev]

On 04/28, JP Kobryn wrote:
> This patch adds test coverage for resizing datasec maps. There are two
> tests which run a bpf program that sums the elements in the datasec array.
> After the datasec array is resized, each elements is assigned a value of 1
> so that the sum will be equal to the length of the array. Assertions are
> done to verify this. The first test attempts to resize to an aligned
> length while the second attempts to resize to a mis-aligned length where
> rounding up is expected to occur. The third test attempts to resize maps
> that do not meet the necessary criteria and assertions are done to confirm
> error codes are returned.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>

Acked-by: Stanislav Fomichev <sdf@google.com>

> ---
>  .../bpf/prog_tests/global_map_resize.c        | 187 ++++++++++++++++++
>  .../bpf/progs/test_global_map_resize.c        |  33 ++++
>  2 files changed, 220 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/global_map_resize.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_map_resize.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_map_resize.c b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> new file mode 100644
> index 000000000000..f38df37664a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <errno.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +#include "test_global_map_resize.skel.h"
> +#include "test_progs.h"
> +
> +static void run_program(void)
> +{
> +	(void)syscall(__NR_getpid);
> +}
> +
> +static int setup(struct test_global_map_resize **skel)
> +{
> +	if (!skel)
> +		return -1;
> +
> +	*skel = test_global_map_resize__open();
> +	if (!ASSERT_OK_PTR(skel, "test_global_map_resize__open"))
> +		return -1;
> +
> +	(*skel)->rodata->pid = getpid();
> +
> +	return 0;
> +}
> +
> +static void teardown(struct test_global_map_resize **skel)
> +{
> +	if (skel && *skel)
> +		test_global_map_resize__destroy(*skel);
> +}
> +
> +static int resize_test(struct test_global_map_resize *skel,
> +		__u32 element_sz, __u32 desired_sz)
> +{
> +	int ret = 0;
> +	struct bpf_map *map;
> +	__u32 initial_sz, actual_sz;
> +	size_t nr_elements;
> +	int *initial_val;
> +	size_t initial_val_sz;
> +
> +	map = skel->maps.data_my_array;
> +
> +	initial_sz = bpf_map__value_size(map);
> +	ASSERT_EQ(initial_sz, element_sz, "initial size");
> +
> +	/* round up desired size to align with element size */
> +	desired_sz = roundup(desired_sz, element_sz);
> +	ret = bpf_map__set_value_size(map, desired_sz);
> +	if (!ASSERT_OK(ret, "bpf_map__set_value_size"))
> +		return ret;
> +
> +	/* refresh map pointer to avoid invalidation issues */
> +	map = skel->maps.data_my_array;
> +
> +	actual_sz = bpf_map__value_size(map);
> +	ASSERT_EQ(actual_sz, desired_sz, "resize");
> +
> +	/* set the expected number of elements based on the resized array */
> +	nr_elements = roundup(actual_sz, element_sz) / element_sz;
> +	skel->rodata->n = nr_elements;
> +
> +	/* create array for initial map value */
> +	initial_val_sz = element_sz * nr_elements;
> +	initial_val = malloc(initial_val_sz);
> +	if (!ASSERT_OK_PTR(initial_val, "malloc initial_val")) {
> +		ret = -ENOMEM;
> +
> +		goto cleanup;
> +	}
> +
> +	/* fill array with ones */
> +	for (int i = 0; i < nr_elements; ++i)
> +		initial_val[i] = 1;
> +
> +	/* set initial value */
> +	ASSERT_EQ(initial_val_sz, actual_sz, "initial value size");
> +
> +	ret = bpf_map__set_initial_value(map, initial_val, initial_val_sz);
> +	if (!ASSERT_OK(ret, "bpf_map__set_initial_val"))
> +		goto cleanup;
> +
> +	ret = test_global_map_resize__load(skel);
> +	if (!ASSERT_OK(ret, "test_global_map_resize__load"))
> +		goto cleanup;
> +
> +	ret = test_global_map_resize__attach(skel);
> +	if (!ASSERT_OK(ret, "test_global_map_resize__attach"))
> +		goto cleanup;
> +
> +	/* run the bpf program which will sum the contents of the array */
> +	run_program();
> +
> +	if (!ASSERT_EQ(skel->bss->sum, nr_elements, "sum"))
> +		goto cleanup;
> +
> +cleanup:
> +	if (initial_val)
> +		free(initial_val);
> +
> +	return ret;
> +}
> +
> +static void global_map_resize_aligned_subtest(void)
> +{
> +	struct test_global_map_resize *skel;
> +	const __u32 element_sz = (__u32)sizeof(int);
> +	const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2;
> +
> +	/* preliminary check that desired_sz aligns with element_sz */
> +	if (!ASSERT_EQ(desired_sz % element_sz, 0, "alignment"))
> +		return;
> +
> +	if (setup(&skel))
> +		goto teardown;
> +
> +	if (resize_test(skel, element_sz, desired_sz))
> +		goto teardown;
> +
> +teardown:
> +	teardown(&skel);
> +}
> +
> +static void global_map_resize_roundup_subtest(void)
> +{
> +	struct test_global_map_resize *skel;
> +	const __u32 element_sz = (__u32)sizeof(int);
> +	/* set desired size a fraction of element size beyond an aligned size */
> +	const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2 + element_sz / 2;
> +
> +	/* preliminary check that desired_sz does NOT align with element_sz */
> +	if (!ASSERT_NEQ(desired_sz % element_sz, 0, "alignment"))
> +		return;
> +
> +	if (setup(&skel))
> +		goto teardown;
> +
> +	if (resize_test(skel, element_sz, desired_sz))
> +		goto teardown;
> +
> +teardown:
> +	teardown(&skel);
> +}
> +
> +static void global_map_resize_invalid_subtest(void)
> +{
> +	int err;
> +	struct test_global_map_resize *skel;
> +	struct bpf_map *map;
> +	const __u32 desired_sz = 8192;
> +
> +	if (setup(&skel))
> +		goto teardown;
> +
> +	/* attempt to resize a global datasec map which is an array
> +	 * BUT is with a var in same datasec
> +	 */
> +	map = skel->maps.data_my_array_and_var;
> +	err = bpf_map__set_value_size(map, desired_sz);
> +	if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> +		goto teardown;
> +
> +	/* attempt to resize a global datasec map which is NOT an array */
> +	map = skel->maps.data_my_non_array;
> +	err = bpf_map__set_value_size(map, desired_sz);
> +	if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> +		goto teardown;
> +
> +teardown:
> +	teardown(&skel);
> +}
> +
> +void test_global_map_resize(void)
> +{
> +	if (test__start_subtest("global_map_resize_aligned"))
> +		global_map_resize_aligned_subtest();
> +
> +	if (test__start_subtest("global_map_resize_roundup"))
> +		global_map_resize_roundup_subtest();
> +
> +	if (test__start_subtest("global_map_resize_invalid"))
> +		global_map_resize_invalid_subtest();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> new file mode 100644
> index 000000000000..cffbba1b6020
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +const volatile pid_t pid;
> +const volatile size_t n;
> +
> +int my_array[1] SEC(".data.my_array");
> +
> +int my_array_with_neighbor[1] SEC(".data.my_array_and_var");
> +int my_var_with_neighbor SEC(".data.my_array_and_var");
> +
> +int my_non_array SEC(".data.my_non_array");
> +
> +int sum = 0;
> +
> +SEC("tp/syscalls/sys_enter_getpid")
> +int array_sum(void *ctx)
> +{
> +	if (pid != (bpf_get_current_pid_tgid() >> 32))
> +		return 0;
> +
> +	sum = 0;
> +
> +	for (size_t i = 0; i < n; ++i)
> +		sum += my_array[i];
> +
> +	return 0;
> +}
> -- 
> 2.40.0
> 

Re: [PATCH bpf-next 2/2] libbpf: selftests for resizing datasec maps

[Andrii Nakryiko]

On Fri, Apr 28, 2023 at 3:28 PM JP Kobryn <inwardvessel@gmail.com> wrote:
>
> This patch adds test coverage for resizing datasec maps. There are two
> tests which run a bpf program that sums the elements in the datasec array.
> After the datasec array is resized, each elements is assigned a value of 1
> so that the sum will be equal to the length of the array. Assertions are
> done to verify this. The first test attempts to resize to an aligned
> length while the second attempts to resize to a mis-aligned length where
> rounding up is expected to occur. The third test attempts to resize maps
> that do not meet the necessary criteria and assertions are done to confirm
> error codes are returned.
>
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> ---
>  .../bpf/prog_tests/global_map_resize.c        | 187 ++++++++++++++++++
>  .../bpf/progs/test_global_map_resize.c        |  33 ++++
>  2 files changed, 220 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/global_map_resize.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_map_resize.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_map_resize.c b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> new file mode 100644
> index 000000000000..f38df37664a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <errno.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +#include "test_global_map_resize.skel.h"
> +#include "test_progs.h"
> +
> +static void run_program(void)
> +{
> +       (void)syscall(__NR_getpid);
> +}
> +
> +static int setup(struct test_global_map_resize **skel)
> +{
> +       if (!skel)
> +               return -1;
> +
> +       *skel = test_global_map_resize__open();
> +       if (!ASSERT_OK_PTR(skel, "test_global_map_resize__open"))
> +               return -1;
> +
> +       (*skel)->rodata->pid = getpid();
> +

this simple logic is also so common in lots of tests, that it doesn't
make sense to have dedicated setup() routine for that (just more
jumping around the code to know what's going on). Please inline in
respective callers.

> +       return 0;
> +}
> +
> +static void teardown(struct test_global_map_resize **skel)
> +{
> +       if (skel && *skel)
> +               test_global_map_resize__destroy(*skel);


skeleton's destroy handles NULL, so this whole teardown() is not
necessary, just call destroy() directly in respective functions

> +}
> +
> +static int resize_test(struct test_global_map_resize *skel,
> +               __u32 element_sz, __u32 desired_sz)
> +{
> +       int ret = 0;
> +       struct bpf_map *map;
> +       __u32 initial_sz, actual_sz;
> +       size_t nr_elements;
> +       int *initial_val;
> +       size_t initial_val_sz;
> +
> +       map = skel->maps.data_my_array;
> +
> +       initial_sz = bpf_map__value_size(map);
> +       ASSERT_EQ(initial_sz, element_sz, "initial size");
> +
> +       /* round up desired size to align with element size */
> +       desired_sz = roundup(desired_sz, element_sz);
> +       ret = bpf_map__set_value_size(map, desired_sz);
> +       if (!ASSERT_OK(ret, "bpf_map__set_value_size"))
> +               return ret;
> +
> +       /* refresh map pointer to avoid invalidation issues */
> +       map = skel->maps.data_my_array;
> +
> +       actual_sz = bpf_map__value_size(map);
> +       ASSERT_EQ(actual_sz, desired_sz, "resize");
> +
> +       /* set the expected number of elements based on the resized array */
> +       nr_elements = roundup(actual_sz, element_sz) / element_sz;
> +       skel->rodata->n = nr_elements;
> +
> +       /* create array for initial map value */
> +       initial_val_sz = element_sz * nr_elements;
> +       initial_val = malloc(initial_val_sz);
> +       if (!ASSERT_OK_PTR(initial_val, "malloc initial_val")) {
> +               ret = -ENOMEM;
> +
> +               goto cleanup;
> +       }
> +
> +       /* fill array with ones */
> +       for (int i = 0; i < nr_elements; ++i)
> +               initial_val[i] = 1;
> +
> +       /* set initial value */
> +       ASSERT_EQ(initial_val_sz, actual_sz, "initial value size");
> +
> +       ret = bpf_map__set_initial_value(map, initial_val, initial_val_sz);
> +       if (!ASSERT_OK(ret, "bpf_map__set_initial_val"))
> +               goto cleanup;


it would be good to demonstrate that it's still possible to use BPF
skeleton to initialize everything, you'll just need to reassign
pointer:

skel->data_my_array = bpf_map__initial_value(skel->maps.data_my_array, &new_sz);
skel->data_my_array.my_var[i] = 123;

can you please add this case as well?

> +
> +       ret = test_global_map_resize__load(skel);
> +       if (!ASSERT_OK(ret, "test_global_map_resize__load"))
> +               goto cleanup;
> +
> +       ret = test_global_map_resize__attach(skel);
> +       if (!ASSERT_OK(ret, "test_global_map_resize__attach"))
> +               goto cleanup;
> +
> +       /* run the bpf program which will sum the contents of the array */
> +       run_program();
> +
> +       if (!ASSERT_EQ(skel->bss->sum, nr_elements, "sum"))
> +               goto cleanup;
> +
> +cleanup:
> +       if (initial_val)
> +               free(initial_val);
> +
> +       return ret;
> +}
> +
> +static void global_map_resize_aligned_subtest(void)
> +{
> +       struct test_global_map_resize *skel;
> +       const __u32 element_sz = (__u32)sizeof(int);
> +       const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2;
> +
> +       /* preliminary check that desired_sz aligns with element_sz */
> +       if (!ASSERT_EQ(desired_sz % element_sz, 0, "alignment"))
> +               return;
> +
> +       if (setup(&skel))
> +               goto teardown;
> +
> +       if (resize_test(skel, element_sz, desired_sz))
> +               goto teardown;
> +
> +teardown:
> +       teardown(&skel);
> +}
> +
> +static void global_map_resize_roundup_subtest(void)
> +{
> +       struct test_global_map_resize *skel;
> +       const __u32 element_sz = (__u32)sizeof(int);
> +       /* set desired size a fraction of element size beyond an aligned size */
> +       const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2 + element_sz / 2;
> +
> +       /* preliminary check that desired_sz does NOT align with element_sz */
> +       if (!ASSERT_NEQ(desired_sz % element_sz, 0, "alignment"))
> +               return;
> +
> +       if (setup(&skel))
> +               goto teardown;
> +
> +       if (resize_test(skel, element_sz, desired_sz))
> +               goto teardown;
> +
> +teardown:
> +       teardown(&skel);
> +}
> +
> +static void global_map_resize_invalid_subtest(void)
> +{
> +       int err;
> +       struct test_global_map_resize *skel;
> +       struct bpf_map *map;
> +       const __u32 desired_sz = 8192;
> +
> +       if (setup(&skel))
> +               goto teardown;
> +
> +       /* attempt to resize a global datasec map which is an array
> +        * BUT is with a var in same datasec
> +        */
> +       map = skel->maps.data_my_array_and_var;
> +       err = bpf_map__set_value_size(map, desired_sz);
> +       if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> +               goto teardown;
> +
> +       /* attempt to resize a global datasec map which is NOT an array */
> +       map = skel->maps.data_my_non_array;
> +       err = bpf_map__set_value_size(map, desired_sz);
> +       if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> +               goto teardown;
> +
> +teardown:
> +       teardown(&skel);
> +}
> +
> +void test_global_map_resize(void)
> +{
> +       if (test__start_subtest("global_map_resize_aligned"))
> +               global_map_resize_aligned_subtest();
> +
> +       if (test__start_subtest("global_map_resize_roundup"))
> +               global_map_resize_roundup_subtest();
> +
> +       if (test__start_subtest("global_map_resize_invalid"))
> +               global_map_resize_invalid_subtest();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> new file mode 100644
> index 000000000000..cffbba1b6020
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +const volatile pid_t pid;
> +const volatile size_t n;
> +
> +int my_array[1] SEC(".data.my_array");
> +
> +int my_array_with_neighbor[1] SEC(".data.my_array_and_var");
> +int my_var_with_neighbor SEC(".data.my_array_and_var");
> +
> +int my_non_array SEC(".data.my_non_array");
> +
> +int sum = 0;
> +
> +SEC("tp/syscalls/sys_enter_getpid")
> +int array_sum(void *ctx)
> +{
> +       if (pid != (bpf_get_current_pid_tgid() >> 32))
> +               return 0;
> +
> +       sum = 0;
> +
> +       for (size_t i = 0; i < n; ++i)
> +               sum += my_array[i];
> +
> +       return 0;
> +}
> --
> 2.40.0
>