[PATCH bpf-next v2 0/2] Support static initialization of BPF_MAP_TYPE_PROG_ARRAY

[PATCH bpf-next v2 0/2] Support static initialization of BPF_MAP_TYPE_PROG_ARRAY

[Hengqi Chen]

Make libbpf support static initialization of BPF_MAP_TYPE_PROG_ARRAY
with a syntax similar to map-in-map initialization:

    SEC("socket")
    int tailcall_1(void *ctx)
    {
        return 0;
    }

    struct {
        __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
        __uint(max_entries, 2);
        __uint(key_size, sizeof(__u32));
        __array(values, int (void *));
    } prog_array_init SEC(".maps") = {
        .values = {
            [1] = (void *)&tailcall_1,
        },
    };

v1->v2:
  - Add stricter checks on relos collect, some renamings (Andrii)
  - Update selftest to check tailcall result (Andrii)

Hengqi Chen (2):
  libbpf: Support static initialization of BPF_MAP_TYPE_PROG_ARRAY
  selftests/bpf: Test BPF_MAP_TYPE_PROG_ARRAY static initialization

 tools/lib/bpf/libbpf.c                        | 139 ++++++++++++++----
 .../bpf/prog_tests/prog_array_init.c          |  32 ++++
 .../bpf/progs/test_prog_array_init.c          |  39 +++++
 3 files changed, 182 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/prog_array_init.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_prog_array_init.c

--
2.30.2

[PATCH bpf-next v2 1/2] libbpf: Support static initialization of BPF_MAP_TYPE_PROG_ARRAY

[Hengqi Chen]

Support static initialization of BPF_MAP_TYPE_PROG_ARRAY with a
syntax similar to map-in-map initialization ([0]):

    SEC("socket")
    int tailcall_1(void *ctx)
    {
        return 0;
    }

    struct {
        __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
        __uint(max_entries, 2);
        __uint(key_size, sizeof(__u32));
        __array(values, int (void *));
    } prog_array_init SEC(".maps") = {
        .values = {
            [1] = (void *)&tailcall_1,
        },
    };

Here's the relevant part of libbpf debug log showing what's
going on with prog-array initialization:

libbpf: sec '.relsocket': collecting relocation for section(3) 'socket'
libbpf: sec '.relsocket': relo #0: insn #2 against 'prog_array_init'
libbpf: prog 'entry': found map 0 (prog_array_init, sec 4, off 0) for insn #0
libbpf: .maps relo #0: for 3 value 0 rel->r_offset 32 name 53 ('tailcall_1')
libbpf: .maps relo #0: map 'prog_array_init' slot [1] points to prog 'tailcall_1'
libbpf: map 'prog_array_init': created successfully, fd=5
libbpf: map 'prog_array_init': slot [1] set to prog 'tailcall_1' fd=6

  [0] Closes: https://github.com/libbpf/libbpf/issues/354

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 tools/lib/bpf/libbpf.c | 139 ++++++++++++++++++++++++++++++++---------
 1 file changed, 111 insertions(+), 28 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 672671879b21..c7ccc04eabed 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2277,6 +2277,9 @@ int parse_btf_map_def(const char *map_name, struct btf *btf,
 			map_def->parts |= MAP_DEF_VALUE_SIZE | MAP_DEF_VALUE_TYPE;
 		}
 		else if (strcmp(name, "values") == 0) {
+			bool is_map_in_map = bpf_map_type__is_map_in_map(map_def->map_type);
+			bool is_prog_array = map_def->map_type == BPF_MAP_TYPE_PROG_ARRAY;
+			const char *desc = is_map_in_map ? "map-in-map inner" : "prog-array value";
 			char inner_map_name[128];
 			int err;

@@ -2290,8 +2293,8 @@ int parse_btf_map_def(const char *map_name, struct btf *btf,
 					map_name, name);
 				return -EINVAL;
 			}
