diff mbox series

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

Message ID 20230428222754.183432-2-inwardvessel@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series libbpf: capability for resizing datasec maps | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 10 maintainers not CCed: daniel@iogearbox.net yhs@fb.com kpsingh@kernel.org martin.lau@linux.dev john.fastabend@gmail.com song@kernel.org sdf@google.com jolsa@kernel.org haoluo@google.com ast@kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning CHECK: Alignment should match open parenthesis CHECK: multiple assignments should be avoided WARNING: line length of 86 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-36 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix

Commit Message

JP Kobryn April 28, 2023, 10:27 p.m. UTC
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(+)

Comments

Stanislav Fomichev April 28, 2023, 11:58 p.m. UTC | #1
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?
JP Kobryn May 1, 2023, 4:50 p.m. UTC | #2
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).
--
Stanislav Fomichev May 1, 2023, 6:23 p.m. UTC | #3
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>
Andrii Nakryiko May 1, 2023, 11:49 p.m. UTC | #4
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
>
diff mbox series

Patch

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)