[bpf-next] libbpf: auto-attach skeletons struct_ops

Commit Message

Mykyta Yatsenko [e-mail] June 5, 2024, 5:51 p.m. UTC

From: Mykyta Yatsenko <[e-mail]>

Similarly to `bpf_program`, support `bpf_map` automatic attachment in
`bpf_object__attach_skeleton`. Currently only struct_ops maps could be
attached.

Bpftool
Code-generate links in skeleton struct for struct_ops maps.
Similarly to `bpf_program_skeleton`, set links in `bpf_map_skeleton`.

Libbpf
Extending `bpf_map` with new `autoattach` field to support enabling or
disabling autoattach functionality, introducing getter/setter for this
field.
Extending `bpf_object__(attach|detach)_skeleton` with
attaching/detaching struct_ops maps.

Signed-off-by: Mykyta Yatsenko <[e-mail]>
---
 tools/bpf/bpftool/gen.c  | 36 +++++++++++++++++++---
 tools/lib/bpf/libbpf.c   | 65 ++++++++++++++++++++++++++++++++++++++--
 tools/lib/bpf/libbpf.h   | 20 +++++++++++++
 tools/lib/bpf/libbpf.map |  2 ++
 4 files changed, 116 insertions(+), 7 deletions(-)

Comments

Andrii Nakryiko June 6, 2024, 5:09 p.m. UTC | #1

On Wed, Jun 5, 2024 at 10:51 AM Mykyta Yatsenko
<[e-mail]> wrote:
>
> From: Mykyta Yatsenko <[e-mail]>
>
> Similarly to `bpf_program`, support `bpf_map` automatic attachment in
> `bpf_object__attach_skeleton`. Currently only struct_ops maps could be
> attached.
>
> Bpftool
> Code-generate links in skeleton struct for struct_ops maps.
> Similarly to `bpf_program_skeleton`, set links in `bpf_map_skeleton`.
>
> Libbpf
> Extending `bpf_map` with new `autoattach` field to support enabling or
> disabling autoattach functionality, introducing getter/setter for this
> field.
> Extending `bpf_object__(attach|detach)_skeleton` with
> attaching/detaching struct_ops maps.
>
> Signed-off-by: Mykyta Yatsenko <[e-mail]>
> ---
>  tools/bpf/bpftool/gen.c  | 36 +++++++++++++++++++---
>  tools/lib/bpf/libbpf.c   | 65 ++++++++++++++++++++++++++++++++++++++--
>  tools/lib/bpf/libbpf.h   | 20 +++++++++++++
>  tools/lib/bpf/libbpf.map |  2 ++
>  4 files changed, 116 insertions(+), 7 deletions(-)
>

This looks great and finished (see minor nit below which I fixed up
while applying). I know you had a selftest ready, why didn't you
submit it? Please follow up with a selftest so we have this
functionality executed in our selftests. Thanks!

[...]

> +               /* only struct_ops maps can be attached */
> +               if (!bpf_map__is_struct_ops(map))
> +                       continue;
> +               *link = bpf_map__attach_struct_ops(map);
> +
> +               if (!*link) {
> +                       const int errcode = errno;
> +
> +                       pr_warn("struct_ops %s: failed to auto-attach: %d\n", bpf_map__name(map),
> +                               errcode);
> +                       return libbpf_err(-errcode);

there is err variable already in this function, I switched to using it here

> +               }
> +       }
> +
>         return 0;
>  }
>

[...]

[e-mail] June 6, 2024, 5:10 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[e-mail]>:

On Wed,  5 Jun 2024 18:51:35 +0100 you wrote:
> From: Mykyta Yatsenko <[e-mail]>
> 
> Similarly to `bpf_program`, support `bpf_map` automatic attachment in
> `bpf_object__attach_skeleton`. Currently only struct_ops maps could be
> attached.
> 
> Bpftool
> Code-generate links in skeleton struct for struct_ops maps.
> Similarly to `bpf_program_skeleton`, set links in `bpf_map_skeleton`.
> 
> [...]

Here is the summary with links:
  - [bpf-next] libbpf: auto-attach skeletons struct_ops
    https://git.kernel.org/bpf/bpf-next/c/08ac454e258e

You are awesome, thank you!

Patch

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index b3979ddc0189..a8d4db0d8044 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -848,7 +848,7 @@  static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
 }
 
 static void
-codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped)
+codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool populate_links)
 {
 	struct bpf_map *map;
 	char ident[256];
@@ -888,6 +888,14 @@  codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped)
 			printf("\ts->maps[%zu].mmaped = (void **)&obj->%s;\n",
 				i, ident);
 		}
+
+		if (populate_links && bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS) {
+			codegen("\
+				\n\
+					s->maps[%zu].link = &obj->links.%s;\n\
+				",
+				i, ident);
+		}
 		i++;
 	}
 }
@@ -1141,7 +1149,7 @@  static void gen_st_ops_shadow_init(struct btf *btf, struct bpf_object *obj)
 static int do_skeleton(int argc, char **argv)
 {
 	char header_guard[MAX_OBJ_NAME_LEN + sizeof("__SKEL_H__")];
-	size_t map_cnt = 0, prog_cnt = 0, file_sz, mmap_sz;
+	size_t map_cnt = 0, prog_cnt = 0, attach_map_cnt = 0, file_sz, mmap_sz;
 	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
 	char obj_name[MAX_OBJ_NAME_LEN] = "", *obj_data;
 	struct bpf_object *obj = NULL;
@@ -1225,6 +1233,10 @@  static int do_skeleton(int argc, char **argv)
 			      bpf_map__name(map));
 			continue;
 		}
+
+		if (bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS)
+			attach_map_cnt++;
+
 		map_cnt++;
 	}
 	bpf_object__for_each_program(prog, obj) {
@@ -1297,6 +1309,9 @@  static int do_skeleton(int argc, char **argv)
 				       bpf_program__name(prog));
 		}
 		printf("\t} progs;\n");
+	}
+
+	if (prog_cnt + attach_map_cnt) {
 		printf("\tstruct {\n");
 		bpf_object__for_each_program(prog, obj) {
 			if (use_loader)
@@ -1306,6 +1321,19 @@  static int do_skeleton(int argc, char **argv)
 				printf("\t\tstruct bpf_link *%s;\n",
 				       bpf_program__name(prog));
 		}
+
+		bpf_object__for_each_map(map, obj) {
+			if (!get_map_ident(map, ident, sizeof(ident)))
+				continue;
+			if (bpf_map__type(map) != BPF_MAP_TYPE_STRUCT_OPS)
+				continue;
+
+			if (use_loader)
+				printf("t\tint %s_fd;\n", ident);
+			else
+				printf("\t\tstruct bpf_link *%s;\n", ident);
+		}
+
 		printf("\t} links;\n");
 	}
 
@@ -1448,7 +1476,7 @@  static int do_skeleton(int argc, char **argv)
 		obj_name
 	);
 
-	codegen_maps_skeleton(obj, map_cnt, true /*mmaped*/);
+	codegen_maps_skeleton(obj, map_cnt, true /*mmaped*/, true /*links*/);
 	codegen_progs_skeleton(obj, prog_cnt, true /*populate_links*/);
 
 	codegen("\
@@ -1786,7 +1814,7 @@  static int do_subskeleton(int argc, char **argv)
 		}
 	}
 
-	codegen_maps_skeleton(obj, map_cnt, false /*mmaped*/);
+	codegen_maps_skeleton(obj, map_cnt, false /*mmaped*/, false /*links*/);
 	codegen_progs_skeleton(obj, prog_cnt, false /*links*/);
 
 	codegen("\
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d1627a2ca30b..ba9cb939c654 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -573,6 +573,7 @@  struct bpf_map {
 	bool reused;
 	bool autocreate;
 	__u64 map_extra;
+	bool autoattach;
 };
 
 enum extern_type {
@@ -1400,6 +1401,7 @@  static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
 		map->def.value_size = type->size;
 		map->def.max_entries = 1;
 		map->def.map_flags = strcmp(sec_name, STRUCT_OPS_LINK_SEC) == 0 ? BPF_F_LINK : 0;
+		map->autoattach = true;
 
 		map->st_ops = calloc(1, sizeof(*map->st_ops));
 		if (!map->st_ops)
@@ -4819,6 +4821,20 @@  int bpf_map__set_autocreate(struct bpf_map *map, bool autocreate)
 	return 0;
 }
 
