libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF

PATCH In review Quentin Monnet 2 participants 14 messages Jul 29, 2021 – Jul 30, 2021

Subsystems (8)

Networking Network drivers Kselftest BPF +4 more subsystems

lore

Share & tools

lore

Quentin Monnet

author

2021-07-29 16:20 UTC #0 lore raw

v3 cover libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF

As part of the effort to move towards a v1.0 for libbpf [0], this set improves some confusing function names related to BTF loading from and to the kernel:

- btf__load() becomes btf__load_into_kernel().
- btf__get_from_id becomes btf__load_from_kernel_by_id().
- A new version btf__load_from_kernel_by_id_split() extends the former to add support for split BTF.

The old functions are marked for deprecation for the next minor version (0.6) of libbpf.

The last patch is a trivial change to bpftool to add support for dumping split BTF objects by referencing them by their id (and not only by their BTF path).

[0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis

v3:
- Use libbpf_err_ptr() in btf__load_from_kernel_by_id(), ERR_PTR() in bpftool's get_map_kv_btf().
- Move the definition of btf__load_from_kernel_by_id() closer to the btf__parse() group in btf.h (move the legacy function with it).
- Fix a bug on the return value in libbpf_find_prog_btf_id(), as a new patch.
- Move the btf__free() fixes to their own patch.
- Add "Fixes:" tags to relevant patches.
- Re-introduce deprecation (removed in v2) for the legacy functions, as a new macro LIBBPF_DEPRECATED_SINCE(major, minor, message).

v2:
- Remove deprecation marking of legacy functions (patch 4/6 from v1).
- Make btf__load_from_kernel_by_id{,_split}() return the btf struct, adjust surrounding code and call btf__free() when missing.
- Add new functions to v0.5.0 API (and not v0.6.0).

Quentin Monnet (8):
libbpf: return non-null error on failures in libbpf_find_prog_btf_id()
libbpf: rename btf__load() as btf__load_into_kernel()
libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()
tools: free BTF objects at various locations
tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()
libbpf: prepare deprecation of btf__get_from_id(), btf__load()
libbpf: add split BTF support for btf__load_from_kernel_by_id()
tools: bpftool: support dumping split BTF by id

 tools/bpf/bpftool/btf.c                      |  8 ++---
 tools/bpf/bpftool/btf_dumper.c               |  6 ++--
 tools/bpf/bpftool/map.c                      | 14 ++++-----
 tools/bpf/bpftool/prog.c                     | 29 +++++++++++------
 tools/lib/bpf/Makefile                       |  3 ++
 tools/lib/bpf/btf.c                          | 33 ++++++++++++++------
 tools/lib/bpf/btf.h                          |  7 ++++-
 tools/lib/bpf/libbpf.c                       | 11 ++++---
 tools/lib/bpf/libbpf.map                     |  3 ++
 tools/lib/bpf/libbpf_common.h                | 19 +++++++++++
 tools/perf/util/bpf-event.c                  | 11 ++++---
 tools/perf/util/bpf_counter.c                | 12 +++++--
 tools/testing/selftests/bpf/prog_tests/btf.c |  4 ++-
 13 files changed, 113 insertions(+), 47 deletions(-)

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #1 lore raw

PATCH v3 1/8 libbpf: return non-null error on failures in libbpf_find_prog_btf_id()

Variable "err" is initialised to -EINVAL so that this error code is returned when something goes wrong in libbpf_find_prog_btf_id(). However, a recent change in the function made use of the variable in such a way that it is set to 0 if retrieving linear information on the program is successful, and this 0 value remains if we error out on failures at later stages.

Let's fix this by setting err to -EINVAL later in the function.

Fixes e9fc3ce99b34 ("libbpf: Streamline error reporting for high-level APIs")

Signed-off-by Quentin Monnet <[email protected]>

---
 tools/lib/bpf/libbpf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

tools/lib/bpf/libbpf.c +3 -1

index a1ca6fb0c6d8..7b2b5d261a08 100644
@@ -8317,7 +8317,7 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd)
 	struct bpf_prog_info_linear *info_linear;
 	struct bpf_prog_info *info;
 	struct btf *btf = NULL;
-	int err = -EINVAL;
+	int err;
 
 	info_linear = bpf_program__get_prog_info_linear(attach_prog_fd, 0);
 	err = libbpf_get_error(info_linear);

@@ -8326,6 +8326,8 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd)
 			attach_prog_fd);
 		return err;
 	}
+
+	err = -EINVAL;
 	info = &info_linear->info;
 	if (!info->btf_id) {
 		pr_warn("The target program doesn't have BTF\n");

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #2 lore raw

PATCH v3 2/8 libbpf: rename btf__load() as btf__load_into_kernel()

In reply to Quentin Monnet

As part of the effort to move towards a v1.0 for libbpf, rename btf__load() function, used to "upload" BTF information into the kernel, as btf__load_into_kernel(). This new name better reflects what the function does.

References:

- https://github.com/libbpf/libbpf/issues/278
- https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis

Signed-off-by Quentin Monnet <[email protected]>

Acked-by John Fastabend <[email protected]>

---
 tools/lib/bpf/btf.c      | 3 ++-
 tools/lib/bpf/btf.h      | 1 +
 tools/lib/bpf/libbpf.c   | 2 +-
 tools/lib/bpf/libbpf.map | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

tools/lib/bpf/btf.c +2 -1

index b46760b93bb4..7e0de560490e 100644
@@ -1180,7 +1180,7 @@ int btf__finalize_data(struct bpf_object *obj, struct btf *btf)
 
 static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endian);
 
-int btf__load(struct btf *btf)
+int btf__load_into_kernel(struct btf *btf)
 {
 	__u32 log_buf_size = 0, raw_size;
 	char *log_buf = NULL;

@@ -1228,6 +1228,7 @@ int btf__load(struct btf *btf)
 	free(log_buf);
 	return libbpf_err(err);
 }
+int btf__load(struct btf *) __attribute__((alias("btf__load_into_kernel")));
 
 int btf__fd(const struct btf *btf)
 {

tools/lib/bpf/btf.h +1 -0

index 374e9f15de2e..fd8a21d936ef 100644
@@ -46,6 +46,7 @@ LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_b
 
 LIBBPF_API int btf__finalize_data(struct bpf_object *obj, struct btf *btf);
 LIBBPF_API int btf__load(struct btf *btf);
+LIBBPF_API int btf__load_into_kernel(struct btf *btf);
 LIBBPF_API __s32 btf__find_by_name(const struct btf *btf,
 				   const char *type_name);
 LIBBPF_API __s32 btf__find_by_name_kind(const struct btf *btf,

tools/lib/bpf/libbpf.c +1 -1

index 7b2b5d261a08..9a657d6d7da3 100644
@@ -2769,7 +2769,7 @@ static int bpf_object__sanitize_and_load_btf(struct bpf_object *obj)
 		 */
 		btf__set_fd(kern_btf, 0);
 	} else {
-		err = btf__load(kern_btf);
+		err = btf__load_into_kernel(kern_btf);
 	}
 	if (sanitize) {
 		if (!err) {

tools/lib/bpf/libbpf.map +1 -0

index c240d488eb5e..4d80eb8c56b0 100644
@@ -375,6 +375,7 @@ LIBBPF_0.5.0 {
 		bpf_map_lookup_and_delete_elem_flags;
 		bpf_program__attach_kprobe_opts;
 		bpf_object__gen_loader;
+		btf__load_into_kernel;
 		btf_dump__dump_type_data;
 		libbpf_set_strict_mode;
 } LIBBPF_0.4.0;

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #3 lore raw

PATCH v3 3/8 libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()

In reply to Quentin Monnet

Rename function btf__get_from_id() as btf__load_from_kernel_by_id() to better indicate what the function does. Change the new function so that, instead of requiring a pointer to the pointer to update and returning with an error code, it takes a single argument (the id of the BTF object) and returns the corresponding pointer. This is more in line with the existing constructors.

The other tools calling the (soon-to-be) deprecated btf__get_from_id() function will be updated in a future commit.

References:

- https://github.com/libbpf/libbpf/issues/278
- https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis

Signed-off-by Quentin Monnet <[email protected]>

Acked-by John Fastabend <[email protected]>

---
 tools/lib/bpf/btf.c      | 25 +++++++++++++++++--------
 tools/lib/bpf/btf.h      |  3 ++-
 tools/lib/bpf/libbpf.c   |  5 +++--
 tools/lib/bpf/libbpf.map |  1 +
 4 files changed, 23 insertions(+), 11 deletions(-)

tools/lib/bpf/btf.c +17 -8

index 7e0de560490e..948c29fee447 100644
@@ -1383,21 +1383,30 @@ struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf)
 	return btf;
 }
 
+struct btf *btf__load_from_kernel_by_id(__u32 id)
+{
+	struct btf *btf;
+	int btf_fd;
+
+	btf_fd = bpf_btf_get_fd_by_id(id);
+	if (btf_fd < 0)
+		return libbpf_err_ptr(-errno);
+
+	btf = btf_get_from_fd(btf_fd, NULL);
+	close(btf_fd);
+
+	return libbpf_ptr(btf);
+}
+
 int btf__get_from_id(__u32 id, struct btf **btf)
 {
 	struct btf *res;
-	int err, btf_fd;
+	int err;
 
 	*btf = NULL;
-	btf_fd = bpf_btf_get_fd_by_id(id);
-	if (btf_fd < 0)
-		return libbpf_err(-errno);
-
-	res = btf_get_from_fd(btf_fd, NULL);
+	res = btf__load_from_kernel_by_id(id);
 	err = libbpf_get_error(res);
 
-	close(btf_fd);
-
 	if (err)
 		return libbpf_err(err);

tools/lib/bpf/btf.h +2 -1

index fd8a21d936ef..698afde03c2e 100644
@@ -43,6 +43,8 @@ LIBBPF_API struct btf *btf__parse_elf(const char *path, struct btf_ext **btf_ext
 LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_btf);
 LIBBPF_API struct btf *btf__parse_raw(const char *path);
 LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
+LIBBPF_API struct btf *btf__load_from_kernel_by_id(__u32 id);
+LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf);
 
 LIBBPF_API int btf__finalize_data(struct bpf_object *obj, struct btf *btf);
 LIBBPF_API int btf__load(struct btf *btf);

@@ -67,7 +69,6 @@ LIBBPF_API void btf__set_fd(struct btf *btf, int fd);
 LIBBPF_API const void *btf__get_raw_data(const struct btf *btf, __u32 *size);
 LIBBPF_API const char *btf__name_by_offset(const struct btf *btf, __u32 offset);
 LIBBPF_API const char *btf__str_by_offset(const struct btf *btf, __u32 offset);
-LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf);
 LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name,
 				    __u32 expected_key_size,
 				    __u32 expected_value_size,

tools/lib/bpf/libbpf.c +3 -2

index 9a657d6d7da3..313883179919 100644
@@ -8316,7 +8316,7 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd)
 {
 	struct bpf_prog_info_linear *info_linear;
 	struct bpf_prog_info *info;
-	struct btf *btf = NULL;
+	struct btf *btf;
 	int err;
 
 	info_linear = bpf_program__get_prog_info_linear(attach_prog_fd, 0);

@@ -8333,7 +8333,8 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd)
 		pr_warn("The target program doesn't have BTF\n");
 		goto out;
 	}
-	if (btf__get_from_id(info->btf_id, &btf)) {
+	btf = btf__load_from_kernel_by_id(info->btf_id);
+	if (libbpf_get_error(btf)) {
 		pr_warn("Failed to get BTF of the program\n");
 		goto out;
 	}

tools/lib/bpf/libbpf.map +1 -0

index 4d80eb8c56b0..3a9c6939301e 100644
@@ -375,6 +375,7 @@ LIBBPF_0.5.0 {
 		bpf_map_lookup_and_delete_elem_flags;
 		bpf_program__attach_kprobe_opts;
 		bpf_object__gen_loader;
+		btf__load_from_kernel_by_id;
 		btf__load_into_kernel;
 		btf_dump__dump_type_data;
 		libbpf_set_strict_mode;

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #4 lore raw

PATCH v3 4/8 tools: free BTF objects at various locations

In reply to Quentin Monnet

Make sure to call btf__free() (and not simply free(), which does not free all pointers stored in the struct) on pointers to struct btf objects retrieved at various locations.

These were found while updating the calls to btf__get_from_id().

Fixes 999d82cbc044 ("tools/bpf: enhance test_btf file testing to test func info")

Fixes 254471e57a86 ("tools/bpf: bpftool: add support for func types")

Fixes 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")

Fixes d56354dc4909 ("perf tools: Save bpf_prog_info and BTF of new BPF programs")

Fixes 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")

Fixes fa853c4b839e ("perf stat: Enable counting events for BPF programs")

Signed-off-by Quentin Monnet <[email protected]>

---
 tools/bpf/bpftool/prog.c                     | 5 ++++-
 tools/perf/util/bpf-event.c                  | 4 ++--
 tools/perf/util/bpf_counter.c                | 3 ++-
 tools/testing/selftests/bpf/prog_tests/btf.c | 1 +
 4 files changed, 9 insertions(+), 4 deletions(-)

tools/bpf/bpftool/prog.c +4 -1

index cc48726740ad..9d709b427665 100644
@@ -781,6 +781,8 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
 		kernel_syms_destroy(&dd);
 	}
 
+	btf__free(btf);
+
 	return 0;
 }

