* [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi [not found] <20250623134342.227347-1-chen.dylane@linux.dev> @ 2025-06-23 13:43 ` Tao Chen 2025-06-23 22:14 ` Andrii Nakryiko 2025-06-23 13:43 ` [PATCH bpf-next v5 3/3] bpf: Add show_fdinfo for kprobe_multi Tao Chen 2025-06-23 17:55 ` [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo Alexei Starovoitov 2 siblings, 1 reply; 11+ messages in thread From: Tao Chen @ 2025-06-23 13:43 UTC (permalink / raw) To: kpsingh, mattbobrowski, song, jolsa, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers Cc: bpf, linux-kernel, linux-trace-kernel, Tao Chen Show uprobe_multi link info with fdinfo, the info as follows: link_type: uprobe_multi link_id: 10 prog_tag: 7db356c03e61a4d4 prog_id: 42 uprobe_cnt: 3 pid: 0 path: /home/dylane/bpf/tools/testing/selftests/bpf/test_progs offset ref_ctr_offset cookie 0xa69f13 0x0 2 0xa69f1e 0x0 3 0xa69f29 0x0 1 Signed-off-by: Tao Chen <chen.dylane@linux.dev> --- kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 8ecb1a9f85d..90209dda819 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -3171,10 +3171,54 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, return err; } +#ifdef CONFIG_PROC_FS +static void bpf_uprobe_multi_show_fdinfo(const struct bpf_link *link, + struct seq_file *seq) +{ + struct bpf_uprobe_multi_link *umulti_link; + char *p, *buf; + + umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); + + buf = kmalloc(PATH_MAX, GFP_KERNEL); + if (!buf) + return; + + p = d_path(&umulti_link->path, buf, PATH_MAX); + if (IS_ERR(p)) { + kfree(buf); + return; + } + + seq_printf(seq, + "uprobe_cnt:\t%u\n" + "pid:\t%u\n" + "path:\t%s\n", + umulti_link->cnt, + umulti_link->task ? task_pid_nr_ns(umulti_link->task, + task_active_pid_ns(current)) : 0, + p); + + seq_printf(seq, "%-16s %-16s %-16s\n", "offset", "ref_ctr_offset", "cookie"); + for (int i = 0; i < umulti_link->cnt; i++) { + seq_printf(seq, + "%#-16llx %#-16lx %-16llu\n", + umulti_link->uprobes[i].offset, + umulti_link->uprobes[i].ref_ctr_offset, + umulti_link->uprobes[i].cookie); + } + + kfree(buf); +} +#endif + static const struct bpf_link_ops bpf_uprobe_multi_link_lops = { .release = bpf_uprobe_multi_link_release, .dealloc_deferred = bpf_uprobe_multi_link_dealloc, .fill_link_info = bpf_uprobe_multi_link_fill_link_info, +#ifdef CONFIG_PROC_FS + .show_fdinfo = bpf_uprobe_multi_show_fdinfo, +#endif }; static int uprobe_prog_run(struct bpf_uprobe *uprobe, -- 2.48.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi 2025-06-23 13:43 ` [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi Tao Chen @ 2025-06-23 22:14 ` Andrii Nakryiko 2025-06-24 8:42 ` Tao Chen 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2025-06-23 22:14 UTC (permalink / raw) To: Tao Chen Cc: kpsingh, mattbobrowski, song, jolsa, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel On Mon, Jun 23, 2025 at 6:45 AM Tao Chen <chen.dylane@linux.dev> wrote: > > Show uprobe_multi link info with fdinfo, the info as follows: > > link_type: uprobe_multi > link_id: 10 > prog_tag: 7db356c03e61a4d4 > prog_id: 42 > uprobe_cnt: 3 > pid: 0 > path: /home/dylane/bpf/tools/testing/selftests/bpf/test_progs > offset ref_ctr_offset cookie > 0xa69f13 0x0 2 > 0xa69f1e 0x0 3 > 0xa69f29 0x0 1 > > Signed-off-by: Tao Chen <chen.dylane@linux.dev> > --- > kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 8ecb1a9f85d..90209dda819 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -3171,10 +3171,54 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, > return err; > } > > +#ifdef CONFIG_PROC_FS > +static void bpf_uprobe_multi_show_fdinfo(const struct bpf_link *link, > + struct seq_file *seq) > +{ > + struct bpf_uprobe_multi_link *umulti_link; > + char *p, *buf; > + > + umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); > + > + buf = kmalloc(PATH_MAX, GFP_KERNEL); > + if (!buf) > + return; > + > + p = d_path(&umulti_link->path, buf, PATH_MAX); > + if (IS_ERR(p)) { > + kfree(buf); > + return; > + } > + > + seq_printf(seq, > + "uprobe_cnt:\t%u\n" > + "pid:\t%u\n" > + "path:\t%s\n", > + umulti_link->cnt, > + umulti_link->task ? task_pid_nr_ns(umulti_link->task, > + task_active_pid_ns(current)) : 0, nit: either keep stuff like this on single line, or if it's too long or awkward, add a local variable. Formatting it like this is very hard to follow and looks pretty ugly... > + p); > + > + seq_printf(seq, "%-16s %-16s %-16s\n", "offset", "ref_ctr_offset", "cookie"); > + for (int i = 0; i < umulti_link->cnt; i++) { > + seq_printf(seq, > + "%#-16llx %#-16lx %-16llu\n", > + umulti_link->uprobes[i].offset, > + umulti_link->uprobes[i].ref_ctr_offset, > + umulti_link->uprobes[i].cookie); > + } > + > + kfree(buf); > +} > +#endif > + > static const struct bpf_link_ops bpf_uprobe_multi_link_lops = { > .release = bpf_uprobe_multi_link_release, > .dealloc_deferred = bpf_uprobe_multi_link_dealloc, > .fill_link_info = bpf_uprobe_multi_link_fill_link_info, > +#ifdef CONFIG_PROC_FS > + .show_fdinfo = bpf_uprobe_multi_show_fdinfo, > +#endif > }; > > static int uprobe_prog_run(struct bpf_uprobe *uprobe, > -- > 2.48.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi 2025-06-23 22:14 ` Andrii Nakryiko @ 2025-06-24 8:42 ` Tao Chen 0 siblings, 0 replies; 11+ messages in thread From: Tao Chen @ 2025-06-24 8:42 UTC (permalink / raw) To: Andrii Nakryiko Cc: kpsingh, mattbobrowski, song, jolsa, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel 在 2025/6/24 06:14, Andrii Nakryiko 写道: > On Mon, Jun 23, 2025 at 6:45 AM Tao Chen <chen.dylane@linux.dev> wrote: >> >> Show uprobe_multi link info with fdinfo, the info as follows: >> >> link_type: uprobe_multi >> link_id: 10 >> prog_tag: 7db356c03e61a4d4 >> prog_id: 42 >> uprobe_cnt: 3 >> pid: 0 >> path: /home/dylane/bpf/tools/testing/selftests/bpf/test_progs >> offset ref_ctr_offset cookie >> 0xa69f13 0x0 2 >> 0xa69f1e 0x0 3 >> 0xa69f29 0x0 1 >> >> Signed-off-by: Tao Chen <chen.dylane@linux.dev> >> --- >> kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >> index 8ecb1a9f85d..90209dda819 100644 >> --- a/kernel/trace/bpf_trace.c >> +++ b/kernel/trace/bpf_trace.c >> @@ -3171,10 +3171,54 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, >> return err; >> } >> >> +#ifdef CONFIG_PROC_FS >> +static void bpf_uprobe_multi_show_fdinfo(const struct bpf_link *link, >> + struct seq_file *seq) >> +{ >> + struct bpf_uprobe_multi_link *umulti_link; >> + char *p, *buf; >> + >> + umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); >> + >> + buf = kmalloc(PATH_MAX, GFP_KERNEL); >> + if (!buf) >> + return; >> + >> + p = d_path(&umulti_link->path, buf, PATH_MAX); >> + if (IS_ERR(p)) { >> + kfree(buf); >> + return; >> + } >> + >> + seq_printf(seq, >> + "uprobe_cnt:\t%u\n" >> + "pid:\t%u\n" >> + "path:\t%s\n", >> + umulti_link->cnt, >> + umulti_link->task ? task_pid_nr_ns(umulti_link->task, >> + task_active_pid_ns(current)) : 0, > > nit: either keep stuff like this on single line, or if it's too long > or awkward, add a local variable. Formatting it like this is very hard > to follow and looks pretty ugly... > will fix it in v6, thanks. >> + p); >> + >> + seq_printf(seq, "%-16s %-16s %-16s\n", "offset", "ref_ctr_offset", "cookie"); >> + for (int i = 0; i < umulti_link->cnt; i++) { >> + seq_printf(seq, >> + "%#-16llx %#-16lx %-16llu\n", >> + umulti_link->uprobes[i].offset, >> + umulti_link->uprobes[i].ref_ctr_offset, >> + umulti_link->uprobes[i].cookie); >> + } >> + >> + kfree(buf); >> +} >> +#endif >> + >> static const struct bpf_link_ops bpf_uprobe_multi_link_lops = { >> .release = bpf_uprobe_multi_link_release, >> .dealloc_deferred = bpf_uprobe_multi_link_dealloc, >> .fill_link_info = bpf_uprobe_multi_link_fill_link_info, >> +#ifdef CONFIG_PROC_FS >> + .show_fdinfo = bpf_uprobe_multi_show_fdinfo, >> +#endif >> }; >> >> static int uprobe_prog_run(struct bpf_uprobe *uprobe, >> -- >> 2.48.1 >> -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next v5 3/3] bpf: Add show_fdinfo for kprobe_multi [not found] <20250623134342.227347-1-chen.dylane@linux.dev> 2025-06-23 13:43 ` [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi Tao Chen @ 2025-06-23 13:43 ` Tao Chen 2025-06-23 17:55 ` [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo Alexei Starovoitov 2 siblings, 0 replies; 11+ messages in thread From: Tao Chen @ 2025-06-23 13:43 UTC (permalink / raw) To: kpsingh, mattbobrowski, song, jolsa, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, sdf, haoluo, rostedt, mhiramat, mathieu.desnoyers Cc: bpf, linux-kernel, linux-trace-kernel, Tao Chen Show kprobe_multi link info with fdinfo, the info as follows: link_type: kprobe_multi link_id: 1 prog_tag: 33be53a4fd673e1d prog_id: 21 kprobe_cnt: 8 missed: 0 cookie func 1 bpf_fentry_test1+0x0/0x20 7 bpf_fentry_test2+0x0/0x20 2 bpf_fentry_test3+0x0/0x20 3 bpf_fentry_test4+0x0/0x20 4 bpf_fentry_test5+0x0/0x20 5 bpf_fentry_test6+0x0/0x20 6 bpf_fentry_test7+0x0/0x20 8 bpf_fentry_test8+0x0/0x10 Signed-off-by: Tao Chen <chen.dylane@linux.dev> --- kernel/trace/bpf_trace.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 90209dda819..957e5b319ef 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2623,10 +2623,37 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, return err; } +#ifdef CONFIG_PROC_FS +static void bpf_kprobe_multi_show_fdinfo(const struct bpf_link *link, + struct seq_file *seq) +{ + struct bpf_kprobe_multi_link *kmulti_link; + + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); + + seq_printf(seq, + "kprobe_cnt:\t%u\n" + "missed:\t%lu\n", + kmulti_link->cnt, + kmulti_link->fp.nmissed); + + seq_printf(seq, "%-16s %-16s\n", "cookie", "func"); + for (int i = 0; i < kmulti_link->cnt; i++) { + seq_printf(seq, + "%-16llu %-16pS\n", + kmulti_link->cookies[i], + (void *)kmulti_link->addrs[i]); + } +} +#endif + static const struct bpf_link_ops bpf_kprobe_multi_link_lops = { .release = bpf_kprobe_multi_link_release, .dealloc_deferred = bpf_kprobe_multi_link_dealloc, .fill_link_info = bpf_kprobe_multi_link_fill_link_info, +#ifdef CONFIG_PROC_FS + .show_fdinfo = bpf_kprobe_multi_show_fdinfo, +#endif }; static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv) -- 2.48.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo [not found] <20250623134342.227347-1-chen.dylane@linux.dev> 2025-06-23 13:43 ` [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi Tao Chen 2025-06-23 13:43 ` [PATCH bpf-next v5 3/3] bpf: Add show_fdinfo for kprobe_multi Tao Chen @ 2025-06-23 17:55 ` Alexei Starovoitov 2025-06-23 20:59 ` Andrii Nakryiko 2 siblings, 1 reply; 11+ messages in thread From: Alexei Starovoitov @ 2025-06-23 17:55 UTC (permalink / raw) To: Tao Chen Cc: KP Singh, Matt Bobrowski, Song Liu, Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: > > Alexei suggested, 'link_type' can be more precise and differentiate > for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes > kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we > can show it more concretely. > > link_type: kprobe_multi > link_id: 1 > prog_tag: d2b307e915f0dd37 > ... > link_type: kretprobe_multi > link_id: 2 > prog_tag: ab9ea0545870781d > ... > link_type: uprobe_multi > link_id: 9 > prog_tag: e729f789e34a8eca > ... > link_type: uretprobe_multi > link_id: 10 > prog_tag: 7db356c03e61a4d4 > > Signed-off-by: Tao Chen <chen.dylane@linux.dev> > --- > include/linux/trace_events.h | 10 ++++++++++ > kernel/bpf/syscall.c | 9 ++++++++- > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+), 1 deletion(-) > > Change list: > v4 -> v5: > - Add patch1 to show precise link_type for > {uprobe,kprobe}_multi.(Alexei) > - patch2,3 just remove type field, which will be showed in > link_type > v4: > https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev > > v3 -> v4: > - use %pS to print func info.(Alexei) > v3: > https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev > > v2 -> v3: > - show info in one line for multi events.(Jiri) > v2: > https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev > > v1 -> v2: > - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) > - print func name is more readable and security for kprobe_multi.(Alexei) > v1: > https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index fa9cf4292df..951c91babbc 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, > unsigned long *missed); > int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > #else > static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > { > @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > { > return -EOPNOTSUPP; > } > +static inline void > +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > +{ > +} > +static inline void > +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > +{ > +} > #endif > > enum { > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 51ba1a7aa43..43b821b37bc 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > const struct bpf_prog *prog = link->prog; > enum bpf_link_type type = link->type; > char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > + char link_type[64] = {}; > > if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > + else > + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); > + seq_printf(m, "link_type:\t%s\n", link_type); New callbacks just to print a string? Let's find a different way. How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? (There is a 7 byte hole there anyway) and checking flags inline. Jiri, Andrii, better ideas? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-23 17:55 ` [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo Alexei Starovoitov @ 2025-06-23 20:59 ` Andrii Nakryiko 2025-06-24 8:16 ` Jiri Olsa 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2025-06-23 20:59 UTC (permalink / raw) To: Alexei Starovoitov Cc: Tao Chen, KP Singh, Matt Bobrowski, Song Liu, Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: > > > > Alexei suggested, 'link_type' can be more precise and differentiate > > for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes > > kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we > > can show it more concretely. > > > > link_type: kprobe_multi > > link_id: 1 > > prog_tag: d2b307e915f0dd37 > > ... > > link_type: kretprobe_multi > > link_id: 2 > > prog_tag: ab9ea0545870781d > > ... > > link_type: uprobe_multi > > link_id: 9 > > prog_tag: e729f789e34a8eca > > ... > > link_type: uretprobe_multi > > link_id: 10 > > prog_tag: 7db356c03e61a4d4 > > > > Signed-off-by: Tao Chen <chen.dylane@linux.dev> > > --- > > include/linux/trace_events.h | 10 ++++++++++ > > kernel/bpf/syscall.c | 9 ++++++++- > > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > > 3 files changed, 46 insertions(+), 1 deletion(-) > > > > Change list: > > v4 -> v5: > > - Add patch1 to show precise link_type for > > {uprobe,kprobe}_multi.(Alexei) > > - patch2,3 just remove type field, which will be showed in > > link_type > > v4: > > https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev > > > > v3 -> v4: > > - use %pS to print func info.(Alexei) > > v3: > > https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev > > > > v2 -> v3: > > - show info in one line for multi events.(Jiri) > > v2: > > https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev > > > > v1 -> v2: > > - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) > > - print func name is more readable and security for kprobe_multi.(Alexei) > > v1: > > https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev > > > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > > index fa9cf4292df..951c91babbc 100644 > > --- a/include/linux/trace_events.h > > +++ b/include/linux/trace_events.h > > @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, > > unsigned long *missed); > > int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > > int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > > +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > > +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > > #else > > static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > > { > > @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > > { > > return -EOPNOTSUPP; > > } > > +static inline void > > +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > > +{ > > +} > > +static inline void > > +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > > +{ > > +} > > #endif > > > > enum { > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 51ba1a7aa43..43b821b37bc 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > > const struct bpf_prog *prog = link->prog; > > enum bpf_link_type type = link->type; > > char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > > + char link_type[64] = {}; > > > > if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > > - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > > + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > > + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > > + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > > + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > > + else > > + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); > > + seq_printf(m, "link_type:\t%s\n", link_type); > > New callbacks just to print a string? > Let's find a different way. > > How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? > (There is a 7 byte hole there anyway) > and checking flags inline. > > Jiri, Andrii, > > better ideas? We can just remember original attr->link_create.attach_type in bpf_link itself, and then have a small helper that will accept link type and attach type, and fill out link type representation based on those two. Internally we can do the special-casing of uprobe vs uretprobe and kprobe vs kretprobe transparently to all the other code. And use that here in show_fdinfo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-23 20:59 ` Andrii Nakryiko @ 2025-06-24 8:16 ` Jiri Olsa 2025-06-24 8:41 ` Tao Chen 0 siblings, 1 reply; 11+ messages in thread From: Jiri Olsa @ 2025-06-24 8:16 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Tao Chen, KP Singh, Matt Bobrowski, Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel On Mon, Jun 23, 2025 at 01:59:18PM -0700, Andrii Nakryiko wrote: > On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: > > > > > > Alexei suggested, 'link_type' can be more precise and differentiate > > > for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes > > > kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we > > > can show it more concretely. > > > > > > link_type: kprobe_multi > > > link_id: 1 > > > prog_tag: d2b307e915f0dd37 > > > ... > > > link_type: kretprobe_multi > > > link_id: 2 > > > prog_tag: ab9ea0545870781d > > > ... > > > link_type: uprobe_multi > > > link_id: 9 > > > prog_tag: e729f789e34a8eca > > > ... > > > link_type: uretprobe_multi > > > link_id: 10 > > > prog_tag: 7db356c03e61a4d4 > > > > > > Signed-off-by: Tao Chen <chen.dylane@linux.dev> > > > --- > > > include/linux/trace_events.h | 10 ++++++++++ > > > kernel/bpf/syscall.c | 9 ++++++++- > > > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > > > 3 files changed, 46 insertions(+), 1 deletion(-) > > > > > > Change list: > > > v4 -> v5: > > > - Add patch1 to show precise link_type for > > > {uprobe,kprobe}_multi.(Alexei) > > > - patch2,3 just remove type field, which will be showed in > > > link_type > > > v4: > > > https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev > > > > > > v3 -> v4: > > > - use %pS to print func info.(Alexei) > > > v3: > > > https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev > > > > > > v2 -> v3: > > > - show info in one line for multi events.(Jiri) > > > v2: > > > https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev > > > > > > v1 -> v2: > > > - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) > > > - print func name is more readable and security for kprobe_multi.(Alexei) > > > v1: > > > https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev > > > > > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > > > index fa9cf4292df..951c91babbc 100644 > > > --- a/include/linux/trace_events.h > > > +++ b/include/linux/trace_events.h > > > @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, > > > unsigned long *missed); > > > int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > > > int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > > > +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > > > +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > > > #else > > > static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > > > { > > > @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > > > { > > > return -EOPNOTSUPP; > > > } > > > +static inline void > > > +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > > > +{ > > > +} > > > +static inline void > > > +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > > > +{ > > > +} > > > #endif > > > > > > enum { > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > > index 51ba1a7aa43..43b821b37bc 100644 > > > --- a/kernel/bpf/syscall.c > > > +++ b/kernel/bpf/syscall.c > > > @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > > > const struct bpf_prog *prog = link->prog; > > > enum bpf_link_type type = link->type; > > > char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > > > + char link_type[64] = {}; > > > > > > if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > > > - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > > > + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > > > + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > > > + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > > > + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > > > + else > > > + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); > > > + seq_printf(m, "link_type:\t%s\n", link_type); > > > > New callbacks just to print a string? > > Let's find a different way. > > > > How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? > > (There is a 7 byte hole there anyway) > > and checking flags inline. > > > > Jiri, Andrii, > > > > better ideas? > > We can just remember original attr->link_create.attach_type in > bpf_link itself, and then have a small helper that will accept link > type and attach type, and fill out link type representation based on > those two. Internally we can do the special-casing of uprobe vs > uretprobe and kprobe vs kretprobe transparently to all the other code. > And use that here in show_fdinfo but you'd still need the flags, no? to find out if it's return probe I tried what Alexei suggested and it seems ok and simple enough jirka --- diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 5dd556e89cce..287c956cdbd2 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1702,6 +1702,7 @@ struct bpf_link { * link's semantics is determined by target attach hook */ bool sleepable; + u32 flags; /* rcu is used before freeing, work can be used to schedule that * RCU-based freeing before that, so they never overlap */ diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 56500381c28a..f1d9ee9717a1 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3228,7 +3228,14 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ? + "kretprobe_multi" : "kprobe_multi"); + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ? + "uretprobe_multi" : "uprobe_multi"); + else + seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); } else { WARN_ONCE(1, "missing BPF_LINK_TYPE(...) for link type %u\n", type); seq_printf(m, "link_type:\t<%u>\n", type); diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 0a06ea6638fe..81d7a4e5ae15 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2466,7 +2466,6 @@ struct bpf_kprobe_multi_link { u32 cnt; u32 mods_cnt; struct module **mods; - u32 flags; }; struct bpf_kprobe_multi_run_ctx { @@ -2586,7 +2585,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); info->kprobe_multi.count = kmulti_link->cnt; - info->kprobe_multi.flags = kmulti_link->flags; + info->kprobe_multi.flags = kmulti_link->link.flags; info->kprobe_multi.missed = kmulti_link->fp.nmissed; if (!uaddrs) @@ -2976,7 +2975,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr link->addrs = addrs; link->cookies = cookies; link->cnt = cnt; - link->flags = flags; + link->link.flags = flags; if (cookies) { /* @@ -3045,7 +3044,6 @@ struct bpf_uprobe_multi_link { struct path path; struct bpf_link link; u32 cnt; - u32 flags; struct bpf_uprobe *uprobes; struct task_struct *task; }; @@ -3109,7 +3107,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); info->uprobe_multi.count = umulti_link->cnt; - info->uprobe_multi.flags = umulti_link->flags; + info->uprobe_multi.flags = umulti_link->link.flags; info->uprobe_multi.pid = umulti_link->task ? task_pid_nr_ns(umulti_link->task, task_active_pid_ns(current)) : 0; @@ -3369,7 +3367,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr link->uprobes = uprobes; link->path = path; link->task = task; - link->flags = flags; + link->link.flags = flags; bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI, &bpf_uprobe_multi_link_lops, prog); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-24 8:16 ` Jiri Olsa @ 2025-06-24 8:41 ` Tao Chen 2025-06-24 15:46 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: Tao Chen @ 2025-06-24 8:41 UTC (permalink / raw) To: Jiri Olsa, Andrii Nakryiko Cc: Alexei Starovoitov, KP Singh, Matt Bobrowski, Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel 在 2025/6/24 16:16, Jiri Olsa 写道: > On Mon, Jun 23, 2025 at 01:59:18PM -0700, Andrii Nakryiko wrote: >> On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov >> <alexei.starovoitov@gmail.com> wrote: >>> >>> On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: >>>> >>>> Alexei suggested, 'link_type' can be more precise and differentiate >>>> for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes >>>> kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we >>>> can show it more concretely. >>>> >>>> link_type: kprobe_multi >>>> link_id: 1 >>>> prog_tag: d2b307e915f0dd37 >>>> ... >>>> link_type: kretprobe_multi >>>> link_id: 2 >>>> prog_tag: ab9ea0545870781d >>>> ... >>>> link_type: uprobe_multi >>>> link_id: 9 >>>> prog_tag: e729f789e34a8eca >>>> ... >>>> link_type: uretprobe_multi >>>> link_id: 10 >>>> prog_tag: 7db356c03e61a4d4 >>>> >>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev> >>>> --- >>>> include/linux/trace_events.h | 10 ++++++++++ >>>> kernel/bpf/syscall.c | 9 ++++++++- >>>> kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ >>>> 3 files changed, 46 insertions(+), 1 deletion(-) >>>> >>>> Change list: >>>> v4 -> v5: >>>> - Add patch1 to show precise link_type for >>>> {uprobe,kprobe}_multi.(Alexei) >>>> - patch2,3 just remove type field, which will be showed in >>>> link_type >>>> v4: >>>> https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev >>>> >>>> v3 -> v4: >>>> - use %pS to print func info.(Alexei) >>>> v3: >>>> https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev >>>> >>>> v2 -> v3: >>>> - show info in one line for multi events.(Jiri) >>>> v2: >>>> https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev >>>> >>>> v1 -> v2: >>>> - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) >>>> - print func name is more readable and security for kprobe_multi.(Alexei) >>>> v1: >>>> https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev >>>> >>>> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h >>>> index fa9cf4292df..951c91babbc 100644 >>>> --- a/include/linux/trace_events.h >>>> +++ b/include/linux/trace_events.h >>>> @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, >>>> unsigned long *missed); >>>> int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); >>>> int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); >>>> +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); >>>> +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); >>>> #else >>>> static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) >>>> { >>>> @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) >>>> { >>>> return -EOPNOTSUPP; >>>> } >>>> +static inline void >>>> +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) >>>> +{ >>>> +} >>>> +static inline void >>>> +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) >>>> +{ >>>> +} >>>> #endif >>>> >>>> enum { >>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>>> index 51ba1a7aa43..43b821b37bc 100644 >>>> --- a/kernel/bpf/syscall.c >>>> +++ b/kernel/bpf/syscall.c >>>> @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) >>>> const struct bpf_prog *prog = link->prog; >>>> enum bpf_link_type type = link->type; >>>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; >>>> + char link_type[64] = {}; >>>> >>>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { >>>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); >>>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) >>>> + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); >>>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) >>>> + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); >>>> + else >>>> + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); >>>> + seq_printf(m, "link_type:\t%s\n", link_type); >>> >>> New callbacks just to print a string? >>> Let's find a different way. >>> >>> How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? >>> (There is a 7 byte hole there anyway) >>> and checking flags inline. >>> >>> Jiri, Andrii, >>> >>> better ideas? >> >> We can just remember original attr->link_create.attach_type in >> bpf_link itself, and then have a small helper that will accept link >> type and attach type, and fill out link type representation based on >> those two. Internally we can do the special-casing of uprobe vs >> uretprobe and kprobe vs kretprobe transparently to all the other code. >> And use that here in show_fdinfo > > but you'd still need the flags, no? to find out if it's return probe > > I tried what Alexei suggested and it seems ok and simple enough > > jirka > > > --- > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 5dd556e89cce..287c956cdbd2 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1702,6 +1702,7 @@ struct bpf_link { > * link's semantics is determined by target attach hook > */ > bool sleepable; > + u32 flags; > /* rcu is used before freeing, work can be used to schedule that > * RCU-based freeing before that, so they never overlap > */ > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 56500381c28a..f1d9ee9717a1 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3228,7 +3228,14 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > > if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ? > + "kretprobe_multi" : "kprobe_multi"); > + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ? > + "uretprobe_multi" : "uprobe_multi"); > + else > + seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > } else { > WARN_ONCE(1, "missing BPF_LINK_TYPE(...) for link type %u\n", type); > seq_printf(m, "link_type:\t<%u>\n", type); > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 0a06ea6638fe..81d7a4e5ae15 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2466,7 +2466,6 @@ struct bpf_kprobe_multi_link { > u32 cnt; > u32 mods_cnt; > struct module **mods; > - u32 flags; > }; > > struct bpf_kprobe_multi_run_ctx { > @@ -2586,7 +2585,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > > kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > info->kprobe_multi.count = kmulti_link->cnt; > - info->kprobe_multi.flags = kmulti_link->flags; > + info->kprobe_multi.flags = kmulti_link->link.flags; > info->kprobe_multi.missed = kmulti_link->fp.nmissed; > > if (!uaddrs) > @@ -2976,7 +2975,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > link->addrs = addrs; > link->cookies = cookies; > link->cnt = cnt; > - link->flags = flags; > + link->link.flags = flags; > > if (cookies) { > /* > @@ -3045,7 +3044,6 @@ struct bpf_uprobe_multi_link { > struct path path; > struct bpf_link link; > u32 cnt; > - u32 flags; > struct bpf_uprobe *uprobes; > struct task_struct *task; > }; > @@ -3109,7 +3107,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, > > umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); > info->uprobe_multi.count = umulti_link->cnt; > - info->uprobe_multi.flags = umulti_link->flags; > + info->uprobe_multi.flags = umulti_link->link.flags; > info->uprobe_multi.pid = umulti_link->task ? > task_pid_nr_ns(umulti_link->task, task_active_pid_ns(current)) : 0; > > @@ -3369,7 +3367,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > link->uprobes = uprobes; > link->path = path; > link->task = task; > - link->flags = flags; > + link->link.flags = flags; > > bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI, > &bpf_uprobe_multi_link_lops, prog); Hi, Jiri, Andrii, Jiri's patch looks more simple, and i see other struct xx_links wrap bpf_link, which have attach_type field like: struct sockmap_link { struct bpf_link link; struct bpf_map *map; enum bpf_attach_type attach_type; }; If we create attach_type filed in bpf_link, maybe these struct xx_link should also be modified. BTW, as Jiri said, we still can not find return probe type from attach_type. -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-24 8:41 ` Tao Chen @ 2025-06-24 15:46 ` Andrii Nakryiko 2025-06-24 16:12 ` Tao Chen 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2025-06-24 15:46 UTC (permalink / raw) To: Tao Chen Cc: Jiri Olsa, Alexei Starovoitov, KP Singh, Matt Bobrowski, Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel On Tue, Jun 24, 2025 at 1:41 AM Tao Chen <chen.dylane@linux.dev> wrote: > > 在 2025/6/24 16:16, Jiri Olsa 写道: > > On Mon, Jun 23, 2025 at 01:59:18PM -0700, Andrii Nakryiko wrote: > >> On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov > >> <alexei.starovoitov@gmail.com> wrote: > >>> > >>> On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: > >>>> > >>>> Alexei suggested, 'link_type' can be more precise and differentiate > >>>> for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes > >>>> kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we > >>>> can show it more concretely. > >>>> > >>>> link_type: kprobe_multi > >>>> link_id: 1 > >>>> prog_tag: d2b307e915f0dd37 > >>>> ... > >>>> link_type: kretprobe_multi > >>>> link_id: 2 > >>>> prog_tag: ab9ea0545870781d > >>>> ... > >>>> link_type: uprobe_multi > >>>> link_id: 9 > >>>> prog_tag: e729f789e34a8eca > >>>> ... > >>>> link_type: uretprobe_multi > >>>> link_id: 10 > >>>> prog_tag: 7db356c03e61a4d4 > >>>> > >>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > >>>> --- > >>>> include/linux/trace_events.h | 10 ++++++++++ > >>>> kernel/bpf/syscall.c | 9 ++++++++- > >>>> kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > >>>> 3 files changed, 46 insertions(+), 1 deletion(-) > >>>> > >>>> Change list: > >>>> v4 -> v5: > >>>> - Add patch1 to show precise link_type for > >>>> {uprobe,kprobe}_multi.(Alexei) > >>>> - patch2,3 just remove type field, which will be showed in > >>>> link_type > >>>> v4: > >>>> https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev > >>>> > >>>> v3 -> v4: > >>>> - use %pS to print func info.(Alexei) > >>>> v3: > >>>> https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev > >>>> > >>>> v2 -> v3: > >>>> - show info in one line for multi events.(Jiri) > >>>> v2: > >>>> https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev > >>>> > >>>> v1 -> v2: > >>>> - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) > >>>> - print func name is more readable and security for kprobe_multi.(Alexei) > >>>> v1: > >>>> https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev > >>>> > >>>> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > >>>> index fa9cf4292df..951c91babbc 100644 > >>>> --- a/include/linux/trace_events.h > >>>> +++ b/include/linux/trace_events.h > >>>> @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, > >>>> unsigned long *missed); > >>>> int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > >>>> int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > >>>> +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > >>>> +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > >>>> #else > >>>> static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > >>>> { > >>>> @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > >>>> { > >>>> return -EOPNOTSUPP; > >>>> } > >>>> +static inline void > >>>> +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > >>>> +{ > >>>> +} > >>>> +static inline void > >>>> +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > >>>> +{ > >>>> +} > >>>> #endif > >>>> > >>>> enum { > >>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > >>>> index 51ba1a7aa43..43b821b37bc 100644 > >>>> --- a/kernel/bpf/syscall.c > >>>> +++ b/kernel/bpf/syscall.c > >>>> @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > >>>> const struct bpf_prog *prog = link->prog; > >>>> enum bpf_link_type type = link->type; > >>>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > >>>> + char link_type[64] = {}; > >>>> > >>>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > >>>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > >>>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > >>>> + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > >>>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > >>>> + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > >>>> + else > >>>> + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); > >>>> + seq_printf(m, "link_type:\t%s\n", link_type); > >>> > >>> New callbacks just to print a string? > >>> Let's find a different way. > >>> > >>> How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? > >>> (There is a 7 byte hole there anyway) > >>> and checking flags inline. > >>> > >>> Jiri, Andrii, > >>> > >>> better ideas? > >> > >> We can just remember original attr->link_create.attach_type in > >> bpf_link itself, and then have a small helper that will accept link > >> type and attach type, and fill out link type representation based on > >> those two. Internally we can do the special-casing of uprobe vs > >> uretprobe and kprobe vs kretprobe transparently to all the other code. > >> And use that here in show_fdinfo > > > > but you'd still need the flags, no? to find out if it's return probe > > > > I tried what Alexei suggested and it seems ok and simple enough > > > > jirka > > > > > > --- > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index 5dd556e89cce..287c956cdbd2 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -1702,6 +1702,7 @@ struct bpf_link { > > * link's semantics is determined by target attach hook > > */ > > bool sleepable; > > + u32 flags; > > /* rcu is used before freeing, work can be used to schedule that > > * RCU-based freeing before that, so they never overlap > > */ > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 56500381c28a..f1d9ee9717a1 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3228,7 +3228,14 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > > char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > > > > if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > > - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > > + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > > + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ? > > + "kretprobe_multi" : "kprobe_multi"); > > + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > > + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ? > > + "uretprobe_multi" : "uprobe_multi"); > > + else > > + seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > > } else { > > WARN_ONCE(1, "missing BPF_LINK_TYPE(...) for link type %u\n", type); > > seq_printf(m, "link_type:\t<%u>\n", type); > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 0a06ea6638fe..81d7a4e5ae15 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2466,7 +2466,6 @@ struct bpf_kprobe_multi_link { > > u32 cnt; > > u32 mods_cnt; > > struct module **mods; > > - u32 flags; > > }; > > > > struct bpf_kprobe_multi_run_ctx { > > @@ -2586,7 +2585,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > > > > kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > > info->kprobe_multi.count = kmulti_link->cnt; > > - info->kprobe_multi.flags = kmulti_link->flags; > > + info->kprobe_multi.flags = kmulti_link->link.flags; > > info->kprobe_multi.missed = kmulti_link->fp.nmissed; > > > > if (!uaddrs) > > @@ -2976,7 +2975,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > link->addrs = addrs; > > link->cookies = cookies; > > link->cnt = cnt; > > - link->flags = flags; > > + link->link.flags = flags; > > > > if (cookies) { > > /* > > @@ -3045,7 +3044,6 @@ struct bpf_uprobe_multi_link { > > struct path path; > > struct bpf_link link; > > u32 cnt; > > - u32 flags; > > struct bpf_uprobe *uprobes; > > struct task_struct *task; > > }; > > @@ -3109,7 +3107,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, > > > > umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); > > info->uprobe_multi.count = umulti_link->cnt; > > - info->uprobe_multi.flags = umulti_link->flags; > > + info->uprobe_multi.flags = umulti_link->link.flags; > > info->uprobe_multi.pid = umulti_link->task ? > > task_pid_nr_ns(umulti_link->task, task_active_pid_ns(current)) : 0; > > > > @@ -3369,7 +3367,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > link->uprobes = uprobes; > > link->path = path; > > link->task = task; > > - link->flags = flags; > > + link->link.flags = flags; > > > > bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI, > > &bpf_uprobe_multi_link_lops, prog); > > Hi, Jiri, Andrii, > > Jiri's patch looks more simple, and i see other struct xx_links wrap > bpf_link, which have attach_type field like: > struct sockmap_link { > struct bpf_link link; > struct bpf_map *map; > enum bpf_attach_type attach_type; > }; > If we create attach_type filed in bpf_link, maybe these struct xx_link > should also be modified. BTW, as Jiri said, we still can not find return > probe type from attach_type. You are right, I somehow was under impression that ret vs non-retprobe comes from attach type as well. Ok, moving flags into common bpf_link struct sounds good to me. I'd still move attach_type into bpf_link, together with flags, for generality (and update all those links that already include attach_type as you mentioned). We can make it a single-byte field to not increase bpf_link size unnecessarily (by using bitfield size). > > -- > Best Regards > Tao Chen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-24 15:46 ` Andrii Nakryiko @ 2025-06-24 16:12 ` Tao Chen 2025-06-24 19:41 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: Tao Chen @ 2025-06-24 16:12 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, Alexei Starovoitov, KP Singh, Matt Bobrowski, Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel 在 2025/6/24 23:46, Andrii Nakryiko 写道: > On Tue, Jun 24, 2025 at 1:41 AM Tao Chen <chen.dylane@linux.dev> wrote: >> >> 在 2025/6/24 16:16, Jiri Olsa 写道: >>> On Mon, Jun 23, 2025 at 01:59:18PM -0700, Andrii Nakryiko wrote: >>>> On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov >>>> <alexei.starovoitov@gmail.com> wrote: >>>>> >>>>> On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: >>>>>> >>>>>> Alexei suggested, 'link_type' can be more precise and differentiate >>>>>> for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes >>>>>> kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we >>>>>> can show it more concretely. >>>>>> >>>>>> link_type: kprobe_multi >>>>>> link_id: 1 >>>>>> prog_tag: d2b307e915f0dd37 >>>>>> ... >>>>>> link_type: kretprobe_multi >>>>>> link_id: 2 >>>>>> prog_tag: ab9ea0545870781d >>>>>> ... >>>>>> link_type: uprobe_multi >>>>>> link_id: 9 >>>>>> prog_tag: e729f789e34a8eca >>>>>> ... >>>>>> link_type: uretprobe_multi >>>>>> link_id: 10 >>>>>> prog_tag: 7db356c03e61a4d4 >>>>>> >>>>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev> >>>>>> --- >>>>>> include/linux/trace_events.h | 10 ++++++++++ >>>>>> kernel/bpf/syscall.c | 9 ++++++++- >>>>>> kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ >>>>>> 3 files changed, 46 insertions(+), 1 deletion(-) >>>>>> >>>>>> Change list: >>>>>> v4 -> v5: >>>>>> - Add patch1 to show precise link_type for >>>>>> {uprobe,kprobe}_multi.(Alexei) >>>>>> - patch2,3 just remove type field, which will be showed in >>>>>> link_type >>>>>> v4: >>>>>> https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev >>>>>> >>>>>> v3 -> v4: >>>>>> - use %pS to print func info.(Alexei) >>>>>> v3: >>>>>> https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev >>>>>> >>>>>> v2 -> v3: >>>>>> - show info in one line for multi events.(Jiri) >>>>>> v2: >>>>>> https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev >>>>>> >>>>>> v1 -> v2: >>>>>> - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) >>>>>> - print func name is more readable and security for kprobe_multi.(Alexei) >>>>>> v1: >>>>>> https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev >>>>>> >>>>>> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h >>>>>> index fa9cf4292df..951c91babbc 100644 >>>>>> --- a/include/linux/trace_events.h >>>>>> +++ b/include/linux/trace_events.h >>>>>> @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, >>>>>> unsigned long *missed); >>>>>> int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); >>>>>> int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); >>>>>> +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); >>>>>> +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); >>>>>> #else >>>>>> static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) >>>>>> { >>>>>> @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) >>>>>> { >>>>>> return -EOPNOTSUPP; >>>>>> } >>>>>> +static inline void >>>>>> +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) >>>>>> +{ >>>>>> +} >>>>>> +static inline void >>>>>> +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) >>>>>> +{ >>>>>> +} >>>>>> #endif >>>>>> >>>>>> enum { >>>>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>>>>> index 51ba1a7aa43..43b821b37bc 100644 >>>>>> --- a/kernel/bpf/syscall.c >>>>>> +++ b/kernel/bpf/syscall.c >>>>>> @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) >>>>>> const struct bpf_prog *prog = link->prog; >>>>>> enum bpf_link_type type = link->type; >>>>>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; >>>>>> + char link_type[64] = {}; >>>>>> >>>>>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { >>>>>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); >>>>>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) >>>>>> + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); >>>>>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) >>>>>> + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); >>>>>> + else >>>>>> + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); >>>>>> + seq_printf(m, "link_type:\t%s\n", link_type); >>>>> >>>>> New callbacks just to print a string? >>>>> Let's find a different way. >>>>> >>>>> How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? >>>>> (There is a 7 byte hole there anyway) >>>>> and checking flags inline. >>>>> >>>>> Jiri, Andrii, >>>>> >>>>> better ideas? >>>> >>>> We can just remember original attr->link_create.attach_type in >>>> bpf_link itself, and then have a small helper that will accept link >>>> type and attach type, and fill out link type representation based on >>>> those two. Internally we can do the special-casing of uprobe vs >>>> uretprobe and kprobe vs kretprobe transparently to all the other code. >>>> And use that here in show_fdinfo >>> >>> but you'd still need the flags, no? to find out if it's return probe >>> >>> I tried what Alexei suggested and it seems ok and simple enough >>> >>> jirka >>> >>> >>> --- >>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >>> index 5dd556e89cce..287c956cdbd2 100644 >>> --- a/include/linux/bpf.h >>> +++ b/include/linux/bpf.h >>> @@ -1702,6 +1702,7 @@ struct bpf_link { >>> * link's semantics is determined by target attach hook >>> */ >>> bool sleepable; >>> + u32 flags; >>> /* rcu is used before freeing, work can be used to schedule that >>> * RCU-based freeing before that, so they never overlap >>> */ >>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>> index 56500381c28a..f1d9ee9717a1 100644 >>> --- a/kernel/bpf/syscall.c >>> +++ b/kernel/bpf/syscall.c >>> @@ -3228,7 +3228,14 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) >>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; >>> >>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { >>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); >>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) >>> + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ? >>> + "kretprobe_multi" : "kprobe_multi"); >>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) >>> + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ? >>> + "uretprobe_multi" : "uprobe_multi"); >>> + else >>> + seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); >>> } else { >>> WARN_ONCE(1, "missing BPF_LINK_TYPE(...) for link type %u\n", type); >>> seq_printf(m, "link_type:\t<%u>\n", type); >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >>> index 0a06ea6638fe..81d7a4e5ae15 100644 >>> --- a/kernel/trace/bpf_trace.c >>> +++ b/kernel/trace/bpf_trace.c >>> @@ -2466,7 +2466,6 @@ struct bpf_kprobe_multi_link { >>> u32 cnt; >>> u32 mods_cnt; >>> struct module **mods; >>> - u32 flags; >>> }; >>> >>> struct bpf_kprobe_multi_run_ctx { >>> @@ -2586,7 +2585,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, >>> >>> kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); >>> info->kprobe_multi.count = kmulti_link->cnt; >>> - info->kprobe_multi.flags = kmulti_link->flags; >>> + info->kprobe_multi.flags = kmulti_link->link.flags; >>> info->kprobe_multi.missed = kmulti_link->fp.nmissed; >>> >>> if (!uaddrs) >>> @@ -2976,7 +2975,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr >>> link->addrs = addrs; >>> link->cookies = cookies; >>> link->cnt = cnt; >>> - link->flags = flags; >>> + link->link.flags = flags; >>> >>> if (cookies) { >>> /* >>> @@ -3045,7 +3044,6 @@ struct bpf_uprobe_multi_link { >>> struct path path; >>> struct bpf_link link; >>> u32 cnt; >>> - u32 flags; >>> struct bpf_uprobe *uprobes; >>> struct task_struct *task; >>> }; >>> @@ -3109,7 +3107,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, >>> >>> umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); >>> info->uprobe_multi.count = umulti_link->cnt; >>> - info->uprobe_multi.flags = umulti_link->flags; >>> + info->uprobe_multi.flags = umulti_link->link.flags; >>> info->uprobe_multi.pid = umulti_link->task ? >>> task_pid_nr_ns(umulti_link->task, task_active_pid_ns(current)) : 0; >>> >>> @@ -3369,7 +3367,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr >>> link->uprobes = uprobes; >>> link->path = path; >>> link->task = task; >>> - link->flags = flags; >>> + link->link.flags = flags; >>> >>> bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI, >>> &bpf_uprobe_multi_link_lops, prog); >> >> Hi, Jiri, Andrii, >> >> Jiri's patch looks more simple, and i see other struct xx_links wrap >> bpf_link, which have attach_type field like: >> struct sockmap_link { >> struct bpf_link link; >> struct bpf_map *map; >> enum bpf_attach_type attach_type; >> }; >> If we create attach_type filed in bpf_link, maybe these struct xx_link >> should also be modified. BTW, as Jiri said, we still can not find return >> probe type from attach_type. > > You are right, I somehow was under impression that ret vs non-retprobe > comes from attach type as well. > > Ok, moving flags into common bpf_link struct sounds good to me. I'd > still move attach_type into bpf_link, together with flags, for > generality (and update all those links that already include > attach_type as you mentioned). We can make it a single-byte field to > not increase bpf_link size unnecessarily (by using bitfield size). > Well，can we complete this in two steps? 1. Create a common field in bpf_link used for flags or attach_type, and realise the precise link_type feature as Jiri and Alexei said, the review of this part has been revised almost completely. 2. Move the attach_type from struct bpf_xx_link into bpf_link, this will involve a lot of changes, i will send a separate patchset to finish it. >> >> -- >> Best Regards >> Tao Chen -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo 2025-06-24 16:12 ` Tao Chen @ 2025-06-24 19:41 ` Andrii Nakryiko 0 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2025-06-24 19:41 UTC (permalink / raw) To: Tao Chen Cc: Jiri Olsa, Alexei Starovoitov, KP Singh, Matt Bobrowski, Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard, Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf, LKML, linux-trace-kernel On Tue, Jun 24, 2025 at 9:13 AM Tao Chen <chen.dylane@linux.dev> wrote: > > 在 2025/6/24 23:46, Andrii Nakryiko 写道: > > On Tue, Jun 24, 2025 at 1:41 AM Tao Chen <chen.dylane@linux.dev> wrote: > >> > >> 在 2025/6/24 16:16, Jiri Olsa 写道: > >>> On Mon, Jun 23, 2025 at 01:59:18PM -0700, Andrii Nakryiko wrote: > >>>> On Mon, Jun 23, 2025 at 10:56 AM Alexei Starovoitov > >>>> <alexei.starovoitov@gmail.com> wrote: > >>>>> > >>>>> On Mon, Jun 23, 2025 at 6:44 AM Tao Chen <chen.dylane@linux.dev> wrote: > >>>>>> > >>>>>> Alexei suggested, 'link_type' can be more precise and differentiate > >>>>>> for human in fdinfo. In fact BPF_LINK_TYPE_KPROBE_MULTI includes > >>>>>> kretprobe_multi type, the same as BPF_LINK_TYPE_UPROBE_MULTI, so we > >>>>>> can show it more concretely. > >>>>>> > >>>>>> link_type: kprobe_multi > >>>>>> link_id: 1 > >>>>>> prog_tag: d2b307e915f0dd37 > >>>>>> ... > >>>>>> link_type: kretprobe_multi > >>>>>> link_id: 2 > >>>>>> prog_tag: ab9ea0545870781d > >>>>>> ... > >>>>>> link_type: uprobe_multi > >>>>>> link_id: 9 > >>>>>> prog_tag: e729f789e34a8eca > >>>>>> ... > >>>>>> link_type: uretprobe_multi > >>>>>> link_id: 10 > >>>>>> prog_tag: 7db356c03e61a4d4 > >>>>>> > >>>>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev> > >>>>>> --- > >>>>>> include/linux/trace_events.h | 10 ++++++++++ > >>>>>> kernel/bpf/syscall.c | 9 ++++++++- > >>>>>> kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > >>>>>> 3 files changed, 46 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> Change list: > >>>>>> v4 -> v5: > >>>>>> - Add patch1 to show precise link_type for > >>>>>> {uprobe,kprobe}_multi.(Alexei) > >>>>>> - patch2,3 just remove type field, which will be showed in > >>>>>> link_type > >>>>>> v4: > >>>>>> https://lore.kernel.org/bpf/20250619034257.70520-1-chen.dylane@linux.dev > >>>>>> > >>>>>> v3 -> v4: > >>>>>> - use %pS to print func info.(Alexei) > >>>>>> v3: > >>>>>> https://lore.kernel.org/bpf/20250616130233.451439-1-chen.dylane@linux.dev > >>>>>> > >>>>>> v2 -> v3: > >>>>>> - show info in one line for multi events.(Jiri) > >>>>>> v2: > >>>>>> https://lore.kernel.org/bpf/20250615150514.418581-1-chen.dylane@linux.dev > >>>>>> > >>>>>> v1 -> v2: > >>>>>> - replace 'func_cnt' with 'uprobe_cnt'.(Andrii) > >>>>>> - print func name is more readable and security for kprobe_multi.(Alexei) > >>>>>> v1: > >>>>>> https://lore.kernel.org/bpf/20250612115556.295103-1-chen.dylane@linux.dev > >>>>>> > >>>>>> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > >>>>>> index fa9cf4292df..951c91babbc 100644 > >>>>>> --- a/include/linux/trace_events.h > >>>>>> +++ b/include/linux/trace_events.h > >>>>>> @@ -780,6 +780,8 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, > >>>>>> unsigned long *missed); > >>>>>> int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > >>>>>> int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog); > >>>>>> +void bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > >>>>>> +void bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len); > >>>>>> #else > >>>>>> static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > >>>>>> { > >>>>>> @@ -832,6 +834,14 @@ bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > >>>>>> { > >>>>>> return -EOPNOTSUPP; > >>>>>> } > >>>>>> +static inline void > >>>>>> +bpf_kprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > >>>>>> +{ > >>>>>> +} > >>>>>> +static inline void > >>>>>> +bpf_uprobe_multi_link_type_show(const struct bpf_link *link, char *link_type, int len) > >>>>>> +{ > >>>>>> +} > >>>>>> #endif > >>>>>> > >>>>>> enum { > >>>>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > >>>>>> index 51ba1a7aa43..43b821b37bc 100644 > >>>>>> --- a/kernel/bpf/syscall.c > >>>>>> +++ b/kernel/bpf/syscall.c > >>>>>> @@ -3226,9 +3226,16 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > >>>>>> const struct bpf_prog *prog = link->prog; > >>>>>> enum bpf_link_type type = link->type; > >>>>>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > >>>>>> + char link_type[64] = {}; > >>>>>> > >>>>>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > >>>>>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > >>>>>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > >>>>>> + bpf_kprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > >>>>>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > >>>>>> + bpf_uprobe_multi_link_type_show(link, link_type, sizeof(link_type)); > >>>>>> + else > >>>>>> + strscpy(link_type, bpf_link_type_strs[type], sizeof(link_type)); > >>>>>> + seq_printf(m, "link_type:\t%s\n", link_type); > >>>>> > >>>>> New callbacks just to print a string? > >>>>> Let's find a different way. > >>>>> > >>>>> How about moving 'flags' from bpf_[ku]probe_multi_link into bpf_link ? > >>>>> (There is a 7 byte hole there anyway) > >>>>> and checking flags inline. > >>>>> > >>>>> Jiri, Andrii, > >>>>> > >>>>> better ideas? > >>>> > >>>> We can just remember original attr->link_create.attach_type in > >>>> bpf_link itself, and then have a small helper that will accept link > >>>> type and attach type, and fill out link type representation based on > >>>> those two. Internally we can do the special-casing of uprobe vs > >>>> uretprobe and kprobe vs kretprobe transparently to all the other code. > >>>> And use that here in show_fdinfo > >>> > >>> but you'd still need the flags, no? to find out if it's return probe > >>> > >>> I tried what Alexei suggested and it seems ok and simple enough > >>> > >>> jirka > >>> > >>> > >>> --- > >>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h > >>> index 5dd556e89cce..287c956cdbd2 100644 > >>> --- a/include/linux/bpf.h > >>> +++ b/include/linux/bpf.h > >>> @@ -1702,6 +1702,7 @@ struct bpf_link { > >>> * link's semantics is determined by target attach hook > >>> */ > >>> bool sleepable; > >>> + u32 flags; > >>> /* rcu is used before freeing, work can be used to schedule that > >>> * RCU-based freeing before that, so they never overlap > >>> */ > >>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > >>> index 56500381c28a..f1d9ee9717a1 100644 > >>> --- a/kernel/bpf/syscall.c > >>> +++ b/kernel/bpf/syscall.c > >>> @@ -3228,7 +3228,14 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) > >>> char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; > >>> > >>> if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) { > >>> - seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > >>> + if (link->type == BPF_LINK_TYPE_KPROBE_MULTI) > >>> + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ? > >>> + "kretprobe_multi" : "kprobe_multi"); > >>> + else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI) > >>> + seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ? > >>> + "uretprobe_multi" : "uprobe_multi"); > >>> + else > >>> + seq_printf(m, "link_type:\t%s\n", bpf_link_type_strs[type]); > >>> } else { > >>> WARN_ONCE(1, "missing BPF_LINK_TYPE(...) for link type %u\n", type); > >>> seq_printf(m, "link_type:\t<%u>\n", type); > >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > >>> index 0a06ea6638fe..81d7a4e5ae15 100644 > >>> --- a/kernel/trace/bpf_trace.c > >>> +++ b/kernel/trace/bpf_trace.c > >>> @@ -2466,7 +2466,6 @@ struct bpf_kprobe_multi_link { > >>> u32 cnt; > >>> u32 mods_cnt; > >>> struct module **mods; > >>> - u32 flags; > >>> }; > >>> > >>> struct bpf_kprobe_multi_run_ctx { > >>> @@ -2586,7 +2585,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > >>> > >>> kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > >>> info->kprobe_multi.count = kmulti_link->cnt; > >>> - info->kprobe_multi.flags = kmulti_link->flags; > >>> + info->kprobe_multi.flags = kmulti_link->link.flags; > >>> info->kprobe_multi.missed = kmulti_link->fp.nmissed; > >>> > >>> if (!uaddrs) > >>> @@ -2976,7 +2975,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > >>> link->addrs = addrs; > >>> link->cookies = cookies; > >>> link->cnt = cnt; > >>> - link->flags = flags; > >>> + link->link.flags = flags; > >>> > >>> if (cookies) { > >>> /* > >>> @@ -3045,7 +3044,6 @@ struct bpf_uprobe_multi_link { > >>> struct path path; > >>> struct bpf_link link; > >>> u32 cnt; > >>> - u32 flags; > >>> struct bpf_uprobe *uprobes; > >>> struct task_struct *task; > >>> }; > >>> @@ -3109,7 +3107,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, > >>> > >>> umulti_link = container_of(link, struct bpf_uprobe_multi_link, link); > >>> info->uprobe_multi.count = umulti_link->cnt; > >>> - info->uprobe_multi.flags = umulti_link->flags; > >>> + info->uprobe_multi.flags = umulti_link->link.flags; > >>> info->uprobe_multi.pid = umulti_link->task ? > >>> task_pid_nr_ns(umulti_link->task, task_active_pid_ns(current)) : 0; > >>> > >>> @@ -3369,7 +3367,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > >>> link->uprobes = uprobes; > >>> link->path = path; > >>> link->task = task; > >>> - link->flags = flags; > >>> + link->link.flags = flags; > >>> > >>> bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI, > >>> &bpf_uprobe_multi_link_lops, prog); > >> > >> Hi, Jiri, Andrii, > >> > >> Jiri's patch looks more simple, and i see other struct xx_links wrap > >> bpf_link, which have attach_type field like: > >> struct sockmap_link { > >> struct bpf_link link; > >> struct bpf_map *map; > >> enum bpf_attach_type attach_type; > >> }; > >> If we create attach_type filed in bpf_link, maybe these struct xx_link > >> should also be modified. BTW, as Jiri said, we still can not find return > >> probe type from attach_type. > > > > You are right, I somehow was under impression that ret vs non-retprobe > > comes from attach type as well. > > > > Ok, moving flags into common bpf_link struct sounds good to me. I'd > > still move attach_type into bpf_link, together with flags, for > > generality (and update all those links that already include > > attach_type as you mentioned). We can make it a single-byte field to > > not increase bpf_link size unnecessarily (by using bitfield size). > > > > Well，can we complete this in two steps? > sure, of course > 1. Create a common field in bpf_link used for flags or attach_type, and > realise the precise link_type feature as Jiri and Alexei said, the > review of this part has been revised almost completely. > > 2. Move the attach_type from struct bpf_xx_link into bpf_link, this will > involve a lot of changes, i will send a separate patchset to finish it. > > >> > >> -- > >> Best Regards > >> Tao Chen > > > -- > Best Regards > Tao Chen ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-24 19:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250623134342.227347-1-chen.dylane@linux.dev>
2025-06-23 13:43 ` [PATCH bpf-next v5 2/3] bpf: Add show_fdinfo for uprobe_multi Tao Chen
2025-06-23 22:14 ` Andrii Nakryiko
2025-06-24 8:42 ` Tao Chen
2025-06-23 13:43 ` [PATCH bpf-next v5 3/3] bpf: Add show_fdinfo for kprobe_multi Tao Chen
2025-06-23 17:55 ` [PATCH bpf-next v5 1/3] bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo Alexei Starovoitov
2025-06-23 20:59 ` Andrii Nakryiko
2025-06-24 8:16 ` Jiri Olsa
2025-06-24 8:41 ` Tao Chen
2025-06-24 15:46 ` Andrii Nakryiko
2025-06-24 16:12 ` Tao Chen
2025-06-24 19:41 ` 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).