hi,
this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
kprobe program through fprobe API [1] instroduced by Masami.
The fprobe API allows to attach probe on multiple functions at once very
fast, because it works on top of ftrace. On the other hand this limits
the probe point to the function entry or return.
With bpftrace support I see following attach speed:
# perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
Attaching 2 probes...
Attaching 3342 functions
...
1.4960 +- 0.0285 seconds time elapsed ( +- 1.91% )
v2 changes:
- based on latest fprobe changes [1]
- renaming the uapi interface to kprobe multi
- adding support for sort_r to pass user pointer for swap functions
and using that in cookie support to keep just single functions array
- moving new link to kernel/trace/bpf_trace.c file
- using single fprobe callback function for entry and exit
- using kvzalloc, libbpf_ensure_mem functions
- adding new k[ret]probe.multi sections instead of using current kprobe
- used glob_match from test_progs.c, added '?' matching
- move bpf_get_func_ip verifier inline change to seprate change
- couple of other minor fixes
Also available at:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
bpf/kprobe_multi
thanks,
jirka
[1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
---
Jiri Olsa (10):
lib/sort: Add priv pointer to swap function
bpf: Add multi kprobe link
bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
bpf: Add support to inline bpf_get_func_ip helper on x86
bpf: Add cookie support to programs attached with kprobe multi link
libbpf: Add libbpf_kallsyms_parse function
libbpf: Add bpf_link_create support for multi kprobes
libbpf: Add bpf_program__attach_kprobe_opts support for multi kprobes
selftest/bpf: Add kprobe_multi attach test
selftest/bpf: Add kprobe_multi test for bpf_cookie values
include/linux/bpf_types.h | 1 +
include/linux/sort.h | 4 +-
include/linux/trace_events.h | 6 ++
include/linux/types.h | 1 +
include/uapi/linux/bpf.h | 14 ++++
kernel/bpf/syscall.c | 26 ++++++--
kernel/bpf/verifier.c | 21 +++++-
kernel/trace/bpf_trace.c | 331 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
lib/sort.c | 42 +++++++++---
tools/include/uapi/linux/bpf.h | 14 ++++
tools/lib/bpf/bpf.c | 7 ++
tools/lib/bpf/bpf.h | 9 ++-
tools/lib/bpf/libbpf.c | 192 +++++++++++++++++++++++++++++++++++++++++++++--------
tools/lib/bpf/libbpf_internal.h | 5 ++
tools/testing/selftests/bpf/prog_tests/bpf_cookie.c | 72 ++++++++++++++++++++
tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 115 ++++++++++++++++++++++++++++++++
tools/testing/selftests/bpf/progs/kprobe_multi.c | 58 ++++++++++++++++
tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c | 62 +++++++++++++++++
18 files changed, 930 insertions(+), 50 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c
create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
Adding support to call bpf_get_func_ip helper from kprobe
programs attached by multi kprobe link.
Signed-off-by: Jiri Olsa <[email protected]>
---
kernel/trace/bpf_trace.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index df3771bfd6e5..64891b7b0885 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1037,6 +1037,18 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe = {
.arg1_type = ARG_PTR_TO_CTX,
};
+BPF_CALL_1(bpf_get_func_ip_kprobe_multi, struct pt_regs *, regs)
+{
+ return instruction_pointer(regs);
+}
+
+static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
+ .func = bpf_get_func_ip_kprobe_multi,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+};
+
BPF_CALL_1(bpf_get_attach_cookie_trace, void *, ctx)
{
struct bpf_trace_run_ctx *run_ctx;
@@ -1280,7 +1292,9 @@ kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_override_return_proto;
#endif
case BPF_FUNC_get_func_ip:
- return &bpf_get_func_ip_proto_kprobe;
+ return prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
+ &bpf_get_func_ip_proto_kprobe_multi :
+ &bpf_get_func_ip_proto_kprobe;
case BPF_FUNC_get_attach_cookie:
return &bpf_get_attach_cookie_proto_trace;
default:
--
2.35.1
Adding bpf_cookie test for programs attached by kprobe_multi links.
Signed-off-by: Jiri Olsa <[email protected]>
---
.../selftests/bpf/prog_tests/bpf_cookie.c | 72 +++++++++++++++++++
.../bpf/progs/kprobe_multi_bpf_cookie.c | 62 ++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index cd10df6cd0fc..edfb9f8736c6 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -7,6 +7,7 @@
#include <unistd.h>
#include <test_progs.h>
#include "test_bpf_cookie.skel.h"
+#include "kprobe_multi_bpf_cookie.skel.h"
/* uprobe attach point */
static void trigger_func(void)
@@ -63,6 +64,75 @@ static void kprobe_subtest(struct test_bpf_cookie *skel)
bpf_link__destroy(retlink2);
}
+static void kprobe_multi_subtest(void)
+{
+ DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
+ int err, prog_fd, link1_fd = -1, link2_fd = -1;
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ struct kprobe_multi_bpf_cookie *skel = NULL;
+ __u64 addrs[8], cookies[8];
+
+ skel = kprobe_multi_bpf_cookie__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+ goto cleanup;
+
+ kallsyms_find("bpf_fentry_test1", &addrs[0]);
+ kallsyms_find("bpf_fentry_test2", &addrs[1]);
+ kallsyms_find("bpf_fentry_test3", &addrs[2]);
+ kallsyms_find("bpf_fentry_test4", &addrs[3]);
+ kallsyms_find("bpf_fentry_test5", &addrs[4]);
+ kallsyms_find("bpf_fentry_test6", &addrs[5]);
+ kallsyms_find("bpf_fentry_test7", &addrs[6]);
+ kallsyms_find("bpf_fentry_test8", &addrs[7]);
+
+ cookies[0] = 1;
+ cookies[1] = 2;
+ cookies[2] = 3;
+ cookies[3] = 4;
+ cookies[4] = 5;
+ cookies[5] = 6;
+ cookies[6] = 7;
+ cookies[7] = 8;
+
+ opts.kprobe_multi.addrs = ptr_to_u64(&addrs);
+ opts.kprobe_multi.cnt = 8;
+ opts.kprobe_multi.cookies = ptr_to_u64(&cookies);
+ prog_fd = bpf_program__fd(skel->progs.test2);
+
+ link1_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, &opts);
+ if (!ASSERT_GE(link1_fd, 0, "link1_fd"))
+ return;
+
+ cookies[0] = 8;
+ cookies[1] = 7;
+ cookies[2] = 6;
+ cookies[3] = 5;
+ cookies[4] = 4;
+ cookies[5] = 3;
+ cookies[6] = 2;
+ cookies[7] = 1;
+
+ opts.flags = BPF_F_KPROBE_MULTI_RETURN;
+ prog_fd = bpf_program__fd(skel->progs.test3);
+
+ link2_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, &opts);
+ if (!ASSERT_GE(link2_fd, 0, "link2_fd"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.test1);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ ASSERT_EQ(skel->bss->test2_result, 8, "test2_result");
+ ASSERT_EQ(skel->bss->test3_result, 8, "test3_result");
+
+cleanup:
+ close(link1_fd);
+ close(link2_fd);
+ kprobe_multi_bpf_cookie__destroy(skel);
+}
+
static void uprobe_subtest(struct test_bpf_cookie *skel)
{
DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
@@ -249,6 +319,8 @@ void test_bpf_cookie(void)
if (test__start_subtest("kprobe"))
kprobe_subtest(skel);
+ if (test__start_subtest("multi_kprobe"))
+ kprobe_multi_subtest();
if (test__start_subtest("uprobe"))
uprobe_subtest(skel);
if (test__start_subtest("tracepoint"))
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
new file mode 100644
index 000000000000..d6f8454ba093
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+extern const void bpf_fentry_test1 __ksym;
+extern const void bpf_fentry_test2 __ksym;
+extern const void bpf_fentry_test3 __ksym;
+extern const void bpf_fentry_test4 __ksym;
+extern const void bpf_fentry_test5 __ksym;
+extern const void bpf_fentry_test6 __ksym;
+extern const void bpf_fentry_test7 __ksym;
+extern const void bpf_fentry_test8 __ksym;
+
+/* No tests, just to trigger bpf_fentry_test* through tracing test_run */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(test1)
+{
+ return 0;
+}
+
+__u64 test2_result = 0;
+
+SEC("kprobe.multi/bpf_fentry_tes??")
+int test2(struct pt_regs *ctx)
+{
+ __u64 cookie = bpf_get_attach_cookie(ctx);
+ __u64 addr = bpf_get_func_ip(ctx);
+
+ test2_result += (const void *) addr == &bpf_fentry_test1 && cookie == 1;
+ test2_result += (const void *) addr == &bpf_fentry_test2 && cookie == 2;
+ test2_result += (const void *) addr == &bpf_fentry_test3 && cookie == 3;
+ test2_result += (const void *) addr == &bpf_fentry_test4 && cookie == 4;
+ test2_result += (const void *) addr == &bpf_fentry_test5 && cookie == 5;
+ test2_result += (const void *) addr == &bpf_fentry_test6 && cookie == 6;
+ test2_result += (const void *) addr == &bpf_fentry_test7 && cookie == 7;
+ test2_result += (const void *) addr == &bpf_fentry_test8 && cookie == 8;
+
+ return 0;
+}
+
+__u64 test3_result = 0;
+
+SEC("kretprobe.multi/bpf_fentry_test*")
+int test3(struct pt_regs *ctx)
+{
+ __u64 cookie = bpf_get_attach_cookie(ctx);
+ __u64 addr = bpf_get_func_ip(ctx);
+
+ test3_result += (const void *) addr == &bpf_fentry_test1 && cookie == 8;
+ test3_result += (const void *) addr == &bpf_fentry_test2 && cookie == 7;
+ test3_result += (const void *) addr == &bpf_fentry_test3 && cookie == 6;
+ test3_result += (const void *) addr == &bpf_fentry_test4 && cookie == 5;
+ test3_result += (const void *) addr == &bpf_fentry_test5 && cookie == 4;
+ test3_result += (const void *) addr == &bpf_fentry_test6 && cookie == 3;
+ test3_result += (const void *) addr == &bpf_fentry_test7 && cookie == 2;
+ test3_result += (const void *) addr == &bpf_fentry_test8 && cookie == 1;
+
+ return 0;
+}
--
2.35.1
Adding kprobe_multi attach test that uses new fprobe interface to
attach kprobe program to multiple functions.
The test is attaching programs to bpf_fentry_test* functions and
uses single trampoline program bpf_prog_test_run to trigger
bpf_fentry_test* functions.
Signed-off-by: Jiri Olsa <[email protected]>
---
.../bpf/prog_tests/kprobe_multi_test.c | 115 ++++++++++++++++++
.../selftests/bpf/progs/kprobe_multi.c | 58 +++++++++
2 files changed, 173 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
new file mode 100644
index 000000000000..f5567c3865d4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "kprobe_multi.skel.h"
+#include "trace_helpers.h"
+
+static void test_skel_api(void)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ struct kprobe_multi *skel = NULL;
+ int err, prog_fd;
+
+ skel = kprobe_multi__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "kprobe_multi__open_and_load"))
+ goto cleanup;
+
+ err = kprobe_multi__attach(skel);
+ if (!ASSERT_OK(err, "kprobe_multi__attach"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.test1);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ ASSERT_EQ(skel->bss->test2_result, 8, "test2_result");
+ ASSERT_EQ(skel->bss->test3_result, 8, "test3_result");
+
+cleanup:
+ kprobe_multi__destroy(skel);
+}
+
+static void test_link_api(struct bpf_link_create_opts *opts)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ int err, prog_fd, link1_fd = -1, link2_fd = -1;
+ struct kprobe_multi *skel = NULL;
+
+ skel = kprobe_multi__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.test2);
+ link1_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, opts);
+ if (!ASSERT_GE(link1_fd, 0, "link_fd"))
+ goto cleanup;
+
+ opts->kprobe_multi.flags = BPF_F_KPROBE_MULTI_RETURN;
+ prog_fd = bpf_program__fd(skel->progs.test3);
+ link2_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, opts);
+ if (!ASSERT_GE(link2_fd, 0, "link_fd"))
+ goto cleanup;
+
+ skel->bss->test2_result = 0;
+ skel->bss->test3_result = 0;
+
+ prog_fd = bpf_program__fd(skel->progs.test1);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ ASSERT_EQ(skel->bss->test2_result, 8, "test2_result");
+ ASSERT_EQ(skel->bss->test3_result, 8, "test3_result");
+
+cleanup:
+ if (link1_fd != -1)
+ close(link1_fd);
+ if (link2_fd != -1)
+ close(link2_fd);
+ kprobe_multi__destroy(skel);
+}
+
+static void test_link_api_addrs(void)
+{
+ DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
+ __u64 addrs[8];
+
+ kallsyms_find("bpf_fentry_test1", &addrs[0]);
+ kallsyms_find("bpf_fentry_test2", &addrs[1]);
+ kallsyms_find("bpf_fentry_test3", &addrs[2]);
+ kallsyms_find("bpf_fentry_test4", &addrs[3]);
+ kallsyms_find("bpf_fentry_test5", &addrs[4]);
+ kallsyms_find("bpf_fentry_test6", &addrs[5]);
+ kallsyms_find("bpf_fentry_test7", &addrs[6]);
+ kallsyms_find("bpf_fentry_test8", &addrs[7]);
+
+ opts.kprobe_multi.addrs = (__u64) addrs;
+ opts.kprobe_multi.cnt = 8;
+ test_link_api(&opts);
+}
+
+static void test_link_api_syms(void)
+{
+ DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
+ const char *syms[8] = {
+ "bpf_fentry_test1",
+ "bpf_fentry_test2",
+ "bpf_fentry_test3",
+ "bpf_fentry_test4",
+ "bpf_fentry_test5",
+ "bpf_fentry_test6",
+ "bpf_fentry_test7",
+ "bpf_fentry_test8",
+ };
+
+ opts.kprobe_multi.syms = (__u64) syms;
+ opts.kprobe_multi.cnt = 8;
+ test_link_api(&opts);
+}
+
+void test_kprobe_multi_test(void)
+{
+ test_skel_api();
+ test_link_api_syms();
+ test_link_api_addrs();
+}
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi.c b/tools/testing/selftests/bpf/progs/kprobe_multi.c
new file mode 100644
index 000000000000..71318c65931c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+extern const void bpf_fentry_test1 __ksym;
+extern const void bpf_fentry_test2 __ksym;
+extern const void bpf_fentry_test3 __ksym;
+extern const void bpf_fentry_test4 __ksym;
+extern const void bpf_fentry_test5 __ksym;
+extern const void bpf_fentry_test6 __ksym;
+extern const void bpf_fentry_test7 __ksym;
+extern const void bpf_fentry_test8 __ksym;
+
+/* No tests, just to trigger bpf_fentry_test* through tracing test_run */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(test1)
+{
+ return 0;
+}
+
+__u64 test2_result = 0;
+
+SEC("kprobe.multi/bpf_fentry_test?")
+int test2(struct pt_regs *ctx)
+{
+ __u64 addr = bpf_get_func_ip(ctx);
+
+ test2_result += (const void *) addr == &bpf_fentry_test1 ||
+ (const void *) addr == &bpf_fentry_test2 ||
+ (const void *) addr == &bpf_fentry_test3 ||
+ (const void *) addr == &bpf_fentry_test4 ||
+ (const void *) addr == &bpf_fentry_test5 ||
+ (const void *) addr == &bpf_fentry_test6 ||
+ (const void *) addr == &bpf_fentry_test7 ||
+ (const void *) addr == &bpf_fentry_test8;
+ return 0;
+}
+
+__u64 test3_result = 0;
+
+SEC("kretprobe.multi/bpf_fentry_test*")
+int test3(struct pt_regs *ctx)
+{
+ __u64 addr = bpf_get_func_ip(ctx);
+
+ test3_result += (const void *) addr == &bpf_fentry_test1 ||
+ (const void *) addr == &bpf_fentry_test2 ||
+ (const void *) addr == &bpf_fentry_test3 ||
+ (const void *) addr == &bpf_fentry_test4 ||
+ (const void *) addr == &bpf_fentry_test5 ||
+ (const void *) addr == &bpf_fentry_test6 ||
+ (const void *) addr == &bpf_fentry_test7 ||
+ (const void *) addr == &bpf_fentry_test8;
+ return 0;
+}
--
2.35.1
Move the kallsyms parsing in internal libbpf_kallsyms_parse
function, so it can be used from other places.
It will be used in following changes.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++-------------
tools/lib/bpf/libbpf_internal.h | 5 +++
2 files changed, 43 insertions(+), 24 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad43b6ce825e..fb530b004a0d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7172,12 +7172,10 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj)
return 0;
}
-static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
+int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
{
char sym_type, sym_name[500];
unsigned long long sym_addr;
- const struct btf_type *t;
- struct extern_desc *ext;
int ret, err = 0;
FILE *f;
@@ -7196,35 +7194,51 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
if (ret != 3) {
pr_warn("failed to read kallsyms entry: %d\n", ret);
err = -EINVAL;
- goto out;
+ break;
}
- ext = find_extern_by_name(obj, sym_name);
- if (!ext || ext->type != EXT_KSYM)
- continue;
-
- t = btf__type_by_id(obj->btf, ext->btf_id);
- if (!btf_is_var(t))
- continue;
-
- if (ext->is_set && ext->ksym.addr != sym_addr) {
- pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
- sym_name, ext->ksym.addr, sym_addr);
- err = -EINVAL;
- goto out;
- }
- if (!ext->is_set) {
- ext->is_set = true;
- ext->ksym.addr = sym_addr;
- pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
- }
+ err = cb(sym_addr, sym_type, sym_name, ctx);
+ if (err)
+ break;
}
-out:
fclose(f);
return err;
}
+static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
+ const char *sym_name, void *ctx)
+{
+ struct bpf_object *obj = ctx;
+ const struct btf_type *t;
+ struct extern_desc *ext;
+
+ ext = find_extern_by_name(obj, sym_name);
+ if (!ext || ext->type != EXT_KSYM)
+ return 0;
+
+ t = btf__type_by_id(obj->btf, ext->btf_id);
+ if (!btf_is_var(t))
+ return 0;
+
+ if (ext->is_set && ext->ksym.addr != sym_addr) {
+ pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
+ sym_name, ext->ksym.addr, sym_addr);
+ return -EINVAL;
+ }
+ if (!ext->is_set) {
+ ext->is_set = true;
+ ext->ksym.addr = sym_addr;
+ pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
+ }
+ return 0;
+}
+
+static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
+{
+ return libbpf_kallsyms_parse(kallsyms_cb, obj);
+}
+
static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
__u16 kind, struct btf **res_btf,
struct module_btf **res_mod_btf)
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4fda8bdf0a0d..b6247dc7f8eb 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -449,6 +449,11 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
extern enum libbpf_strict_mode libbpf_mode;
+typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
+ const char *sym_name, void *ctx);
+
+int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
+
/* handle direct returned errors */
static inline int libbpf_err(int ret)
{
--
2.35.1
Adding support to have priv pointer in swap callback function.
Following the initial change on cmp callback functions [1]
and adding SWAP_WRAPPER macro to identify sort call of sort_r.
Cc: Rasmus Villemoes <[email protected]>
[1] 4333fb96ca10 ("media: lib/sort.c: implement sort() variant taking context argument")
Signed-off-by: Jiri Olsa <[email protected]>
---
include/linux/sort.h | 2 +-
include/linux/types.h | 1 +
lib/sort.c | 40 ++++++++++++++++++++++++++++++----------
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/include/linux/sort.h b/include/linux/sort.h
index b5898725fe9d..e163287ac6c1 100644
--- a/include/linux/sort.h
+++ b/include/linux/sort.h
@@ -6,7 +6,7 @@
void sort_r(void *base, size_t num, size_t size,
cmp_r_func_t cmp_func,
- swap_func_t swap_func,
+ swap_r_func_t swap_func,
const void *priv);
void sort(void *base, size_t num, size_t size,
diff --git a/include/linux/types.h b/include/linux/types.h
index ac825ad90e44..ea8cf60a8a79 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -226,6 +226,7 @@ struct callback_head {
typedef void (*rcu_callback_t)(struct rcu_head *head);
typedef void (*call_rcu_func_t)(struct rcu_head *head, rcu_callback_t func);
+typedef void (*swap_r_func_t)(void *a, void *b, int size, const void *priv);
typedef void (*swap_func_t)(void *a, void *b, int size);
typedef int (*cmp_r_func_t)(const void *a, const void *b, const void *priv);
diff --git a/lib/sort.c b/lib/sort.c
index aa18153864d2..b399bf10d675 100644
--- a/lib/sort.c
+++ b/lib/sort.c
@@ -122,16 +122,27 @@ static void swap_bytes(void *a, void *b, size_t n)
* a pointer, but small integers make for the smallest compare
* instructions.
*/
-#define SWAP_WORDS_64 (swap_func_t)0
-#define SWAP_WORDS_32 (swap_func_t)1
-#define SWAP_BYTES (swap_func_t)2
+#define SWAP_WORDS_64 (swap_r_func_t)0
+#define SWAP_WORDS_32 (swap_r_func_t)1
+#define SWAP_BYTES (swap_r_func_t)2
+#define SWAP_WRAPPER (swap_r_func_t)3
+
+struct wrapper {
+ cmp_func_t cmp;
+ swap_func_t swap;
+};
/*
* The function pointer is last to make tail calls most efficient if the
* compiler decides not to inline this function.
*/
-static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
+static void do_swap(void *a, void *b, size_t size, swap_r_func_t swap_func, const void *priv)
{
+ if (swap_func == SWAP_WRAPPER) {
+ ((const struct wrapper *)priv)->swap(a, b, (int)size);
+ return;
+ }
+
if (swap_func == SWAP_WORDS_64)
swap_words_64(a, b, size);
else if (swap_func == SWAP_WORDS_32)
@@ -139,7 +150,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
else if (swap_func == SWAP_BYTES)
swap_bytes(a, b, size);
else
- swap_func(a, b, (int)size);
+ swap_func(a, b, (int)size, priv);
}
#define _CMP_WRAPPER ((cmp_r_func_t)0L)
@@ -147,7 +158,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
static int do_cmp(const void *a, const void *b, cmp_r_func_t cmp, const void *priv)
{
if (cmp == _CMP_WRAPPER)
- return ((cmp_func_t)(priv))(a, b);
+ return ((const struct wrapper *)priv)->cmp(a, b);
return cmp(a, b, priv);
}
@@ -198,7 +209,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)
*/
void sort_r(void *base, size_t num, size_t size,
cmp_r_func_t cmp_func,
- swap_func_t swap_func,
+ swap_r_func_t swap_func,
const void *priv)
{
/* pre-scale counters for performance */
@@ -208,6 +219,10 @@ void sort_r(void *base, size_t num, size_t size,
if (!a) /* num < 2 || size == 0 */
return;
+ /* called from 'sort' without swap function, let's pick the default */
+ if (swap_func == SWAP_WRAPPER && !((struct wrapper *)priv)->swap)
+ swap_func = NULL;
+
if (!swap_func) {
if (is_aligned(base, size, 8))
swap_func = SWAP_WORDS_64;
@@ -230,7 +245,7 @@ void sort_r(void *base, size_t num, size_t size,
if (a) /* Building heap: sift down --a */
a -= size;
else if (n -= size) /* Sorting: Extract root to --n */
- do_swap(base, base + n, size, swap_func);
+ do_swap(base, base + n, size, swap_func, priv);
else /* Sort complete */
break;
@@ -257,7 +272,7 @@ void sort_r(void *base, size_t num, size_t size,
c = b; /* Where "a" belongs */
while (b != a) { /* Shift it into place */
b = parent(b, lsbit, size);
- do_swap(base + b, base + c, size, swap_func);
+ do_swap(base + b, base + c, size, swap_func, priv);
}
}
}
@@ -267,6 +282,11 @@ void sort(void *base, size_t num, size_t size,
cmp_func_t cmp_func,
swap_func_t swap_func)
{
- return sort_r(base, num, size, _CMP_WRAPPER, swap_func, cmp_func);
+ struct wrapper w = {
+ .cmp = cmp_func,
+ .swap = swap_func,
+ };
+
+ return sort_r(base, num, size, _CMP_WRAPPER, SWAP_WRAPPER, &w);
}
EXPORT_SYMBOL(sort);
--
2.35.1
On Tue, 22 Feb 2022 18:05:51 +0100
Jiri Olsa <[email protected]> wrote:
> Adding support to have priv pointer in swap callback function.
>
> Following the initial change on cmp callback functions [1]
> and adding SWAP_WRAPPER macro to identify sort call of sort_r.
>
This looks good to me.
Reviewed-by: Masami Hiramatsu <[email protected]>
Thank you,
> Cc: Rasmus Villemoes <[email protected]>
> [1] 4333fb96ca10 ("media: lib/sort.c: implement sort() variant taking context argument")
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> include/linux/sort.h | 2 +-
> include/linux/types.h | 1 +
> lib/sort.c | 40 ++++++++++++++++++++++++++++++----------
> 3 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/sort.h b/include/linux/sort.h
> index b5898725fe9d..e163287ac6c1 100644
> --- a/include/linux/sort.h
> +++ b/include/linux/sort.h
> @@ -6,7 +6,7 @@
>
> void sort_r(void *base, size_t num, size_t size,
> cmp_r_func_t cmp_func,
> - swap_func_t swap_func,
> + swap_r_func_t swap_func,
> const void *priv);
>
> void sort(void *base, size_t num, size_t size,
> diff --git a/include/linux/types.h b/include/linux/types.h
> index ac825ad90e44..ea8cf60a8a79 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -226,6 +226,7 @@ struct callback_head {
> typedef void (*rcu_callback_t)(struct rcu_head *head);
> typedef void (*call_rcu_func_t)(struct rcu_head *head, rcu_callback_t func);
>
> +typedef void (*swap_r_func_t)(void *a, void *b, int size, const void *priv);
> typedef void (*swap_func_t)(void *a, void *b, int size);
>
> typedef int (*cmp_r_func_t)(const void *a, const void *b, const void *priv);
> diff --git a/lib/sort.c b/lib/sort.c
> index aa18153864d2..b399bf10d675 100644
> --- a/lib/sort.c
> +++ b/lib/sort.c
> @@ -122,16 +122,27 @@ static void swap_bytes(void *a, void *b, size_t n)
> * a pointer, but small integers make for the smallest compare
> * instructions.
> */
> -#define SWAP_WORDS_64 (swap_func_t)0
> -#define SWAP_WORDS_32 (swap_func_t)1
> -#define SWAP_BYTES (swap_func_t)2
> +#define SWAP_WORDS_64 (swap_r_func_t)0
> +#define SWAP_WORDS_32 (swap_r_func_t)1
> +#define SWAP_BYTES (swap_r_func_t)2
> +#define SWAP_WRAPPER (swap_r_func_t)3
> +
> +struct wrapper {
> + cmp_func_t cmp;
> + swap_func_t swap;
> +};
>
> /*
> * The function pointer is last to make tail calls most efficient if the
> * compiler decides not to inline this function.
> */
> -static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
> +static void do_swap(void *a, void *b, size_t size, swap_r_func_t swap_func, const void *priv)
> {
> + if (swap_func == SWAP_WRAPPER) {
> + ((const struct wrapper *)priv)->swap(a, b, (int)size);
> + return;
> + }
> +
> if (swap_func == SWAP_WORDS_64)
> swap_words_64(a, b, size);
> else if (swap_func == SWAP_WORDS_32)
> @@ -139,7 +150,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
> else if (swap_func == SWAP_BYTES)
> swap_bytes(a, b, size);
> else
> - swap_func(a, b, (int)size);
> + swap_func(a, b, (int)size, priv);
> }
>
> #define _CMP_WRAPPER ((cmp_r_func_t)0L)
> @@ -147,7 +158,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
> static int do_cmp(const void *a, const void *b, cmp_r_func_t cmp, const void *priv)
> {
> if (cmp == _CMP_WRAPPER)
> - return ((cmp_func_t)(priv))(a, b);
> + return ((const struct wrapper *)priv)->cmp(a, b);
> return cmp(a, b, priv);
> }
>
> @@ -198,7 +209,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)
> */
> void sort_r(void *base, size_t num, size_t size,
> cmp_r_func_t cmp_func,
> - swap_func_t swap_func,
> + swap_r_func_t swap_func,
> const void *priv)
> {
> /* pre-scale counters for performance */
> @@ -208,6 +219,10 @@ void sort_r(void *base, size_t num, size_t size,
> if (!a) /* num < 2 || size == 0 */
> return;
>
> + /* called from 'sort' without swap function, let's pick the default */
> + if (swap_func == SWAP_WRAPPER && !((struct wrapper *)priv)->swap)
> + swap_func = NULL;
> +
> if (!swap_func) {
> if (is_aligned(base, size, 8))
> swap_func = SWAP_WORDS_64;
> @@ -230,7 +245,7 @@ void sort_r(void *base, size_t num, size_t size,
> if (a) /* Building heap: sift down --a */
> a -= size;
> else if (n -= size) /* Sorting: Extract root to --n */
> - do_swap(base, base + n, size, swap_func);
> + do_swap(base, base + n, size, swap_func, priv);
> else /* Sort complete */
> break;
>
> @@ -257,7 +272,7 @@ void sort_r(void *base, size_t num, size_t size,
> c = b; /* Where "a" belongs */
> while (b != a) { /* Shift it into place */
> b = parent(b, lsbit, size);
> - do_swap(base + b, base + c, size, swap_func);
> + do_swap(base + b, base + c, size, swap_func, priv);
> }
> }
> }
> @@ -267,6 +282,11 @@ void sort(void *base, size_t num, size_t size,
> cmp_func_t cmp_func,
> swap_func_t swap_func)
> {
> - return sort_r(base, num, size, _CMP_WRAPPER, swap_func, cmp_func);
> + struct wrapper w = {
> + .cmp = cmp_func,
> + .swap = swap_func,
> + };
> +
> + return sort_r(base, num, size, _CMP_WRAPPER, SWAP_WRAPPER, &w);
> }
> EXPORT_SYMBOL(sort);
> --
> 2.35.1
>
--
Masami Hiramatsu <[email protected]>
On Tue, Feb 22, 2022 at 9:06 AM Jiri Olsa <[email protected]> wrote:
>
> hi,
> this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> kprobe program through fprobe API [1] instroduced by Masami.
>
> The fprobe API allows to attach probe on multiple functions at once very
> fast, because it works on top of ftrace. On the other hand this limits
> the probe point to the function entry or return.
>
>
> With bpftrace support I see following attach speed:
>
> # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
> Attaching 2 probes...
> Attaching 3342 functions
> ...
>
> 1.4960 +- 0.0285 seconds time elapsed ( +- 1.91% )
>
>
> v2 changes:
> - based on latest fprobe changes [1]
> - renaming the uapi interface to kprobe multi
> - adding support for sort_r to pass user pointer for swap functions
> and using that in cookie support to keep just single functions array
> - moving new link to kernel/trace/bpf_trace.c file
> - using single fprobe callback function for entry and exit
> - using kvzalloc, libbpf_ensure_mem functions
> - adding new k[ret]probe.multi sections instead of using current kprobe
> - used glob_match from test_progs.c, added '?' matching
> - move bpf_get_func_ip verifier inline change to seprate change
> - couple of other minor fixes
>
>
I think it's shaping up pretty well. Great work, Jiri! Can't wait to
adopt this in retsnoop. See below about dependency on Masami's
patches.
> Also available at:
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> bpf/kprobe_multi
>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
Masami, Jiri, Steven, what would be the logistics here? What's the
plan for getting this upstream? Any idea about timelines? I really
hope it won't take as long as it took for kretprobe stack trace
capturing fixes last year to land. Can we take Masami's changes
through bpf-next tree? If yes, Steven, can you please review and give
your acks? Thanks for understanding!
> ---
> Jiri Olsa (10):
> lib/sort: Add priv pointer to swap function
> bpf: Add multi kprobe link
> bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
> bpf: Add support to inline bpf_get_func_ip helper on x86
> bpf: Add cookie support to programs attached with kprobe multi link
> libbpf: Add libbpf_kallsyms_parse function
> libbpf: Add bpf_link_create support for multi kprobes
> libbpf: Add bpf_program__attach_kprobe_opts support for multi kprobes
> selftest/bpf: Add kprobe_multi attach test
> selftest/bpf: Add kprobe_multi test for bpf_cookie values
>
> include/linux/bpf_types.h | 1 +
> include/linux/sort.h | 4 +-
> include/linux/trace_events.h | 6 ++
> include/linux/types.h | 1 +
> include/uapi/linux/bpf.h | 14 ++++
> kernel/bpf/syscall.c | 26 ++++++--
> kernel/bpf/verifier.c | 21 +++++-
> kernel/trace/bpf_trace.c | 331 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/sort.c | 42 +++++++++---
> tools/include/uapi/linux/bpf.h | 14 ++++
> tools/lib/bpf/bpf.c | 7 ++
> tools/lib/bpf/bpf.h | 9 ++-
> tools/lib/bpf/libbpf.c | 192 +++++++++++++++++++++++++++++++++++++++++++++--------
> tools/lib/bpf/libbpf_internal.h | 5 ++
> tools/testing/selftests/bpf/prog_tests/bpf_cookie.c | 72 ++++++++++++++++++++
> tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 115 ++++++++++++++++++++++++++++++++
> tools/testing/selftests/bpf/progs/kprobe_multi.c | 58 ++++++++++++++++
> tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c | 62 +++++++++++++++++
> 18 files changed, 930 insertions(+), 50 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c
> create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
On Tue, Feb 22, 2022 at 9:06 AM Jiri Olsa <[email protected]> wrote:
>
> Adding support to call bpf_get_func_ip helper from kprobe
> programs attached by multi kprobe link.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
LGTM.
Acked-by: Andrii Nakryiko <[email protected]>
> kernel/trace/bpf_trace.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index df3771bfd6e5..64891b7b0885 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1037,6 +1037,18 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> +BPF_CALL_1(bpf_get_func_ip_kprobe_multi, struct pt_regs *, regs)
> +{
> + return instruction_pointer(regs);
> +}
> +
> +static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
> + .func = bpf_get_func_ip_kprobe_multi,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> +};
> +
> BPF_CALL_1(bpf_get_attach_cookie_trace, void *, ctx)
> {
> struct bpf_trace_run_ctx *run_ctx;
> @@ -1280,7 +1292,9 @@ kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_override_return_proto;
> #endif
> case BPF_FUNC_get_func_ip:
> - return &bpf_get_func_ip_proto_kprobe;
> + return prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
> + &bpf_get_func_ip_proto_kprobe_multi :
> + &bpf_get_func_ip_proto_kprobe;
> case BPF_FUNC_get_attach_cookie:
> return &bpf_get_attach_cookie_proto_trace;
> default:
> --
> 2.35.1
>
On Tue, Feb 22, 2022 at 9:07 AM Jiri Olsa <[email protected]> wrote:
>
> Move the kallsyms parsing in internal libbpf_kallsyms_parse
> function, so it can be used from other places.
>
> It will be used in following changes.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
LGTM
Acked-by: Andrii Nakryiko <[email protected]>
> tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++-------------
> tools/lib/bpf/libbpf_internal.h | 5 +++
> 2 files changed, 43 insertions(+), 24 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad43b6ce825e..fb530b004a0d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7172,12 +7172,10 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj)
> return 0;
> }
>
> -static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
> +int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
> {
> char sym_type, sym_name[500];
> unsigned long long sym_addr;
> - const struct btf_type *t;
> - struct extern_desc *ext;
> int ret, err = 0;
> FILE *f;
>
> @@ -7196,35 +7194,51 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
> if (ret != 3) {
> pr_warn("failed to read kallsyms entry: %d\n", ret);
> err = -EINVAL;
> - goto out;
> + break;
> }
>
> - ext = find_extern_by_name(obj, sym_name);
> - if (!ext || ext->type != EXT_KSYM)
> - continue;
> -
> - t = btf__type_by_id(obj->btf, ext->btf_id);
> - if (!btf_is_var(t))
> - continue;
> -
> - if (ext->is_set && ext->ksym.addr != sym_addr) {
> - pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
> - sym_name, ext->ksym.addr, sym_addr);
> - err = -EINVAL;
> - goto out;
> - }
> - if (!ext->is_set) {
> - ext->is_set = true;
> - ext->ksym.addr = sym_addr;
> - pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
> - }
> + err = cb(sym_addr, sym_type, sym_name, ctx);
> + if (err)
> + break;
> }
>
> -out:
> fclose(f);
> return err;
> }
>
> +static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
> + const char *sym_name, void *ctx)
> +{
> + struct bpf_object *obj = ctx;
> + const struct btf_type *t;
> + struct extern_desc *ext;
> +
> + ext = find_extern_by_name(obj, sym_name);
> + if (!ext || ext->type != EXT_KSYM)
> + return 0;
> +
> + t = btf__type_by_id(obj->btf, ext->btf_id);
> + if (!btf_is_var(t))
> + return 0;
> +
> + if (ext->is_set && ext->ksym.addr != sym_addr) {
> + pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
> + sym_name, ext->ksym.addr, sym_addr);
> + return -EINVAL;
> + }
> + if (!ext->is_set) {
> + ext->is_set = true;
> + ext->ksym.addr = sym_addr;
> + pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
> + }
> + return 0;
> +}
> +
> +static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
> +{
> + return libbpf_kallsyms_parse(kallsyms_cb, obj);
> +}
> +
> static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
> __u16 kind, struct btf **res_btf,
> struct module_btf **res_mod_btf)
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 4fda8bdf0a0d..b6247dc7f8eb 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -449,6 +449,11 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
>
> extern enum libbpf_strict_mode libbpf_mode;
>
> +typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
> + const char *sym_name, void *ctx);
> +
> +int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
> +
> /* handle direct returned errors */
> static inline int libbpf_err(int ret)
> {
> --
> 2.35.1
>
On Tue, Feb 22, 2022 at 9:08 AM Jiri Olsa <[email protected]> wrote:
>
> Adding kprobe_multi attach test that uses new fprobe interface to
> attach kprobe program to multiple functions.
>
> The test is attaching programs to bpf_fentry_test* functions and
> uses single trampoline program bpf_prog_test_run to trigger
> bpf_fentry_test* functions.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
subj typo: selftest -> selftests
> .../bpf/prog_tests/kprobe_multi_test.c | 115 ++++++++++++++++++
> .../selftests/bpf/progs/kprobe_multi.c | 58 +++++++++
> 2 files changed, 173 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c
>
[...]
> +
> +static void test_link_api_addrs(void)
> +{
> + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> + __u64 addrs[8];
> +
> + kallsyms_find("bpf_fentry_test1", &addrs[0]);
> + kallsyms_find("bpf_fentry_test2", &addrs[1]);
> + kallsyms_find("bpf_fentry_test3", &addrs[2]);
> + kallsyms_find("bpf_fentry_test4", &addrs[3]);
> + kallsyms_find("bpf_fentry_test5", &addrs[4]);
> + kallsyms_find("bpf_fentry_test6", &addrs[5]);
> + kallsyms_find("bpf_fentry_test7", &addrs[6]);
> + kallsyms_find("bpf_fentry_test8", &addrs[7]);
ASSERT_OK() that symbols are found? It also sucks that we re-parse
kallsyms so much...
maybe use load_kallsyms() to pre-cache? We should also teach
load_kallsyms() to not reload kallsyms more than once
> +
> + opts.kprobe_multi.addrs = (__u64) addrs;
> + opts.kprobe_multi.cnt = 8;
ARRAY_SIZE()?
> + test_link_api(&opts);
> +}
> +
> +static void test_link_api_syms(void)
> +{
> + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
nit: just LIBBPF_OPTS
> + const char *syms[8] = {
> + "bpf_fentry_test1",
> + "bpf_fentry_test2",
> + "bpf_fentry_test3",
> + "bpf_fentry_test4",
> + "bpf_fentry_test5",
> + "bpf_fentry_test6",
> + "bpf_fentry_test7",
> + "bpf_fentry_test8",
> + };
> +
> + opts.kprobe_multi.syms = (__u64) syms;
> + opts.kprobe_multi.cnt = 8;
ARRAY_SIZE() ?
> + test_link_api(&opts);
> +}
> +
> +void test_kprobe_multi_test(void)
> +{
> + test_skel_api();
> + test_link_api_syms();
> + test_link_api_addrs();
model as subtests?
> +}
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi.c b/tools/testing/selftests/bpf/progs/kprobe_multi.c
> new file mode 100644
> index 000000000000..71318c65931c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_multi.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern const void bpf_fentry_test1 __ksym;
> +extern const void bpf_fentry_test2 __ksym;
> +extern const void bpf_fentry_test3 __ksym;
> +extern const void bpf_fentry_test4 __ksym;
> +extern const void bpf_fentry_test5 __ksym;
> +extern const void bpf_fentry_test6 __ksym;
> +extern const void bpf_fentry_test7 __ksym;
> +extern const void bpf_fentry_test8 __ksym;
> +
> +/* No tests, just to trigger bpf_fentry_test* through tracing test_run */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(test1)
> +{
> + return 0;
> +}
> +
> +__u64 test2_result = 0;
> +
> +SEC("kprobe.multi/bpf_fentry_test?")
> +int test2(struct pt_regs *ctx)
> +{
> + __u64 addr = bpf_get_func_ip(ctx);
> +
> + test2_result += (const void *) addr == &bpf_fentry_test1 ||
> + (const void *) addr == &bpf_fentry_test2 ||
> + (const void *) addr == &bpf_fentry_test3 ||
> + (const void *) addr == &bpf_fentry_test4 ||
> + (const void *) addr == &bpf_fentry_test5 ||
> + (const void *) addr == &bpf_fentry_test6 ||
> + (const void *) addr == &bpf_fentry_test7 ||
> + (const void *) addr == &bpf_fentry_test8;
> + return 0;
> +}
> +
> +__u64 test3_result = 0;
> +
> +SEC("kretprobe.multi/bpf_fentry_test*")
> +int test3(struct pt_regs *ctx)
> +{
> + __u64 addr = bpf_get_func_ip(ctx);
> +
> + test3_result += (const void *) addr == &bpf_fentry_test1 ||
> + (const void *) addr == &bpf_fentry_test2 ||
> + (const void *) addr == &bpf_fentry_test3 ||
> + (const void *) addr == &bpf_fentry_test4 ||
> + (const void *) addr == &bpf_fentry_test5 ||
> + (const void *) addr == &bpf_fentry_test6 ||
> + (const void *) addr == &bpf_fentry_test7 ||
> + (const void *) addr == &bpf_fentry_test8;
> + return 0;
> +}
> --
> 2.35.1
>
On Tue, Feb 22, 2022 at 9:08 AM Jiri Olsa <[email protected]> wrote:
>
> Adding bpf_cookie test for programs attached by kprobe_multi links.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> .../selftests/bpf/prog_tests/bpf_cookie.c | 72 +++++++++++++++++++
> .../bpf/progs/kprobe_multi_bpf_cookie.c | 62 ++++++++++++++++
> 2 files changed, 134 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index cd10df6cd0fc..edfb9f8736c6 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> @@ -7,6 +7,7 @@
> #include <unistd.h>
> #include <test_progs.h>
> #include "test_bpf_cookie.skel.h"
> +#include "kprobe_multi_bpf_cookie.skel.h"
>
> /* uprobe attach point */
> static void trigger_func(void)
> @@ -63,6 +64,75 @@ static void kprobe_subtest(struct test_bpf_cookie *skel)
> bpf_link__destroy(retlink2);
> }
>
> +static void kprobe_multi_subtest(void)
> +{
> + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> + int err, prog_fd, link1_fd = -1, link2_fd = -1;
> + LIBBPF_OPTS(bpf_test_run_opts, topts);
consistency ftw, LIBBPF_OPTS
> + struct kprobe_multi_bpf_cookie *skel = NULL;
> + __u64 addrs[8], cookies[8];
> +
[..]
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
> new file mode 100644
> index 000000000000..d6f8454ba093
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern const void bpf_fentry_test1 __ksym;
> +extern const void bpf_fentry_test2 __ksym;
> +extern const void bpf_fentry_test3 __ksym;
> +extern const void bpf_fentry_test4 __ksym;
> +extern const void bpf_fentry_test5 __ksym;
> +extern const void bpf_fentry_test6 __ksym;
> +extern const void bpf_fentry_test7 __ksym;
> +extern const void bpf_fentry_test8 __ksym;
> +
> +/* No tests, just to trigger bpf_fentry_test* through tracing test_run */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(test1)
> +{
> + return 0;
> +}
> +
> +__u64 test2_result = 0;
> +
> +SEC("kprobe.multi/bpf_fentry_tes??")
> +int test2(struct pt_regs *ctx)
> +{
> + __u64 cookie = bpf_get_attach_cookie(ctx);
> + __u64 addr = bpf_get_func_ip(ctx);
> +
> + test2_result += (const void *) addr == &bpf_fentry_test1 && cookie == 1;
> + test2_result += (const void *) addr == &bpf_fentry_test2 && cookie == 2;
> + test2_result += (const void *) addr == &bpf_fentry_test3 && cookie == 3;
> + test2_result += (const void *) addr == &bpf_fentry_test4 && cookie == 4;
> + test2_result += (const void *) addr == &bpf_fentry_test5 && cookie == 5;
> + test2_result += (const void *) addr == &bpf_fentry_test6 && cookie == 6;
> + test2_result += (const void *) addr == &bpf_fentry_test7 && cookie == 7;
> + test2_result += (const void *) addr == &bpf_fentry_test8 && cookie == 8;
this is not parallel mode friendly
let's filter by pid, but also it's best to do count locally and just
assign it (so that multiple calls of the program still produce the
same value, instead of constantly increasing global variable with each
run)
> +
> + return 0;
> +}
> +
> +__u64 test3_result = 0;
> +
> +SEC("kretprobe.multi/bpf_fentry_test*")
> +int test3(struct pt_regs *ctx)
> +{
> + __u64 cookie = bpf_get_attach_cookie(ctx);
> + __u64 addr = bpf_get_func_ip(ctx);
> +
> + test3_result += (const void *) addr == &bpf_fentry_test1 && cookie == 8;
> + test3_result += (const void *) addr == &bpf_fentry_test2 && cookie == 7;
> + test3_result += (const void *) addr == &bpf_fentry_test3 && cookie == 6;
> + test3_result += (const void *) addr == &bpf_fentry_test4 && cookie == 5;
> + test3_result += (const void *) addr == &bpf_fentry_test5 && cookie == 4;
> + test3_result += (const void *) addr == &bpf_fentry_test6 && cookie == 3;
> + test3_result += (const void *) addr == &bpf_fentry_test7 && cookie == 2;
> + test3_result += (const void *) addr == &bpf_fentry_test8 && cookie == 1;
> +
> + return 0;
> +}
> --
> 2.35.1
>
On Sat, 5 Mar 2022 20:09:39 -0500
Steven Rostedt <[email protected]> wrote:
> On Fri, 4 Mar 2022 15:10:55 -0800
> Andrii Nakryiko <[email protected]> wrote:
>
> > Masami, Jiri, Steven, what would be the logistics here? What's the
> > plan for getting this upstream? Any idea about timelines? I really
> > hope it won't take as long as it took for kretprobe stack trace
> > capturing fixes last year to land. Can we take Masami's changes
> > through bpf-next tree? If yes, Steven, can you please review and give
> > your acks? Thanks for understanding!
>
> Yeah, I'll start looking at it this week. I just started a new job and
> that's been taking up a lot of my time and limiting what I can look at
> upstream.
Let me update my series, I found some issues in the selftest.
I'll send v9 soon.
Thank you!
--
Masami Hiramatsu
On Fri, Mar 04, 2022 at 03:11:26PM -0800, Andrii Nakryiko wrote:
> On Tue, Feb 22, 2022 at 9:08 AM Jiri Olsa <[email protected]> wrote:
> >
> > Adding bpf_cookie test for programs attached by kprobe_multi links.
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
> > .../selftests/bpf/prog_tests/bpf_cookie.c | 72 +++++++++++++++++++
> > .../bpf/progs/kprobe_multi_bpf_cookie.c | 62 ++++++++++++++++
> > 2 files changed, 134 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > index cd10df6cd0fc..edfb9f8736c6 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > @@ -7,6 +7,7 @@
> > #include <unistd.h>
> > #include <test_progs.h>
> > #include "test_bpf_cookie.skel.h"
> > +#include "kprobe_multi_bpf_cookie.skel.h"
> >
> > /* uprobe attach point */
> > static void trigger_func(void)
> > @@ -63,6 +64,75 @@ static void kprobe_subtest(struct test_bpf_cookie *skel)
> > bpf_link__destroy(retlink2);
> > }
> >
> > +static void kprobe_multi_subtest(void)
> > +{
> > + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > + int err, prog_fd, link1_fd = -1, link2_fd = -1;
> > + LIBBPF_OPTS(bpf_test_run_opts, topts);
>
> consistency ftw, LIBBPF_OPTS
ok
>
>
> > + struct kprobe_multi_bpf_cookie *skel = NULL;
> > + __u64 addrs[8], cookies[8];
> > +
>
> [..]
>
> > diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
> > new file mode 100644
> > index 000000000000..d6f8454ba093
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_bpf_cookie.c
> > @@ -0,0 +1,62 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +extern const void bpf_fentry_test1 __ksym;
> > +extern const void bpf_fentry_test2 __ksym;
> > +extern const void bpf_fentry_test3 __ksym;
> > +extern const void bpf_fentry_test4 __ksym;
> > +extern const void bpf_fentry_test5 __ksym;
> > +extern const void bpf_fentry_test6 __ksym;
> > +extern const void bpf_fentry_test7 __ksym;
> > +extern const void bpf_fentry_test8 __ksym;
> > +
> > +/* No tests, just to trigger bpf_fentry_test* through tracing test_run */
> > +SEC("fentry/bpf_modify_return_test")
> > +int BPF_PROG(test1)
> > +{
> > + return 0;
> > +}
> > +
> > +__u64 test2_result = 0;
> > +
> > +SEC("kprobe.multi/bpf_fentry_tes??")
> > +int test2(struct pt_regs *ctx)
> > +{
> > + __u64 cookie = bpf_get_attach_cookie(ctx);
> > + __u64 addr = bpf_get_func_ip(ctx);
> > +
> > + test2_result += (const void *) addr == &bpf_fentry_test1 && cookie == 1;
> > + test2_result += (const void *) addr == &bpf_fentry_test2 && cookie == 2;
> > + test2_result += (const void *) addr == &bpf_fentry_test3 && cookie == 3;
> > + test2_result += (const void *) addr == &bpf_fentry_test4 && cookie == 4;
> > + test2_result += (const void *) addr == &bpf_fentry_test5 && cookie == 5;
> > + test2_result += (const void *) addr == &bpf_fentry_test6 && cookie == 6;
> > + test2_result += (const void *) addr == &bpf_fentry_test7 && cookie == 7;
> > + test2_result += (const void *) addr == &bpf_fentry_test8 && cookie == 8;
>
> this is not parallel mode friendly
>
> let's filter by pid, but also it's best to do count locally and just
> assign it (so that multiple calls of the program still produce the
> same value, instead of constantly increasing global variable with each
> run)
ah I did not think of the paralel run, right, will change
thanks,
jirka
On Fri, 4 Mar 2022 15:10:55 -0800
Andrii Nakryiko <[email protected]> wrote:
> Masami, Jiri, Steven, what would be the logistics here? What's the
> plan for getting this upstream? Any idea about timelines? I really
> hope it won't take as long as it took for kretprobe stack trace
> capturing fixes last year to land. Can we take Masami's changes
> through bpf-next tree? If yes, Steven, can you please review and give
> your acks? Thanks for understanding!
Yeah, I'll start looking at it this week. I just started a new job and
that's been taking up a lot of my time and limiting what I can look at
upstream.
-- Steve
On Fri, Mar 04, 2022 at 03:11:23PM -0800, Andrii Nakryiko wrote:
> On Tue, Feb 22, 2022 at 9:08 AM Jiri Olsa <[email protected]> wrote:
> >
> > Adding kprobe_multi attach test that uses new fprobe interface to
> > attach kprobe program to multiple functions.
> >
> > The test is attaching programs to bpf_fentry_test* functions and
> > uses single trampoline program bpf_prog_test_run to trigger
> > bpf_fentry_test* functions.
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
>
> subj typo: selftest -> selftests
ok
>
> > .../bpf/prog_tests/kprobe_multi_test.c | 115 ++++++++++++++++++
> > .../selftests/bpf/progs/kprobe_multi.c | 58 +++++++++
> > 2 files changed, 173 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c
> >
>
> [...]
>
> > +
> > +static void test_link_api_addrs(void)
> > +{
> > + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > + __u64 addrs[8];
> > +
> > + kallsyms_find("bpf_fentry_test1", &addrs[0]);
> > + kallsyms_find("bpf_fentry_test2", &addrs[1]);
> > + kallsyms_find("bpf_fentry_test3", &addrs[2]);
> > + kallsyms_find("bpf_fentry_test4", &addrs[3]);
> > + kallsyms_find("bpf_fentry_test5", &addrs[4]);
> > + kallsyms_find("bpf_fentry_test6", &addrs[5]);
> > + kallsyms_find("bpf_fentry_test7", &addrs[6]);
> > + kallsyms_find("bpf_fentry_test8", &addrs[7]);
>
> ASSERT_OK() that symbols are found? It also sucks that we re-parse
> kallsyms so much...
ok
>
> maybe use load_kallsyms() to pre-cache? We should also teach
> load_kallsyms() to not reload kallsyms more than once
true, it saved many cycles in bpftrace ;-) will check
>
> > +
> > + opts.kprobe_multi.addrs = (__u64) addrs;
> > + opts.kprobe_multi.cnt = 8;
>
> ARRAY_SIZE()?
ok
>
> > + test_link_api(&opts);
> > +}
> > +
> > +static void test_link_api_syms(void)
> > +{
> > + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
>
> nit: just LIBBPF_OPTS
ok
>
> > + const char *syms[8] = {
> > + "bpf_fentry_test1",
> > + "bpf_fentry_test2",
> > + "bpf_fentry_test3",
> > + "bpf_fentry_test4",
> > + "bpf_fentry_test5",
> > + "bpf_fentry_test6",
> > + "bpf_fentry_test7",
> > + "bpf_fentry_test8",
> > + };
> > +
> > + opts.kprobe_multi.syms = (__u64) syms;
> > + opts.kprobe_multi.cnt = 8;
>
> ARRAY_SIZE() ?
ok
>
> > + test_link_api(&opts);
> > +}
> > +
> > +void test_kprobe_multi_test(void)
> > +{
> > + test_skel_api();
> > + test_link_api_syms();
> > + test_link_api_addrs();
>
> model as subtests?
ok
thanks,
jirka
On Sat, Mar 5, 2022 at 5:33 PM Masami Hiramatsu
<[email protected]> wrote:
>
> On Sat, 5 Mar 2022 20:09:39 -0500
> Steven Rostedt <[email protected]> wrote:
>
> > On Fri, 4 Mar 2022 15:10:55 -0800
> > Andrii Nakryiko <[email protected]> wrote:
> >
> > > Masami, Jiri, Steven, what would be the logistics here? What's the
> > > plan for getting this upstream? Any idea about timelines? I really
> > > hope it won't take as long as it took for kretprobe stack trace
> > > capturing fixes last year to land. Can we take Masami's changes
> > > through bpf-next tree? If yes, Steven, can you please review and give
> > > your acks? Thanks for understanding!
> >
> > Yeah, I'll start looking at it this week. I just started a new job and
Thanks, Steven. Greatly appreciated!
> > that's been taking up a lot of my time and limiting what I can look at
> > upstream.
>
> Let me update my series, I found some issues in the selftest.
> I'll send v9 soon.
I haven't checked, but if you haven't based your patches off of
bpf-next tree, please do so in the next revision, so that we can land
your patch set and Jiri's patch set on top without any issues. Thanks!
>
> Thank you!
>
> --
> Masami Hiramatsu