@@ -2002,8 +2004,8 @@ static char *profile_target_name(int tgt_fd)
 	struct bpf_prog_info_linear *info_linear;
 	struct bpf_func_info *func_info;
 	const struct btf_type *t;
+	struct btf *btf = NULL;
 	char *name = NULL;
-	struct btf *btf;
 
 	info_linear = bpf_program__get_prog_info_linear(
 		tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO);

@@ -2027,6 +2029,7 @@ static char *profile_target_name(int tgt_fd)
 	}
 	name = strdup(btf__name_by_offset(btf, t->name_off));
 out:
+	btf__free(btf);
 	free(info_linear);
 	return name;
 }

tools/perf/util/bpf-event.c +2 -2

index cdecda1ddd36..17a9844e4fbf 100644
@@ -296,7 +296,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 
 out:
 	free(info_linear);
-	free(btf);
+	btf__free(btf);
 	return err ? -1 : 0;
 }

@@ -486,7 +486,7 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id)
 	perf_env__fetch_btf(env, btf_id, btf);
 
 out:
-	free(btf);
+	btf__free(btf);
 	close(fd);
 }

tools/perf/util/bpf_counter.c +2 -1

index 8150e03367bb..beca55129b0b 100644
@@ -64,8 +64,8 @@ static char *bpf_target_prog_name(int tgt_fd)
 	struct bpf_prog_info_linear *info_linear;
 	struct bpf_func_info *func_info;
 	const struct btf_type *t;
+	struct btf *btf = NULL;
 	char *name = NULL;
-	struct btf *btf;
 
 	info_linear = bpf_program__get_prog_info_linear(
 		tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO);

@@ -89,6 +89,7 @@ static char *bpf_target_prog_name(int tgt_fd)
 	}
 	name = strdup(btf__name_by_offset(btf, t->name_off));
 out:
+	btf__free(btf);
 	free(info_linear);
 	return name;
 }

tools/testing/selftests/bpf/prog_tests/btf.c +1 -0

index 857e3f26086f..68e415f4d33c 100644
@@ -4386,6 +4386,7 @@ static void do_test_file(unsigned int test_num)
 	fprintf(stderr, "OK");
 
 done:
+	btf__free(btf);
 	free(func_info);
 	bpf_object__close(obj);
 }

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #5 lore raw

PATCH v3 5/8 tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()

In reply to Quentin Monnet

Replace the calls to function btf__get_from_id(), which we plan to deprecate before the library reaches v1.0, with calls to btf__load_from_kernel_by_id() in tools/ (bpftool, perf, selftests). Update the surrounding code accordingly (instead of passing a pointer to the btf struct, get it as a return value from the function).

Signed-off-by Quentin Monnet <[email protected]>

Acked-by John Fastabend <[email protected]>

---
 tools/bpf/bpftool/btf.c                      |  8 ++-----
 tools/bpf/bpftool/btf_dumper.c               |  6 +++--
 tools/bpf/bpftool/map.c                      | 14 ++++++------
 tools/bpf/bpftool/prog.c                     | 24 +++++++++++++-------
 tools/perf/util/bpf-event.c                  |  7 +++---
 tools/perf/util/bpf_counter.c                |  9 ++++++--
 tools/testing/selftests/bpf/prog_tests/btf.c |  3 ++-
 7 files changed, 42 insertions(+), 29 deletions(-)

tools/bpf/bpftool/btf.c +2 -6

index 385d5c955cf3..9162a18e84c0 100644
@@ -580,16 +580,12 @@ static int do_dump(int argc, char **argv)
 	}
 
 	if (!btf) {
-		err = btf__get_from_id(btf_id, &btf);
+		btf = btf__load_from_kernel_by_id(btf_id);
+		err = libbpf_get_error(btf);
 		if (err) {
 			p_err("get btf by id (%u): %s", btf_id, strerror(err));
 			goto done;
 		}
-		if (!btf) {
-			err = -ENOENT;
-			p_err("can't find btf with ID (%u)", btf_id);
-			goto done;
-		}
 	}
 
 	if (dump_c) {

tools/bpf/bpftool/btf_dumper.c +4 -2

index 7ca54d046362..9c25286a5c73 100644
@@ -64,8 +64,10 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
 	}
 	info = &prog_info->info;
 
-	if (!info->btf_id || !info->nr_func_info ||
-	    btf__get_from_id(info->btf_id, &prog_btf))
+	if (!info->btf_id || !info->nr_func_info)
+		goto print;
+	prog_btf = btf__load_from_kernel_by_id(info->btf_id);
+	if (libbpf_get_error(prog_btf))
 		goto print;
 	finfo = u64_to_ptr(info->func_info);
 	func_type = btf__type_by_id(prog_btf, finfo->type_id);

tools/bpf/bpftool/map.c +7 -7

