On Thu, Feb 8, 2024 at 8:07 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> From: Alexei Starovoitov <ast@xxxxxxxxxx>
>
> LLVM automatically places __arena variables into ".arena.1" ELF section.
> When libbpf sees such section it creates internal 'struct bpf_map' LIBBPF_MAP_ARENA
> that is connected to actual BPF_MAP_TYPE_ARENA 'struct bpf_map'.
> They share the same kernel's side bpf map and single map_fd.
> Both are emitted into skeleton. Real arena with the name given by bpf program
> in SEC(".maps") and another with "__arena_internal" name.
> All global variables from ".arena.1" section are accessible from user space
> via skel->arena->name_of_var.
This "real arena" vs "__arena_internal" seems like an unnecessary
complication. When processing .arena.1 ELF section, we search for
explicit BPF_MAP_TYPE_ARENA map, right? Great, at that point, we can
use that map and it's map->mmapable pointer (we mmap() anonymous
memory to hold initial values of global variables). We copy init
values into map->mmapable on actual arena struct bpf_map. Then during
map creation (during load) we do a new mmap(map_fd), taking into
account map_extra. Then memcpy() from the original anonymous mmap into
this arena-linked mmap. Then discard the original mmap.
This way we don't have fake maps anymore, we initialize actual map (we
might need to just remember smaller init mmap_sz, which doesn't seem
like a big complication). WDYT?
BTW, I think bpf_object__load_skeleton() can re-assign skeleton's
arena data pointer, so user accessing skel->arena->var before and
after skeleton load will be getting correct pointer.
>
> For bss/data/rodata the skeleton/libbpf perform the following sequence:
> 1. addr = mmap(MAP_ANONYMOUS)
> 2. user space optionally modifies global vars
> 3. map_fd = bpf_create_map()
> 4. bpf_update_map_elem(map_fd, addr) // to store values into the kernel
> 5. mmap(addr, MAP_FIXED, map_fd)
> after step 5 user spaces see the values it wrote at step 2 at the same addresses
>
> arena doesn't support update_map_elem. Hence skeleton/libbpf do:
> 1. addr = mmap(MAP_ANONYMOUS)
> 2. user space optionally modifies global vars
> 3. map_fd = bpf_create_map(MAP_TYPE_ARENA)
> 4. real_addr = mmap(map->map_extra, MAP_SHARED | MAP_FIXED, map_fd)
> 5. memcpy(real_addr, addr) // this will fault-in and allocate pages
> 6. munmap(addr)
>
> At the end look and feel of global data vs __arena global data is the same from bpf prog pov.
>
> Another complication is:
> struct {
> __uint(type, BPF_MAP_TYPE_ARENA);
> } arena SEC(".maps");
>
> int __arena foo;
> int bar;
>
> ptr1 = &foo; // relocation against ".arena.1" section
> ptr2 = &arena; // relocation against ".maps" section
> ptr3 = &bar; // relocation against ".bss" section
>
> Fo the kernel ptr1 and ptr2 has point to the same arena's map_fd
> while ptr3 points to a different global array's map_fd.
> For the verifier:
> ptr1->type == unknown_scalar
> ptr2->type == const_ptr_to_map
> ptr3->type == ptr_to_map_value
>
> after the verifier and for JIT all 3 ptr-s are normal ld_imm64 insns.
>
> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> ---
> tools/bpf/bpftool/gen.c | 13 ++++-
> tools/lib/bpf/libbpf.c | 102 +++++++++++++++++++++++++++++++++++-----
> 2 files changed, 101 insertions(+), 14 deletions(-)
>
[...]
> @@ -1718,10 +1722,34 @@ static int
> bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
> const char *real_name, int sec_idx, void *data, size_t data_sz)
> {
> + const long page_sz = sysconf(_SC_PAGE_SIZE);
> + struct bpf_map *map, *arena = NULL;
> struct bpf_map_def *def;
> - struct bpf_map *map;
> size_t mmap_sz;
> - int err;
> + int err, i;
> +
> + if (type == LIBBPF_MAP_ARENA) {
> + for (i = 0; i < obj->nr_maps; i++) {
> + map = &obj->maps[i];
> + if (map->def.type != BPF_MAP_TYPE_ARENA)
> + continue;
> + arena = map;
> + real_name = "__arena_internal";
> + mmap_sz = bpf_map_mmap_sz(map);
> + if (roundup(data_sz, page_sz) > mmap_sz) {
> + pr_warn("Declared arena map size %zd is too small to hold"
> + "global __arena variables of size %zd\n",
> + mmap_sz, data_sz);
> + return -E2BIG;
> + }
> + break;
> + }
> + if (!arena) {
> + pr_warn("To use global __arena variables the arena map should"
> + "be declared explicitly in SEC(\".maps\")\n");
> + return -ENOENT;
> + }
so basically here we found arena, let's arena->mmapable =
mmap(MAP_ANONYMOUS) here, memcpy(arena->mmapable, data, data_sz) and
exit early, not doing bpf_object__add_map()?
> + }
>
> map = bpf_object__add_map(obj);
> if (IS_ERR(map))
> @@ -1732,6 +1760,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
> map->sec_offset = 0;
> map->real_name = strdup(real_name);
> map->name = internal_map_name(obj, real_name);
> + map->arena = arena;
> if (!map->real_name || !map->name) {
> zfree(&map->real_name);
> zfree(&map->name);
[...]
> @@ -13437,6 +13508,11 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
> continue;
> }
>
> + if (map->arena) {
> + *mmaped = map->arena->mmaped;
> + continue;
> + }
> +
yep, I was going to suggest that we can fix up this pointer in
load_skeleton, nice
> if (map->def.map_flags & BPF_F_RDONLY_PROG)
> prot = PROT_READ;
> else
> --
> 2.34.1
>
