* [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
@ 2023-06-25 1:13 Jackie Liu
2023-06-25 1:56 ` Jiri Olsa
2023-06-26 23:46 ` Andrii Nakryiko
0 siblings, 2 replies; 5+ messages in thread
From: Jackie Liu @ 2023-06-25 1:13 UTC (permalink / raw)
To: olsajiri, andrii; +Cc: martin.lau, song, yhs, bpf, liuyun01
From: Jackie Liu <liuyun01@kylinos.cn>
When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.
Use available_filter_functions_addrs check first, if failed, fallback to
kallsyms.
Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/tree/master/src/funccount
Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
tools/lib/bpf/libbpf.c | 81 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 75 insertions(+), 6 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a27f6e9ccce7..fca5d2e412c5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10107,6 +10107,12 @@ static const char *tracefs_uprobe_events(void)
return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
}
+static const char *tracefs_available_filter_functions_addrs(void)
+{
+ return use_debugfs() ? DEBUGFS"/available_filter_functions_addrs" :
+ TRACEFS"/available_filter_functions_addrs";
+}
+
static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
const char *kfunc_name, size_t offset)
{
@@ -10422,9 +10428,8 @@ struct kprobe_multi_resolve {
size_t cnt;
};
-static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
- const char *sym_name, void *ctx)
+static int ftrace_resolve_kprobe_multi_cb(unsigned long long sym_addr,
+ const char *sym_name, void *ctx)
{
struct kprobe_multi_resolve *res = ctx;
int err;
@@ -10441,6 +10446,63 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
return 0;
}
+static int
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+ const char *sym_name, void *ctx)
+{
+ return ftrace_resolve_kprobe_multi_cb(sym_addr, sym_name, ctx);
+}
+
+typedef int (*available_kprobe_cb_t)(unsigned long long sym_addr,
+ const char *sym_name, void *ctx);
+
+static int
+libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
+{
+ unsigned long long sym_addr;
+ char sym_name[256];
+ FILE *f;
+ int ret, err = 0;
+ const char *available_path = tracefs_available_filter_functions_addrs();
+
+ f = fopen(available_path, "r");
+ if (!f) {
+ err = -errno;
+ pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
+ available_path);
+ return err;
+ }
+
+ while (true) {
+ ret = fscanf(f, "%llx %255s%*[^\n]\n", &sym_addr, sym_name);
+ if (ret == EOF && feof(f))
+ break;
+ if (ret != 2) {
+ pr_warn("failed to read available kprobe entry: %d\n",
+ ret);
+ err = -EINVAL;
+ break;
+ }
+
+ err = cb(sym_addr, sym_name, ctx);
+ if (err)
+ break;
+ }
+
+ fclose(f);
+ return err;
+}
+
+static void kprobe_multi_resolve_reinit(struct kprobe_multi_resolve *res)
+{
+ free(res->addrs);
+
+ /* reset to zero, when fallback */
+ res->cap = 0;
+ res->cnt = 0;
+ res->addrs = NULL;
+}
+
struct bpf_link *
bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
const char *pattern,
@@ -10477,9 +10539,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
return libbpf_err_ptr(-EINVAL);
if (pattern) {
- err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
- if (err)
- goto error;
+ err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
+ &res);
+ if (err) {
+ /* fallback to kallsyms */
+ kprobe_multi_resolve_reinit(&res);
+ err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
+ &res);
+ if (err)
+ goto error;
+ }
if (!res.cnt) {
err = -ENOENT;
goto error;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
2023-06-25 1:13 [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs Jackie Liu
@ 2023-06-25 1:56 ` Jiri Olsa
2023-06-26 23:46 ` Andrii Nakryiko
1 sibling, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2023-06-25 1:56 UTC (permalink / raw)
To: Jackie Liu; +Cc: olsajiri, andrii, martin.lau, song, yhs, bpf, liuyun01
On Sun, Jun 25, 2023 at 09:13:26AM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> When using regular expression matching with "kprobe multi", it scans all
> the functions under "/proc/kallsyms" that can be matched. However, not all
> of them can be traced by kprobe.multi. If any one of the functions fails
> to be traced, it will result in the failure of all functions. The best
> approach is to filter out the functions that cannot be traced to ensure
> proper tracking of the functions.
>
> Use available_filter_functions_addrs check first, if failed, fallback to
> kallsyms.
>
> Here is the test eBPF program [1].
> [1] https://github.com/JackieLiu1/ketones/tree/master/src/funccount
>
> Suggested-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
> ---
> tools/lib/bpf/libbpf.c | 81 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index a27f6e9ccce7..fca5d2e412c5 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10107,6 +10107,12 @@ static const char *tracefs_uprobe_events(void)
> return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
> }
>
> +static const char *tracefs_available_filter_functions_addrs(void)
> +{
> + return use_debugfs() ? DEBUGFS"/available_filter_functions_addrs" :
> + TRACEFS"/available_filter_functions_addrs";
> +}
> +
> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
> const char *kfunc_name, size_t offset)
> {
> @@ -10422,9 +10428,8 @@ struct kprobe_multi_resolve {
> size_t cnt;
> };
>
> -static int
> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> - const char *sym_name, void *ctx)
> +static int ftrace_resolve_kprobe_multi_cb(unsigned long long sym_addr,
> + const char *sym_name, void *ctx)
> {
> struct kprobe_multi_resolve *res = ctx;
> int err;
> @@ -10441,6 +10446,63 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> return 0;
> }
>
> +static int
> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> + const char *sym_name, void *ctx)
> +{
> + return ftrace_resolve_kprobe_multi_cb(sym_addr, sym_name, ctx);
> +}
> +
> +typedef int (*available_kprobe_cb_t)(unsigned long long sym_addr,
> + const char *sym_name, void *ctx);
> +
> +static int
> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
> +{
> + unsigned long long sym_addr;
> + char sym_name[256];
> + FILE *f;
> + int ret, err = 0;
> + const char *available_path = tracefs_available_filter_functions_addrs();
> +
> + f = fopen(available_path, "r");
> + if (!f) {
> + err = -errno;
> + pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
> + available_path);
> + return err;
> + }
> +
> + while (true) {
> + ret = fscanf(f, "%llx %255s%*[^\n]\n", &sym_addr, sym_name);
> + if (ret == EOF && feof(f))
> + break;
> + if (ret != 2) {
> + pr_warn("failed to read available kprobe entry: %d\n",
> + ret);
> + err = -EINVAL;
> + break;
> + }
> +
> + err = cb(sym_addr, sym_name, ctx);
> + if (err)
> + break;
> + }
> +
> + fclose(f);
> + return err;
> +}
> +
> +static void kprobe_multi_resolve_reinit(struct kprobe_multi_resolve *res)
> +{
> + free(res->addrs);
> +
> + /* reset to zero, when fallback */
> + res->cap = 0;
> + res->cnt = 0;
> + res->addrs = NULL;
> +}
> +
> struct bpf_link *
> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> const char *pattern,
> @@ -10477,9 +10539,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> return libbpf_err_ptr(-EINVAL);
>
> if (pattern) {
> - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> - if (err)
> - goto error;
> + err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
> + &res);
> + if (err) {
> + /* fallback to kallsyms */
> + kprobe_multi_resolve_reinit(&res);
> + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
> + &res);
> + if (err)
> + goto error;
> + }
> if (!res.cnt) {
> err = -ENOENT;
> goto error;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
2023-06-25 1:13 [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs Jackie Liu
2023-06-25 1:56 ` Jiri Olsa
@ 2023-06-26 23:46 ` Andrii Nakryiko
2023-06-27 1:37 ` Jackie Liu
1 sibling, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2023-06-26 23:46 UTC (permalink / raw)
To: Jackie Liu; +Cc: olsajiri, andrii, martin.lau, song, yhs, bpf, liuyun01
On Sat, Jun 24, 2023 at 6:14 PM Jackie Liu <liu.yun@linux.dev> wrote:
>
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> When using regular expression matching with "kprobe multi", it scans all
> the functions under "/proc/kallsyms" that can be matched. However, not all
> of them can be traced by kprobe.multi. If any one of the functions fails
> to be traced, it will result in the failure of all functions. The best
> approach is to filter out the functions that cannot be traced to ensure
> proper tracking of the functions.
>
> Use available_filter_functions_addrs check first, if failed, fallback to
> kallsyms.
>
This is good, but it also doesn't address the original problem. On
older kernels that don't have available_filter_functions_addrs, we'll
still be trying to attach to non-attachable kprobes?
So I think we'll still need to add available_filter_functions-based
filtering on top of kallsyms. I guess we'll have to collect all
symbols+addr from kallsyms, sort them, then go over
available_filter_functions and do binary search. If element is not
found, we'll mark it for removal. Then last pass to filter out marked
entries to keep only addrs to be passed to kernel?
> Here is the test eBPF program [1].
> [1] https://github.com/JackieLiu1/ketones/tree/master/src/funccount
>
> Suggested-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
> tools/lib/bpf/libbpf.c | 81 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index a27f6e9ccce7..fca5d2e412c5 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10107,6 +10107,12 @@ static const char *tracefs_uprobe_events(void)
> return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
> }
>
> +static const char *tracefs_available_filter_functions_addrs(void)
> +{
> + return use_debugfs() ? DEBUGFS"/available_filter_functions_addrs" :
> + TRACEFS"/available_filter_functions_addrs";
> +}
> +
> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
> const char *kfunc_name, size_t offset)
> {
> @@ -10422,9 +10428,8 @@ struct kprobe_multi_resolve {
> size_t cnt;
> };
>
> -static int
> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> - const char *sym_name, void *ctx)
> +static int ftrace_resolve_kprobe_multi_cb(unsigned long long sym_addr,
> + const char *sym_name, void *ctx)
> {
> struct kprobe_multi_resolve *res = ctx;
> int err;
> @@ -10441,6 +10446,63 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> return 0;
> }
>
> +static int
> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> + const char *sym_name, void *ctx)
> +{
> + return ftrace_resolve_kprobe_multi_cb(sym_addr, sym_name, ctx);
> +}
> +
> +typedef int (*available_kprobe_cb_t)(unsigned long long sym_addr,
> + const char *sym_name, void *ctx);
> +
> +static int
> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
> +{
> + unsigned long long sym_addr;
> + char sym_name[256];
> + FILE *f;
> + int ret, err = 0;
> + const char *available_path = tracefs_available_filter_functions_addrs();
> +
> + f = fopen(available_path, "r");
> + if (!f) {
> + err = -errno;
> + pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
> + available_path);
if this is expected to happen, we shouldn't pr_warn() and pollute log output
> + return err;
> + }
> +
> + while (true) {
> + ret = fscanf(f, "%llx %255s%*[^\n]\n", &sym_addr, sym_name);
> + if (ret == EOF && feof(f))
> + break;
> + if (ret != 2) {
> + pr_warn("failed to read available kprobe entry: %d\n",
> + ret);
> + err = -EINVAL;
> + break;
> + }
> +
> + err = cb(sym_addr, sym_name, ctx);
> + if (err)
> + break;
> + }
> +
> + fclose(f);
> + return err;
> +}
> +
> +static void kprobe_multi_resolve_reinit(struct kprobe_multi_resolve *res)
> +{
> + free(res->addrs);
> +
> + /* reset to zero, when fallback */
> + res->cap = 0;
> + res->cnt = 0;
> + res->addrs = NULL;
> +}
> +
> struct bpf_link *
> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> const char *pattern,
> @@ -10477,9 +10539,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> return libbpf_err_ptr(-EINVAL);
>
> if (pattern) {
> - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> - if (err)
> - goto error;
> + err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
> + &res);
> + if (err) {
> + /* fallback to kallsyms */
> + kprobe_multi_resolve_reinit(&res);
instead of try, fail, try something else approach, can we check that
tracefs_available_filter_functions_addrs() exists and is readable, and
if yes -- commit to it. If something fails -- to bad. If it doesn't
exist, fallback to kallsyms. And we won't need ugly
kprobe_multi_resolve_reinit() hack.
> + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
> + &res);
> + if (err)
> + goto error;
> + }
> if (!res.cnt) {
> err = -ENOENT;
> goto error;
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
2023-06-26 23:46 ` Andrii Nakryiko
@ 2023-06-27 1:37 ` Jackie Liu
2023-06-30 19:01 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Jackie Liu @ 2023-06-27 1:37 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: olsajiri, andrii, martin.lau, song, yhs, bpf, liuyun01
在 2023/6/27 07:46, Andrii Nakryiko 写道:
> On Sat, Jun 24, 2023 at 6:14 PM Jackie Liu <liu.yun@linux.dev> wrote:
>>
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> When using regular expression matching with "kprobe multi", it scans all
>> the functions under "/proc/kallsyms" that can be matched. However, not all
>> of them can be traced by kprobe.multi. If any one of the functions fails
>> to be traced, it will result in the failure of all functions. The best
>> approach is to filter out the functions that cannot be traced to ensure
>> proper tracking of the functions.
>>
>> Use available_filter_functions_addrs check first, if failed, fallback to
>> kallsyms.
>>
>
> This is good, but it also doesn't address the original problem. On
> older kernels that don't have available_filter_functions_addrs, we'll
> still be trying to attach to non-attachable kprobes?
>
> So I think we'll still need to add available_filter_functions-based
> filtering on top of kallsyms. I guess we'll have to collect all
> symbols+addr from kallsyms, sort them, then go over
> available_filter_functions and do binary search. If element is not
> found, we'll mark it for removal. Then last pass to filter out marked
> entries to keep only addrs to be passed to kernel?
The first patch I submitted was to re-search available_filter_functions
in the case of kallsyms matching. The link is at:
https://lore.kernel.org/all/20230523132547.94384-1-liu.yun@linux.dev/,
but it is very slow and takes about 5s to start. If necessary, I can
continue to add it to the V3 patch. Maybe you have other better
algorithms?
--
Jackie Liu
>
>
>> Here is the test eBPF program [1].
>> [1] https://github.com/JackieLiu1/ketones/tree/master/src/funccount
>>
>> Suggested-by: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>> tools/lib/bpf/libbpf.c | 81 ++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 75 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index a27f6e9ccce7..fca5d2e412c5 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -10107,6 +10107,12 @@ static const char *tracefs_uprobe_events(void)
>> return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
>> }
>>
>> +static const char *tracefs_available_filter_functions_addrs(void)
>> +{
>> + return use_debugfs() ? DEBUGFS"/available_filter_functions_addrs" :
>> + TRACEFS"/available_filter_functions_addrs";
>> +}
>> +
>> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>> const char *kfunc_name, size_t offset)
>> {
>> @@ -10422,9 +10428,8 @@ struct kprobe_multi_resolve {
>> size_t cnt;
>> };
>>
>> -static int
>> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> - const char *sym_name, void *ctx)
>> +static int ftrace_resolve_kprobe_multi_cb(unsigned long long sym_addr,
>> + const char *sym_name, void *ctx)
>> {
>> struct kprobe_multi_resolve *res = ctx;
>> int err;
>> @@ -10441,6 +10446,63 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> return 0;
>> }
>>
>> +static int
>> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> + const char *sym_name, void *ctx)
>> +{
>> + return ftrace_resolve_kprobe_multi_cb(sym_addr, sym_name, ctx);
>> +}
>> +
>> +typedef int (*available_kprobe_cb_t)(unsigned long long sym_addr,
>> + const char *sym_name, void *ctx);
>> +
>> +static int
>> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
>> +{
>> + unsigned long long sym_addr;
>> + char sym_name[256];
>> + FILE *f;
>> + int ret, err = 0;
>> + const char *available_path = tracefs_available_filter_functions_addrs();
>> +
>> + f = fopen(available_path, "r");
>> + if (!f) {
>> + err = -errno;
>> + pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
>> + available_path);
>
> if this is expected to happen, we shouldn't pr_warn() and pollute log output
>
>> + return err;
>> + }
>> +
>> + while (true) {
>> + ret = fscanf(f, "%llx %255s%*[^\n]\n", &sym_addr, sym_name);
>> + if (ret == EOF && feof(f))
>> + break;
>> + if (ret != 2) {
>> + pr_warn("failed to read available kprobe entry: %d\n",
>> + ret);
>> + err = -EINVAL;
>> + break;
>> + }
>> +
>> + err = cb(sym_addr, sym_name, ctx);
>> + if (err)
>> + break;
>> + }
>> +
>> + fclose(f);
>> + return err;
>> +}
>> +
>> +static void kprobe_multi_resolve_reinit(struct kprobe_multi_resolve *res)
>> +{
>> + free(res->addrs);
>> +
>> + /* reset to zero, when fallback */
>> + res->cap = 0;
>> + res->cnt = 0;
>> + res->addrs = NULL;
>> +}
>> +
>> struct bpf_link *
>> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> const char *pattern,
>> @@ -10477,9 +10539,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> return libbpf_err_ptr(-EINVAL);
>>
>> if (pattern) {
>> - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
>> - if (err)
>> - goto error;
>> + err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
>> + &res);
>> + if (err) {
>> + /* fallback to kallsyms */
>> + kprobe_multi_resolve_reinit(&res);
>
> instead of try, fail, try something else approach, can we check that
> tracefs_available_filter_functions_addrs() exists and is readable, and
> if yes -- commit to it. If something fails -- to bad. If it doesn't
> exist, fallback to kallsyms. And we won't need ugly
> kprobe_multi_resolve_reinit() hack.
Sounds good.
>
>> + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
>> + &res);
>> + if (err)
>> + goto error;
>> + }
>> if (!res.cnt) {
>> err = -ENOENT;
>> goto error;
>> --
>> 2.25.1
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
2023-06-27 1:37 ` Jackie Liu
@ 2023-06-30 19:01 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2023-06-30 19:01 UTC (permalink / raw)
To: Jackie Liu; +Cc: olsajiri, andrii, martin.lau, song, yhs, bpf, liuyun01
On Mon, Jun 26, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
>
>
>
> 在 2023/6/27 07:46, Andrii Nakryiko 写道:
> > On Sat, Jun 24, 2023 at 6:14 PM Jackie Liu <liu.yun@linux.dev> wrote:
> >>
> >> From: Jackie Liu <liuyun01@kylinos.cn>
> >>
> >> When using regular expression matching with "kprobe multi", it scans all
> >> the functions under "/proc/kallsyms" that can be matched. However, not all
> >> of them can be traced by kprobe.multi. If any one of the functions fails
> >> to be traced, it will result in the failure of all functions. The best
> >> approach is to filter out the functions that cannot be traced to ensure
> >> proper tracking of the functions.
> >>
> >> Use available_filter_functions_addrs check first, if failed, fallback to
> >> kallsyms.
> >>
> >
> > This is good, but it also doesn't address the original problem. On
> > older kernels that don't have available_filter_functions_addrs, we'll
> > still be trying to attach to non-attachable kprobes?
> >
> > So I think we'll still need to add available_filter_functions-based
> > filtering on top of kallsyms. I guess we'll have to collect all
> > symbols+addr from kallsyms, sort them, then go over
> > available_filter_functions and do binary search. If element is not
> > found, we'll mark it for removal. Then last pass to filter out marked
> > entries to keep only addrs to be passed to kernel?
>
> The first patch I submitted was to re-search available_filter_functions
> in the case of kallsyms matching. The link is at:
>
> https://lore.kernel.org/all/20230523132547.94384-1-liu.yun@linux.dev/,
>
> but it is very slow and takes about 5s to start. If necessary, I can
> continue to add it to the V3 patch. Maybe you have other better
> algorithms?
Of course it's very slow. For every matched kallsyms symbol you are
reopening/reparsing/scanning linearly entire
available_filter_functions. You need to make sure that you parse both
kallsyms and available_filter_functions just once, and then join them
together to keep only symbols in both files.
See my suggestion above about sorting and binary search. That's one
way to do it efficiently.
>
> --
> Jackie Liu
>
> >
> >
> >> Here is the test eBPF program [1].
> >> [1] https://github.com/JackieLiu1/ketones/tree/master/src/funccount
> >>
> >> Suggested-by: Jiri Olsa <jolsa@kernel.org>
> >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >> ---
> >> tools/lib/bpf/libbpf.c | 81 ++++++++++++++++++++++++++++++++++++++----
> >> 1 file changed, 75 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index a27f6e9ccce7..fca5d2e412c5 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -10107,6 +10107,12 @@ static const char *tracefs_uprobe_events(void)
> >> return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
> >> }
> >>
> >> +static const char *tracefs_available_filter_functions_addrs(void)
> >> +{
> >> + return use_debugfs() ? DEBUGFS"/available_filter_functions_addrs" :
> >> + TRACEFS"/available_filter_functions_addrs";
> >> +}
> >> +
> >> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
> >> const char *kfunc_name, size_t offset)
> >> {
> >> @@ -10422,9 +10428,8 @@ struct kprobe_multi_resolve {
> >> size_t cnt;
> >> };
> >>
> >> -static int
> >> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >> - const char *sym_name, void *ctx)
> >> +static int ftrace_resolve_kprobe_multi_cb(unsigned long long sym_addr,
> >> + const char *sym_name, void *ctx)
> >> {
> >> struct kprobe_multi_resolve *res = ctx;
> >> int err;
> >> @@ -10441,6 +10446,63 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >> return 0;
> >> }
> >>
> >> +static int
> >> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >> + const char *sym_name, void *ctx)
> >> +{
> >> + return ftrace_resolve_kprobe_multi_cb(sym_addr, sym_name, ctx);
> >> +}
> >> +
> >> +typedef int (*available_kprobe_cb_t)(unsigned long long sym_addr,
> >> + const char *sym_name, void *ctx);
> >> +
> >> +static int
> >> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
> >> +{
> >> + unsigned long long sym_addr;
> >> + char sym_name[256];
> >> + FILE *f;
> >> + int ret, err = 0;
> >> + const char *available_path = tracefs_available_filter_functions_addrs();
> >> +
> >> + f = fopen(available_path, "r");
> >> + if (!f) {
> >> + err = -errno;
> >> + pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
> >> + available_path);
> >
> > if this is expected to happen, we shouldn't pr_warn() and pollute log output
> >
> >> + return err;
> >> + }
> >> +
> >> + while (true) {
> >> + ret = fscanf(f, "%llx %255s%*[^\n]\n", &sym_addr, sym_name);
> >> + if (ret == EOF && feof(f))
> >> + break;
> >> + if (ret != 2) {
> >> + pr_warn("failed to read available kprobe entry: %d\n",
> >> + ret);
> >> + err = -EINVAL;
> >> + break;
> >> + }
> >> +
> >> + err = cb(sym_addr, sym_name, ctx);
> >> + if (err)
> >> + break;
> >> + }
> >> +
> >> + fclose(f);
> >> + return err;
> >> +}
> >> +
> >> +static void kprobe_multi_resolve_reinit(struct kprobe_multi_resolve *res)
> >> +{
> >> + free(res->addrs);
> >> +
> >> + /* reset to zero, when fallback */
> >> + res->cap = 0;
> >> + res->cnt = 0;
> >> + res->addrs = NULL;
> >> +}
> >> +
> >> struct bpf_link *
> >> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >> const char *pattern,
> >> @@ -10477,9 +10539,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >> return libbpf_err_ptr(-EINVAL);
> >>
> >> if (pattern) {
> >> - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> >> - if (err)
> >> - goto error;
> >> + err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
> >> + &res);
> >> + if (err) {
> >> + /* fallback to kallsyms */
> >> + kprobe_multi_resolve_reinit(&res);
> >
> > instead of try, fail, try something else approach, can we check that
> > tracefs_available_filter_functions_addrs() exists and is readable, and
> > if yes -- commit to it. If something fails -- to bad. If it doesn't
> > exist, fallback to kallsyms. And we won't need ugly
> > kprobe_multi_resolve_reinit() hack.
>
> Sounds good.
>
> >
> >> + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
> >> + &res);
> >> + if (err)
> >> + goto error;
> >> + }
> >> if (!res.cnt) {
> >> err = -ENOENT;
> >> goto error;
> >> --
> >> 2.25.1
> >>
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-30 19:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-25 1:13 [PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs Jackie Liu
2023-06-25 1:56 ` Jiri Olsa
2023-06-26 23:46 ` Andrii Nakryiko
2023-06-27 1:37 ` Jackie Liu
2023-06-30 19:01 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).