-			if (!bpf_map_type__is_map_in_map(map_def->map_type)) {
-				pr_warn("map '%s': should be map-in-map.\n",
+			if (!is_map_in_map && !is_prog_array) {
+				pr_warn("map '%s': should be map-in-map or prog-array.\n",
 					map_name);
 				return -ENOTSUP;
 			}
@@ -2303,22 +2306,30 @@ int parse_btf_map_def(const char *map_name, struct btf *btf,
 			map_def->value_size = 4;
 			t = btf__type_by_id(btf, m->type);
 			if (!t) {
-				pr_warn("map '%s': map-in-map inner type [%d] not found.\n",
-					map_name, m->type);
+				pr_warn("map '%s': %s type [%d] not found.\n",
+					map_name, desc, m->type);
 				return -EINVAL;
 			}
 			if (!btf_is_array(t) || btf_array(t)->nelems) {
-				pr_warn("map '%s': map-in-map inner spec is not a zero-sized array.\n",
-					map_name);
+				pr_warn("map '%s': %s spec is not a zero-sized array.\n",
+					map_name, desc);
 				return -EINVAL;
 			}
 			t = skip_mods_and_typedefs(btf, btf_array(t)->type, NULL);
 			if (!btf_is_ptr(t)) {
-				pr_warn("map '%s': map-in-map inner def is of unexpected kind %s.\n",
-					map_name, btf_kind_str(t));
+				pr_warn("map '%s': %s def is of unexpected kind %s.\n",
+					map_name, desc, btf_kind_str(t));
 				return -EINVAL;
 			}
 			t = skip_mods_and_typedefs(btf, t->type, NULL);
+			if (is_prog_array) {
+				if (!btf_is_func_proto(t)) {
+					pr_warn("map '%s': prog-array value def is of unexpected kind %s.\n",
+						map_name, btf_kind_str(t));
+					return -EINVAL;
+				}
+				continue;
+			}
 			if (!btf_is_struct(t)) {
 				pr_warn("map '%s': map-in-map inner def is of unexpected kind %s.\n",
 					map_name, btf_kind_str(t));
@@ -4940,7 +4951,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
 	return err;
 }

-static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)
+static int init_map_in_map_slots(struct bpf_object *obj, struct bpf_map *map)
 {
 	const struct bpf_map *targ_map;
 	unsigned int i;
@@ -4952,6 +4963,7 @@ static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)

 		targ_map = map->init_slots[i];
 		fd = bpf_map__fd(targ_map);
+
 		if (obj->gen_loader) {
 			pr_warn("// TODO map_update_elem: idx %td key %d value==map_idx %td\n",
 				map - obj->maps, i, targ_map - obj->maps);
@@ -4962,8 +4974,7 @@ static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)
 		if (err) {
 			err = -errno;
 			pr_warn("map '%s': failed to initialize slot [%d] to map '%s' fd=%d: %d\n",
-				map->name, i, targ_map->name,
-				fd, err);
+				map->name, i, targ_map->name, fd, err);
 			return err;
 		}
 		pr_debug("map '%s': slot [%d] set to map '%s' fd=%d\n",
@@ -4976,6 +4987,59 @@ static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)
 	return 0;
 }

+static int init_prog_array_slots(struct bpf_object *obj, struct bpf_map *map)
+{
+	const struct bpf_program *targ_prog;
+	unsigned int i;
+	int fd, err;
+
+	if (obj->gen_loader)
+		return -ENOTSUP;
+
+	for (i = 0; i < map->init_slots_sz; i++) {
+		if (!map->init_slots[i])
+			continue;
+
+		targ_prog = map->init_slots[i];
+		fd = bpf_program__fd(targ_prog);
+
+		err = bpf_map_update_elem(map->fd, &i, &fd, 0);
+		if (err) {
+			err = -errno;
+			pr_warn("map '%s': failed to initialize slot [%d] to prog '%s' fd=%d: %d\n",
+				map->name, i, targ_prog->name, fd, err);
+			return err;
+		}
+		pr_debug("map '%s': slot [%d] set to prog '%s' fd=%d\n",
+			 map->name, i, targ_prog->name, fd);
+	}
+
+	zfree(&map->init_slots);
+	map->init_slots_sz = 0;
+
+	return 0;
+}
+
+static int bpf_object_init_prog_arrays(struct bpf_object *obj)
+{
+	struct bpf_map *map;
+	int i, err;
+
+	for (i = 0; i < obj->nr_maps; i++) {
+		map = &obj->maps[i];
+
+		if (!map->init_slots_sz || map->def.type != BPF_MAP_TYPE_PROG_ARRAY)
+			continue;
+
+		err = init_prog_array_slots(obj, map);
+		if (err < 0) {
+			zclose(map->fd);
+			return err;
+		}
+	}
+	return 0;
+}
+
 static int
 bpf_object__create_maps(struct bpf_object *obj)
 {
@@ -5042,8 +5106,8 @@ bpf_object__create_maps(struct bpf_object *obj)
 				}
 			}

