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

[Stanislav Fomichev <sdf@google.com>]

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>