Re: [PATCH bpf-next v2 0/5] libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF
From: Quentin Monnet <hidden>
Date: 2021-07-27 11:39:39
Also in:
bpf
Subsystem:
bpf [general] (safe dynamic programs and tools), bpf [library] (libbpf), the rest · Maintainers:
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Linus Torvalds
2021-07-23 08:51 UTC-0700 ~ Andrii Nakryiko [off-list ref]
On Fri, Jul 23, 2021 at 2:58 AM Quentin Monnet [off-list ref] wrote:quoted
2021-07-22 19:45 UTC-0700 ~ Andrii Nakryiko [off-list ref]quoted
On Thu, Jul 22, 2021 at 5:58 PM Andrii Nakryiko [off-list ref] wrote:quoted
On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet [off-list ref] wrote:quoted
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 not removed or marked as deprecated yet, there should be in a future libbpf version.Oh, and I was thinking about this whole deprecation having to be done in two steps. It's super annoying to keep track of that. Ideally, we'd have some macro that can mark API deprecated "in the future", when actual libbpf version is >= to defined version. So something like this: LIBBPF_DEPRECATED_AFTER(V(0,5), "API that will be marked deprecated in v0.6")Better: LIBBPF_DEPRECATED_SINCE(0, 6, "API that will be marked deprecated in v0.6")
So I've been looking into this, and it's not _that_ simple to do. Unless I missed something about preprocessing macros, I cannot bake a "#if" in a "#define", to have the attribute printed if and only if the current version is >= 0.6 in this example. I've come up with something, but it is not optimal because I have to write a check and macros for each version number used with the LIBBPF_DEPRECATED_SINCE macro. If we really wanted to automate that part I guess we could generate a header with those macros from the Makefile and include it in libbpf_common.h, but that does not really look much cleaner to me. Here's my current code, below - does it correspond to what you had in mind? Or did you think of something else? ------
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index ec14aa725bb0..095d5dc30d50 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile@@ -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
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index cf8490f95641..8b6b5442dbd8 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h@@ -45,7 +45,8 @@ 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_API int btf__get_from_id(__u32 id, struct btf **btf); +LIBBPF_API LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__load_from_kernel_by_id instead") +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);
diff --git a/tools/lib/bpf/libbpf_common.h b/tools/lib/bpf/libbpf_common.h
index 947d8bd8a7bb..9ba9f8135dc8 100644
--- a/tools/lib/bpf/libbpf_common.h
+++ b/tools/lib/bpf/libbpf_common.h@@ -17,6 +17,28 @@ #define LIBBPF_DEPRECATED(msg) __attribute__((deprecated(msg))) +#ifndef LIBBPF_DEPRECATED_SINCE +#define __LIBBPF_VERSION_CHECK(major, minor) \ + LIBBPF_MAJOR_VERSION > major || \ + (LIBBPF_MAJOR_VERSION == major && LIBBPF_MINOR_VERSION >= minor) + +/* Add checks for other versions below when planning deprecation of API symbols + * with the LIBBPF_DEPRECATED_SINCE macro. + */ +#if __LIBBPF_VERSION_CHECK(0, 6) +#define __LIBBPF_MARK_DEPRECATED_0_6(X) X +#else +#define __LIBBPF_MARK_DEPRECATED_0_6(X) +#endif + +#define __LIBBPF_DEPRECATED_SINCE(major, minor, msg) \ + __LIBBPF_MARK_DEPRECATED_ ## major ## _ ## minor (LIBBPF_DEPRECATED("v" # major "." # minor "+, " msg)) + +/* Mark a symbol as deprecated when libbpf version is >= {major}.{minor} */ +#define LIBBPF_DEPRECATED_SINCE(major, minor, msg) \ + __LIBBPF_DEPRECATED_SINCE(major, minor, msg) +#endif /* LIBBPF_DEPRECATED_SINCE */ + /* Helper macro to declare and initialize libbpf options struct * * This dance with uninitialized declaration, followed by memset to zero,