* Fix repeated legacy kprobes on same function
@ 2021-12-24 4:01 Qiang Wang
2021-12-24 6:57 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Qiang Wang @ 2021-12-24 4:01 UTC (permalink / raw)
To: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
kpsingh, zhouchengming, songmuchun, duanxiongchun, shekairui
Cc: netdev, bpf, linux-kernel
If repeated legacy kprobes on same function in one process,
libbpf will register using the same probe name and got -EBUSY
error. So append index to the probe name format to fix this
problem.
And fix a bug in commit 46ed5fc33db9, which wrongly used the
func_name instead of probe_name to register.
Fixes: 46ed5fc33db9 ("libbpf: Refactor and simplify legacy kprobe code")
Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
Signed-off-by: Qiang Wang <wangqiang.wq.frank@bytedance.com>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
tools/lib/bpf/libbpf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7c74342bb668..7d1097958459 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9634,7 +9634,8 @@ static int append_to_file(const char *file, const
char *fmt, ...)
static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
const char *kfunc_name, size_t
offset)
{
- snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(),
kfunc_name, offset);
+ static int index = 0;
+ snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(),
kfunc_name, offset, index++);
}
static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
@@ -9735,7 +9736,7 @@ bpf_program__attach_kprobe_opts(const struct
bpf_program *prog,
gen_kprobe_legacy_event_name(probe_name,
sizeof(probe_name),
func_name, offset);
- legacy_probe = strdup(func_name);
+ legacy_probe = strdup(probe_name);
if (!legacy_probe)
return libbpf_err_ptr(-ENOMEM);
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: Fix repeated legacy kprobes on same function
2021-12-24 4:01 Fix repeated legacy kprobes on same function Qiang Wang
@ 2021-12-24 6:57 ` Andrii Nakryiko
2021-12-24 7:38 ` [External] " Chengming Zhou
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-12-24 6:57 UTC (permalink / raw)
To: Qiang Wang
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, zhouchengming,
Muchun Song, duanxiongchun, shekairui, Networking, bpf,
open list
On Thu, Dec 23, 2021 at 8:01 PM Qiang Wang
<wangqiang.wq.frank@bytedance.com> wrote:
>
> If repeated legacy kprobes on same function in one process,
> libbpf will register using the same probe name and got -EBUSY
> error. So append index to the probe name format to fix this
> problem.
>
> And fix a bug in commit 46ed5fc33db9, which wrongly used the
> func_name instead of probe_name to register.
>
> Fixes: 46ed5fc33db9 ("libbpf: Refactor and simplify legacy kprobe code")
> Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
> Signed-off-by: Qiang Wang <wangqiang.wq.frank@bytedance.com>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
>
> ---
> tools/lib/bpf/libbpf.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 7c74342bb668..7d1097958459 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9634,7 +9634,8 @@ static int append_to_file(const char *file, const
> char *fmt, ...)
> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
> const char *kfunc_name, size_t
> offset)
> {
> - snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(),
> kfunc_name, offset);
> + static int index = 0;
> + snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(),
> kfunc_name, offset, index++);
BCC doesn't add this auto-increment (which is also not thread-safe)
and it seems like that works fine for all users.
What is the use case where you'd like to attach to the same kernel
function multiple times with legacy kprobe?
> }
>
> static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
> @@ -9735,7 +9736,7 @@ bpf_program__attach_kprobe_opts(const struct
> bpf_program *prog,
> gen_kprobe_legacy_event_name(probe_name,
> sizeof(probe_name),
> func_name, offset);
>
> - legacy_probe = strdup(func_name);
> + legacy_probe = strdup(probe_name);
please send this as a separate fix
> if (!legacy_probe)
> return libbpf_err_ptr(-ENOMEM);
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [External] Re: Fix repeated legacy kprobes on same function
2021-12-24 6:57 ` Andrii Nakryiko
@ 2021-12-24 7:38 ` Chengming Zhou
0 siblings, 0 replies; 3+ messages in thread
From: Chengming Zhou @ 2021-12-24 7:38 UTC (permalink / raw)
To: Andrii Nakryiko, Qiang Wang
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Muchun Song,
duanxiongchun, shekairui, Networking, bpf, open list
On 2021/12/24 2:57 下午, Andrii Nakryiko wrote:
> On Thu, Dec 23, 2021 at 8:01 PM Qiang Wang
> <wangqiang.wq.frank@bytedance.com> wrote:
>>
>> If repeated legacy kprobes on same function in one process,
>> libbpf will register using the same probe name and got -EBUSY
>> error. So append index to the probe name format to fix this
>> problem.
>>
>> And fix a bug in commit 46ed5fc33db9, which wrongly used the
>> func_name instead of probe_name to register.
>>
>> Fixes: 46ed5fc33db9 ("libbpf: Refactor and simplify legacy kprobe code")
>> Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
>> Signed-off-by: Qiang Wang <wangqiang.wq.frank@bytedance.com>
>> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
>>
>> ---
>> tools/lib/bpf/libbpf.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 7c74342bb668..7d1097958459 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -9634,7 +9634,8 @@ static int append_to_file(const char *file, const
>> char *fmt, ...)
>> static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>> const char *kfunc_name, size_t
>> offset)
>> {
>> - snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(),
>> kfunc_name, offset);
>> + static int index = 0;
>> + snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(),
>> kfunc_name, offset, index++);
>
> BCC doesn't add this auto-increment (which is also not thread-safe)
> and it seems like that works fine for all users.
>
Yes, BCC has the same problem, we will send a fix patch to BCC later.
We thought libbpf was used in single-threaded environment, so will
change to use __sync_fetch_and_add() to keep thread-safe. Thanks for
pointing this out.
> What is the use case where you'd like to attach to the same kernel
> function multiple times with legacy kprobe?
>
We used many different BPF modules writen by different people in one
monitor process to analyze all data, there maybe repeated legacy kprobes
on the same function. So we want to add a unique index suffix to support
this use case.
>> }
>>
>> static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
>> @@ -9735,7 +9736,7 @@ bpf_program__attach_kprobe_opts(const struct
>> bpf_program *prog,
>> gen_kprobe_legacy_event_name(probe_name,
>> sizeof(probe_name),
>> func_name, offset);
>>
>> - legacy_probe = strdup(func_name);
>> + legacy_probe = strdup(probe_name);
>
> please send this as a separate fix
>
Ok, will do.
Thanks.
>> if (!legacy_probe)
>> return libbpf_err_ptr(-ENOMEM);
>>
>> --
>> 2.20.1
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-24 7:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-24 4:01 Fix repeated legacy kprobes on same function Qiang Wang
2021-12-24 6:57 ` Andrii Nakryiko
2021-12-24 7:38 ` [External] " Chengming Zhou
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.