index 09ae0381205b..7e7f748bb0be 100644
@@ -807,10 +807,11 @@ static struct btf *get_map_kv_btf(const struct bpf_map_info *info)
 	} else if (info->btf_value_type_id) {
 		int err;
 
-		err = btf__get_from_id(info->btf_id, &btf);
-		if (err || !btf) {
+		btf = btf__load_from_kernel_by_id(info->btf_id);
+		err = libbpf_get_error(btf);
+		if (err) {
 			p_err("failed to get btf");
-			btf = err ? ERR_PTR(err) : ERR_PTR(-ESRCH);
+			btf = ERR_PTR(err);
 		}
 	}

@@ -1039,11 +1040,10 @@ static void print_key_value(struct bpf_map_info *info, void *key,
 			    void *value)
 {
 	json_writer_t *btf_wtr;
-	struct btf *btf = NULL;
-	int err;
+	struct btf *btf;
 
-	err = btf__get_from_id(info->btf_id, &btf);
-	if (err) {
+	btf = btf__load_from_kernel_by_id(info->btf_id);
+	if (libbpf_get_error(btf)) {
 		p_err("failed to get btf");
 		return;
 	}

tools/bpf/bpftool/prog.c +16 -8

index 9d709b427665..b1996b8f1d42 100644
@@ -249,10 +249,10 @@ static void show_prog_metadata(int fd, __u32 num_maps)
 	struct bpf_map_info map_info;
 	struct btf_var_secinfo *vsi;
 	bool printed_header = false;
-	struct btf *btf = NULL;
 	unsigned int i, vlen;
 	void *value = NULL;
 	const char *name;
+	struct btf *btf;
 	int err;
 
 	if (!num_maps)

@@ -263,8 +263,8 @@ static void show_prog_metadata(int fd, __u32 num_maps)
 	if (!value)
 		return;
 
-	err = btf__get_from_id(map_info.btf_id, &btf);
-	if (err || !btf)
+	btf = btf__load_from_kernel_by_id(map_info.btf_id);
+	if (libbpf_get_error(btf))
 		goto out_free;
 
 	t_datasec = btf__type_by_id(btf, map_info.btf_value_type_id);

@@ -646,9 +646,12 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
 		member_len = info->xlated_prog_len;
 	}
 
-	if (info->btf_id && btf__get_from_id(info->btf_id, &btf)) {
-		p_err("failed to get btf");
-		return -1;
+	if (info->btf_id) {
+		btf = btf__load_from_kernel_by_id(info->btf_id);
+		if (libbpf_get_error(btf)) {
+			p_err("failed to get btf");
+			return -1;
+		}
 	}
 
 	func_info = u64_to_ptr(info->func_info);

@@ -2014,12 +2017,17 @@ static char *profile_target_name(int tgt_fd)
 		return NULL;
 	}
 
-	if (info_linear->info.btf_id == 0 ||
-	    btf__get_from_id(info_linear->info.btf_id, &btf)) {
+	if (info_linear->info.btf_id == 0) {
 		p_err("prog FD %d doesn't have valid btf", tgt_fd);
 		goto out;
 	}
 
+	btf = btf__load_from_kernel_by_id(info_linear->info.btf_id);
+	if (libbpf_get_error(btf)) {
+		p_err("failed to load btf for prog FD %d", tgt_fd);
+		goto out;
+	}
+
 	func_info = u64_to_ptr(info_linear->info.func_info);
 	t = btf__type_by_id(btf, func_info[0].type_id);
 	if (!t) {

tools/perf/util/bpf-event.c +4 -3

index 17a9844e4fbf..996d025b8ed8 100644
@@ -223,10 +223,10 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 			free(info_linear);
 			return -1;
 		}
-		if (btf__get_from_id(info->btf_id, &btf)) {
+		btf = btf__load_from_kernel_by_id(info->btf_id);
+		if (libbpf_get_error(btf)) {
 			pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info->btf_id);
 			err = -1;
-			btf = NULL;
 			goto out;
 		}
 		perf_env__fetch_btf(env, info->btf_id, btf);

@@ -478,7 +478,8 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id)
 	if (btf_id == 0)
 		goto out;
 
-	if (btf__get_from_id(btf_id, &btf)) {
+	btf = btf__load_from_kernel_by_id(btf_id);
+	if (libbpf_get_error(btf)) {
 		pr_debug("%s: failed to get BTF of id %u, aborting\n",
 			 __func__, btf_id);
 		goto out;

tools/perf/util/bpf_counter.c +7 -2

index beca55129b0b..ba0f20853651 100644
@@ -74,12 +74,17 @@ static char *bpf_target_prog_name(int tgt_fd)
 		return NULL;
 	}
 
-	if (info_linear->info.btf_id == 0 ||
-	    btf__get_from_id(info_linear->info.btf_id, &btf)) {
+	if (info_linear->info.btf_id == 0) {
 		pr_debug("prog FD %d doesn't have valid btf\n", tgt_fd);
 		goto out;
 	}
 
+	btf = btf__load_from_kernel_by_id(info_linear->info.btf_id);
+	if (libbpf_get_error(btf)) {
+		pr_debug("failed to load btf for prog FD %d\n", tgt_fd);
+		goto out;
+	}
+
 	func_info = u64_to_ptr(info_linear->info.func_info);
 	t = btf__type_by_id(btf, func_info[0].type_id);
 	if (!t) {

tools/testing/selftests/bpf/prog_tests/btf.c +2 -1

index 68e415f4d33c..649f87382c8d 100644
@@ -4350,7 +4350,8 @@ static void do_test_file(unsigned int test_num)
 		goto done;
 	}
 
-	err = btf__get_from_id(info.btf_id, &btf);
+	btf = btf__load_from_kernel_by_id(info.btf_id);
+	err = libbpf_get_error(btf);
 	if (CHECK(err, "cannot get btf from kernel, err: %d", err))
 		goto done;

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #6 lore raw

PATCH v3 6/8 libbpf: prepare deprecation of btf__get_from_id(), btf__load()

In reply to Quentin Monnet

Introduce a macro LIBBPF_DEPRECATED_SINCE(major, minor, message) to prepare the deprecation of two API functions. This macro mark the functions as deprecated when libbpf's version reaches the values passed as an argument.

Prepare deprecation for btf__get_from_id() and btf__load(), respectively replaced by btf__load_from_kernel_by_id() and btf__load_into_kernel(), for version 0.6 of the library.

References:

- https://github.com/libbpf/libbpf/issues/278
- https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis

Side notes:

- Because of the constraints from the preprocessor, we have to write a few lines of macro magic for each version used to prepare deprecation (0.6 for now).
- Checkpatch complains about the absence of parentheses around the definition for LIBBPF_DEPRECATED_SINCE, but the compiler profusely complains if we attempt to add them.

Signed-off-by Quentin Monnet <[email protected]>

---
 tools/lib/bpf/Makefile        |  3 +++
 tools/lib/bpf/btf.h           |  2 ++
 tools/lib/bpf/libbpf_common.h | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+)

tools/lib/bpf/Makefile +3 -0

index ec14aa725bb0..095d5dc30d50 100644
@@ -8,6 +8,7 @@ LIBBPF_VERSION := $(shell \
 	grep -oE '^LIBBPF_([0-9.]+)' libbpf.map | \
 	sort -rV | head -n1 | cut -d'_' -f2)
 LIBBPF_MAJOR_VERSION := $(firstword $(subst ., ,$(LIBBPF_VERSION)))
+LIBBPF_MINOR_VERSION := $(firstword $(subst ., ,$(subst $(LIBBPF_MAJOR_VERSION)., ,$(LIBBPF_VERSION))))
 
 MAKEFLAGS += --no-print-directory

@@ -86,6 +87,8 @@ override CFLAGS += -Werror -Wall
 override CFLAGS += $(INCLUDES)
 override CFLAGS += -fvisibility=hidden
 override CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
+override CFLAGS += -DLIBBPF_MAJOR_VERSION=$(LIBBPF_MAJOR_VERSION)
+override CFLAGS += -DLIBBPF_MINOR_VERSION=$(LIBBPF_MINOR_VERSION)
 
 # flags specific for shared library
 SHLIB_FLAGS := -DSHARED -fPIC

tools/lib/bpf/btf.h +2 -0

index 698afde03c2e..a6039ca66895 100644
@@ -44,9 +44,11 @@ LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_b
 LIBBPF_API struct btf *btf__parse_raw(const char *path);
 LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
 LIBBPF_API struct btf *btf__load_from_kernel_by_id(__u32 id);
+LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__load_from_kernel_by_id instead")
 LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf);
 
 LIBBPF_API int btf__finalize_data(struct bpf_object *obj, struct btf *btf);
+LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__load_into_kernel instead")
 LIBBPF_API int btf__load(struct btf *btf);
 LIBBPF_API int btf__load_into_kernel(struct btf *btf);
 LIBBPF_API __s32 btf__find_by_name(const struct btf *btf,

tools/lib/bpf/libbpf_common.h +19 -0

index 947d8bd8a7bb..7218d6156ed7 100644
@@ -17,6 +17,25 @@
 
 #define LIBBPF_DEPRECATED(msg) __attribute__((deprecated(msg)))
 
+#define __LIBBPF_GET_VERSION(major, minor) (((major) << 8) + (minor))
+#define __LIBBPF_CURRENT_VERSION					    \
+	__LIBBPF_GET_VERSION(LIBBPF_MAJOR_VERSION, LIBBPF_MINOR_VERSION)
+#define __LIBBPF_CURRENT_VERSION_GEQ(major, minor)			    \
+	(__LIBBPF_CURRENT_VERSION >= __LIBBPF_GET_VERSION(major, minor))
+/* Add checks for other versions below when planning deprecation of API symbols
+ * with the LIBBPF_DEPRECATED_SINCE macro.
+ */
+#if __LIBBPF_CURRENT_VERSION_GEQ(0, 6)
+#define __LIBBPF_MARK_DEPRECATED_0_6(X) X
+#else
+#define __LIBBPF_MARK_DEPRECATED_0_6(X)
+#endif
+
+/* Mark a symbol as deprecated when libbpf version is >= {major}.{minor} */
+#define LIBBPF_DEPRECATED_SINCE(major, minor, msg)			    \
+	__LIBBPF_MARK_DEPRECATED_ ## major ## _ ## minor		    \
+		(LIBBPF_DEPRECATED("v" # major "." # minor "+, " msg))
+
 /* Helper macro to declare and initialize libbpf options struct
  *
  * This dance with uninitialized declaration, followed by memset to zero,

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #7 lore raw

PATCH v3 7/8 libbpf: add split BTF support for btf__load_from_kernel_by_id()

In reply to Quentin Monnet

Add a new API function btf__load_from_kernel_by_id_split(), which takes a pointer to a base BTF object in order to support split BTF objects when retrieving BTF information from the kernel.

Reference: https://github.com/libbpf/libbpf/issues/314

Signed-off-by Quentin Monnet <[email protected]>

Acked-by John Fastabend <[email protected]>

---
 tools/lib/bpf/btf.c      | 9 +++++++--
 tools/lib/bpf/btf.h      | 1 +
 tools/lib/bpf/libbpf.map | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

tools/lib/bpf/btf.c +7 -2

index 948c29fee447..cafa4f6bd9b1 100644
@@ -1383,7 +1383,7 @@ struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf)
 	return btf;
 }
 
-struct btf *btf__load_from_kernel_by_id(__u32 id)
+struct btf *btf__load_from_kernel_by_id_split(__u32 id, struct btf *base_btf)
 {
 	struct btf *btf;
 	int btf_fd;

@@ -1392,12 +1392,17 @@ struct btf *btf__load_from_kernel_by_id(__u32 id)
 	if (btf_fd < 0)
 		return libbpf_err_ptr(-errno);
 
-	btf = btf_get_from_fd(btf_fd, NULL);
+	btf = btf_get_from_fd(btf_fd, base_btf);
 	close(btf_fd);
 
 	return libbpf_ptr(btf);
 }
 
+struct btf *btf__load_from_kernel_by_id(__u32 id)
+{
+	return btf__load_from_kernel_by_id_split(id, NULL);
+}
+
 int btf__get_from_id(__u32 id, struct btf **btf)
 {
 	struct btf *res;

tools/lib/bpf/btf.h +1 -0

index a6039ca66895..358046ffc7bd 100644
@@ -44,6 +44,7 @@ LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_b
 LIBBPF_API struct btf *btf__parse_raw(const char *path);
 LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
 LIBBPF_API struct btf *btf__load_from_kernel_by_id(__u32 id);
+LIBBPF_API struct btf *btf__load_from_kernel_by_id_split(__u32 id, struct btf *base_btf);
 LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__load_from_kernel_by_id instead")
 LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf);

tools/lib/bpf/libbpf.map +1 -0

index 3a9c6939301e..5aca3686ca5e 100644
@@ -376,6 +376,7 @@ LIBBPF_0.5.0 {
 		bpf_program__attach_kprobe_opts;
 		bpf_object__gen_loader;
 		btf__load_from_kernel_by_id;
+		btf__load_from_kernel_by_id_split;
 		btf__load_into_kernel;
 		btf_dump__dump_type_data;
 		libbpf_set_strict_mode;

Signature · 1 line

2.30.2

Quentin Monnet

author

2021-07-29 16:20 UTC #8 lore raw

PATCH v3 8/8 tools: bpftool: support dumping split BTF by id

In reply to Quentin Monnet

Split BTF objects are typically BTF objects for kernel modules, which are incrementally built on top of kernel BTF instead of redefining all kernel symbols they need. We can use bpftool with its -B command-line option to dump split BTF objects. It works well when the handle provided for the BTF object to dump is a "path" to the BTF object, typically under /sys/kernel/btf, because bpftool internally calls btf__parse_split() which can take a "base_btf" pointer and resolve the BTF reconstruction (although in that case, the "-B" option is unnecessary because bpftool performs autodetection).

However, it did not work so far when passing the BTF object through its id, because bpftool would call btf__get_from_id() which did not provide a way to pass a "base_btf" pointer.

In other words, the following works:

    # bpftool btf dump file /sys/kernel/btf/i2c_smbus -B /sys/kernel/btf/vmlinux

But this was not possible:

    # bpftool btf dump id 6 -B /sys/kernel/btf/vmlinux

The libbpf API has recently changed, and btf__get_from_id() has been deprecated in favour of btf__load_from_kernel_by_id() and its version with support for split BTF, btf__load_from_kernel_by_id_split(). Let's update bpftool to make it able to dump the BTF object in the second case as well.

Signed-off-by Quentin Monnet <[email protected]>

Acked-by John Fastabend <[email protected]>

---
 tools/bpf/bpftool/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/bpf/bpftool/btf.c +1 -1

index 9162a18e84c0..0ce3643278d4 100644
@@ -580,7 +580,7 @@ static int do_dump(int argc, char **argv)
 	}
 
 	if (!btf) {
-		btf = btf__load_from_kernel_by_id(btf_id);
+		btf = btf__load_from_kernel_by_id_split(btf_id, base_btf);
 		err = libbpf_get_error(btf);
 		if (err) {
 			p_err("get btf by id (%u): %s", btf_id, strerror(err));

Signature · 1 line

2.30.2

Andrii Nakryiko

2021-07-30 00:32 UTC #9 lore raw

In reply to Quentin Monnet

Quoted 62 lines · Quentin Monnet

As part of the effort to move towards a v1.0 for libbpf [0], this set improves some confusing function names related to BTF loading from and to the kernel:
- btf__load() becomes btf__load_into_kernel().
- btf__get_from_id becomes btf__load_from_kernel_by_id().
- A new version btf__load_from_kernel_by_id_split() extends the former to add support for split BTF.
The old functions are marked for deprecation for the next minor version (0.6) of libbpf.
The last patch is a trivial change to bpftool to add support for dumping split BTF objects by referencing them by their id (and not only by their BTF path).
[0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis
v3:
- Use libbpf_err_ptr() in btf__load_from_kernel_by_id(), ERR_PTR() in bpftool's get_map_kv_btf().
- Move the definition of btf__load_from_kernel_by_id() closer to the btf__parse() group in btf.h (move the legacy function with it).
- Fix a bug on the return value in libbpf_find_prog_btf_id(), as a new patch.
- Move the btf__free() fixes to their own patch.
- Add "Fixes:" tags to relevant patches.
- Re-introduce deprecation (removed in v2) for the legacy functions, as a new macro LIBBPF_DEPRECATED_SINCE(major, minor, message).
v2:
- Remove deprecation marking of legacy functions (patch 4/6 from v1).
- Make btf__load_from_kernel_by_id{,_split}() return the btf struct, adjust surrounding code and call btf__free() when missing.
- Add new functions to v0.5.0 API (and not v0.6.0).
Quentin Monnet (8):
libbpf: return non-null error on failures in libbpf_find_prog_btf_id()
libbpf: rename btf__load() as btf__load_into_kernel()
libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()
tools: free BTF objects at various locations
tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()
libbpf: prepare deprecation of btf__get_from_id(), btf__load()
libbpf: add split BTF support for btf__load_from_kernel_by_id()
tools: bpftool: support dumping split BTF by id
 tools/bpf/bpftool/btf.c                      |  8 ++---
 tools/bpf/bpftool/btf_dumper.c               |  6 ++--
 tools/bpf/bpftool/map.c                      | 14 ++++-----
 tools/bpf/bpftool/prog.c                     | 29 +++++++++++------
 tools/lib/bpf/Makefile                       |  3 ++
 tools/lib/bpf/btf.c                          | 33 ++++++++++++++------
 tools/lib/bpf/btf.h                          |  7 ++++-
 tools/lib/bpf/libbpf.c                       | 11 ++++---
 tools/lib/bpf/libbpf.map                     |  3 ++
 tools/lib/bpf/libbpf_common.h                | 19 +++++++++++
 tools/perf/util/bpf-event.c                  | 11 ++++---
 tools/perf/util/bpf_counter.c                | 12 +++++--
 tools/testing/selftests/bpf/prog_tests/btf.c |  4 ++-
 13 files changed, 113 insertions(+), 47 deletions(-)

Quoted 3 lines

--
2.30.2

I dropped patch #7 with deprecations and LIBBPF_DEPRECATED_SINCE and applied to bpf-next.

Current LIBBPF_DEPRECATED_SINCE approach doesn't work (and you should have caught this when you built selftests/bpf, what happened there?). bpftool build generates warnings like this:

In file included from /data/users/andriin/linux/tools/lib/bpf/libbpf.h:20,
from xlated_dumper.c:10:
/data/users/andriin/linux/tools/lib/bpf/libbpf_common.h:22:23:
warning: "LIBBPF_MAJOR_VERSION" is not defined, evaluates to 0 [-Wundef]
__LIBBPF_GET_VERSION(LIBBPF_MAJOR_VERSION, LIBBPF_MINOR_VERSION)

                       ^~~~~~~~~~~~~~~~~~~~

And it makes total sense. LIBBPF_DEPRECATED_SINCE() assumes LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION is defined at compilation time of the *application that is using libbpf*, not just libbpf's compilation time. And that's clearly a bogus assumption which we can't and shouldn't make. The right approach will be to define LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION in some sort of auto-generated header, included from libbpf_common.h and installed as part of libbpf package.

Anyways, I've removed all the LIBBPF_DEPRECATED_SINCE stuff and applied all the rest, as it looks good and is a useful addition. We should work some more on deprecation helpers, though.

Quentin Monnet

author

2021-07-30 15:23 UTC #10 lore raw

2021-07-29 17:31 UTC-0700 ~ Andrii Nakryiko <[email protected]>

Quoted 74 lines · Quentin Monnet

Quoted 62 lines
As part of the effort to move towards a v1.0 for libbpf [0], this set improves some confusing function names related to BTF loading from and to the kernel:
- btf__load() becomes btf__load_into_kernel().
- btf__get_from_id becomes btf__load_from_kernel_by_id().
- A new version btf__load_from_kernel_by_id_split() extends the former to add support for split BTF.
The old functions are marked for deprecation for the next minor version (0.6) of libbpf.
The last patch is a trivial change to bpftool to add support for dumping split BTF objects by referencing them by their id (and not only by their BTF path).
[0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis
v3:
- Use libbpf_err_ptr() in btf__load_from_kernel_by_id(), ERR_PTR() in bpftool's get_map_kv_btf().
- Move the definition of btf__load_from_kernel_by_id() closer to the btf__parse() group in btf.h (move the legacy function with it).
- Fix a bug on the return value in libbpf_find_prog_btf_id(), as a new patch.
- Move the btf__free() fixes to their own patch.
- Add "Fixes:" tags to relevant patches.
- Re-introduce deprecation (removed in v2) for the legacy functions, as a new macro LIBBPF_DEPRECATED_SINCE(major, minor, message).
v2:
- Remove deprecation marking of legacy functions (patch 4/6 from v1).
- Make btf__load_from_kernel_by_id{,_split}() return the btf struct, adjust surrounding code and call btf__free() when missing.
- Add new functions to v0.5.0 API (and not v0.6.0).
Quentin Monnet (8):
libbpf: return non-null error on failures in libbpf_find_prog_btf_id()
libbpf: rename btf__load() as btf__load_into_kernel()
libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()
tools: free BTF objects at various locations
tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()
libbpf: prepare deprecation of btf__get_from_id(), btf__load()
libbpf: add split BTF support for btf__load_from_kernel_by_id()
tools: bpftool: support dumping split BTF by id
 tools/bpf/bpftool/btf.c                      |  8 ++---
 tools/bpf/bpftool/btf_dumper.c               |  6 ++--
 tools/bpf/bpftool/map.c                      | 14 ++++-----
 tools/bpf/bpftool/prog.c                     | 29 +++++++++++------
 tools/lib/bpf/Makefile                       |  3 ++
 tools/lib/bpf/btf.c                          | 33 ++++++++++++++------
 tools/lib/bpf/btf.h                          |  7 ++++-
 tools/lib/bpf/libbpf.c                       | 11 ++++---
 tools/lib/bpf/libbpf.map                     |  3 ++
 tools/lib/bpf/libbpf_common.h                | 19 +++++++++++
 tools/perf/util/bpf-event.c                  | 11 ++++---
 tools/perf/util/bpf_counter.c                | 12 +++++--
 tools/testing/selftests/bpf/prog_tests/btf.c |  4 ++-
 13 files changed, 113 insertions(+), 47 deletions(-)
Quoted 3 lines
--
2.30.2
I dropped patch #7 with deprecations and LIBBPF_DEPRECATED_SINCE and applied to bpf-next.
Current LIBBPF_DEPRECATED_SINCE approach doesn't work (and you should have caught this when you built selftests/bpf, what happened there?). bpftool build generates warnings like this:

Quoted 7 lines

In file included from /data/users/andriin/linux/tools/lib/bpf/libbpf.h:20,
from xlated_dumper.c:10:
/data/users/andriin/linux/tools/lib/bpf/libbpf_common.h:22:23:
warning: "LIBBPF_MAJOR_VERSION" is not defined, evaluates to 0 [-Wundef]
__LIBBPF_GET_VERSION(LIBBPF_MAJOR_VERSION, LIBBPF_MINOR_VERSION)
                       ^~~~~~~~~~~~~~~~~~~~

Apologies, I didn't realise the change would impact external applications.

Quoted 9 lines

And it makes total sense. LIBBPF_DEPRECATED_SINCE() assumes LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION is defined at compilation time of the *application that is using libbpf*, not just libbpf's compilation time. And that's clearly a bogus assumption which we can't and shouldn't make. The right approach will be to define LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION in some sort of auto-generated header, included from libbpf_common.h and installed as part of libbpf package.

So generating this header is easy. Installing it with the other headers is simple too. It becomes a bit trickier when we build outside of the directory (it seems I need to pass -I$(OUTPUT) to build libbpf).

The step I'm most struggling with at the moment is bpftool, which bootstraps a first version of itself before building libbpf, by looking at the headers directly in libbpf's directory. It means that the generated header with the version number has not yet been generated. Do you think it is worth changing bpftool's build steps to implement this deprecation helper?

Alternatively, wouldn't it make more sense to have a script in the GitHub repo for libbpf, and to run it once during the release process of a new version to update, say, the version number, or even the deprecation status directly?

Quoted 3 lines

Anyways, I've removed all the LIBBPF_DEPRECATED_SINCE stuff and applied all the rest, as it looks good and is a useful addition.

Thanks.
Quentin

Andrii Nakryiko

2021-07-30 17:24 UTC #11 lore raw

Quoted 84 lines · Quentin Monnet

2021-07-29 17:31 UTC-0700 ~ Andrii Nakryiko <[email protected]>
Quoted 74 lines · Quentin Monnet
Quoted 62 lines
As part of the effort to move towards a v1.0 for libbpf [0], this set improves some confusing function names related to BTF loading from and to the kernel:
- btf__load() becomes btf__load_into_kernel().
- btf__get_from_id becomes btf__load_from_kernel_by_id().
- A new version btf__load_from_kernel_by_id_split() extends the former to add support for split BTF.
The old functions are marked for deprecation for the next minor version (0.6) of libbpf.
The last patch is a trivial change to bpftool to add support for dumping split BTF objects by referencing them by their id (and not only by their BTF path).
[0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis
v3:
- Use libbpf_err_ptr() in btf__load_from_kernel_by_id(), ERR_PTR() in bpftool's get_map_kv_btf().
- Move the definition of btf__load_from_kernel_by_id() closer to the btf__parse() group in btf.h (move the legacy function with it).
- Fix a bug on the return value in libbpf_find_prog_btf_id(), as a new patch.
- Move the btf__free() fixes to their own patch.
- Add "Fixes:" tags to relevant patches.
- Re-introduce deprecation (removed in v2) for the legacy functions, as a new macro LIBBPF_DEPRECATED_SINCE(major, minor, message).
v2:
- Remove deprecation marking of legacy functions (patch 4/6 from v1).
- Make btf__load_from_kernel_by_id{,_split}() return the btf struct, adjust surrounding code and call btf__free() when missing.
- Add new functions to v0.5.0 API (and not v0.6.0).
Quentin Monnet (8):
libbpf: return non-null error on failures in libbpf_find_prog_btf_id()
libbpf: rename btf__load() as btf__load_into_kernel()
libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()
tools: free BTF objects at various locations
tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()
libbpf: prepare deprecation of btf__get_from_id(), btf__load()
libbpf: add split BTF support for btf__load_from_kernel_by_id()
tools: bpftool: support dumping split BTF by id
 tools/bpf/bpftool/btf.c                      |  8 ++---
 tools/bpf/bpftool/btf_dumper.c               |  6 ++--
 tools/bpf/bpftool/map.c                      | 14 ++++-----
 tools/bpf/bpftool/prog.c                     | 29 +++++++++++------
 tools/lib/bpf/Makefile                       |  3 ++
 tools/lib/bpf/btf.c                          | 33 ++++++++++++++------
 tools/lib/bpf/btf.h                          |  7 ++++-
 tools/lib/bpf/libbpf.c                       | 11 ++++---
 tools/lib/bpf/libbpf.map                     |  3 ++
 tools/lib/bpf/libbpf_common.h                | 19 +++++++++++
 tools/perf/util/bpf-event.c                  | 11 ++++---
 tools/perf/util/bpf_counter.c                | 12 +++++--
 tools/testing/selftests/bpf/prog_tests/btf.c |  4 ++-
 13 files changed, 113 insertions(+), 47 deletions(-)
Quoted 3 lines
--
2.30.2
I dropped patch #7 with deprecations and LIBBPF_DEPRECATED_SINCE and applied to bpf-next.
Current LIBBPF_DEPRECATED_SINCE approach doesn't work (and you should have caught this when you built selftests/bpf, what happened there?). bpftool build generates warnings like this:
Quoted 7 lines
In file included from /data/users/andriin/linux/tools/lib/bpf/libbpf.h:20,
from xlated_dumper.c:10:
/data/users/andriin/linux/tools/lib/bpf/libbpf_common.h:22:23:
warning: "LIBBPF_MAJOR_VERSION" is not defined, evaluates to 0 [-Wundef]
__LIBBPF_GET_VERSION(LIBBPF_MAJOR_VERSION, LIBBPF_MINOR_VERSION)
                       ^~~~~~~~~~~~~~~~~~~~

Quoted 1 line

Apologies, I didn't realise the change would impact external applications.

It doesn't matter, we expect everyone to compile selftest (just make in tools/testing/selftests/bpf) and run at least test_progs, preferably also test_maps and test_verifier. Especially with vmtest.sh script it's quite simple (once you get latest Clang and pahole compiled locally). We obviously have CI and maintainers as the last line of defense, but that should be the last line of defense, not the main line :)

Quoted 11 lines

Quoted 10 lines
And it makes total sense. LIBBPF_DEPRECATED_SINCE() assumes LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION is defined at compilation time of the *application that is using libbpf*, not just libbpf's compilation time. And that's clearly a bogus assumption which we can't and shouldn't make. The right approach will be to define LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION in some sort of auto-generated header, included from libbpf_common.h and installed as part of libbpf package.

Quoted 3 lines

So generating this header is easy. Installing it with the other headers is simple too. It becomes a bit trickier when we build outside of the directory (it seems I need to pass -I$(OUTPUT) to build libbpf).

Not sure why using the header is tricky. We auto-generate bpf_helper_defs.h, which is included from bpf_helpers.h, which is included in every single libbpf-using application. Works good with no extra magic.

Quoted 7 lines

The step I'm most struggling with at the moment is bpftool, which bootstraps a first version of itself before building libbpf, by looking at the headers directly in libbpf's directory. It means that the generated header with the version number has not yet been generated. Do you think it is worth changing bpftool's build steps to implement this deprecation helper?

If it doesn't do that already, bpftool should do make install for libbpf, not just build. Install will put all the headers, generated or otherwise, into a designated destination folder, which should be passed as -I parameter. But that should be already happening due to bpf_helper_defs.h.

Quoted 5 lines

Alternatively, wouldn't it make more sense to have a script in the GitHub repo for libbpf, and to run it once during the release process of a new version to update, say, the version number, or even the deprecation status directly?

I'd like to avoid extra manual steps that I or someone else will definitely forget from time to time. Again, taking bpf_helper_defs.h as a precedent. In the kernel repo we auto-generate it during build. But when we sync libbpf to Github, we copy and check-in bpf_helper_defs.h, so it's always available there (and will get installed on make install or during packaging). We should do the same for this new header (libbpf_version.h?).

Quoted 7 lines

Quoted 4 lines
Anyways, I've removed all the LIBBPF_DEPRECATED_SINCE stuff and applied all the rest, as it looks good and is a useful addition.
Thanks.
Quentin

Quentin Monnet

author

2021-07-30 20:23 UTC #12 lore raw

On Fri, 30 Jul 2021 at 18:24, Andrii Nakryiko <[email protected]> wrote:

[...]

Quoted 5 lines

Quoted 5 lines
Quoted 4 lines
The right approach will be to define LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION in some sort of auto-generated header, included from libbpf_common.h and installed as part of libbpf package.

Quoted 8 lines

Quoted 3 lines
So generating this header is easy. Installing it with the other headers is simple too. It becomes a bit trickier when we build outside of the directory (it seems I need to pass -I$(OUTPUT) to build libbpf).
Not sure why using the header is tricky. We auto-generate bpf_helper_defs.h, which is included from bpf_helpers.h, which is included in every single libbpf-using application. Works good with no extra magic.

bpf_helper_defs.h is the first thing I looked at, and I processed libbpf_version.h just like it. But there is a difference:
bpf_helper_defs.h is _not_ included in libbpf itself, nor is it needed in bpftool at the bootstrap stage (it is only included from the eBPF skeletons for profiling or showing PIDs etc., which are compiled after libbpf). The version header is needed in both cases.

Quoted 9 lines

Quoted 8 lines
The step I'm most struggling with at the moment is bpftool, which bootstraps a first version of itself before building libbpf, by looking at the headers directly in libbpf's directory. It means that the generated header with the version number has not yet been generated. Do you think it is worth changing bpftool's build steps to implement this deprecation helper?

Quoted 5 lines

If it doesn't do that already, bpftool should do make install for libbpf, not just build. Install will put all the headers, generated or otherwise, into a designated destination folder, which should be passed as -I parameter. But that should be already happening due to bpf_helper_defs.h.

bpftool does not run "make install". It compiles libbpf passing "OUTPUT=$(LIBBPF_OUTPUT)", sets LIBBPF_PATH to the same directory, and then adds "-I$(LIBBPF_PATH)" for accessing bpf_helper_defs.h and compile its eBPF programs. It is possible to include libbpf_version.h the same way, but only after libbpf has been compiled, after the bootstrap.

I'll look into updating the Makefile to compile and install libbpf before the bootstrap, when I have some time.

Andrii Nakryiko

2021-07-30 21:54 UTC #13 lore raw

Quoted 19 lines · Quentin Monnet

On Fri, 30 Jul 2021 at 18:24, Andrii Nakryiko <[email protected]> wrote:
[...]
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The right approach will be to define LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION in some sort of auto-generated header, included from libbpf_common.h and installed as part of libbpf package.
Quoted 8 lines
Quoted 3 lines
So generating this header is easy. Installing it with the other headers is simple too. It becomes a bit trickier when we build outside of the directory (it seems I need to pass -I$(OUTPUT) to build libbpf).
Not sure why using the header is tricky. We auto-generate bpf_helper_defs.h, which is included from bpf_helpers.h, which is included in every single libbpf-using application. Works good with no extra magic.

Quoted 6 lines

bpf_helper_defs.h is the first thing I looked at, and I processed libbpf_version.h just like it. But there is a difference:
bpf_helper_defs.h is _not_ included in libbpf itself, nor is it needed in bpftool at the bootstrap stage (it is only included from the eBPF skeletons for profiling or showing PIDs etc., which are compiled after libbpf). The version header is needed in both cases.

Oh, in that sense. Yeah, sure, I didn't think that would qualify as tricky. But yeah, -I$(OUTPUT) or something along those lines is fine.

Quoted 16 lines

Quoted 10 lines
Quoted 9 lines
The step I'm most struggling with at the moment is bpftool, which bootstraps a first version of itself before building libbpf, by looking at the headers directly in libbpf's directory. It means that the generated header with the version number has not yet been generated. Do you think it is worth changing bpftool's build steps to implement this deprecation helper?
Quoted 5 lines
If it doesn't do that already, bpftool should do make install for libbpf, not just build. Install will put all the headers, generated or otherwise, into a designated destination folder, which should be passed as -I parameter. But that should be already happening due to bpf_helper_defs.h.

Quoted 8 lines

bpftool does not run "make install". It compiles libbpf passing "OUTPUT=$(LIBBPF_OUTPUT)", sets LIBBPF_PATH to the same directory, and then adds "-I$(LIBBPF_PATH)" for accessing bpf_helper_defs.h and compile its eBPF programs. It is possible to include libbpf_version.h the same way, but only after libbpf has been compiled, after the bootstrap.
I'll look into updating the Makefile to compile and install libbpf before the bootstrap, when I have some time.

Cool, make install is the best way as it prevents accidental usage of libbpf's internal header. So it's a good change to make.