+int bpf_map__set_autoattach(struct bpf_map *map, bool autoattach)
+{
+	if (!bpf_map__is_struct_ops(map))
+		return libbpf_err(-EINVAL);
+
+	map->autoattach = autoattach;
+	return 0;
+}
+
+bool bpf_map__autoattach(const struct bpf_map *map)
+{
+	return map->autoattach;
+}
+
 int bpf_map__reuse_fd(struct bpf_map *map, int fd)
 {
 	struct bpf_map_info info;
@@ -12900,8 +12916,10 @@  struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
 	__u32 zero = 0;
 	int err, fd;
 
-	if (!bpf_map__is_struct_ops(map))
+	if (!bpf_map__is_struct_ops(map)) {
+		pr_warn("map '%s': can't attach non-struct_ops map\n", map->name);
 		return libbpf_err_ptr(-EINVAL);
+	}
 
 	if (map->fd < 0) {
 		pr_warn("map '%s': can't attach BPF map without FD (was it created?)\n", map->name);
@@ -13945,6 +13963,36 @@  int bpf_object__attach_skeleton(struct bpf_object_skeleton *s)
 		 */
 	}
 
+	/* Skeleton is created with earlier version of bpftool
+	 * which does not support auto-attachment
+	 */
+	if (s->map_skel_sz < sizeof(struct bpf_map_skeleton))
+		return 0;
+
+	for (i = 0; i < s->map_cnt; i++) {
+		struct bpf_map *map = *s->maps[i].map;
+		struct bpf_link **link = s->maps[i].link;
+
+		if (!map->autocreate || !map->autoattach)
+			continue;
+
+		if (*link)
+			continue;
+
+		/* only struct_ops maps can be attached */
+		if (!bpf_map__is_struct_ops(map))
+			continue;
+		*link = bpf_map__attach_struct_ops(map);
+
+		if (!*link) {
+			const int errcode = errno;
+
+			pr_warn("struct_ops %s: failed to auto-attach: %d\n", bpf_map__name(map),
+				errcode);
+			return libbpf_err(-errcode);
+		}
+	}
+
 	return 0;
 }
 
@@ -13958,6 +14006,18 @@  void bpf_object__detach_skeleton(struct bpf_object_skeleton *s)
 		bpf_link__destroy(*link);
 		*link = NULL;
 	}
+
+	if (s->map_skel_sz < sizeof(struct bpf_map_skeleton))
+		return;
+
+	for (i = 0; i < s->map_cnt; i++) {
+		struct bpf_link **link = s->maps[i].link;
+
+		if (link) {
+			bpf_link__destroy(*link);
+			*link = NULL;
+		}
+	}
 }
 
 void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
@@ -13965,8 +14025,7 @@  void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
 	if (!s)
 		return;
 
-	if (s->progs)
-		bpf_object__detach_skeleton(s);
+	bpf_object__detach_skeleton(s);
 	if (s->obj)
 		bpf_object__close(*s->obj);
 	free(s->maps);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 26e4e35528c5..641d6ec876e2 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -978,6 +978,25 @@  bpf_object__prev_map(const struct bpf_object *obj, const struct bpf_map *map);
 LIBBPF_API int bpf_map__set_autocreate(struct bpf_map *map, bool autocreate);
 LIBBPF_API bool bpf_map__autocreate(const struct bpf_map *map);
 
+/**
+ * @brief **bpf_map__set_autoattach()** sets whether libbpf has to auto-attach
+ * map during BPF skeleton attach phase.
+ * @param map the BPF map instance
+ * @param autoattach whether to attach map during BPF skeleton attach phase
+ * @return 0 on success; negative error code, otherwise
+ *
+ */
+LIBBPF_API int bpf_map__set_autoattach(struct bpf_map *map, bool autoattach);
+
+/**
+ * @brief **bpf_map__autoattach()** returns whether BPF map is configured to
+ * auto-attach during BPF skeleton attach phase.
+ * @param map the BPF map instance
+ * @return true if map is set to auto-attach during skeleton attach phase; false otherwise
+ *
+ */
+LIBBPF_API bool bpf_map__autoattach(const struct bpf_map *map);
+
 /**
  * @brief **bpf_map__fd()** gets the file descriptor of the passed
  * BPF map
@@ -1672,6 +1691,7 @@  struct bpf_map_skeleton {
 	const char *name;
 	struct bpf_map **map;
 	void **mmaped;
+	struct bpf_link **link;
 };
 
 struct bpf_prog_skeleton {
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index c1ce8aa3520b..40595233dc7f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -419,6 +419,8 @@  LIBBPF_1.4.0 {
 
 LIBBPF_1.5.0 {
 	global:
+		bpf_map__autoattach;
+		bpf_map__set_autoattach;
 		bpf_program__attach_sockmap;
 		ring__consume_n;
 		ring_buffer__consume_n;