-			if (map->init_slots_sz) {
-				err = init_map_slots(obj, map);
+			if (map->init_slots_sz && map->def.type != BPF_MAP_TYPE_PROG_ARRAY) {
+				err = init_map_in_map_slots(obj, map);
 				if (err < 0) {
 					zclose(map->fd);
 					goto err_out;
@@ -6189,9 +6253,11 @@ static int bpf_object__collect_map_relos(struct bpf_object *obj,
 	int i, j, nrels, new_sz;
 	const struct btf_var_secinfo *vi = NULL;
 	const struct btf_type *sec, *var, *def;
-	struct bpf_map *map = NULL, *targ_map;
+	struct bpf_map *map = NULL, *targ_map = NULL;
+	struct bpf_program *targ_prog = NULL;
+	bool is_prog_array, is_map_in_map;
 	const struct btf_member *member;
-	const char *name, *mname;
+	const char *name, *mname, *type;
 	unsigned int moff;
 	Elf64_Sym *sym;
 	Elf64_Rel *rel;
@@ -6218,11 +6284,6 @@ static int bpf_object__collect_map_relos(struct bpf_object *obj,
 			return -LIBBPF_ERRNO__FORMAT;
 		}
 		name = elf_sym_str(obj, sym->st_name) ?: "<?>";
-		if (sym->st_shndx != obj->efile.btf_maps_shndx) {
-			pr_warn(".maps relo #%d: '%s' isn't a BTF-defined map\n",
-				i, name);
-			return -LIBBPF_ERRNO__RELOC;
-		}

 		pr_debug(".maps relo #%d: for %zd value %zd rel->r_offset %zu name %d ('%s')\n",
 			 i, (ssize_t)(rel->r_info >> 32), (size_t)sym->st_value,
@@ -6244,18 +6305,39 @@ static int bpf_object__collect_map_relos(struct bpf_object *obj,
 			return -EINVAL;
 		}

-		if (!bpf_map_type__is_map_in_map(map->def.type))
+		is_map_in_map = bpf_map_type__is_map_in_map(map->def.type);
+		is_prog_array = map->def.type == BPF_MAP_TYPE_PROG_ARRAY;
+		type = is_map_in_map ? "map" : "prog";
+		if (!is_map_in_map && !is_prog_array)
 			return -EINVAL;
+		if (is_map_in_map && sym->st_shndx != obj->efile.btf_maps_shndx) {
+			pr_warn(".maps relo #%d: '%s' isn't a BTF-defined map\n",
+				i, name);
+			return -LIBBPF_ERRNO__RELOC;
+		}
 		if (map->def.type == BPF_MAP_TYPE_HASH_OF_MAPS &&
 		    map->def.key_size != sizeof(int)) {
 			pr_warn(".maps relo #%d: hash-of-maps '%s' should have key size %zu.\n",
 				i, map->name, sizeof(int));
 			return -EINVAL;
 		}
-
-		targ_map = bpf_object__find_map_by_name(obj, name);
-		if (!targ_map)
-			return -ESRCH;
+		if (is_map_in_map) {
+			targ_map = bpf_object__find_map_by_name(obj, name);
+			if (!targ_map)
+				return -ESRCH;
+		}
+		if (is_prog_array) {
+			targ_prog = bpf_object__find_program_by_name(obj, name);
+			if (!targ_prog)
+				return -ESRCH;
+			if (targ_prog->sec_idx != sym->st_shndx ||
+			    targ_prog->sec_insn_off * 8 != sym->st_value ||
+			    prog_is_subprog(obj, targ_prog)) {
+				pr_warn(".maps relo #%d: '%s' isn't an entry-point BPF program\n",
+					i, name);
+				return -LIBBPF_ERRNO__RELOC;
+			}
+		}

 		var = btf__type_by_id(obj->btf, vi->type);
 		def = skip_mods_and_typedefs(obj->btf, var->type, NULL);
@@ -6287,10 +6369,10 @@ static int bpf_object__collect_map_relos(struct bpf_object *obj,
 			       (new_sz - map->init_slots_sz) * host_ptr_sz);
 			map->init_slots_sz = new_sz;
 		}
-		map->init_slots[moff] = targ_map;
+		map->init_slots[moff] = is_map_in_map ? (void *)targ_map : (void *)targ_prog;

-		pr_debug(".maps relo #%d: map '%s' slot [%d] points to map '%s'\n",
-			 i, map->name, moff, name);
+		pr_debug(".maps relo #%d: map '%s' slot [%d] points to %s '%s'\n",
+			 i, map->name, moff, type, name);
 	}

 	return 0;
@@ -7304,6 +7386,7 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
 	err = err ? : bpf_object__create_maps(obj);
 	err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : attr->target_btf_path);
 	err = err ? : bpf_object__load_progs(obj, attr->log_level);
+	err = err ? : bpf_object_init_prog_arrays(obj);

 	if (obj->gen_loader) {
 		/* reset FDs */
--
2.30.2

Re: [PATCH bpf-next v2 1/2] libbpf: Support static initialization of BPF_MAP_TYPE_PROG_ARRAY

[Andrii Nakryiko]

On Sun, Nov 28, 2021 at 6:17 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Support static initialization of BPF_MAP_TYPE_PROG_ARRAY with a
> syntax similar to map-in-map initialization ([0]):
>
>     SEC("socket")
>     int tailcall_1(void *ctx)
>     {
>         return 0;
>     }
>
>     struct {
>         __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
>         __uint(max_entries, 2);
>         __uint(key_size, sizeof(__u32));
>         __array(values, int (void *));
>     } prog_array_init SEC(".maps") = {
>         .values = {
>             [1] = (void *)&tailcall_1,
>         },
>     };
>
> Here's the relevant part of libbpf debug log showing what's
> going on with prog-array initialization:
>
> libbpf: sec '.relsocket': collecting relocation for section(3) 'socket'
> libbpf: sec '.relsocket': relo #0: insn #2 against 'prog_array_init'
> libbpf: prog 'entry': found map 0 (prog_array_init, sec 4, off 0) for insn #0
> libbpf: .maps relo #0: for 3 value 0 rel->r_offset 32 name 53 ('tailcall_1')
> libbpf: .maps relo #0: map 'prog_array_init' slot [1] points to prog 'tailcall_1'
> libbpf: map 'prog_array_init': created successfully, fd=5
> libbpf: map 'prog_array_init': slot [1] set to prog 'tailcall_1' fd=6
>
>   [0] Closes: https://github.com/libbpf/libbpf/issues/354
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 139 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 111 insertions(+), 28 deletions(-)
>

[...]

> -               targ_map = bpf_object__find_map_by_name(obj, name);
> -               if (!targ_map)
> -                       return -ESRCH;
> +               if (is_map_in_map) {
> +                       targ_map = bpf_object__find_map_by_name(obj, name);
> +                       if (!targ_map)
> +                               return -ESRCH;
> +               }
> +               if (is_prog_array) {
> +                       targ_prog = bpf_object__find_program_by_name(obj, name);
> +                       if (!targ_prog)
> +                               return -ESRCH;
> +                       if (targ_prog->sec_idx != sym->st_shndx ||
> +                           targ_prog->sec_insn_off * 8 != sym->st_value ||
> +                           prog_is_subprog(obj, targ_prog)) {
> +                               pr_warn(".maps relo #%d: '%s' isn't an entry-point BPF program\n",
> +                                       i, name);
> +                               return -LIBBPF_ERRNO__RELOC;
> +                       }
> +               }

I've slightly adjusted this part to have an exhaustive if
(is_map_in_map) else if (is_prog_array) else { return -EINVAL; }. I've
also added an error message when someone is trying to put non-BPF
program into BPF_MAP_TYPE_PROG_ARRAY, like this:

  libbpf: .maps relo #0: 'value' isn't a valid program reference

Awesome work, applied to bpf-next, thanks.

>
>                 var = btf__type_by_id(obj->btf, vi->type);
>                 def = skip_mods_and_typedefs(obj->btf, var->type, NULL);
> @@ -6287,10 +6369,10 @@ static int bpf_object__collect_map_relos(struct bpf_object *obj,
>                                (new_sz - map->init_slots_sz) * host_ptr_sz);
>                         map->init_slots_sz = new_sz;
>                 }
> -               map->init_slots[moff] = targ_map;
> +               map->init_slots[moff] = is_map_in_map ? (void *)targ_map : (void *)targ_prog;
>
> -               pr_debug(".maps relo #%d: map '%s' slot [%d] points to map '%s'\n",
> -                        i, map->name, moff, name);
> +               pr_debug(".maps relo #%d: map '%s' slot [%d] points to %s '%s'\n",
> +                        i, map->name, moff, type, name);
>         }
>
>         return 0;
> @@ -7304,6 +7386,7 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
>         err = err ? : bpf_object__create_maps(obj);
>         err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : attr->target_btf_path);
>         err = err ? : bpf_object__load_progs(obj, attr->log_level);
> +       err = err ? : bpf_object_init_prog_arrays(obj);
>
>         if (obj->gen_loader) {
>                 /* reset FDs */
> --
> 2.30.2

[PATCH bpf-next v2 2/2] selftests/bpf: Test BPF_MAP_TYPE_PROG_ARRAY static initialization

[Hengqi Chen]

Add testcase for BPF_MAP_TYPE_PROG_ARRAY static initialization.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 .../bpf/prog_tests/prog_array_init.c          | 32 +++++++++++++++
 .../bpf/progs/test_prog_array_init.c          | 39 +++++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/prog_array_init.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_prog_array_init.c

diff --git a/tools/testing/selftests/bpf/prog_tests/prog_array_init.c b/tools/testing/selftests/bpf/prog_tests/prog_array_init.c
new file mode 100644
index 000000000000..fc4657619739
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/prog_array_init.c
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021 Hengqi Chen */
+
+#include <test_progs.h>
+#include "test_prog_array_init.skel.h"
+
+void test_prog_array_init(void)
+{
+	struct test_prog_array_init *skel;
+	int err;
+
+	skel = test_prog_array_init__open();
+	if (!ASSERT_OK_PTR(skel, "could not open BPF object"))
+		return;
+
+	skel->rodata->my_pid = getpid();
+
+	err = test_prog_array_init__load(skel);
+	if (!ASSERT_OK(err, "could not load BPF object"))
+		goto cleanup;
+
+	skel->links.entry = bpf_program__attach_raw_tracepoint(skel->progs.entry, "sys_enter");
+	if (!ASSERT_OK_PTR(skel->links.entry, "could not attach BPF program"))
+		goto cleanup;
+
+	usleep(1);
+
+	ASSERT_EQ(skel->bss->value, 42, "unexpected value");
+
+cleanup:
+	test_prog_array_init__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_prog_array_init.c b/tools/testing/selftests/bpf/progs/test_prog_array_init.c
new file mode 100644
index 000000000000..2cd138356126
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_prog_array_init.c
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021 Hengqi Chen */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+const volatile pid_t my_pid = 0;
+int value = 0;
+
+SEC("raw_tp/sys_enter")
+int tailcall_1(void *ctx)
+{
+	value = 42;
+	return 0;
+}
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+	__uint(max_entries, 2);
+	__uint(key_size, sizeof(__u32));
+	__array(values, int (void *));
+} prog_array_init SEC(".maps") = {
+	.values = {
+		[1] = (void *)&tailcall_1,
+	},
+};
+
+SEC("raw_tp/sys_enter")
+int entry(void *ctx)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != my_pid)
+		return 0;
+
+	bpf_tail_call(ctx, &prog_array_init, 1);
+	return 0;
+}
--
2.30.2