* [PATCH bpf-next v3 1/8] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
@ 2024-03-26 4:14 ` Yonghong Song
2024-03-26 4:14 ` [PATCH bpf-next v3 2/8] libbpf: Mark libbpf_kallsyms_parse static function Yonghong Song
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:14 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Replace CHECK with ASSERT macros for ksyms tests.
This test failed earlier with clang lto kernel, but the
issue is gone with latest code base. But replacing
CHECK with ASSERT still improves code as ASSERT is
preferred in selftests.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../testing/selftests/bpf/prog_tests/ksyms.c | 30 +++++++------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms.c b/tools/testing/selftests/bpf/prog_tests/ksyms.c
index b295969b263b..dc7aab532fb1 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms.c
@@ -5,8 +5,6 @@
#include "test_ksyms.skel.h"
#include <sys/stat.h>
-static int duration;
-
void test_ksyms(void)
{
const char *btf_path = "/sys/kernel/btf/vmlinux";
@@ -18,43 +16,37 @@ void test_ksyms(void)
int err;
err = kallsyms_find("bpf_link_fops", &link_fops_addr);
- if (CHECK(err == -EINVAL, "kallsyms_fopen", "failed to open: %d\n", errno))
+ if (!ASSERT_NEQ(err, -EINVAL, "bpf_link_fops: kallsyms_fopen"))
return;
- if (CHECK(err == -ENOENT, "ksym_find", "symbol 'bpf_link_fops' not found\n"))
+ if (!ASSERT_NEQ(err, -ENOENT, "bpf_link_fops: ksym_find"))
return;
err = kallsyms_find("__per_cpu_start", &per_cpu_start_addr);
- if (CHECK(err == -EINVAL, "kallsyms_fopen", "failed to open: %d\n", errno))
+ if (!ASSERT_NEQ(err, -EINVAL, "__per_cpu_start: kallsyms_fopen"))
return;
- if (CHECK(err == -ENOENT, "ksym_find", "symbol 'per_cpu_start' not found\n"))
+ if (!ASSERT_NEQ(err, -ENOENT, "__per_cpu_start: ksym_find"))
return;
- if (CHECK(stat(btf_path, &st), "stat_btf", "err %d\n", errno))
+ if (!ASSERT_OK(stat(btf_path, &st), "stat_btf"))
return;
btf_size = st.st_size;
skel = test_ksyms__open_and_load();
- if (CHECK(!skel, "skel_open", "failed to open and load skeleton\n"))
+ if (!ASSERT_OK_PTR(skel, "test_ksyms__open_and_load"))
return;
err = test_ksyms__attach(skel);
- if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+ if (!ASSERT_OK(err, "test_ksyms__attach"))
goto cleanup;
/* trigger tracepoint */
usleep(1);
data = skel->data;
- CHECK(data->out__bpf_link_fops != link_fops_addr, "bpf_link_fops",
- "got 0x%llx, exp 0x%llx\n",
- data->out__bpf_link_fops, link_fops_addr);
- CHECK(data->out__bpf_link_fops1 != 0, "bpf_link_fops1",
- "got %llu, exp %llu\n", data->out__bpf_link_fops1, (__u64)0);
- CHECK(data->out__btf_size != btf_size, "btf_size",
- "got %llu, exp %llu\n", data->out__btf_size, btf_size);
- CHECK(data->out__per_cpu_start != per_cpu_start_addr, "__per_cpu_start",
- "got %llu, exp %llu\n", data->out__per_cpu_start,
- per_cpu_start_addr);
+ ASSERT_EQ(data->out__bpf_link_fops, link_fops_addr, "bpf_link_fops");
+ ASSERT_EQ(data->out__bpf_link_fops1, 0, "bpf_link_fops1");
+ ASSERT_EQ(data->out__btf_size, btf_size, "btf_size");
+ ASSERT_EQ(data->out__per_cpu_start, per_cpu_start_addr, "__per_cpu_start");
cleanup:
test_ksyms__destroy(skel);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 2/8] libbpf: Mark libbpf_kallsyms_parse static function
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
2024-03-26 4:14 ` [PATCH bpf-next v3 1/8] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test Yonghong Song
@ 2024-03-26 4:14 ` Yonghong Song
2024-03-26 4:14 ` [PATCH bpf-next v3 3/8] libbpf: Handle <orig_name>.llvm.<hash> symbol properly Yonghong Song
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:14 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Currently libbpf_kallsyms_parse() function is declared as a global
function but actually it is not a API and there is no external
users in bpftool/bpf-selftests. So let us mark the function as
static.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/lib/bpf/libbpf.c | 5 ++++-
tools/lib/bpf/libbpf_internal.h | 5 -----
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8eb2cd4ef288..1f5581b9d11b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7982,7 +7982,10 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj)
return 0;
}
-int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
+typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
+ const char *sym_name, void *ctx);
+
+static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
{
char sym_type, sym_name[500];
unsigned long long sym_addr;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 864b36177424..a0dcfb82e455 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -518,11 +518,6 @@ int btf_ext_visit_str_offs(struct btf_ext *btf_ext, str_off_visit_fn visit, void
__s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
__u32 kind);
-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.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 3/8] libbpf: Handle <orig_name>.llvm.<hash> symbol properly
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
2024-03-26 4:14 ` [PATCH bpf-next v3 1/8] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test Yonghong Song
2024-03-26 4:14 ` [PATCH bpf-next v3 2/8] libbpf: Mark libbpf_kallsyms_parse static function Yonghong Song
@ 2024-03-26 4:14 ` Yonghong Song
2024-03-26 4:15 ` [PATCH bpf-next v3 4/8] selftests/bpf: Refactor some functions for kprobe_multi_test Yonghong Song
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:14 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
With CONFIG_LTO_CLANG_THIN enabled, with some of previous
version of kernel code base ([1]), I hit the following
error:
test_ksyms:PASS:kallsyms_fopen 0 nsec
test_ksyms:FAIL:ksym_find symbol 'bpf_link_fops' not found
#118 ksyms:FAIL
The reason is that 'bpf_link_fops' is renamed to
bpf_link_fops.llvm.8325593422554671469
Due to cross-file inlining, the static variable 'bpf_link_fops'
in syscall.c is used by a function in another file. To avoid
potential duplicated names, the llvm added suffix
'.llvm.<hash>' ([2]) to 'bpf_link_fops' variable.
Such renaming caused a problem in libbpf if 'bpf_link_fops'
is used in bpf prog as a ksym but 'bpf_link_fops' does not
match any symbol in /proc/kallsyms.
To fix this issue, libbpf needs to understand that suffix '.llvm.<hash>'
is caused by clang lto kernel and to process such symbols properly.
With latest bpf-next code base built with CONFIG_LTO_CLANG_THIN,
I cannot reproduce the above failure any more. But such an issue
could happen with other symbols or in the future for bpf_link_fops symbol.
For example, with my current kernel, I got the following from
/proc/kallsyms:
ffffffff84782154 d __func__.net_ratelimit.llvm.6135436931166841955
ffffffff85f0a500 d tk_core.llvm.726630847145216431
ffffffff85fdb960 d __fs_reclaim_map.llvm.10487989720912350772
ffffffff864c7300 d fake_dst_ops.llvm.54750082607048300
I could not easily create a selftest to test newly-added
libbpf functionality with a static C test since I do not know
which symbol is cross-file inlined. But based on my particular kernel,
the following test change can run successfully.
> diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms.c b/tools/testing/selftests/bpf/prog_tests/ksyms.c
> index 6a86d1f07800..904a103f7b1d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ksyms.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ksyms.c
> @@ -42,6 +42,7 @@ void test_ksyms(void)
> ASSERT_EQ(data->out__bpf_link_fops, link_fops_addr, "bpf_link_fops");
> ASSERT_EQ(data->out__bpf_link_fops1, 0, "bpf_link_fops1");
> ASSERT_EQ(data->out__btf_size, btf_size, "btf_size");
> + ASSERT_NEQ(data->out__fake_dst_ops, 0, "fake_dst_ops");
> ASSERT_EQ(data->out__per_cpu_start, per_cpu_start_addr, "__per_cpu_start");
>
> cleanup:
> diff --git a/tools/testing/selftests/bpf/progs/test_ksyms.c b/tools/testing/selftests/bpf/progs/test_ksyms.c
> index 6c9cbb5a3bdf..fe91eef54b66 100644
> --- a/tools/testing/selftests/bpf/progs/test_ksyms.c
> +++ b/tools/testing/selftests/bpf/progs/test_ksyms.c
> @@ -9,11 +9,13 @@ __u64 out__bpf_link_fops = -1;
> __u64 out__bpf_link_fops1 = -1;
> __u64 out__btf_size = -1;
> __u64 out__per_cpu_start = -1;
> +__u64 out__fake_dst_ops = -1;
>
> extern const void bpf_link_fops __ksym;
> extern const void __start_BTF __ksym;
> extern const void __stop_BTF __ksym;
> extern const void __per_cpu_start __ksym;
> +extern const void fake_dst_ops __ksym;
> /* non-existing symbol, weak, default to zero */
> extern const void bpf_link_fops1 __ksym __weak;
>
> @@ -23,6 +25,7 @@ int handler(const void *ctx)
> out__bpf_link_fops = (__u64)&bpf_link_fops;
> out__btf_size = (__u64)(&__stop_BTF - &__start_BTF);
> out__per_cpu_start = (__u64)&__per_cpu_start;
> + out__fake_dst_ops = (__u64)&fake_dst_ops;
>
> out__bpf_link_fops1 = (__u64)&bpf_link_fops1;
This patch fixed the issue in libbpf such that
the suffix '.llvm.<hash>' will be ignored during comparison of
bpf prog ksym vs. symbols in /proc/kallsyms, this resolved the issue.
Currently, only static variables in /proc/kallsyms are checked
with '.llvm.<hash>' suffix since in bpf programs function ksyms
with '.llvm.<hash>' suffix are most likely kfunc's and unlikely
to be cross-file inlined.
Note that currently kernel does not support gcc build with lto.
[1] https://lore.kernel.org/bpf/20240302165017.1627295-1-yonghong.song@linux.dev/
[2] https://github.com/llvm/llvm-project/blob/release/18.x/llvm/include/llvm/IR/ModuleSummaryIndex.h#L1714-L1719
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/lib/bpf/libbpf.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1f5581b9d11b..370818849e37 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1966,6 +1966,20 @@ static struct extern_desc *find_extern_by_name(const struct bpf_object *obj,
return NULL;
}
+static struct extern_desc *find_extern_by_name_with_len(const struct bpf_object *obj,
+ const void *name, int len)
+{
+ const char *ext_name;
+ int i;
+
+ for (i = 0; i < obj->nr_extern; i++) {
+ ext_name = obj->externs[i].name;
+ if (strlen(ext_name) == len && strncmp(ext_name, name, len) == 0)
+ return &obj->externs[i];
+ }
+ return NULL;
+}
+
static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
char value)
{
@@ -8025,8 +8039,13 @@ static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
struct bpf_object *obj = ctx;
const struct btf_type *t;
struct extern_desc *ext;
+ char *res;
- ext = find_extern_by_name(obj, sym_name);
+ res = strstr(sym_name, ".llvm.");
+ if (sym_type == 'd' && res)
+ ext = find_extern_by_name_with_len(obj, sym_name, res - sym_name);
+ else
+ ext = find_extern_by_name(obj, sym_name);
if (!ext || ext->type != EXT_KSYM)
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 4/8] selftests/bpf: Refactor some functions for kprobe_multi_test
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (2 preceding siblings ...)
2024-03-26 4:14 ` [PATCH bpf-next v3 3/8] libbpf: Handle <orig_name>.llvm.<hash> symbol properly Yonghong Song
@ 2024-03-26 4:15 ` Yonghong Song
2024-03-26 4:15 ` [PATCH bpf-next v3 5/8] selftests/bpf: Refactor trace helper func load_kallsyms_local() Yonghong Song
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:15 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Refactor some functions in kprobe_multi_test.c to extract
some helper functions who will be used in later patches
to avoid code duplication.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../bpf/prog_tests/kprobe_multi_test.c | 94 +++++++++++--------
1 file changed, 57 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 05000810e28e..46e28edda595 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -336,6 +336,37 @@ static bool symbol_equal(long key1, long key2, void *ctx __maybe_unused)
return strcmp((const char *) key1, (const char *) key2) == 0;
}
+static bool is_invalid_entry(char *buf, bool kernel)
+{
+ if (kernel && strchr(buf, '['))
+ return true;
+ if (!kernel && !strchr(buf, '['))
+ return true;
+ return false;
+}
+
+static bool skip_entry(char *name)
+{
+ /*
+ * We attach to almost all kernel functions and some of them
+ * will cause 'suspicious RCU usage' when fprobe is attached
+ * to them. Filter out the current culprits - arch_cpu_idle
+ * default_idle and rcu_* functions.
+ */
+ if (!strcmp(name, "arch_cpu_idle"))
+ return true;
+ if (!strcmp(name, "default_idle"))
+ return true;
+ if (!strncmp(name, "rcu_", 4))
+ return true;
+ if (!strcmp(name, "bpf_dispatcher_xdp_func"))
+ return true;
+ if (!strncmp(name, "__ftrace_invalid_address__",
+ sizeof("__ftrace_invalid_address__") - 1))
+ return true;
+ return false;
+}
+
static int get_syms(char ***symsp, size_t *cntp, bool kernel)
{
size_t cap = 0, cnt = 0, i;
@@ -368,30 +399,13 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
}
while (fgets(buf, sizeof(buf), f)) {
- if (kernel && strchr(buf, '['))
- continue;
- if (!kernel && !strchr(buf, '['))
+ if (is_invalid_entry(buf, kernel))
continue;
free(name);
if (sscanf(buf, "%ms$*[^\n]\n", &name) != 1)
continue;
- /*
- * We attach to almost all kernel functions and some of them
- * will cause 'suspicious RCU usage' when fprobe is attached
- * to them. Filter out the current culprits - arch_cpu_idle
- * default_idle and rcu_* functions.
- */
- if (!strcmp(name, "arch_cpu_idle"))
- continue;
- if (!strcmp(name, "default_idle"))
- continue;
- if (!strncmp(name, "rcu_", 4))
- continue;
- if (!strcmp(name, "bpf_dispatcher_xdp_func"))
- continue;
- if (!strncmp(name, "__ftrace_invalid_address__",
- sizeof("__ftrace_invalid_address__") - 1))
+ if (skip_entry(name))
continue;
err = hashmap__add(map, name, 0);
@@ -426,34 +440,20 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
return err;
}
-static void test_kprobe_multi_bench_attach(bool kernel)
+static void do_bench_test(struct kprobe_multi_empty *skel, struct bpf_kprobe_multi_opts *opts)
{
- LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
- struct kprobe_multi_empty *skel = NULL;
long attach_start_ns, attach_end_ns;
long detach_start_ns, detach_end_ns;
double attach_delta, detach_delta;
struct bpf_link *link = NULL;
- char **syms = NULL;
- size_t cnt = 0, i;
-
- if (!ASSERT_OK(get_syms(&syms, &cnt, kernel), "get_syms"))
- return;
-
- skel = kprobe_multi_empty__open_and_load();
- if (!ASSERT_OK_PTR(skel, "kprobe_multi_empty__open_and_load"))
- goto cleanup;
-
- opts.syms = (const char **) syms;
- opts.cnt = cnt;
attach_start_ns = get_time_ns();
link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_empty,
- NULL, &opts);
+ NULL, opts);
attach_end_ns = get_time_ns();
if (!ASSERT_OK_PTR(link, "bpf_program__attach_kprobe_multi_opts"))
- goto cleanup;
+ return;
detach_start_ns = get_time_ns();
bpf_link__destroy(link);
@@ -462,9 +462,29 @@ static void test_kprobe_multi_bench_attach(bool kernel)
attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
detach_delta = (detach_end_ns - detach_start_ns) / 1000000000.0;
- printf("%s: found %lu functions\n", __func__, cnt);
+ printf("%s: found %lu functions\n", __func__, opts->cnt);
printf("%s: attached in %7.3lfs\n", __func__, attach_delta);
printf("%s: detached in %7.3lfs\n", __func__, detach_delta);
+}
+
+static void test_kprobe_multi_bench_attach(bool kernel)
+{
+ LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+ struct kprobe_multi_empty *skel = NULL;
+ char **syms = NULL;
+ size_t cnt = 0, i;
+
+ if (!ASSERT_OK(get_syms(&syms, &cnt, kernel), "get_syms"))
+ return;
+
+ skel = kprobe_multi_empty__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "kprobe_multi_empty__open_and_load"))
+ goto cleanup;
+
+ opts.syms = (const char **) syms;
+ opts.cnt = cnt;
+
+ do_bench_test(skel, &opts);
cleanup:
kprobe_multi_empty__destroy(skel);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 5/8] selftests/bpf: Refactor trace helper func load_kallsyms_local()
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (3 preceding siblings ...)
2024-03-26 4:15 ` [PATCH bpf-next v3 4/8] selftests/bpf: Refactor some functions for kprobe_multi_test Yonghong Song
@ 2024-03-26 4:15 ` Yonghong Song
2024-03-26 4:15 ` [PATCH bpf-next v3 6/8] selftests/bpf: Add {load,search}_kallsyms_custom_local() Yonghong Song
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:15 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Refactor trace helper function load_kallsyms_local() such that
it invokes a common function with a compare function as input.
The common function will be used later for other local functions.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/testing/selftests/bpf/trace_helpers.c | 19 ++++++++++++-------
tools/testing/selftests/bpf/trace_helpers.h | 2 ++
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index 27fd7ed3e4b0..fc9de4a89aea 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -61,12 +61,7 @@ void free_kallsyms_local(struct ksyms *ksyms)
free(ksyms);
}
-static int ksym_cmp(const void *p1, const void *p2)
-{
- return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
-}
-
-struct ksyms *load_kallsyms_local(void)
+static struct ksyms *load_kallsyms_local_common(ksym_cmp_t cmp_cb)
{
FILE *f;
char func[256], buf[256];
@@ -100,7 +95,7 @@ struct ksyms *load_kallsyms_local(void)
goto error;
}
fclose(f);
- qsort(ksyms->syms, ksyms->sym_cnt, sizeof(struct ksym), ksym_cmp);
+ qsort(ksyms->syms, ksyms->sym_cnt, sizeof(struct ksym), cmp_cb);
return ksyms;
error:
@@ -109,6 +104,16 @@ struct ksyms *load_kallsyms_local(void)
return NULL;
}
+static int ksym_cmp(const void *p1, const void *p2)
+{
+ return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
+}
+
+struct ksyms *load_kallsyms_local(void)
+{
+ return load_kallsyms_local_common(ksym_cmp);
+}
+
int load_kallsyms(void)
{
pthread_mutex_lock(&ksyms_mutex);
diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h
index 04fd1da7079d..9da0d0484eed 100644
--- a/tools/testing/selftests/bpf/trace_helpers.h
+++ b/tools/testing/selftests/bpf/trace_helpers.h
@@ -13,6 +13,8 @@ struct ksym {
};
struct ksyms;
+typedef int (*ksym_cmp_t)(const void *p1, const void *p2);
+
int load_kallsyms(void);
struct ksym *ksym_search(long key);
long ksym_get_addr(const char *name);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 6/8] selftests/bpf: Add {load,search}_kallsyms_custom_local()
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (4 preceding siblings ...)
2024-03-26 4:15 ` [PATCH bpf-next v3 5/8] selftests/bpf: Refactor trace helper func load_kallsyms_local() Yonghong Song
@ 2024-03-26 4:15 ` Yonghong Song
2024-03-26 4:15 ` [PATCH bpf-next v3 7/8] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel Yonghong Song
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:15 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
These two functions allow selftests to do loading/searching
kallsyms based on their specific compare functions.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/testing/selftests/bpf/trace_helpers.c | 27 +++++++++++++++++++++
tools/testing/selftests/bpf/trace_helpers.h | 5 ++++
2 files changed, 32 insertions(+)
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index fc9de4a89aea..7f45b4cb41fe 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -114,6 +114,11 @@ struct ksyms *load_kallsyms_local(void)
return load_kallsyms_local_common(ksym_cmp);
}
+struct ksyms *load_kallsyms_custom_local(ksym_cmp_t cmp_cb)
+{
+ return load_kallsyms_local_common(cmp_cb);
+}
+
int load_kallsyms(void)
{
pthread_mutex_lock(&ksyms_mutex);
@@ -153,6 +158,28 @@ struct ksym *ksym_search_local(struct ksyms *ksyms, long key)
return &ksyms->syms[0];
}
+struct ksym *search_kallsyms_custom_local(struct ksyms *ksyms, const void *p,
+ ksym_search_cmp_t cmp_cb)
+{
+ int start = 0, mid, end = ksyms->sym_cnt;
+ struct ksym *ks;
+ int result;
+
+ while (start < end) {
+ mid = start + (end - start) / 2;
+ ks = &ksyms->syms[mid];
+ result = cmp_cb(p, ks);
+ if (result < 0)
+ end = mid;
+ else if (result > 0)
+ start = mid + 1;
+ else
+ return ks;
+ }
+
+ return NULL;
+}
+
struct ksym *ksym_search(long key)
{
if (!ksyms)
diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h
index 9da0d0484eed..d1ed71789049 100644
--- a/tools/testing/selftests/bpf/trace_helpers.h
+++ b/tools/testing/selftests/bpf/trace_helpers.h
@@ -14,6 +14,7 @@ struct ksym {
struct ksyms;
typedef int (*ksym_cmp_t)(const void *p1, const void *p2);
+typedef int (*ksym_search_cmp_t)(const void *p1, const struct ksym *p2);
int load_kallsyms(void);
struct ksym *ksym_search(long key);
@@ -24,6 +25,10 @@ struct ksym *ksym_search_local(struct ksyms *ksyms, long key);
long ksym_get_addr_local(struct ksyms *ksyms, const char *name);
void free_kallsyms_local(struct ksyms *ksyms);
+struct ksyms *load_kallsyms_custom_local(ksym_cmp_t cmp_cb);
+struct ksym *search_kallsyms_custom_local(struct ksyms *ksyms, const void *p1,
+ ksym_search_cmp_t cmp_cb);
+
/* open kallsyms and find addresses on the fly, faster than load + search. */
int kallsyms_find(const char *sym, unsigned long long *addr);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 7/8] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (5 preceding siblings ...)
2024-03-26 4:15 ` [PATCH bpf-next v3 6/8] selftests/bpf: Add {load,search}_kallsyms_custom_local() Yonghong Song
@ 2024-03-26 4:15 ` Yonghong Song
2024-03-26 4:15 ` [PATCH bpf-next v3 8/8] selftests/bpf: Add a kprobe_multi subtest to use addrs instead of syms Yonghong Song
2024-03-28 2:00 ` [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel patchwork-bot+netdevbpf
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:15 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
In my locally build clang LTO kernel (enabling CONFIG_LTO and
CONFIG_LTO_CLANG_THIN), kprobe_multi_bench_attach/kernel subtest
failed like:
test_kprobe_multi_bench_attach:PASS:get_syms 0 nsec
test_kprobe_multi_bench_attach:PASS:kprobe_multi_empty__open_and_load 0 nsec
libbpf: prog 'test_kprobe_empty': failed to attach: No such process
test_kprobe_multi_bench_attach:FAIL:bpf_program__attach_kprobe_multi_opts unexpected error: -3
#117/1 kprobe_multi_bench_attach/kernel:FAIL
There are multiple symbols in /sys/kernel/debug/tracing/available_filter_functions
are renamed in /proc/kallsyms due to cross file inlining. One example is for
static function __access_remote_vm in mm/memory.c.
In a non-LTO kernel, we have the following call stack:
ptrace_access_vm (global, kernel/ptrace.c)
access_remote_vm (global, mm/memory.c)
__access_remote_vm (static, mm/memory.c)
With LTO kernel, it is possible that access_remote_vm() is inlined by
ptrace_access_vm(). So we end up with the following call stack:
ptrace_access_vm (global, kernel/ptrace.c)
__access_remote_vm (static, mm/memory.c)
The compiler renames __access_remote_vm to __access_remote_vm.llvm.<hash>
to prevent potential name collision.
The kernel bpf_kprobe_multi_link_attach() and ftrace_lookup_symbols() try
to find addresses based on /proc/kallsyms, hence the current test failed
with LTO kenrel.
This patch consulted /proc/kallsyms to find the corresponding entries
for the ksym and this solved the issue.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../bpf/prog_tests/kprobe_multi_test.c | 62 ++++++++++++++-----
1 file changed, 48 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 46e28edda595..3b9059164360 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -367,15 +367,49 @@ static bool skip_entry(char *name)
return false;
}
+/* Do comparision by ignoring '.llvm.<hash>' suffixes. */
+static int compare_name(const char *name1, const char *name2)
+{
+ const char *res1, *res2;
+ int len1, len2;
+
+ res1 = strstr(name1, ".llvm.");
+ res2 = strstr(name2, ".llvm.");
+ len1 = res1 ? res1 - name1 : strlen(name1);
+ len2 = res2 ? res2 - name2 : strlen(name2);
+
+ if (len1 == len2)
+ return strncmp(name1, name2, len1);
+ if (len1 < len2)
+ return strncmp(name1, name2, len1) <= 0 ? -1 : 1;
+ return strncmp(name1, name2, len2) >= 0 ? 1 : -1;
+}
+
+static int load_kallsyms_compare(const void *p1, const void *p2)
+{
+ return compare_name(((const struct ksym *)p1)->name, ((const struct ksym *)p2)->name);
+}
+
+static int search_kallsyms_compare(const void *p1, const struct ksym *p2)
+{
+ return compare_name(p1, p2->name);
+}
+
static int get_syms(char ***symsp, size_t *cntp, bool kernel)
{
- size_t cap = 0, cnt = 0, i;
- char *name = NULL, **syms = NULL;
+ size_t cap = 0, cnt = 0;
+ char *name = NULL, *ksym_name, **syms = NULL;
struct hashmap *map;
+ struct ksyms *ksyms;
+ struct ksym *ks;
char buf[256];
FILE *f;
int err = 0;
+ ksyms = load_kallsyms_custom_local(load_kallsyms_compare);
+ if (!ASSERT_OK_PTR(ksyms, "load_kallsyms_custom_local"))
+ return -EINVAL;
+
/*
* The available_filter_functions contains many duplicates,
* but other than that all symbols are usable in kprobe multi
@@ -408,7 +442,14 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
if (skip_entry(name))
continue;
- err = hashmap__add(map, name, 0);
+ ks = search_kallsyms_custom_local(ksyms, name, search_kallsyms_compare);
+ if (!ks) {
+ err = -EINVAL;
+ goto error;
+ }
+
+ ksym_name = ks->name;
+ err = hashmap__add(map, ksym_name, 0);
if (err == -EEXIST) {
err = 0;
continue;
@@ -421,8 +462,7 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
if (err)
goto error;
- syms[cnt++] = name;
- name = NULL;
+ syms[cnt++] = ksym_name;
}
*symsp = syms;
@@ -432,11 +472,8 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
free(name);
fclose(f);
hashmap__free(map);
- if (err) {
- for (i = 0; i < cnt; i++)
- free(syms[i]);
+ if (err)
free(syms);
- }
return err;
}
@@ -472,7 +509,7 @@ static void test_kprobe_multi_bench_attach(bool kernel)
LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
struct kprobe_multi_empty *skel = NULL;
char **syms = NULL;
- size_t cnt = 0, i;
+ size_t cnt = 0;
if (!ASSERT_OK(get_syms(&syms, &cnt, kernel), "get_syms"))
return;
@@ -488,11 +525,8 @@ static void test_kprobe_multi_bench_attach(bool kernel)
cleanup:
kprobe_multi_empty__destroy(skel);
- if (syms) {
- for (i = 0; i < cnt; i++)
- free(syms[i]);
+ if (syms)
free(syms);
- }
}
static void test_attach_override(void)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 8/8] selftests/bpf: Add a kprobe_multi subtest to use addrs instead of syms
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (6 preceding siblings ...)
2024-03-26 4:15 ` [PATCH bpf-next v3 7/8] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel Yonghong Song
@ 2024-03-26 4:15 ` Yonghong Song
2024-03-28 2:00 ` [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel patchwork-bot+netdevbpf
8 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2024-03-26 4:15 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Get addrs directly from available_filter_functions_addrs and
send to the kernel during kprobe_multi_attach. This avoids
consultation of /proc/kallsyms. But available_filter_functions_addrs
is introduced in 6.5, i.e., it is introduced recently,
so I skip the test if the kernel does not support it.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../bpf/prog_tests/kprobe_multi_test.c | 98 +++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 3b9059164360..51628455b6f5 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -477,6 +477,69 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
return err;
}
+static int get_addrs(unsigned long **addrsp, size_t *cntp, bool kernel)
+{
+ unsigned long *addr, *addrs, *tmp_addrs;
+ int err = 0, max_cnt, inc_cnt;
+ char *name = NULL;
+ size_t cnt = 0;
+ char buf[256];
+ FILE *f;
+
+ if (access("/sys/kernel/tracing/trace", F_OK) == 0)
+ f = fopen("/sys/kernel/tracing/available_filter_functions_addrs", "r");
+ else
+ f = fopen("/sys/kernel/debug/tracing/available_filter_functions_addrs", "r");
+
+ if (!f)
+ return -ENOENT;
+
+ /* In my local setup, the number of entries is 50k+ so Let us initially
+ * allocate space to hold 64k entries. If 64k is not enough, incrementally
+ * increase 1k each time.
+ */
+ max_cnt = 65536;
+ inc_cnt = 1024;
+ addrs = malloc(max_cnt * sizeof(long));
+ if (addrs == NULL) {
+ err = -ENOMEM;
+ goto error;
+ }
+
+ while (fgets(buf, sizeof(buf), f)) {
+ if (is_invalid_entry(buf, kernel))
+ continue;
+
+ free(name);
+ if (sscanf(buf, "%p %ms$*[^\n]\n", &addr, &name) != 2)
+ continue;
+ if (skip_entry(name))
+ continue;
+
+ if (cnt == max_cnt) {
+ max_cnt += inc_cnt;
+ tmp_addrs = realloc(addrs, max_cnt);
+ if (!tmp_addrs) {
+ err = -ENOMEM;
+ goto error;
+ }
+ addrs = tmp_addrs;
+ }
+
+ addrs[cnt++] = (unsigned long)addr;
+ }
+
+ *addrsp = addrs;
+ *cntp = cnt;
+
+error:
+ free(name);
+ fclose(f);
+ if (err)
+ free(addrs);
+ return err;
+}
+
static void do_bench_test(struct kprobe_multi_empty *skel, struct bpf_kprobe_multi_opts *opts)
{
long attach_start_ns, attach_end_ns;
@@ -529,6 +592,37 @@ static void test_kprobe_multi_bench_attach(bool kernel)
free(syms);
}
+static void test_kprobe_multi_bench_attach_addr(bool kernel)
+{
+ LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+ struct kprobe_multi_empty *skel = NULL;
+ unsigned long *addrs = NULL;
+ size_t cnt = 0;
+ int err;
+
+ err = get_addrs(&addrs, &cnt, kernel);
+ if (err == -ENOENT) {
+ test__skip();
+ return;
+ }
+
+ if (!ASSERT_OK(err, "get_addrs"))
+ return;
+
+ skel = kprobe_multi_empty__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "kprobe_multi_empty__open_and_load"))
+ goto cleanup;
+
+ opts.addrs = addrs;
+ opts.cnt = cnt;
+
+ do_bench_test(skel, &opts);
+
+cleanup:
+ kprobe_multi_empty__destroy(skel);
+ free(addrs);
+}
+
static void test_attach_override(void)
{
struct kprobe_multi_override *skel = NULL;
@@ -569,6 +663,10 @@ void serial_test_kprobe_multi_bench_attach(void)
test_kprobe_multi_bench_attach(true);
if (test__start_subtest("modules"))
test_kprobe_multi_bench_attach(false);
+ if (test__start_subtest("kernel"))
+ test_kprobe_multi_bench_attach_addr(true);
+ if (test__start_subtest("modules"))
+ test_kprobe_multi_bench_attach_addr(false);
}
void test_kprobe_multi_test(void)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel
2024-03-26 4:14 [PATCH bpf-next v3 0/8] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
` (7 preceding siblings ...)
2024-03-26 4:15 ` [PATCH bpf-next v3 8/8] selftests/bpf: Add a kprobe_multi subtest to use addrs instead of syms Yonghong Song
@ 2024-03-28 2:00 ` patchwork-bot+netdevbpf
8 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-28 2:00 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, ast, andrii, daniel, kernel-team, martin.lau
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Mon, 25 Mar 2024 21:14:43 -0700 you wrote:
> With a LTO kernel built with clang, with one of earlier version of kernel,
> I encountered two test failures, ksyms and kprobe_multi_bench_attach/kernel.
> Now with latest bpf-next, only kprobe_multi_bench_attach/kernel failed.
> But it is possible in the future ksyms selftest may fail again.
>
> Both test failures are due to static variable/function renaming
> due to cross-file inlining. For Ksyms failure, the solution is
> to strip .llvm.<hash> suffixes for symbols in /proc/kallsyms before
> comparing against the ksym in bpf program.
> For kprobe_multi_bench_attach/kernel failure, the solution is
> to either provide names in /proc/kallsyms to the kernel or
> ignore those names who have .llvm.<hash> suffix since the kernel
> sym name comparison is against /proc/kallsyms.
>
> [...]
Here is the summary with links:
- [bpf-next,v3,1/8] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test
https://git.kernel.org/bpf/bpf-next/c/779012842209
- [bpf-next,v3,2/8] libbpf: Mark libbpf_kallsyms_parse static function
https://git.kernel.org/bpf/bpf-next/c/2c8bc4cf4d03
- [bpf-next,v3,3/8] libbpf: Handle <orig_name>.llvm.<hash> symbol properly
https://git.kernel.org/bpf/bpf-next/c/afc6524702c5
- [bpf-next,v3,4/8] selftests/bpf: Refactor some functions for kprobe_multi_test
https://git.kernel.org/bpf/bpf-next/c/de2c50ce88ec
- [bpf-next,v3,5/8] selftests/bpf: Refactor trace helper func load_kallsyms_local()
https://git.kernel.org/bpf/bpf-next/c/8cfdd77e94e6
- [bpf-next,v3,6/8] selftests/bpf: Add {load,search}_kallsyms_custom_local()
https://git.kernel.org/bpf/bpf-next/c/3fa5b1626130
- [bpf-next,v3,7/8] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel
https://git.kernel.org/bpf/bpf-next/c/ba4a347e17d0
- [bpf-next,v3,8/8] selftests/bpf: Add a kprobe_multi subtest to use addrs instead of syms
https://git.kernel.org/bpf/bpf-next/c/aa17f462133f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread