* [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs
@ 2021-11-11 5:36 Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 1/9] bpftool: normalize compile rules to specify output file last Andrii Nakryiko
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
This patch set continues the work of revamping libbpf APIs that are not
extensible, as they were added before we figured out all the intricacies of
building APIs that can preserve ABI compatibility (both backward and forward).
What makes them tricky is that (most of) these APIs are actively used by
multiple applications, so we need to be careful about refactoring them. See
individual patches for details, but the general approach is similar to
previous bpf_prog_load() API revamp. The biggest different and complexity is
in changing btf_dump__new(), because function overloading through macro magic
doesn't work based on number of arguments, as both new and old APIs have
4 arguments. Because of that, another overloading approach is taken; overload
happens based on argument types.
I've validated manually (by using local test_progs-shared flavor that is
compiling test_progs against libbpf as a shared library) that compiling "old
application" (selftests before being adapted to using new variants of revamped
APIs) are compiled and successfully run against newest libbpf version as well
as the older libbpf version (provided no new variants are used). All these
scenarios seem to be working as expected.
v1->v2:
- add explicit printf_fn NULL check in btf_dump__new() (Alexei);
- replaced + with || in __builtin_choose_expr() (Alexei);
- dropped test_progs-shared flavor (Alexei).
Andrii Nakryiko (9):
bpftool: normalize compile rules to specify output file last
selftests/bpf: minor cleanups and normalization of Makefile
libbpf: turn btf_dedup_opts into OPTS-based struct
libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
libbpf: make perf_buffer__new() use OPTS-based interface
selftests/bpf: migrate all deprecated perf_buffer uses
selftests/bpf: update btf_dump__new() uses to v1.0+ variant
tools/runqslower: update perf_buffer__new() calls
bpftool: update btf_dump__new() and perf_buffer__new_raw() calls
tools/bpf/bpftool/Makefile | 16 ++--
tools/bpf/bpftool/btf.c | 2 +-
tools/bpf/bpftool/gen.c | 2 +-
tools/bpf/bpftool/map_perf_ring.c | 9 +-
tools/bpf/runqslower/runqslower.c | 6 +-
tools/lib/bpf/btf.c | 46 ++++++----
tools/lib/bpf/btf.h | 71 +++++++++++++--
tools/lib/bpf/btf_dump.c | 31 +++++--
tools/lib/bpf/libbpf.c | 70 +++++++++++----
tools/lib/bpf/libbpf.h | 86 ++++++++++++++++---
tools/lib/bpf/libbpf.map | 8 ++
tools/lib/bpf/linker.c | 4 +-
tools/testing/selftests/bpf/Makefile | 32 +++----
.../selftests/bpf/benchs/bench_ringbufs.c | 8 +-
tools/testing/selftests/bpf/btf_helpers.c | 4 +-
tools/testing/selftests/bpf/prog_tests/btf.c | 46 ++--------
.../bpf/prog_tests/btf_dedup_split.c | 6 +-
.../selftests/bpf/prog_tests/btf_dump.c | 33 +++----
.../selftests/bpf/prog_tests/btf_split.c | 4 +-
.../bpf/prog_tests/get_stack_raw_tp.c | 5 +-
.../selftests/bpf/prog_tests/kfree_skb.c | 6 +-
.../selftests/bpf/prog_tests/perf_buffer.c | 6 +-
.../selftests/bpf/prog_tests/xdp_bpf2bpf.c | 7 +-
.../selftests/bpf/test_tcpnotify_user.c | 4 +-
24 files changed, 318 insertions(+), 194 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 1/9] bpftool: normalize compile rules to specify output file last
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 2/9] selftests/bpf: minor cleanups and normalization of Makefile Andrii Nakryiko
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
When dealing with verbose Makefile output, it's extremely confusing when
compiler invocation commands don't specify -o <output.o> as the last
argument. Normalize bpftool's Makefile to do just that, as most other
BPF-related Makefiles are already doing that.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 622568c7a9b8..31dfef6a4121 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -177,7 +177,8 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF)
-I$(if $(OUTPUT),$(OUTPUT),.) \
-I$(srctree)/tools/include/uapi/ \
-I$(LIBBPF_INCLUDE) \
- -g -O2 -Wall -target bpf -c $< -o $@ && $(LLVM_STRIP) -g $@
+ -g -O2 -Wall -target bpf -c $< -o $@
+ $(Q)$(LLVM_STRIP) -g $@
$(OUTPUT)%.skel.h: $(OUTPUT)%.bpf.o $(BPFTOOL_BOOTSTRAP)
$(QUIET_GEN)$(BPFTOOL_BOOTSTRAP) gen skeleton $< > $@
@@ -192,10 +193,10 @@ endif
CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
- $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
+ $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD $< -o $@
$(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
- $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<
+ $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD $< -o $@
$(OUTPUT)feature.o:
ifneq ($(feature-zlib), 1)
@@ -203,17 +204,16 @@ ifneq ($(feature-zlib), 1)
endif
$(BPFTOOL_BOOTSTRAP): $(BOOTSTRAP_OBJS) $(LIBBPF_BOOTSTRAP)
- $(QUIET_LINK)$(HOSTCC) $(CFLAGS) $(LDFLAGS) -o $@ $(BOOTSTRAP_OBJS) \
- $(LIBS_BOOTSTRAP)
+ $(QUIET_LINK)$(HOSTCC) $(CFLAGS) $(LDFLAGS) $(BOOTSTRAP_OBJS) $(LIBS_BOOTSTRAP) -o $@
$(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
- $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
+ $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(OBJS) $(LIBS) -o $@
$(BOOTSTRAP_OUTPUT)%.o: %.c $(LIBBPF_INTERNAL_HDRS) | $(BOOTSTRAP_OUTPUT)
- $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
+ $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD $< -o $@
$(OUTPUT)%.o: %.c
- $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<
+ $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD $< -o $@
feature-detect-clean:
$(call QUIET_CLEAN, feature-detect)
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 2/9] selftests/bpf: minor cleanups and normalization of Makefile
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 1/9] bpftool: normalize compile rules to specify output file last Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 3/9] libbpf: turn btf_dedup_opts into OPTS-based struct Andrii Nakryiko
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Few clean ups and single-line simplifications. Also split CLEAN command
into multiple $(RM) invocations as it gets dangerously close to too long
argument list. Make sure that -o <output.o> is used always as the last
argument for saner verbose make output.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 32 ++++++++++++++--------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 0468ea57650d..0470802c907c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -45,10 +45,8 @@ ifneq ($(BPF_GCC),)
TEST_GEN_PROGS += test_progs-bpf_gcc
endif
-TEST_GEN_FILES = test_lwt_ip_encap.o \
- test_tc_edt.o
-TEST_FILES = xsk_prereqs.sh \
- $(wildcard progs/btf_dump_test_case_*.c)
+TEST_GEN_FILES = test_lwt_ip_encap.o test_tc_edt.o
+TEST_FILES = xsk_prereqs.sh $(wildcard progs/btf_dump_test_case_*.c)
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -107,7 +105,10 @@ endif
OVERRIDE_TARGETS := 1
override define CLEAN
$(call msg,CLEAN)
- $(Q)$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
+ $(Q)$(RM) -r $(TEST_GEN_PROGS)
+ $(Q)$(RM) -r $(TEST_GEN_PROGS_EXTENDED)
+ $(Q)$(RM) -r $(TEST_GEN_FILES)
+ $(Q)$(RM) -r $(EXTRA_CLEAN)
$(Q)$(MAKE) -C bpf_testmod clean
$(Q)$(MAKE) docs-clean
endef
@@ -169,7 +170,7 @@ $(OUTPUT)/%:%.c
$(OUTPUT)/urandom_read: urandom_read.c
$(call msg,BINARY,,$@)
- $(Q)$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS) -Wl,--build-id=sha1
+ $(Q)$(CC) $(LDFLAGS) $< $(LDLIBS) -Wl,--build-id=sha1 -o $@
$(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
$(call msg,MOD,,$@)
@@ -232,16 +233,16 @@ docs-clean:
prefix= OUTPUT=$(OUTPUT)/ DESTDIR=$(OUTPUT)/ $@
$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
- ../../../include/uapi/linux/bpf.h \
+ $(APIDIR)/linux/bpf.h \
| $(BUILD_DIR)/libbpf
$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
EXTRA_CFLAGS='-g -O0' \
DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
-$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
- ../../../include/uapi/linux/bpf.h \
- | $(HOST_BUILD_DIR)/libbpf
+$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
+ $(APIDIR)/linux/bpf.h \
+ | $(HOST_BUILD_DIR)/libbpf
$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
EXTRA_CFLAGS='-g -O0' \
OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
@@ -305,12 +306,12 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
# $3 - CFLAGS
define CLANG_BPF_BUILD_RULE
$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
- $(Q)$(CLANG) $3 -O2 -target bpf -c $1 -o $2 -mcpu=v3
+ $(Q)$(CLANG) $3 -O2 -target bpf -c $1 -mcpu=v3 -o $2
endef
# Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
define CLANG_NOALU32_BPF_BUILD_RULE
$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
- $(Q)$(CLANG) $3 -O2 -target bpf -c $1 -o $2 -mcpu=v2
+ $(Q)$(CLANG) $3 -O2 -target bpf -c $1 -mcpu=v2 -o $2
endef
# Build BPF object using GCC
define GCC_BPF_BUILD_RULE
@@ -472,13 +473,12 @@ TRUNNER_TESTS_DIR := prog_tests
TRUNNER_BPF_PROGS_DIR := progs
TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
network_helpers.c testing_helpers.c \
- btf_helpers.c flow_dissector_load.h
+ btf_helpers.c flow_dissector_load.h
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
ima_setup.sh \
$(wildcard progs/btf_dump_test_case_*.c)
TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
-TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
-TRUNNER_BPF_CFLAGS += -DENABLE_ATOMICS_TESTS
+TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS
$(eval $(call DEFINE_TEST_RUNNER,test_progs))
# Define test_progs-no_alu32 test runner.
@@ -540,7 +540,7 @@ $(OUTPUT)/bench: $(OUTPUT)/bench.o $(OUTPUT)/testing_helpers.o \
$(OUTPUT)/bench_ringbufs.o \
$(OUTPUT)/bench_bloom_filter_map.o
$(call msg,BINARY,,$@)
- $(Q)$(CC) $(LDFLAGS) -o $@ $(filter %.a %.o,$^) $(LDLIBS)
+ $(Q)$(CC) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@
EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
prog_tests/tests.h map_tests/tests.h verifier/tests.h \
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 3/9] libbpf: turn btf_dedup_opts into OPTS-based struct
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 1/9] bpftool: normalize compile rules to specify output file last Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 2/9] selftests/bpf: minor cleanups and normalization of Makefile Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof Andrii Nakryiko
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
btf__dedup() and struct btf_dedup_opts were added before we figured out
OPTS mechanism. As such, btf_dedup_opts is non-extensible without
breaking an ABI and potentially crashing user application.
Unfortunately, btf__dedup() and btf_dedup_opts are short and succinct
names that would be great to preserve and use going forward. So we use
___libbpf_override() macro approach, used previously for bpf_prog_load()
API, to define a new btf__dedup() variant that accepts only struct btf *
and struct btf_dedup_opts * arguments, and rename the old btf__dedup()
implementation into btf__dedup_deprecated(). This keeps both source and
binary compatibility with old and new applications.
The biggest problem was struct btf_dedup_opts, which wasn't OPTS-based,
and as such doesn't have `size_t sz;` as a first field. But btf__dedup()
is a pretty rarely used API and I believe that the only currently known
users (besides selftests) are libbpf's own bpf_linker and pahole.
Neither use case actually uses options and just passes NULL. So instead
of doing extra hacks, just rewrite struct btf_dedup_opts into OPTS-based
one, move btf_ext argument into those opts (only bpf_linker needs to
dedup btf_ext, so it's not a typical thing to specify), and drop never
used `dont_resolve_fwds` option (it was never used anywhere, AFAIK, it
makes BTF dedup much less useful and efficient).
Just in case, for old implementation, btf__dedup_deprecated(), detect
non-NULL options and error out with helpful message, to help users
migrate, if there are any user playing with btf__dedup().
The last remaining piece is dedup_table_size, which is another
anachronism from very early days of BTF dedup. Since then it has been
reduced to the only valid value, 1, to request forced hash collisions.
This is only used during testing. So instead introduce a bool flag to
force collisions explicitly.
This patch also adapts selftests to new btf__dedup() and btf_dedup_opts
use to avoid selftests breakage.
[0] Closes: https://github.com/libbpf/libbpf/issues/281
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/btf.c | 46 +++++++++++--------
tools/lib/bpf/btf.h | 20 ++++++--
tools/lib/bpf/libbpf.map | 2 +
tools/lib/bpf/linker.c | 4 +-
tools/testing/selftests/bpf/prog_tests/btf.c | 46 +++----------------
.../bpf/prog_tests/btf_dedup_split.c | 6 +--
6 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 7e4c5586bd87..fcec27622e7a 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -2846,8 +2846,7 @@ __u32 btf_ext__line_info_rec_size(const struct btf_ext *btf_ext)
struct btf_dedup;
-static struct btf_dedup *btf_dedup_new(struct btf *btf, struct btf_ext *btf_ext,
- const struct btf_dedup_opts *opts);
+static struct btf_dedup *btf_dedup_new(struct btf *btf, const struct btf_dedup_opts *opts);
static void btf_dedup_free(struct btf_dedup *d);
static int btf_dedup_prep(struct btf_dedup *d);
static int btf_dedup_strings(struct btf_dedup *d);
@@ -2994,12 +2993,17 @@ static int btf_dedup_remap_types(struct btf_dedup *d);
* deduplicating structs/unions is described in greater details in comments for
* `btf_dedup_is_equiv` function.
*/
-int btf__dedup(struct btf *btf, struct btf_ext *btf_ext,
- const struct btf_dedup_opts *opts)
+
+DEFAULT_VERSION(btf__dedup_v0_6_0, btf__dedup, LIBBPF_0.6.0)
+int btf__dedup_v0_6_0(struct btf *btf, const struct btf_dedup_opts *opts)
{
- struct btf_dedup *d = btf_dedup_new(btf, btf_ext, opts);
+ struct btf_dedup *d;
int err;
+ if (!OPTS_VALID(opts, btf_dedup_opts))
+ return libbpf_err(-EINVAL);
+
+ d = btf_dedup_new(btf, opts);
if (IS_ERR(d)) {
pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
return libbpf_err(-EINVAL);
@@ -3051,6 +3055,19 @@ int btf__dedup(struct btf *btf, struct btf_ext *btf_ext,
return libbpf_err(err);
}
+COMPAT_VERSION(bpf__dedup_deprecated, btf__dedup, LIBBPF_0.0.2)
+int btf__dedup_deprecated(struct btf *btf, struct btf_ext *btf_ext, const void *unused_opts)
+{
+ LIBBPF_OPTS(btf_dedup_opts, opts, .btf_ext = btf_ext);
+
+ if (unused_opts) {
+ pr_warn("please use new version of btf__dedup() that supports options\n");
+ return libbpf_err(-ENOTSUP);
+ }
+
+ return btf__dedup(btf, &opts);
+}
+
#define BTF_UNPROCESSED_ID ((__u32)-1)
#define BTF_IN_PROGRESS_ID ((__u32)-2)
@@ -3163,8 +3180,7 @@ static bool btf_dedup_equal_fn(const void *k1, const void *k2, void *ctx)
return k1 == k2;
}
-static struct btf_dedup *btf_dedup_new(struct btf *btf, struct btf_ext *btf_ext,
- const struct btf_dedup_opts *opts)
+static struct btf_dedup *btf_dedup_new(struct btf *btf, const struct btf_dedup_opts *opts)
{
struct btf_dedup *d = calloc(1, sizeof(struct btf_dedup));
hashmap_hash_fn hash_fn = btf_dedup_identity_hash_fn;
@@ -3173,13 +3189,11 @@ static struct btf_dedup *btf_dedup_new(struct btf *btf, struct btf_ext *btf_ext,
if (!d)
return ERR_PTR(-ENOMEM);
- d->opts.dont_resolve_fwds = opts && opts->dont_resolve_fwds;
- /* dedup_table_size is now used only to force collisions in tests */
- if (opts && opts->dedup_table_size == 1)
+ if (OPTS_GET(opts, force_collisions, false))
hash_fn = btf_dedup_collision_hash_fn;
d->btf = btf;
- d->btf_ext = btf_ext;
+ d->btf_ext = OPTS_GET(opts, btf_ext, NULL);
d->dedup_table = hashmap__new(hash_fn, btf_dedup_equal_fn, NULL);
if (IS_ERR(d->dedup_table)) {
@@ -3708,8 +3722,6 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
new_id = cand_id;
break;
}
- if (d->opts.dont_resolve_fwds)
- continue;
if (btf_compat_enum(t, cand)) {
if (btf_is_enum_fwd(t)) {
/* resolve fwd to full enum */
@@ -3952,8 +3964,7 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
return 0;
/* FWD <--> STRUCT/UNION equivalence check, if enabled */
- if (!d->opts.dont_resolve_fwds
- && (cand_kind == BTF_KIND_FWD || canon_kind == BTF_KIND_FWD)
+ if ((cand_kind == BTF_KIND_FWD || canon_kind == BTF_KIND_FWD)
&& cand_kind != canon_kind) {
__u16 real_kind;
__u16 fwd_kind;
@@ -3979,10 +3990,7 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
return btf_equal_int_tag(cand_type, canon_type);
case BTF_KIND_ENUM:
- if (d->opts.dont_resolve_fwds)
- return btf_equal_enum(cand_type, canon_type);
- else
- return btf_compat_enum(cand_type, canon_type);
+ return btf_compat_enum(cand_type, canon_type);
case BTF_KIND_FWD:
case BTF_KIND_FLOAT:
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index bc005ba3ceec..6aae4f62ee0b 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -245,12 +245,24 @@ LIBBPF_API int btf__add_decl_tag(struct btf *btf, const char *value, int ref_typ
int component_idx);
struct btf_dedup_opts {
- unsigned int dedup_table_size;
- bool dont_resolve_fwds;
+ size_t sz;
+ /* optional .BTF.ext info to dedup along the main BTF info */
+ struct btf_ext *btf_ext;
+ /* force hash collisions (used for testing) */
+ bool force_collisions;
+ size_t :0;
};
+#define btf_dedup_opts__last_field force_collisions
+
+LIBBPF_API int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts);
+
+LIBBPF_API int btf__dedup_v0_6_0(struct btf *btf, const struct btf_dedup_opts *opts);
-LIBBPF_API int btf__dedup(struct btf *btf, struct btf_ext *btf_ext,
- const struct btf_dedup_opts *opts);
+LIBBPF_DEPRECATED_SINCE(0, 7, "use btf__dedup() instead")
+LIBBPF_API int btf__dedup_deprecated(struct btf *btf, struct btf_ext *btf_ext, const void *opts);
+#define btf__dedup(...) ___libbpf_overload(___btf_dedup, __VA_ARGS__)
+#define ___btf_dedup3(btf, btf_ext, opts) btf__dedup_deprecated(btf, btf_ext, opts)
+#define ___btf_dedup2(btf, opts) btf__dedup(btf, opts)
struct btf_dump;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index b895861a13c0..edc40bc16f19 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -401,6 +401,8 @@ LIBBPF_0.6.0 {
bpf_program__insns;
btf__add_btf;
btf__add_decl_tag;
+ btf__dedup;
+ btf__dedup_deprecated;
btf__raw_data;
btf__type_cnt;
} LIBBPF_0.5.0;
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index f677dccdeae4..594b206fa674 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -2650,6 +2650,7 @@ static int emit_elf_data_sec(struct bpf_linker *linker, const char *sec_name,
static int finalize_btf(struct bpf_linker *linker)
{
+ LIBBPF_OPTS(btf_dedup_opts, opts);
struct btf *btf = linker->btf;
const void *raw_data;
int i, j, id, err;
@@ -2686,7 +2687,8 @@ static int finalize_btf(struct bpf_linker *linker)
return err;
}
- err = btf__dedup(linker->btf, linker->btf_ext, NULL);
+ opts.btf_ext = linker->btf_ext;
+ err = btf__dedup(linker->btf, &opts);
if (err) {
pr_warn("BTF dedup failed: %d\n", err);
return err;
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index ebd1aa4d09d6..1e8b36d74df2 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -6627,7 +6627,7 @@ struct btf_dedup_test {
struct btf_dedup_opts opts;
};
-const struct btf_dedup_test dedup_tests[] = {
+static struct btf_dedup_test dedup_tests[] = {
{
.descr = "dedup: unused strings filtering",
@@ -6647,9 +6647,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0int\0long"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: strings deduplication",
@@ -6672,9 +6669,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0int\0long int"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: struct example #1",
@@ -6755,9 +6749,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0a\0b\0c\0d\0int\0float\0next\0s"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: struct <-> fwd resolution w/ hash collision",
@@ -6800,8 +6791,7 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_STR_SEC("\0s\0x"),
},
.opts = {
- .dont_resolve_fwds = false,
- .dedup_table_size = 1, /* force hash collisions */
+ .force_collisions = true, /* force hash collisions */
},
},
{
@@ -6847,8 +6837,7 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_STR_SEC("\0s\0x"),
},
.opts = {
- .dont_resolve_fwds = false,
- .dedup_table_size = 1, /* force hash collisions */
+ .force_collisions = true, /* force hash collisions */
},
},
{
@@ -6911,9 +6900,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0A\0B\0C\0D\0E\0F\0G\0H\0I\0J\0K\0L\0M\0N\0O\0P\0Q"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: no int/float duplicates",
@@ -6965,9 +6951,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0int\0some other int\0float"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: enum fwd resolution",
@@ -7009,9 +6992,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0e1\0e1_val\0e2\0e2_val"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: datasec and vars pass-through",
@@ -7054,8 +7034,7 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_STR_SEC("\0.bss\0t"),
},
.opts = {
- .dont_resolve_fwds = false,
- .dedup_table_size = 1
+ .force_collisions = true
},
},
{
@@ -7099,9 +7078,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0t\0a1\0a2\0f\0tag"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: func/func_param tags",
@@ -7152,9 +7128,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0a1\0a2\0f\0tag1\0tag2\0tag3"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: struct/struct_member tags",
@@ -7200,9 +7173,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0t\0m1\0m2\0tag1\0tag2\0tag3"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
{
.descr = "dedup: typedef tags",
@@ -7233,9 +7203,6 @@ const struct btf_dedup_test dedup_tests[] = {
},
BTF_STR_SEC("\0t\0tag1\0tag2\0tag3"),
},
- .opts = {
- .dont_resolve_fwds = false,
- },
},
};
@@ -7293,7 +7260,7 @@ static void dump_btf_strings(const char *strs, __u32 len)
static void do_test_dedup(unsigned int test_num)
{
- const struct btf_dedup_test *test = &dedup_tests[test_num - 1];
+ struct btf_dedup_test *test = &dedup_tests[test_num - 1];
__u32 test_nr_types, expect_nr_types, test_btf_size, expect_btf_size;
const struct btf_header *test_hdr, *expect_hdr;
struct btf *test_btf = NULL, *expect_btf = NULL;
@@ -7337,7 +7304,8 @@ static void do_test_dedup(unsigned int test_num)
goto done;
}
- err = btf__dedup(test_btf, NULL, &test->opts);
+ test->opts.sz = sizeof(test->opts);
+ err = btf__dedup(test_btf, &test->opts);
if (CHECK(err, "btf_dedup failed errno:%d", err)) {
err = -1;
goto done;
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c b/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
index 64554fd33547..9d3b8d7a1537 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
@@ -92,7 +92,7 @@ struct s2 {\n\
int *f3;\n\
};\n\n", "c_dump");
- err = btf__dedup(btf2, NULL, NULL);
+ err = btf__dedup(btf2, NULL);
if (!ASSERT_OK(err, "btf_dedup"))
goto cleanup;
@@ -186,7 +186,7 @@ static void test_split_fwd_resolve() {
"\t'f1' type_id=7 bits_offset=0\n"
"\t'f2' type_id=9 bits_offset=64");
- err = btf__dedup(btf2, NULL, NULL);
+ err = btf__dedup(btf2, NULL);
if (!ASSERT_OK(err, "btf_dedup"))
goto cleanup;
@@ -283,7 +283,7 @@ static void test_split_struct_duped() {
"[13] STRUCT 's3' size=8 vlen=1\n"
"\t'f1' type_id=12 bits_offset=0");
- err = btf__dedup(btf2, NULL, NULL);
+ err = btf__dedup(btf2, NULL);
if (!ASSERT_OK(err, "btf_dedup"))
goto cleanup;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (2 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 3/9] libbpf: turn btf_dedup_opts into OPTS-based struct Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-12-22 14:55 ` Jiri Olsa
2021-11-11 5:36 ` [PATCH v2 bpf-next 5/9] libbpf: make perf_buffer__new() use OPTS-based interface Andrii Nakryiko
` (5 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Change btf_dump__new() and corresponding struct btf_dump_ops structure
to be extensible by using OPTS "framework" ([0]). Given we don't change
the names, we use a similar approach as with bpf_prog_load(), but this
time we ended up with two APIs with the same name and same number of
arguments, so overloading based on number of arguments with
___libbpf_override() doesn't work.
Instead, use "overloading" based on types. In this particular case,
print callback has to be specified, so we detect which argument is
a callback. If it's 4th (last) argument, old implementation of API is
used by user code. If not, it must be 2nd, and thus new implementation
is selected. The rest is handled by the same symbol versioning approach.
btf_ext argument is dropped as it was never used and isn't necessary
either. If in the future we'll need btf_ext, that will be added into
OPTS-based struct btf_dump_opts.
struct btf_dump_opts is reused for both old API and new APIs. ctx field
is marked deprecated in v0.7+ and it's put at the same memory location
as OPTS's sz field. Any user of new-style btf_dump__new() will have to
set sz field and doesn't/shouldn't use ctx, as ctx is now passed along
the callback as mandatory input argument, following the other APIs in
libbpf that accept callbacks consistently.
Again, this is quite ugly in implementation, but is done in the name of
backwards compatibility and uniform and extensible future APIs (at the
same time, sigh). And it will be gone in libbpf 1.0.
[0] Closes: https://github.com/libbpf/libbpf/issues/283
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/btf.h | 51 ++++++++++++++++++++++++++++++++++++----
tools/lib/bpf/btf_dump.c | 31 +++++++++++++++++-------
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 6aae4f62ee0b..45310c65e865 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -267,15 +267,58 @@ LIBBPF_API int btf__dedup_deprecated(struct btf *btf, struct btf_ext *btf_ext, c
struct btf_dump;
struct btf_dump_opts {
- void *ctx;
+ union {
+ size_t sz;
+ void *ctx; /* DEPRECATED: will be gone in v1.0 */
+ };
};
typedef void (*btf_dump_printf_fn_t)(void *ctx, const char *fmt, va_list args);
LIBBPF_API struct btf_dump *btf_dump__new(const struct btf *btf,
- const struct btf_ext *btf_ext,
- const struct btf_dump_opts *opts,
- btf_dump_printf_fn_t printf_fn);
+ btf_dump_printf_fn_t printf_fn,
+ void *ctx,
+ const struct btf_dump_opts *opts);
+
+LIBBPF_API struct btf_dump *btf_dump__new_v0_6_0(const struct btf *btf,
+ btf_dump_printf_fn_t printf_fn,
+ void *ctx,
+ const struct btf_dump_opts *opts);
+
+LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
+ const struct btf_ext *btf_ext,
+ const struct btf_dump_opts *opts,
+ btf_dump_printf_fn_t printf_fn);
+
+/* Choose either btf_dump__new() or btf_dump__new_deprecated() based on the
+ * type of 4th argument. If it's btf_dump's print callback, use deprecated
+ * API; otherwise, choose the new btf_dump__new(). ___libbpf_override()
+ * doesn't work here because both variants have 4 input arguments.
+ *
+ * (void *) casts are necessary to avoid compilation warnings about type
+ * mismatches, because even though __builtin_choose_expr() only ever evaluates
+ * one side the other side still has to satisfy type constraints (this is
+ * compiler implementation limitation which might be lifted eventually,
+ * according to the documentation). So passing struct btf_ext in place of
+ * btf_dump_printf_fn_t would be generating compilation warning. Casting to
+ * void * avoids this issue.
+ *
+ * Also, two type compatibility checks for a function and function pointer are
+ * required because passing function reference into btf_dump__new() as
+ * btf_dump__new(..., my_callback, ...) and as btf_dump__new(...,
+ * &my_callback, ...) (not explicit ampersand in the latter case) actually
+ * differs as far as __builtin_types_compatible_p() is concerned. Thus two
+ * checks are combined to detect callback argument.
+ *
+ * The rest works just like in case of ___libbpf_override() usage with symbol
+ * versioning.
+ */
+#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
+ __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
+ __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
+ btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
+ btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
+
LIBBPF_API void btf_dump__free(struct btf_dump *d);
LIBBPF_API int btf_dump__dump_type(struct btf_dump *d, __u32 id);
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 17db62b5002e..b8cd7e4f557a 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -77,9 +77,8 @@ struct btf_dump_data {
struct btf_dump {
const struct btf *btf;
- const struct btf_ext *btf_ext;
btf_dump_printf_fn_t printf_fn;
- struct btf_dump_opts opts;
+ void *cb_ctx;
int ptr_sz;
bool strip_mods;
bool skip_anon_defs;
@@ -138,29 +137,32 @@ static void btf_dump_printf(const struct btf_dump *d, const char *fmt, ...)
va_list args;
va_start(args, fmt);
- d->printf_fn(d->opts.ctx, fmt, args);
+ d->printf_fn(d->cb_ctx, fmt, args);
va_end(args);
}
static int btf_dump_mark_referenced(struct btf_dump *d);
static int btf_dump_resize(struct btf_dump *d);
-struct btf_dump *btf_dump__new(const struct btf *btf,
- const struct btf_ext *btf_ext,
- const struct btf_dump_opts *opts,
- btf_dump_printf_fn_t printf_fn)
+DEFAULT_VERSION(btf_dump__new_v0_6_0, btf_dump__new, LIBBPF_0.6.0)
+struct btf_dump *btf_dump__new_v0_6_0(const struct btf *btf,
+ btf_dump_printf_fn_t printf_fn,
+ void *ctx,
+ const struct btf_dump_opts *opts)
{
struct btf_dump *d;
int err;
+ if (!printf_fn)
+ return libbpf_err_ptr(-EINVAL);
+
d = calloc(1, sizeof(struct btf_dump));
if (!d)
return libbpf_err_ptr(-ENOMEM);
d->btf = btf;
- d->btf_ext = btf_ext;
d->printf_fn = printf_fn;
- d->opts.ctx = opts ? opts->ctx : NULL;
+ d->cb_ctx = ctx;
d->ptr_sz = btf__pointer_size(btf) ? : sizeof(void *);
d->type_names = hashmap__new(str_hash_fn, str_equal_fn, NULL);
@@ -186,6 +188,17 @@ struct btf_dump *btf_dump__new(const struct btf *btf,
return libbpf_err_ptr(err);
}
+COMPAT_VERSION(btf_dump__new_deprecated, btf_dump__new, LIBBPF_0.0.4)
+struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
+ const struct btf_ext *btf_ext,
+ const struct btf_dump_opts *opts,
+ btf_dump_printf_fn_t printf_fn)
+{
+ if (!printf_fn)
+ return libbpf_err_ptr(-EINVAL);
+ return btf_dump__new_v0_6_0(btf, printf_fn, opts ? opts->ctx : NULL, opts);
+}
+
static int btf_dump_resize(struct btf_dump *d)
{
int err, last_id = btf__type_cnt(d->btf) - 1;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index edc40bc16f19..c51411d46c9e 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -405,4 +405,6 @@ LIBBPF_0.6.0 {
btf__dedup_deprecated;
btf__raw_data;
btf__type_cnt;
+ btf_dump__new;
+ btf_dump__new_deprecated;
} LIBBPF_0.5.0;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 5/9] libbpf: make perf_buffer__new() use OPTS-based interface
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (3 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 6/9] selftests/bpf: migrate all deprecated perf_buffer uses Andrii Nakryiko
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Add new variants of perf_buffer__new() and perf_buffer__new_raw() that
use OPTS-based options for future extensibility ([0]). Given all the
currently used API names are best fits, re-use them and use
___libbpf_override() approach and symbol versioning to preserve ABI and
source code compatibility. struct perf_buffer_opts and struct
perf_buffer_raw_opts are kept as well, but they are restructured such
that they are OPTS-based when used with new APIs. For struct
perf_buffer_raw_opts we keep few fields intact, so we have to also
preserve the memory location of them both when used as OPTS and for
legacy API variants. This is achieved with anonymous padding for OPTS
"incarnation" of the struct. These pads can be eventually used for new
options.
[0] Closes: https://github.com/libbpf/libbpf/issues/311
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 70 +++++++++++++++++++++++++-------
tools/lib/bpf/libbpf.h | 86 ++++++++++++++++++++++++++++++++++------
tools/lib/bpf/libbpf.map | 4 ++
3 files changed, 132 insertions(+), 28 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d869ebee1e27..cc08d677c5c0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10564,11 +10564,18 @@ perf_buffer__open_cpu_buf(struct perf_buffer *pb, struct perf_event_attr *attr,
static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
struct perf_buffer_params *p);
-struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
- const struct perf_buffer_opts *opts)
+DEFAULT_VERSION(perf_buffer__new_v0_6_0, perf_buffer__new, LIBBPF_0.6.0)
+struct perf_buffer *perf_buffer__new_v0_6_0(int map_fd, size_t page_cnt,
+ perf_buffer_sample_fn sample_cb,
+ perf_buffer_lost_fn lost_cb,
+ void *ctx,
+ const struct perf_buffer_opts *opts)
{
struct perf_buffer_params p = {};
- struct perf_event_attr attr = { 0, };
+ struct perf_event_attr attr = {};
+
+ if (!OPTS_VALID(opts, perf_buffer_opts))
+ return libbpf_err_ptr(-EINVAL);
attr.config = PERF_COUNT_SW_BPF_OUTPUT;
attr.type = PERF_TYPE_SOFTWARE;
@@ -10577,29 +10584,62 @@ struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
attr.wakeup_events = 1;
p.attr = &attr;
- p.sample_cb = opts ? opts->sample_cb : NULL;
- p.lost_cb = opts ? opts->lost_cb : NULL;
- p.ctx = opts ? opts->ctx : NULL;
+ p.sample_cb = sample_cb;
+ p.lost_cb = lost_cb;
+ p.ctx = ctx;
return libbpf_ptr(__perf_buffer__new(map_fd, page_cnt, &p));
}
-struct perf_buffer *
-perf_buffer__new_raw(int map_fd, size_t page_cnt,
- const struct perf_buffer_raw_opts *opts)
+COMPAT_VERSION(perf_buffer__new_deprecated, perf_buffer__new, LIBBPF_0.0.4)
+struct perf_buffer *perf_buffer__new_deprecated(int map_fd, size_t page_cnt,
+ const struct perf_buffer_opts *opts)
+{
+ return perf_buffer__new_v0_6_0(map_fd, page_cnt,
+ opts ? opts->sample_cb : NULL,
+ opts ? opts->lost_cb : NULL,
+ opts ? opts->ctx : NULL,
+ NULL);
+}
+
+DEFAULT_VERSION(perf_buffer__new_raw_v0_6_0, perf_buffer__new_raw, LIBBPF_0.6.0)
+struct perf_buffer *perf_buffer__new_raw_v0_6_0(int map_fd, size_t page_cnt,
+ struct perf_event_attr *attr,
+ perf_buffer_event_fn event_cb, void *ctx,
+ const struct perf_buffer_raw_opts *opts)
{
struct perf_buffer_params p = {};
- p.attr = opts->attr;
- p.event_cb = opts->event_cb;
- p.ctx = opts->ctx;
- p.cpu_cnt = opts->cpu_cnt;
- p.cpus = opts->cpus;
- p.map_keys = opts->map_keys;
+ if (page_cnt == 0 || !attr)
+ return libbpf_err_ptr(-EINVAL);
+
+ if (!OPTS_VALID(opts, perf_buffer_raw_opts))
+ return libbpf_err_ptr(-EINVAL);
+
+ p.attr = attr;
+ p.event_cb = event_cb;
+ p.ctx = ctx;
+ p.cpu_cnt = OPTS_GET(opts, cpu_cnt, 0);
+ p.cpus = OPTS_GET(opts, cpus, NULL);
+ p.map_keys = OPTS_GET(opts, map_keys, NULL);
return libbpf_ptr(__perf_buffer__new(map_fd, page_cnt, &p));
}
+COMPAT_VERSION(perf_buffer__new_raw_deprecated, perf_buffer__new_raw, LIBBPF_0.0.4)
+struct perf_buffer *perf_buffer__new_raw_deprecated(int map_fd, size_t page_cnt,
+ const struct perf_buffer_raw_opts *opts)
+{
+ LIBBPF_OPTS(perf_buffer_raw_opts, inner_opts,
+ .cpu_cnt = opts->cpu_cnt,
+ .cpus = opts->cpus,
+ .map_keys = opts->map_keys,
+ );
+
+ return perf_buffer__new_raw_v0_6_0(map_fd, page_cnt, opts->attr,
+ opts->event_cb, opts->ctx, &inner_opts);
+}
+
static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
struct perf_buffer_params *p)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 039058763173..47d4585a79bb 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -775,18 +775,52 @@ typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
/* common use perf buffer options */
struct perf_buffer_opts {
- /* if specified, sample_cb is called for each sample */
- perf_buffer_sample_fn sample_cb;
- /* if specified, lost_cb is called for each batch of lost samples */
- perf_buffer_lost_fn lost_cb;
- /* ctx is provided to sample_cb and lost_cb */
- void *ctx;
+ union {
+ size_t sz;
+ struct { /* DEPRECATED: will be removed in v1.0 */
+ /* if specified, sample_cb is called for each sample */
+ perf_buffer_sample_fn sample_cb;
+ /* if specified, lost_cb is called for each batch of lost samples */
+ perf_buffer_lost_fn lost_cb;
+ /* ctx is provided to sample_cb and lost_cb */
+ void *ctx;
+ };
+ };
};
+#define perf_buffer_opts__last_field sz
+/**
+ * @brief **perf_buffer__new()** creates BPF perfbuf manager for a specified
+ * BPF_PERF_EVENT_ARRAY map
+ * @param map_fd FD of BPF_PERF_EVENT_ARRAY BPF map that will be used by BPF
+ * code to send data over to user-space
+ * @param page_cnt number of memory pages allocated for each per-CPU buffer
+ * @param sample_cb function called on each received data record
+ * @param lost_cb function called when record loss has occurred
+ * @param ctx user-provided extra context passed into *sample_cb* and *lost_cb*
+ * @return a new instance of struct perf_buffer on success, NULL on error with
+ * *errno* containing an error code
+ */
LIBBPF_API struct perf_buffer *
perf_buffer__new(int map_fd, size_t page_cnt,
+ perf_buffer_sample_fn sample_cb, perf_buffer_lost_fn lost_cb, void *ctx,
const struct perf_buffer_opts *opts);
+LIBBPF_API struct perf_buffer *
+perf_buffer__new_v0_6_0(int map_fd, size_t page_cnt,
+ perf_buffer_sample_fn sample_cb, perf_buffer_lost_fn lost_cb, void *ctx,
+ const struct perf_buffer_opts *opts);
+
+LIBBPF_API LIBBPF_DEPRECATED_SINCE(0, 7, "use new variant of perf_buffer__new() instead")
+struct perf_buffer *perf_buffer__new_deprecated(int map_fd, size_t page_cnt,
+ const struct perf_buffer_opts *opts);
+
+#define perf_buffer__new(...) ___libbpf_overload(___perf_buffer_new, __VA_ARGS__)
+#define ___perf_buffer_new6(map_fd, page_cnt, sample_cb, lost_cb, ctx, opts) \
+ perf_buffer__new(map_fd, page_cnt, sample_cb, lost_cb, ctx, opts)
+#define ___perf_buffer_new3(map_fd, page_cnt, opts) \
+ perf_buffer__new_deprecated(map_fd, page_cnt, opts)
+
enum bpf_perf_event_ret {
LIBBPF_PERF_EVENT_DONE = 0,
LIBBPF_PERF_EVENT_ERROR = -1,
@@ -800,12 +834,21 @@ typedef enum bpf_perf_event_ret
/* raw perf buffer options, giving most power and control */
struct perf_buffer_raw_opts {
- /* perf event attrs passed directly into perf_event_open() */
- struct perf_event_attr *attr;
- /* raw event callback */
- perf_buffer_event_fn event_cb;
- /* ctx is provided to event_cb */
- void *ctx;
+ union {
+ struct {
+ size_t sz;
+ long :0;
+ long :0;
+ };
+ struct { /* DEPRECATED: will be removed in v1.0 */
+ /* perf event attrs passed directly into perf_event_open() */
+ struct perf_event_attr *attr;
+ /* raw event callback */
+ perf_buffer_event_fn event_cb;
+ /* ctx is provided to event_cb */
+ void *ctx;
+ };
+ };
/* if cpu_cnt == 0, open all on all possible CPUs (up to the number of
* max_entries of given PERF_EVENT_ARRAY map)
*/
@@ -815,11 +858,28 @@ struct perf_buffer_raw_opts {
/* if cpu_cnt > 0, map_keys specify map keys to set per-CPU FDs for */
int *map_keys;
};
+#define perf_buffer_raw_opts__last_field map_keys
LIBBPF_API struct perf_buffer *
-perf_buffer__new_raw(int map_fd, size_t page_cnt,
+perf_buffer__new_raw(int map_fd, size_t page_cnt, struct perf_event_attr *attr,
+ perf_buffer_event_fn event_cb, void *ctx,
const struct perf_buffer_raw_opts *opts);
+LIBBPF_API struct perf_buffer *
+perf_buffer__new_raw_v0_6_0(int map_fd, size_t page_cnt, struct perf_event_attr *attr,
+ perf_buffer_event_fn event_cb, void *ctx,
+ const struct perf_buffer_raw_opts *opts);
+
+LIBBPF_API LIBBPF_DEPRECATED_SINCE(0, 7, "use new variant of perf_buffer__new_raw() instead")
+struct perf_buffer *perf_buffer__new_raw_deprecated(int map_fd, size_t page_cnt,
+ const struct perf_buffer_raw_opts *opts);
+
+#define perf_buffer__new_raw(...) ___libbpf_overload(___perf_buffer_new_raw, __VA_ARGS__)
+#define ___perf_buffer_new_raw6(map_fd, page_cnt, attr, event_cb, ctx, opts) \
+ perf_buffer__new_raw(map_fd, page_cnt, attr, event_cb, ctx, opts)
+#define ___perf_buffer_new_raw3(map_fd, page_cnt, opts) \
+ perf_buffer__new_raw_deprecated(map_fd, page_cnt, opts)
+
LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
LIBBPF_API int perf_buffer__epoll_fd(const struct perf_buffer *pb);
LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index c51411d46c9e..19396e1c59f9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -407,4 +407,8 @@ LIBBPF_0.6.0 {
btf__type_cnt;
btf_dump__new;
btf_dump__new_deprecated;
+ perf_buffer__new;
+ perf_buffer__new_deprecated;
+ perf_buffer__new_raw;
+ perf_buffer__new_raw_deprecated;
} LIBBPF_0.5.0;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 6/9] selftests/bpf: migrate all deprecated perf_buffer uses
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (4 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 5/9] libbpf: make perf_buffer__new() use OPTS-based interface Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 7/9] selftests/bpf: update btf_dump__new() uses to v1.0+ variant Andrii Nakryiko
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Migrate all old-style perf_buffer__new() and perf_buffer__new_raw()
calls to new v1.0+ variants.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/benchs/bench_ringbufs.c | 8 ++------
tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 5 ++---
tools/testing/selftests/bpf/prog_tests/kfree_skb.c | 6 ++----
tools/testing/selftests/bpf/prog_tests/perf_buffer.c | 6 ++----
tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c | 7 ++-----
tools/testing/selftests/bpf/test_tcpnotify_user.c | 4 +---
6 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
index d167bffac679..52d4a2f91dbd 100644
--- a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
+++ b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
@@ -394,11 +394,6 @@ static void perfbuf_libbpf_setup()
{
struct perfbuf_libbpf_ctx *ctx = &perfbuf_libbpf_ctx;
struct perf_event_attr attr;
- struct perf_buffer_raw_opts pb_opts = {
- .event_cb = perfbuf_process_sample_raw,
- .ctx = (void *)(long)0,
- .attr = &attr,
- };
struct bpf_link *link;
ctx->skel = perfbuf_setup_skeleton();
@@ -423,7 +418,8 @@ static void perfbuf_libbpf_setup()
}
ctx->perfbuf = perf_buffer__new_raw(bpf_map__fd(ctx->skel->maps.perfbuf),
- args.perfbuf_sz, &pb_opts);
+ args.perfbuf_sz, &attr,
+ perfbuf_process_sample_raw, NULL, NULL);
if (!ctx->perfbuf) {
fprintf(stderr, "failed to create perfbuf\n");
exit(1);
diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
index 569fcc6ed660..4184c399d4c6 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
@@ -85,7 +85,6 @@ void test_get_stack_raw_tp(void)
const char *file_err = "./test_get_stack_rawtp_err.o";
const char *prog_name = "raw_tracepoint/sys_enter";
int i, err, prog_fd, exp_cnt = MAX_CNT_RAWTP;
- struct perf_buffer_opts pb_opts = {};
struct perf_buffer *pb = NULL;
struct bpf_link *link = NULL;
struct timespec tv = {0, 10};
@@ -124,8 +123,8 @@ void test_get_stack_raw_tp(void)
if (!ASSERT_OK_PTR(link, "attach_raw_tp"))
goto close_prog;
- pb_opts.sample_cb = get_stack_print_output;
- pb = perf_buffer__new(bpf_map__fd(map), 8, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(map), 8, get_stack_print_output,
+ NULL, NULL, NULL);
if (!ASSERT_OK_PTR(pb, "perf_buf__new"))
goto close_prog;
diff --git a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
index 885413ed5c96..2a49f8fcde06 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c
@@ -66,7 +66,6 @@ void serial_test_kfree_skb(void)
struct bpf_map *perf_buf_map, *global_data;
struct bpf_program *prog, *fentry, *fexit;
struct bpf_object *obj, *obj2 = NULL;
- struct perf_buffer_opts pb_opts = {};
struct perf_buffer *pb = NULL;
int err, kfree_skb_fd;
bool passed = false;
@@ -112,9 +111,8 @@ void serial_test_kfree_skb(void)
goto close_prog;
/* set up perf buffer */
- pb_opts.sample_cb = on_sample;
- pb_opts.ctx = &passed;
- pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1,
+ on_sample, NULL, &passed, NULL);
if (!ASSERT_OK_PTR(pb, "perf_buf__new"))
goto close_prog;
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
index 4e32f3586a75..5fc2b3a0711e 100644
--- a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -47,7 +47,6 @@ void serial_test_perf_buffer(void)
{
int err, on_len, nr_on_cpus = 0, nr_cpus, i, j;
int zero = 0, my_pid = getpid();
- struct perf_buffer_opts pb_opts = {};
struct test_perf_buffer *skel;
cpu_set_t cpu_seen;
struct perf_buffer *pb;
@@ -82,9 +81,8 @@ void serial_test_perf_buffer(void)
goto out_close;
/* set up perf buffer */
- pb_opts.sample_cb = on_sample;
- pb_opts.ctx = &cpu_seen;
- pb = perf_buffer__new(bpf_map__fd(skel->maps.perf_buf_map), 1, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(skel->maps.perf_buf_map), 1,
+ on_sample, NULL, &cpu_seen, NULL);
if (!ASSERT_OK_PTR(pb, "perf_buf__new"))
goto out_close;
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
index 3bd5904b4db5..f99386d1dc4c 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
@@ -49,7 +49,6 @@ void test_xdp_bpf2bpf(void)
struct vip key4 = {.protocol = 6, .family = AF_INET};
struct bpf_program *prog;
struct perf_buffer *pb = NULL;
- struct perf_buffer_opts pb_opts = {};
/* Load XDP program to introspect */
pkt_skel = test_xdp__open_and_load();
@@ -86,10 +85,8 @@ void test_xdp_bpf2bpf(void)
goto out;
/* Set up perf buffer */
- pb_opts.sample_cb = on_sample;
- pb_opts.ctx = &passed;
- pb = perf_buffer__new(bpf_map__fd(ftrace_skel->maps.perf_buf_map),
- 1, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(ftrace_skel->maps.perf_buf_map), 1,
+ on_sample, NULL, &passed, NULL);
if (!ASSERT_OK_PTR(pb, "perf_buf__new"))
goto out;
diff --git a/tools/testing/selftests/bpf/test_tcpnotify_user.c b/tools/testing/selftests/bpf/test_tcpnotify_user.c
index 63111cb082fe..4c5114765b23 100644
--- a/tools/testing/selftests/bpf/test_tcpnotify_user.c
+++ b/tools/testing/selftests/bpf/test_tcpnotify_user.c
@@ -72,7 +72,6 @@ int main(int argc, char **argv)
{
const char *file = "test_tcpnotify_kern.o";
struct bpf_map *perf_map, *global_map;
- struct perf_buffer_opts pb_opts = {};
struct tcpnotify_globals g = {0};
struct perf_buffer *pb = NULL;
const char *cg_path = "/foo";
@@ -117,8 +116,7 @@ int main(int argc, char **argv)
return -1;
}
- pb_opts.sample_cb = dummyfn;
- pb = perf_buffer__new(bpf_map__fd(perf_map), 8, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(perf_map), 8, dummyfn, NULL, NULL, NULL);
if (!pb)
goto err;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 7/9] selftests/bpf: update btf_dump__new() uses to v1.0+ variant
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (5 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 6/9] selftests/bpf: migrate all deprecated perf_buffer uses Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 8/9] tools/runqslower: update perf_buffer__new() calls Andrii Nakryiko
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Update to-be-deprecated forms of btf_dump__new().
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/btf_helpers.c | 4 +--
.../selftests/bpf/prog_tests/btf_dump.c | 33 ++++++++-----------
.../selftests/bpf/prog_tests/btf_split.c | 4 +--
3 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
index 3d1a748d09d8..acb59202486d 100644
--- a/tools/testing/selftests/bpf/btf_helpers.c
+++ b/tools/testing/selftests/bpf/btf_helpers.c
@@ -238,7 +238,6 @@ const char *btf_type_c_dump(const struct btf *btf)
static char buf[16 * 1024];
FILE *buf_file;
struct btf_dump *d = NULL;
- struct btf_dump_opts opts = {};
int err, i;
buf_file = fmemopen(buf, sizeof(buf) - 1, "w");
@@ -247,8 +246,7 @@ const char *btf_type_c_dump(const struct btf *btf)
return NULL;
}
- opts.ctx = buf_file;
- d = btf_dump__new(btf, NULL, &opts, btf_dump_printf);
+ d = btf_dump__new(btf, btf_dump_printf, buf_file, NULL);
if (libbpf_get_error(d)) {
fprintf(stderr, "Failed to create btf_dump instance: %ld\n", libbpf_get_error(d));
goto err_out;
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index a04961942dfa..d6272013a5a3 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -13,25 +13,23 @@ static struct btf_dump_test_case {
const char *name;
const char *file;
bool known_ptr_sz;
- struct btf_dump_opts opts;
} btf_dump_test_cases[] = {
- {"btf_dump: syntax", "btf_dump_test_case_syntax", true, {}},
- {"btf_dump: ordering", "btf_dump_test_case_ordering", false, {}},
- {"btf_dump: padding", "btf_dump_test_case_padding", true, {}},
- {"btf_dump: packing", "btf_dump_test_case_packing", true, {}},
- {"btf_dump: bitfields", "btf_dump_test_case_bitfields", true, {}},
- {"btf_dump: multidim", "btf_dump_test_case_multidim", false, {}},
- {"btf_dump: namespacing", "btf_dump_test_case_namespacing", false, {}},
+ {"btf_dump: syntax", "btf_dump_test_case_syntax", true},
+ {"btf_dump: ordering", "btf_dump_test_case_ordering", false},
+ {"btf_dump: padding", "btf_dump_test_case_padding", true},
+ {"btf_dump: packing", "btf_dump_test_case_packing", true},
+ {"btf_dump: bitfields", "btf_dump_test_case_bitfields", true},
+ {"btf_dump: multidim", "btf_dump_test_case_multidim", false},
+ {"btf_dump: namespacing", "btf_dump_test_case_namespacing", false},
};
-static int btf_dump_all_types(const struct btf *btf,
- const struct btf_dump_opts *opts)
+static int btf_dump_all_types(const struct btf *btf, void *ctx)
{
size_t type_cnt = btf__type_cnt(btf);
struct btf_dump *d;
int err = 0, id;
- d = btf_dump__new(btf, NULL, opts, btf_dump_printf);
+ d = btf_dump__new(btf, btf_dump_printf, ctx, NULL);
err = libbpf_get_error(d);
if (err)
return err;
@@ -88,8 +86,7 @@ static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
goto done;
}
- t->opts.ctx = f;
- err = btf_dump_all_types(btf, &t->opts);
+ err = btf_dump_all_types(btf, f);
fclose(f);
close(fd);
if (CHECK(err, "btf_dump", "failure during C dumping: %d\n", err)) {
@@ -137,7 +134,6 @@ static void test_btf_dump_incremental(void)
{
struct btf *btf = NULL;
struct btf_dump *d = NULL;
- struct btf_dump_opts opts;
int id, err, i;
dump_buf_file = open_memstream(&dump_buf, &dump_buf_sz);
@@ -146,8 +142,7 @@ static void test_btf_dump_incremental(void)
btf = btf__new_empty();
if (!ASSERT_OK_PTR(btf, "new_empty"))
goto err_out;
- opts.ctx = dump_buf_file;
- d = btf_dump__new(btf, NULL, &opts, btf_dump_printf);
+ d = btf_dump__new(btf, btf_dump_printf, dump_buf_file, NULL);
if (!ASSERT_OK(libbpf_get_error(d), "btf_dump__new"))
goto err_out;
@@ -815,7 +810,6 @@ static void test_btf_datasec(struct btf *btf, struct btf_dump *d, char *str,
static void test_btf_dump_datasec_data(char *str)
{
struct btf *btf;
- struct btf_dump_opts opts = { .ctx = str };
char license[4] = "GPL";
struct btf_dump *d;
@@ -823,7 +817,7 @@ static void test_btf_dump_datasec_data(char *str)
if (!ASSERT_OK_PTR(btf, "xdping_kern.o BTF not found"))
return;
- d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
+ d = btf_dump__new(btf, btf_dump_snprintf, str, NULL);
if (!ASSERT_OK_PTR(d, "could not create BTF dump"))
goto out;
@@ -837,7 +831,6 @@ static void test_btf_dump_datasec_data(char *str)
void test_btf_dump() {
char str[STRSIZE];
- struct btf_dump_opts opts = { .ctx = str };
struct btf_dump *d;
struct btf *btf;
int i;
@@ -857,7 +850,7 @@ void test_btf_dump() {
if (!ASSERT_OK_PTR(btf, "no kernel BTF found"))
return;
- d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
+ d = btf_dump__new(btf, btf_dump_snprintf, str, NULL);
if (!ASSERT_OK_PTR(d, "could not create BTF dump"))
return;
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
index b1ffe61f2aa9..eef1158676ed 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
@@ -13,7 +13,6 @@ static void btf_dump_printf(void *ctx, const char *fmt, va_list args)
}
void test_btf_split() {
- struct btf_dump_opts opts;
struct btf_dump *d = NULL;
const struct btf_type *t;
struct btf *btf1, *btf2;
@@ -68,8 +67,7 @@ void test_btf_split() {
dump_buf_file = open_memstream(&dump_buf, &dump_buf_sz);
if (!ASSERT_OK_PTR(dump_buf_file, "dump_memstream"))
return;
- opts.ctx = dump_buf_file;
- d = btf_dump__new(btf2, NULL, &opts, btf_dump_printf);
+ d = btf_dump__new(btf2, btf_dump_printf, dump_buf_file, NULL);
if (!ASSERT_OK_PTR(d, "btf_dump__new"))
goto cleanup;
for (i = 1; i < btf__type_cnt(btf2); i++) {
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 8/9] tools/runqslower: update perf_buffer__new() calls
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (6 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 7/9] selftests/bpf: update btf_dump__new() uses to v1.0+ variant Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 9/9] bpftool: update btf_dump__new() and perf_buffer__new_raw() calls Andrii Nakryiko
2021-11-12 0:59 ` [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Alexei Starovoitov
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Use v1.0+ compatible variant of perf_buffer__new() call to prepare for
deprecation.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/runqslower/runqslower.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/runqslower/runqslower.c b/tools/bpf/runqslower/runqslower.c
index d89715844952..2414cc764461 100644
--- a/tools/bpf/runqslower/runqslower.c
+++ b/tools/bpf/runqslower/runqslower.c
@@ -123,7 +123,6 @@ int main(int argc, char **argv)
.parser = parse_arg,
.doc = argp_program_doc,
};
- struct perf_buffer_opts pb_opts;
struct perf_buffer *pb = NULL;
struct runqslower_bpf *obj;
int err;
@@ -165,9 +164,8 @@ int main(int argc, char **argv)
printf("Tracing run queue latency higher than %llu us\n", env.min_us);
printf("%-8s %-16s %-6s %14s\n", "TIME", "COMM", "PID", "LAT(us)");
- pb_opts.sample_cb = handle_event;
- pb_opts.lost_cb = handle_lost_events;
- pb = perf_buffer__new(bpf_map__fd(obj->maps.events), 64, &pb_opts);
+ pb = perf_buffer__new(bpf_map__fd(obj->maps.events), 64,
+ handle_event, handle_lost_events, NULL, NULL);
err = libbpf_get_error(pb);
if (err) {
pb = NULL;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 bpf-next 9/9] bpftool: update btf_dump__new() and perf_buffer__new_raw() calls
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (7 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 8/9] tools/runqslower: update perf_buffer__new() calls Andrii Nakryiko
@ 2021-11-11 5:36 ` Andrii Nakryiko
2021-11-12 0:59 ` [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Alexei Starovoitov
9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-11 5:36 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Use v1.0-compatible variants of btf_dump and perf_buffer "constructors".
This is also a demonstration of reusing struct perf_buffer_raw_opts as
OPTS-style option struct for new perf_buffer__new_raw() API.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/btf.c | 2 +-
tools/bpf/bpftool/gen.c | 2 +-
tools/bpf/bpftool/map_perf_ring.c | 9 +++------
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 015d2758f826..223ac7676027 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -418,7 +418,7 @@ static int dump_btf_c(const struct btf *btf,
struct btf_dump *d;
int err = 0, i;
- d = btf_dump__new(btf, NULL, NULL, btf_dump_printf);
+ d = btf_dump__new(btf, btf_dump_printf, NULL, NULL);
if (IS_ERR(d))
return PTR_ERR(d);
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 5c18351290f0..89f0e828bbfa 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -218,7 +218,7 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
char sec_ident[256], map_ident[256];
int i, err = 0;
- d = btf_dump__new(btf, NULL, NULL, codegen_btf_dump_printf);
+ d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
if (IS_ERR(d))
return PTR_ERR(d);
diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c
index b98ea702d284..6b0c410152de 100644
--- a/tools/bpf/bpftool/map_perf_ring.c
+++ b/tools/bpf/bpftool/map_perf_ring.c
@@ -124,7 +124,7 @@ int do_event_pipe(int argc, char **argv)
.wakeup_events = 1,
};
struct bpf_map_info map_info = {};
- struct perf_buffer_raw_opts opts = {};
+ LIBBPF_OPTS(perf_buffer_raw_opts, opts);
struct event_pipe_ctx ctx = {
.all_cpus = true,
.cpu = -1,
@@ -190,14 +190,11 @@ int do_event_pipe(int argc, char **argv)
ctx.idx = 0;
}
- opts.attr = &perf_attr;
- opts.event_cb = print_bpf_output;
- opts.ctx = &ctx;
opts.cpu_cnt = ctx.all_cpus ? 0 : 1;
opts.cpus = &ctx.cpu;
opts.map_keys = &ctx.idx;
-
- pb = perf_buffer__new_raw(map_fd, MMAP_PAGE_CNT, &opts);
+ pb = perf_buffer__new_raw(map_fd, MMAP_PAGE_CNT, &perf_attr,
+ print_bpf_output, &ctx, &opts);
err = libbpf_get_error(pb);
if (err) {
p_err("failed to create perf buffer: %s (%d)",
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
` (8 preceding siblings ...)
2021-11-11 5:36 ` [PATCH v2 bpf-next 9/9] bpftool: update btf_dump__new() and perf_buffer__new_raw() calls Andrii Nakryiko
@ 2021-11-12 0:59 ` Alexei Starovoitov
9 siblings, 0 replies; 15+ messages in thread
From: Alexei Starovoitov @ 2021-11-12 0:59 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team
On Wed, Nov 10, 2021 at 09:36:15PM -0800, Andrii Nakryiko wrote:
> This patch set continues the work of revamping libbpf APIs that are not
> extensible, as they were added before we figured out all the intricacies of
> building APIs that can preserve ABI compatibility (both backward and forward).
>
> What makes them tricky is that (most of) these APIs are actively used by
> multiple applications, so we need to be careful about refactoring them. See
> individual patches for details, but the general approach is similar to
> previous bpf_prog_load() API revamp. The biggest different and complexity is
> in changing btf_dump__new(), because function overloading through macro magic
> doesn't work based on number of arguments, as both new and old APIs have
> 4 arguments. Because of that, another overloading approach is taken; overload
> happens based on argument types.
>
> I've validated manually (by using local test_progs-shared flavor that is
> compiling test_progs against libbpf as a shared library) that compiling "old
> application" (selftests before being adapted to using new variants of revamped
> APIs) are compiled and successfully run against newest libbpf version as well
> as the older libbpf version (provided no new variants are used). All these
> scenarios seem to be working as expected.
>
> v1->v2:
> - add explicit printf_fn NULL check in btf_dump__new() (Alexei);
> - replaced + with || in __builtin_choose_expr() (Alexei);
> - dropped test_progs-shared flavor (Alexei).
Applied, Thanks
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
2021-11-11 5:36 ` [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof Andrii Nakryiko
@ 2021-12-22 14:55 ` Jiri Olsa
2021-12-22 22:14 ` Andrii Nakryiko
0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2021-12-22 14:55 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team
On Wed, Nov 10, 2021 at 09:36:19PM -0800, Andrii Nakryiko wrote:
> Change btf_dump__new() and corresponding struct btf_dump_ops structure
> to be extensible by using OPTS "framework" ([0]). Given we don't change
> the names, we use a similar approach as with bpf_prog_load(), but this
> time we ended up with two APIs with the same name and same number of
> arguments, so overloading based on number of arguments with
> ___libbpf_override() doesn't work.
>
> Instead, use "overloading" based on types. In this particular case,
> print callback has to be specified, so we detect which argument is
> a callback. If it's 4th (last) argument, old implementation of API is
> used by user code. If not, it must be 2nd, and thus new implementation
> is selected. The rest is handled by the same symbol versioning approach.
>
> btf_ext argument is dropped as it was never used and isn't necessary
> either. If in the future we'll need btf_ext, that will be added into
> OPTS-based struct btf_dump_opts.
>
> struct btf_dump_opts is reused for both old API and new APIs. ctx field
> is marked deprecated in v0.7+ and it's put at the same memory location
> as OPTS's sz field. Any user of new-style btf_dump__new() will have to
> set sz field and doesn't/shouldn't use ctx, as ctx is now passed along
> the callback as mandatory input argument, following the other APIs in
> libbpf that accept callbacks consistently.
>
> Again, this is quite ugly in implementation, but is done in the name of
> backwards compatibility and uniform and extensible future APIs (at the
> same time, sigh). And it will be gone in libbpf 1.0.
>
> [0] Closes: https://github.com/libbpf/libbpf/issues/283
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/lib/bpf/btf.h | 51 ++++++++++++++++++++++++++++++++++++----
> tools/lib/bpf/btf_dump.c | 31 +++++++++++++++++-------
> tools/lib/bpf/libbpf.map | 2 ++
> 3 files changed, 71 insertions(+), 13 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 6aae4f62ee0b..45310c65e865 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -267,15 +267,58 @@ LIBBPF_API int btf__dedup_deprecated(struct btf *btf, struct btf_ext *btf_ext, c
> struct btf_dump;
>
> struct btf_dump_opts {
> - void *ctx;
> + union {
> + size_t sz;
> + void *ctx; /* DEPRECATED: will be gone in v1.0 */
> + };
> };
>
> typedef void (*btf_dump_printf_fn_t)(void *ctx, const char *fmt, va_list args);
>
> LIBBPF_API struct btf_dump *btf_dump__new(const struct btf *btf,
> - const struct btf_ext *btf_ext,
> - const struct btf_dump_opts *opts,
> - btf_dump_printf_fn_t printf_fn);
> + btf_dump_printf_fn_t printf_fn,
> + void *ctx,
> + const struct btf_dump_opts *opts);
> +
> +LIBBPF_API struct btf_dump *btf_dump__new_v0_6_0(const struct btf *btf,
> + btf_dump_printf_fn_t printf_fn,
> + void *ctx,
> + const struct btf_dump_opts *opts);
> +
> +LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
> + const struct btf_ext *btf_ext,
> + const struct btf_dump_opts *opts,
> + btf_dump_printf_fn_t printf_fn);
> +
> +/* Choose either btf_dump__new() or btf_dump__new_deprecated() based on the
> + * type of 4th argument. If it's btf_dump's print callback, use deprecated
> + * API; otherwise, choose the new btf_dump__new(). ___libbpf_override()
> + * doesn't work here because both variants have 4 input arguments.
> + *
> + * (void *) casts are necessary to avoid compilation warnings about type
> + * mismatches, because even though __builtin_choose_expr() only ever evaluates
> + * one side the other side still has to satisfy type constraints (this is
> + * compiler implementation limitation which might be lifted eventually,
> + * according to the documentation). So passing struct btf_ext in place of
> + * btf_dump_printf_fn_t would be generating compilation warning. Casting to
> + * void * avoids this issue.
> + *
> + * Also, two type compatibility checks for a function and function pointer are
> + * required because passing function reference into btf_dump__new() as
> + * btf_dump__new(..., my_callback, ...) and as btf_dump__new(...,
> + * &my_callback, ...) (not explicit ampersand in the latter case) actually
> + * differs as far as __builtin_types_compatible_p() is concerned. Thus two
> + * checks are combined to detect callback argument.
> + *
> + * The rest works just like in case of ___libbpf_override() usage with symbol
> + * versioning.
> + */
> +#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> + __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> + __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> + btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> + btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
hi,
this change breaks bpftrace g++ build that includes btf.h,
because there's no typeof and __builtin_types_compatible_p in c++
I guess there could be some c++ solution doing similar check,
but I wonder we want to polute btf.h with that, I'll need to
check on that
I think I can add some detection code to bpftrace, to find out
which version of btf_dump__new to use
the build error can be generated with test_cpp.cpp below,
so far I'm using __cplusplus ifdef in btf.h to workaround
the issue
thoughts?
thanks,
jirka
---
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 742a2bf71c5e..bd2d77979571 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -314,11 +314,13 @@ LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
* The rest works just like in case of ___libbpf_override() usage with symbol
* versioning.
*/
+#ifndef __cplusplus
#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
__builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
__builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
+#endif
LIBBPF_API void btf_dump__free(struct btf_dump *d);
diff --git a/tools/testing/selftests/bpf/test_cpp.cpp b/tools/testing/selftests/bpf/test_cpp.cpp
index a8d2e9a87fbf..e00201de2890 100644
--- a/tools/testing/selftests/bpf/test_cpp.cpp
+++ b/tools/testing/selftests/bpf/test_cpp.cpp
@@ -7,9 +7,15 @@
/* do nothing, just make sure we can link successfully */
+static void dump_printf(void *ctx, const char *fmt, va_list args)
+{
+}
+
int main(int argc, char *argv[])
{
+ struct btf_dump_opts opts = { };
struct test_core_extern *skel;
+ struct btf *btf;
/* libbpf.h */
libbpf_set_print(NULL);
@@ -18,7 +24,8 @@ int main(int argc, char *argv[])
bpf_prog_get_fd_by_id(0);
/* btf.h */
- btf__new(NULL, 0);
+ btf = btf__new(NULL, 0);
+ btf_dump__new(btf, dump_printf, nullptr, &opts);
/* BPF skeleton */
skel = test_core_extern__open_and_load();
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
2021-12-22 14:55 ` Jiri Olsa
@ 2021-12-22 22:14 ` Andrii Nakryiko
2021-12-23 10:23 ` Jiri Olsa
0 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2021-12-22 22:14 UTC (permalink / raw)
To: Jiri Olsa
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Wed, Dec 22, 2021 at 6:56 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, Nov 10, 2021 at 09:36:19PM -0800, Andrii Nakryiko wrote:
> > Change btf_dump__new() and corresponding struct btf_dump_ops structure
> > to be extensible by using OPTS "framework" ([0]). Given we don't change
> > the names, we use a similar approach as with bpf_prog_load(), but this
> > time we ended up with two APIs with the same name and same number of
> > arguments, so overloading based on number of arguments with
> > ___libbpf_override() doesn't work.
> >
> > Instead, use "overloading" based on types. In this particular case,
> > print callback has to be specified, so we detect which argument is
> > a callback. If it's 4th (last) argument, old implementation of API is
> > used by user code. If not, it must be 2nd, and thus new implementation
> > is selected. The rest is handled by the same symbol versioning approach.
> >
> > btf_ext argument is dropped as it was never used and isn't necessary
> > either. If in the future we'll need btf_ext, that will be added into
> > OPTS-based struct btf_dump_opts.
> >
> > struct btf_dump_opts is reused for both old API and new APIs. ctx field
> > is marked deprecated in v0.7+ and it's put at the same memory location
> > as OPTS's sz field. Any user of new-style btf_dump__new() will have to
> > set sz field and doesn't/shouldn't use ctx, as ctx is now passed along
> > the callback as mandatory input argument, following the other APIs in
> > libbpf that accept callbacks consistently.
> >
> > Again, this is quite ugly in implementation, but is done in the name of
> > backwards compatibility and uniform and extensible future APIs (at the
> > same time, sigh). And it will be gone in libbpf 1.0.
> >
> > [0] Closes: https://github.com/libbpf/libbpf/issues/283
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/lib/bpf/btf.h | 51 ++++++++++++++++++++++++++++++++++++----
> > tools/lib/bpf/btf_dump.c | 31 +++++++++++++++++-------
> > tools/lib/bpf/libbpf.map | 2 ++
> > 3 files changed, 71 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > index 6aae4f62ee0b..45310c65e865 100644
> > --- a/tools/lib/bpf/btf.h
> > +++ b/tools/lib/bpf/btf.h
> > @@ -267,15 +267,58 @@ LIBBPF_API int btf__dedup_deprecated(struct btf *btf, struct btf_ext *btf_ext, c
> > struct btf_dump;
> >
> > struct btf_dump_opts {
> > - void *ctx;
> > + union {
> > + size_t sz;
> > + void *ctx; /* DEPRECATED: will be gone in v1.0 */
> > + };
> > };
> >
> > typedef void (*btf_dump_printf_fn_t)(void *ctx, const char *fmt, va_list args);
> >
> > LIBBPF_API struct btf_dump *btf_dump__new(const struct btf *btf,
> > - const struct btf_ext *btf_ext,
> > - const struct btf_dump_opts *opts,
> > - btf_dump_printf_fn_t printf_fn);
> > + btf_dump_printf_fn_t printf_fn,
> > + void *ctx,
> > + const struct btf_dump_opts *opts);
> > +
> > +LIBBPF_API struct btf_dump *btf_dump__new_v0_6_0(const struct btf *btf,
> > + btf_dump_printf_fn_t printf_fn,
> > + void *ctx,
> > + const struct btf_dump_opts *opts);
> > +
> > +LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
> > + const struct btf_ext *btf_ext,
> > + const struct btf_dump_opts *opts,
> > + btf_dump_printf_fn_t printf_fn);
> > +
> > +/* Choose either btf_dump__new() or btf_dump__new_deprecated() based on the
> > + * type of 4th argument. If it's btf_dump's print callback, use deprecated
> > + * API; otherwise, choose the new btf_dump__new(). ___libbpf_override()
> > + * doesn't work here because both variants have 4 input arguments.
> > + *
> > + * (void *) casts are necessary to avoid compilation warnings about type
> > + * mismatches, because even though __builtin_choose_expr() only ever evaluates
> > + * one side the other side still has to satisfy type constraints (this is
> > + * compiler implementation limitation which might be lifted eventually,
> > + * according to the documentation). So passing struct btf_ext in place of
> > + * btf_dump_printf_fn_t would be generating compilation warning. Casting to
> > + * void * avoids this issue.
> > + *
> > + * Also, two type compatibility checks for a function and function pointer are
> > + * required because passing function reference into btf_dump__new() as
> > + * btf_dump__new(..., my_callback, ...) and as btf_dump__new(...,
> > + * &my_callback, ...) (not explicit ampersand in the latter case) actually
> > + * differs as far as __builtin_types_compatible_p() is concerned. Thus two
> > + * checks are combined to detect callback argument.
> > + *
> > + * The rest works just like in case of ___libbpf_override() usage with symbol
> > + * versioning.
> > + */
> > +#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> > + __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> > + __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> > + btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> > + btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
>
> hi,
> this change breaks bpftrace g++ build that includes btf.h,
> because there's no typeof and __builtin_types_compatible_p in c++
>
> I guess there could be some c++ solution doing similar check,
> but I wonder we want to polute btf.h with that, I'll need to
> check on that
>
> I think I can add some detection code to bpftrace, to find out
> which version of btf_dump__new to use
>
> the build error can be generated with test_cpp.cpp below,
> so far I'm using __cplusplus ifdef in btf.h to workaround
> the issue
>
> thoughts?
This has been reported before, see [0]. I think the simplest solution
is what you did, just to opt-out on __cplusplus.
Please send the below as a proper two patches and I'll apply them and
sync to Github. Also let's add a comment explaining that
__builtin_types_compatible_p doesn't work with C++ compiler to make it
clear why we do this.
In the [0] I was proposing the following comment, feel free to reuse it.
/* C++ compilers don't support __builtin_types_compatible_p(), so at least
* don't screw up compilation for them and let C++ users pick btf_dump__new vs
* btf_dump__new_deprecated explicitly.
*/
[0] https://github.com/libbpf/libbpf/issues/283#issuecomment-986100727
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 742a2bf71c5e..bd2d77979571 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -314,11 +314,13 @@ LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
> * The rest works just like in case of ___libbpf_override() usage with symbol
> * versioning.
> */
> +#ifndef __cplusplus
> #define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
> +#endif
>
> LIBBPF_API void btf_dump__free(struct btf_dump *d);
>
> diff --git a/tools/testing/selftests/bpf/test_cpp.cpp b/tools/testing/selftests/bpf/test_cpp.cpp
> index a8d2e9a87fbf..e00201de2890 100644
> --- a/tools/testing/selftests/bpf/test_cpp.cpp
> +++ b/tools/testing/selftests/bpf/test_cpp.cpp
> @@ -7,9 +7,15 @@
>
> /* do nothing, just make sure we can link successfully */
>
> +static void dump_printf(void *ctx, const char *fmt, va_list args)
> +{
> +}
> +
> int main(int argc, char *argv[])
> {
> + struct btf_dump_opts opts = { };
> struct test_core_extern *skel;
> + struct btf *btf;
>
> /* libbpf.h */
> libbpf_set_print(NULL);
> @@ -18,7 +24,8 @@ int main(int argc, char *argv[])
> bpf_prog_get_fd_by_id(0);
>
> /* btf.h */
> - btf__new(NULL, 0);
> + btf = btf__new(NULL, 0);
> + btf_dump__new(btf, dump_printf, nullptr, &opts);
>
> /* BPF skeleton */
> skel = test_core_extern__open_and_load();
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
2021-12-22 22:14 ` Andrii Nakryiko
@ 2021-12-23 10:23 ` Jiri Olsa
2021-12-23 13:16 ` Jiri Olsa
0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2021-12-23 10:23 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Wed, Dec 22, 2021 at 02:14:35PM -0800, Andrii Nakryiko wrote:
SNIP
> > > +/* Choose either btf_dump__new() or btf_dump__new_deprecated() based on the
> > > + * type of 4th argument. If it's btf_dump's print callback, use deprecated
> > > + * API; otherwise, choose the new btf_dump__new(). ___libbpf_override()
> > > + * doesn't work here because both variants have 4 input arguments.
> > > + *
> > > + * (void *) casts are necessary to avoid compilation warnings about type
> > > + * mismatches, because even though __builtin_choose_expr() only ever evaluates
> > > + * one side the other side still has to satisfy type constraints (this is
> > > + * compiler implementation limitation which might be lifted eventually,
> > > + * according to the documentation). So passing struct btf_ext in place of
> > > + * btf_dump_printf_fn_t would be generating compilation warning. Casting to
> > > + * void * avoids this issue.
> > > + *
> > > + * Also, two type compatibility checks for a function and function pointer are
> > > + * required because passing function reference into btf_dump__new() as
> > > + * btf_dump__new(..., my_callback, ...) and as btf_dump__new(...,
> > > + * &my_callback, ...) (not explicit ampersand in the latter case) actually
> > > + * differs as far as __builtin_types_compatible_p() is concerned. Thus two
> > > + * checks are combined to detect callback argument.
> > > + *
> > > + * The rest works just like in case of ___libbpf_override() usage with symbol
> > > + * versioning.
> > > + */
> > > +#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> > > + __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> > > + __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> > > + btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> > > + btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
> >
> > hi,
> > this change breaks bpftrace g++ build that includes btf.h,
> > because there's no typeof and __builtin_types_compatible_p in c++
> >
> > I guess there could be some c++ solution doing similar check,
> > but I wonder we want to polute btf.h with that, I'll need to
> > check on that
> >
> > I think I can add some detection code to bpftrace, to find out
> > which version of btf_dump__new to use
> >
> > the build error can be generated with test_cpp.cpp below,
> > so far I'm using __cplusplus ifdef in btf.h to workaround
> > the issue
> >
> > thoughts?
>
> This has been reported before, see [0]. I think the simplest solution
> is what you did, just to opt-out on __cplusplus.
>
> Please send the below as a proper two patches and I'll apply them and
> sync to Github. Also let's add a comment explaining that
> __builtin_types_compatible_p doesn't work with C++ compiler to make it
> clear why we do this.
ok, will send
thanks,
jirka
>
> In the [0] I was proposing the following comment, feel free to reuse it.
>
> /* C++ compilers don't support __builtin_types_compatible_p(), so at least
> * don't screw up compilation for them and let C++ users pick btf_dump__new vs
> * btf_dump__new_deprecated explicitly.
> */
>
> [0] https://github.com/libbpf/libbpf/issues/283#issuecomment-986100727
>
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > index 742a2bf71c5e..bd2d77979571 100644
> > --- a/tools/lib/bpf/btf.h
> > +++ b/tools/lib/bpf/btf.h
> > @@ -314,11 +314,13 @@ LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
> > * The rest works just like in case of ___libbpf_override() usage with symbol
> > * versioning.
> > */
> > +#ifndef __cplusplus
> > #define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> > __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> > __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> > btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> > btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
> > +#endif
> >
> > LIBBPF_API void btf_dump__free(struct btf_dump *d);
> >
> > diff --git a/tools/testing/selftests/bpf/test_cpp.cpp b/tools/testing/selftests/bpf/test_cpp.cpp
> > index a8d2e9a87fbf..e00201de2890 100644
> > --- a/tools/testing/selftests/bpf/test_cpp.cpp
> > +++ b/tools/testing/selftests/bpf/test_cpp.cpp
> > @@ -7,9 +7,15 @@
> >
> > /* do nothing, just make sure we can link successfully */
> >
> > +static void dump_printf(void *ctx, const char *fmt, va_list args)
> > +{
> > +}
> > +
> > int main(int argc, char *argv[])
> > {
> > + struct btf_dump_opts opts = { };
> > struct test_core_extern *skel;
> > + struct btf *btf;
> >
> > /* libbpf.h */
> > libbpf_set_print(NULL);
> > @@ -18,7 +24,8 @@ int main(int argc, char *argv[])
> > bpf_prog_get_fd_by_id(0);
> >
> > /* btf.h */
> > - btf__new(NULL, 0);
> > + btf = btf__new(NULL, 0);
> > + btf_dump__new(btf, dump_printf, nullptr, &opts);
> >
> > /* BPF skeleton */
> > skel = test_core_extern__open_and_load();
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof
2021-12-23 10:23 ` Jiri Olsa
@ 2021-12-23 13:16 ` Jiri Olsa
0 siblings, 0 replies; 15+ messages in thread
From: Jiri Olsa @ 2021-12-23 13:16 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Thu, Dec 23, 2021 at 11:23:13AM +0100, Jiri Olsa wrote:
> On Wed, Dec 22, 2021 at 02:14:35PM -0800, Andrii Nakryiko wrote:
>
> SNIP
>
> > > > +/* Choose either btf_dump__new() or btf_dump__new_deprecated() based on the
> > > > + * type of 4th argument. If it's btf_dump's print callback, use deprecated
> > > > + * API; otherwise, choose the new btf_dump__new(). ___libbpf_override()
> > > > + * doesn't work here because both variants have 4 input arguments.
> > > > + *
> > > > + * (void *) casts are necessary to avoid compilation warnings about type
> > > > + * mismatches, because even though __builtin_choose_expr() only ever evaluates
> > > > + * one side the other side still has to satisfy type constraints (this is
> > > > + * compiler implementation limitation which might be lifted eventually,
> > > > + * according to the documentation). So passing struct btf_ext in place of
> > > > + * btf_dump_printf_fn_t would be generating compilation warning. Casting to
> > > > + * void * avoids this issue.
> > > > + *
> > > > + * Also, two type compatibility checks for a function and function pointer are
> > > > + * required because passing function reference into btf_dump__new() as
> > > > + * btf_dump__new(..., my_callback, ...) and as btf_dump__new(...,
> > > > + * &my_callback, ...) (not explicit ampersand in the latter case) actually
> > > > + * differs as far as __builtin_types_compatible_p() is concerned. Thus two
> > > > + * checks are combined to detect callback argument.
> > > > + *
> > > > + * The rest works just like in case of ___libbpf_override() usage with symbol
> > > > + * versioning.
> > > > + */
> > > > +#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> > > > + __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> > > > + __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> > > > + btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> > > > + btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
> > >
> > > hi,
> > > this change breaks bpftrace g++ build that includes btf.h,
> > > because there's no typeof and __builtin_types_compatible_p in c++
> > >
> > > I guess there could be some c++ solution doing similar check,
> > > but I wonder we want to polute btf.h with that, I'll need to
> > > check on that
> > >
> > > I think I can add some detection code to bpftrace, to find out
> > > which version of btf_dump__new to use
> > >
> > > the build error can be generated with test_cpp.cpp below,
> > > so far I'm using __cplusplus ifdef in btf.h to workaround
> > > the issue
> > >
> > > thoughts?
> >
> > This has been reported before, see [0]. I think the simplest solution
> > is what you did, just to opt-out on __cplusplus.
> >
> > Please send the below as a proper two patches and I'll apply them and
> > sync to Github. Also let's add a comment explaining that
> > __builtin_types_compatible_p doesn't work with C++ compiler to make it
> > clear why we do this.
>
> ok, will send
I see you've already provided the same patch in the github comment,
so I'll put your Signed-off-by in the patch as well
jirka
>
> thanks,
> jirka
>
> >
> > In the [0] I was proposing the following comment, feel free to reuse it.
> >
> > /* C++ compilers don't support __builtin_types_compatible_p(), so at least
> > * don't screw up compilation for them and let C++ users pick btf_dump__new vs
> > * btf_dump__new_deprecated explicitly.
> > */
> >
> > [0] https://github.com/libbpf/libbpf/issues/283#issuecomment-986100727
> >
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > ---
> > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > > index 742a2bf71c5e..bd2d77979571 100644
> > > --- a/tools/lib/bpf/btf.h
> > > +++ b/tools/lib/bpf/btf.h
> > > @@ -314,11 +314,13 @@ LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
> > > * The rest works just like in case of ___libbpf_override() usage with symbol
> > > * versioning.
> > > */
> > > +#ifndef __cplusplus
> > > #define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
> > > __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
> > > __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
> > > btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
> > > btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
> > > +#endif
> > >
> > > LIBBPF_API void btf_dump__free(struct btf_dump *d);
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_cpp.cpp b/tools/testing/selftests/bpf/test_cpp.cpp
> > > index a8d2e9a87fbf..e00201de2890 100644
> > > --- a/tools/testing/selftests/bpf/test_cpp.cpp
> > > +++ b/tools/testing/selftests/bpf/test_cpp.cpp
> > > @@ -7,9 +7,15 @@
> > >
> > > /* do nothing, just make sure we can link successfully */
> > >
> > > +static void dump_printf(void *ctx, const char *fmt, va_list args)
> > > +{
> > > +}
> > > +
> > > int main(int argc, char *argv[])
> > > {
> > > + struct btf_dump_opts opts = { };
> > > struct test_core_extern *skel;
> > > + struct btf *btf;
> > >
> > > /* libbpf.h */
> > > libbpf_set_print(NULL);
> > > @@ -18,7 +24,8 @@ int main(int argc, char *argv[])
> > > bpf_prog_get_fd_by_id(0);
> > >
> > > /* btf.h */
> > > - btf__new(NULL, 0);
> > > + btf = btf__new(NULL, 0);
> > > + btf_dump__new(btf, dump_printf, nullptr, &opts);
> > >
> > > /* BPF skeleton */
> > > skel = test_core_extern__open_and_load();
> > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-12-23 13:16 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 5:36 [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 1/9] bpftool: normalize compile rules to specify output file last Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 2/9] selftests/bpf: minor cleanups and normalization of Makefile Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 3/9] libbpf: turn btf_dedup_opts into OPTS-based struct Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 4/9] libbpf: ensure btf_dump__new() and btf_dump_opts are future-proof Andrii Nakryiko
2021-12-22 14:55 ` Jiri Olsa
2021-12-22 22:14 ` Andrii Nakryiko
2021-12-23 10:23 ` Jiri Olsa
2021-12-23 13:16 ` Jiri Olsa
2021-11-11 5:36 ` [PATCH v2 bpf-next 5/9] libbpf: make perf_buffer__new() use OPTS-based interface Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 6/9] selftests/bpf: migrate all deprecated perf_buffer uses Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 7/9] selftests/bpf: update btf_dump__new() uses to v1.0+ variant Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 8/9] tools/runqslower: update perf_buffer__new() calls Andrii Nakryiko
2021-11-11 5:36 ` [PATCH v2 bpf-next 9/9] bpftool: update btf_dump__new() and perf_buffer__new_raw() calls Andrii Nakryiko
2021-11-12 0:59 ` [PATCH v2 bpf-next 0/9] Future-proof more tricky libbpf APIs Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).