* [PATCHv2 bpf-next 1/9] bpf: Count stats for kprobe_multi programs
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:09 ` Song Liu
2023-09-07 7:13 ` [PATCHv2 bpf-next 2/9] bpf: Add missed value to kprobe_multi link info Jiri Olsa
` (7 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
Adding support to gather missed stats for kprobe_multi
programs due to bpf_prog_active protection.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/trace/bpf_trace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index a7264b2c17ad..279a3d370812 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2710,6 +2710,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
int err;
if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
+ bpf_prog_inc_misses_counter(link->link.prog);
err = 0;
goto out;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 1/9] bpf: Count stats for kprobe_multi programs
2023-09-07 7:13 ` [PATCHv2 bpf-next 1/9] bpf: Count stats for kprobe_multi programs Jiri Olsa
@ 2023-09-07 18:09 ` Song Liu
0 siblings, 0 replies; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
nit: The subject doesn't state "missed stats".
>
> Adding support to gather missed stats for kprobe_multi
> programs due to bpf_prog_active protection.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Other than that
Reviewed-and-tested-by: Song Liu <song@kernel.org>
> ---
> kernel/trace/bpf_trace.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index a7264b2c17ad..279a3d370812 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2710,6 +2710,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> int err;
>
> if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> + bpf_prog_inc_misses_counter(link->link.prog);
> err = 0;
> goto out;
> }
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 2/9] bpf: Add missed value to kprobe_multi link info
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 1/9] bpf: Count stats for kprobe_multi programs Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:15 ` Song Liu
2023-09-07 7:13 ` [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf " Jiri Olsa
` (6 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Hou Tao, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
Add missed value to kprobe_multi link info to hold the stats of missed
kprobe_multi probe.
The missed counter gets incremented when fprobe fails the recursion
check or there's no rethook available for return probe. In either
case the attached bpf program is not executed.
Acked-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/uapi/linux/bpf.h | 1 +
kernel/trace/bpf_trace.c | 1 +
tools/include/uapi/linux/bpf.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 73b155e52204..e5216420ec73 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6530,6 +6530,7 @@ struct bpf_link_info {
__aligned_u64 addrs;
__u32 count; /* in/out: kprobe_multi function count */
__u32 flags;
+ __u64 missed;
} kprobe_multi;
struct {
__u32 type; /* enum bpf_perf_event_type */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 279a3d370812..aec52938c703 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2614,6 +2614,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.missed = kmulti_link->fp.nmissed;
if (!uaddrs)
return 0;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 73b155e52204..e5216420ec73 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6530,6 +6530,7 @@ struct bpf_link_info {
__aligned_u64 addrs;
__u32 count; /* in/out: kprobe_multi function count */
__u32 flags;
+ __u64 missed;
} kprobe_multi;
struct {
__u32 type; /* enum bpf_perf_event_type */
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 2/9] bpf: Add missed value to kprobe_multi link info
2023-09-07 7:13 ` [PATCHv2 bpf-next 2/9] bpf: Add missed value to kprobe_multi link info Jiri Olsa
@ 2023-09-07 18:15 ` Song Liu
0 siblings, 0 replies; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:15 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Add missed value to kprobe_multi link info to hold the stats of missed
> kprobe_multi probe.
>
> The missed counter gets incremented when fprobe fails the recursion
> check or there's no rethook available for return probe. In either
> case the attached bpf program is not executed.
>
> Acked-by: Hou Tao <houtao1@huawei.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-and-tested-by: Song Liu <song@kernel.org>
> ---
> include/uapi/linux/bpf.h | 1 +
> kernel/trace/bpf_trace.c | 1 +
> tools/include/uapi/linux/bpf.h | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 73b155e52204..e5216420ec73 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6530,6 +6530,7 @@ struct bpf_link_info {
> __aligned_u64 addrs;
> __u32 count; /* in/out: kprobe_multi function count */
> __u32 flags;
> + __u64 missed;
> } kprobe_multi;
> struct {
> __u32 type; /* enum bpf_perf_event_type */
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 279a3d370812..aec52938c703 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2614,6 +2614,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.missed = kmulti_link->fp.nmissed;
>
> if (!uaddrs)
> return 0;
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 73b155e52204..e5216420ec73 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -6530,6 +6530,7 @@ struct bpf_link_info {
> __aligned_u64 addrs;
> __u32 count; /* in/out: kprobe_multi function count */
> __u32 flags;
> + __u64 missed;
> } kprobe_multi;
> struct {
> __u32 type; /* enum bpf_perf_event_type */
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 1/9] bpf: Count stats for kprobe_multi programs Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 2/9] bpf: Add missed value to kprobe_multi link info Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:40 ` Song Liu
2023-09-07 7:13 ` [PATCHv2 bpf-next 4/9] bpf: Count missed stats in trace_call_bpf Jiri Olsa
` (5 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
Add missed value to kprobe attached through perf link info to
hold the stats of missed kprobe handler execution.
The kprobe's missed counter gets incremented when kprobe handler
is not executed due to another kprobe running on the same cpu.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/trace_events.h | 6 ++++--
include/uapi/linux/bpf.h | 1 +
kernel/bpf/syscall.c | 14 ++++++++------
kernel/trace/bpf_trace.c | 5 +++--
kernel/trace/trace_kprobe.c | 14 +++++++++++---
tools/include/uapi/linux/bpf.h | 1 +
6 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index c1a0a19d80fb..8e12c63a9db7 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -761,7 +761,8 @@ struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name);
void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp);
int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
u32 *fd_type, const char **buf,
- u64 *probe_offset, u64 *probe_addr);
+ u64 *probe_offset, u64 *probe_addr,
+ 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);
#else
@@ -801,7 +802,7 @@ static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
static inline int bpf_get_perf_event_info(const struct perf_event *event,
u32 *prog_id, u32 *fd_type,
const char **buf, u64 *probe_offset,
- u64 *probe_addr)
+ u64 *probe_addr, unsigned long *missed)
{
return -EOPNOTSUPP;
}
@@ -876,6 +877,7 @@ extern void perf_kprobe_destroy(struct perf_event *event);
extern int bpf_get_kprobe_info(const struct perf_event *event,
u32 *fd_type, const char **symbol,
u64 *probe_offset, u64 *probe_addr,
+ unsigned long *missed,
bool perf_type_tracepoint);
#endif
#ifdef CONFIG_UPROBE_EVENTS
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e5216420ec73..e824b0c50425 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6546,6 +6546,7 @@ struct bpf_link_info {
__u32 name_len;
__u32 offset; /* offset from func_name */
__u64 addr;
+ __u64 missed;
} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
struct {
__aligned_u64 tp_name; /* in/out */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e25e7e059d7d..0c0c93c87927 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3374,7 +3374,7 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
static int bpf_perf_link_fill_common(const struct perf_event *event,
char __user *uname, u32 ulen,
u64 *probe_offset, u64 *probe_addr,
- u32 *fd_type)
+ u32 *fd_type, unsigned long *missed)
{
const char *buf;
u32 prog_id;
@@ -3385,7 +3385,7 @@ static int bpf_perf_link_fill_common(const struct perf_event *event,
return -EINVAL;
err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
- probe_offset, probe_addr);
+ probe_offset, probe_addr, missed);
if (err)
return err;
if (!uname)
@@ -3408,6 +3408,7 @@ static int bpf_perf_link_fill_common(const struct perf_event *event,
static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
struct bpf_link_info *info)
{
+ unsigned long missed;
char __user *uname;
u64 addr, offset;
u32 ulen, type;
@@ -3416,7 +3417,7 @@ static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
uname = u64_to_user_ptr(info->perf_event.kprobe.func_name);
ulen = info->perf_event.kprobe.name_len;
err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
- &type);
+ &type, &missed);
if (err)
return err;
if (type == BPF_FD_TYPE_KRETPROBE)
@@ -3425,6 +3426,7 @@ static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
info->perf_event.type = BPF_PERF_EVENT_KPROBE;
info->perf_event.kprobe.offset = offset;
+ info->perf_event.kprobe.missed = missed;
if (!kallsyms_show_value(current_cred()))
addr = 0;
info->perf_event.kprobe.addr = addr;
@@ -3444,7 +3446,7 @@ static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
uname = u64_to_user_ptr(info->perf_event.uprobe.file_name);
ulen = info->perf_event.uprobe.name_len;
err = bpf_perf_link_fill_common(event, uname, ulen, &offset, &addr,
- &type);
+ &type, NULL);
if (err)
return err;
@@ -3480,7 +3482,7 @@ static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
ulen = info->perf_event.tracepoint.name_len;
info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
- return bpf_perf_link_fill_common(event, uname, ulen, NULL, NULL, NULL);
+ return bpf_perf_link_fill_common(event, uname, ulen, NULL, NULL, NULL, NULL);
}
static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
@@ -4813,7 +4815,7 @@ static int bpf_task_fd_query(const union bpf_attr *attr,
err = bpf_get_perf_event_info(event, &prog_id, &fd_type,
&buf, &probe_offset,
- &probe_addr);
+ &probe_addr, NULL);
if (!err)
err = bpf_task_fd_query_copy(attr, uattr, prog_id,
fd_type, buf,
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index aec52938c703..a9d8634b503c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2384,7 +2384,8 @@ int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
u32 *fd_type, const char **buf,
- u64 *probe_offset, u64 *probe_addr)
+ u64 *probe_offset, u64 *probe_addr,
+ unsigned long *missed)
{
bool is_tracepoint, is_syscall_tp;
struct bpf_prog *prog;
@@ -2419,7 +2420,7 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
#ifdef CONFIG_KPROBE_EVENTS
if (flags & TRACE_EVENT_FL_KPROBE)
err = bpf_get_kprobe_info(event, fd_type, buf,
- probe_offset, probe_addr,
+ probe_offset, probe_addr, missed,
event->attr.type == PERF_TYPE_TRACEPOINT);
#endif
#ifdef CONFIG_UPROBE_EVENTS
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 17c21c0b2dd1..5264fd26f8c7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1188,6 +1188,12 @@ static const struct file_operations kprobe_events_ops = {
.write = probes_write,
};
+static unsigned long trace_kprobe_missed(struct trace_kprobe *tk)
+{
+ return trace_kprobe_is_return(tk) ?
+ tk->rp.kp.nmissed + tk->rp.nmissed : tk->rp.kp.nmissed;
+}
+
/* Probes profiling interfaces */
static int probes_profile_seq_show(struct seq_file *m, void *v)
{
@@ -1199,8 +1205,7 @@ static int probes_profile_seq_show(struct seq_file *m, void *v)
return 0;
tk = to_trace_kprobe(ev);
- nmissed = trace_kprobe_is_return(tk) ?
- tk->rp.kp.nmissed + tk->rp.nmissed : tk->rp.kp.nmissed;
+ nmissed = trace_kprobe_missed(tk);
seq_printf(m, " %-44s %15lu %15lu\n",
trace_probe_name(&tk->tp),
trace_kprobe_nhit(tk),
@@ -1546,7 +1551,8 @@ NOKPROBE_SYMBOL(kretprobe_perf_func);
int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
const char **symbol, u64 *probe_offset,
- u64 *probe_addr, bool perf_type_tracepoint)
+ u64 *probe_addr, unsigned long *missed,
+ bool perf_type_tracepoint)
{
const char *pevent = trace_event_name(event->tp_event);
const char *group = event->tp_event->class->system;
@@ -1565,6 +1571,8 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
*probe_addr = kallsyms_show_value(current_cred()) ?
(unsigned long)tk->rp.kp.addr : 0;
*symbol = tk->symbol;
+ if (missed)
+ *missed = trace_kprobe_missed(tk);
return 0;
}
#endif /* CONFIG_PERF_EVENTS */
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e5216420ec73..e824b0c50425 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6546,6 +6546,7 @@ struct bpf_link_info {
__u32 name_len;
__u32 offset; /* offset from func_name */
__u64 addr;
+ __u64 missed;
} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
struct {
__aligned_u64 tp_name; /* in/out */
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-07 7:13 ` [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf " Jiri Olsa
@ 2023-09-07 18:40 ` Song Liu
2023-09-08 11:43 ` Jiri Olsa
0 siblings, 1 reply; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:40 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Add missed value to kprobe attached through perf link info to
> hold the stats of missed kprobe handler execution.
>
> The kprobe's missed counter gets incremented when kprobe handler
> is not executed due to another kprobe running on the same cpu.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
[...]
The code looks good to me. But I have two thoughts on this (and 2/9).
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e5216420ec73..e824b0c50425 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> __u32 name_len;
> __u32 offset; /* offset from func_name */
> __u64 addr;
> + __u64 missed;
> } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> struct {
> __aligned_u64 tp_name; /* in/out */
1) Shall we add missed for all bpf_link_info? Something like:
diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
index 5a39c7a13499..cf0b8b2a8b39 100644
--- i/include/uapi/linux/bpf.h
+++ w/include/uapi/linux/bpf.h
@@ -6465,6 +6465,7 @@ struct bpf_link_info {
__u32 type;
__u32 id;
__u32 prog_id;
+ __u64 missed;
union {
struct {
__aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
2) "missed" doesn't seem to fit well with other information in
struct bpf_link_info. Other information there are more like stable-ish
information; while missed is a stat that changes over time. Given we
have prog_id in bpf_link_info, do we really need "missed" here?
Thanks,
Song
[...]
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-07 18:40 ` Song Liu
@ 2023-09-08 11:43 ` Jiri Olsa
2023-09-08 16:49 ` Song Liu
2023-09-08 23:22 ` Andrii Nakryiko
0 siblings, 2 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-08 11:43 UTC (permalink / raw)
To: Song Liu
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Add missed value to kprobe attached through perf link info to
> > hold the stats of missed kprobe handler execution.
> >
> > The kprobe's missed counter gets incremented when kprobe handler
> > is not executed due to another kprobe running on the same cpu.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>
> [...]
>
> The code looks good to me. But I have two thoughts on this (and 2/9).
>
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index e5216420ec73..e824b0c50425 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > __u32 name_len;
> > __u32 offset; /* offset from func_name */
> > __u64 addr;
> > + __u64 missed;
> > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > struct {
> > __aligned_u64 tp_name; /* in/out */
>
> 1) Shall we add missed for all bpf_link_info? Something like:
>
> diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> index 5a39c7a13499..cf0b8b2a8b39 100644
> --- i/include/uapi/linux/bpf.h
> +++ w/include/uapi/linux/bpf.h
> @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> __u32 type;
> __u32 id;
> __u32 prog_id;
> + __u64 missed;
> union {
> struct {
> __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
hm, there's lot of links under bpf_link_info, can't really tell if
all could gather 'missed' data.. like I don't think we have any for
standard perf event or perf tracepoint
>
> 2) "missed" doesn't seem to fit well with other information in
> struct bpf_link_info. Other information there are more like stable-ish
> information; while missed is a stat that changes over time. Given we
> have prog_id in bpf_link_info, do we really need "missed" here?
right, OTOH there's recursion_misses/run_time_ns/run_cnt in bpf_prog_info
the bpf link has access to its attach layer, like perf event for kprobe
in perf_link or fprobe for kprobe_multi link... so it's convenient to
reach out from link for these stats and make them available through
bpf_link_info
also there's no other way to get these data for some links
like we could perhaps add some perf event specific interface to retrieve
these stats for kprobes, because we have access to the perf event in user
space, but that's not the case for kprobe_multi link, because there's no
other way to reach the fprobe object
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 11:43 ` Jiri Olsa
@ 2023-09-08 16:49 ` Song Liu
2023-09-10 18:54 ` Jiri Olsa
2023-09-08 23:22 ` Andrii Nakryiko
1 sibling, 1 reply; 27+ messages in thread
From: Song Liu @ 2023-09-08 16:49 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Fri, Sep 8, 2023 at 4:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> > On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Add missed value to kprobe attached through perf link info to
> > > hold the stats of missed kprobe handler execution.
> > >
> > > The kprobe's missed counter gets incremented when kprobe handler
> > > is not executed due to another kprobe running on the same cpu.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >
> > [...]
> >
> > The code looks good to me. But I have two thoughts on this (and 2/9).
> >
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index e5216420ec73..e824b0c50425 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > > __u32 name_len;
> > > __u32 offset; /* offset from func_name */
> > > __u64 addr;
> > > + __u64 missed;
> > > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > > struct {
> > > __aligned_u64 tp_name; /* in/out */
> >
> > 1) Shall we add missed for all bpf_link_info? Something like:
> >
> > diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> > index 5a39c7a13499..cf0b8b2a8b39 100644
> > --- i/include/uapi/linux/bpf.h
> > +++ w/include/uapi/linux/bpf.h
> > @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> > __u32 type;
> > __u32 id;
> > __u32 prog_id;
> > + __u64 missed;
> > union {
> > struct {
> > __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
>
> hm, there's lot of links under bpf_link_info, can't really tell if
> all could gather 'missed' data.. like I don't think we have any for
> standard perf event or perf tracepoint
I thought about the same thing, but didn't get to a conclusion. So
I brought it up for more discussions. Personally, I don't have a
strong preference either way.
>
> >
> > 2) "missed" doesn't seem to fit well with other information in
> > struct bpf_link_info. Other information there are more like stable-ish
> > information; while missed is a stat that changes over time. Given we
> > have prog_id in bpf_link_info, do we really need "missed" here?
>
> right, OTOH there's recursion_misses/run_time_ns/run_cnt in bpf_prog_info
Agreed. bpf_prog_info also contains stats of the program.
> the bpf link has access to its attach layer, like perf event for kprobe
> in perf_link or fprobe for kprobe_multi link... so it's convenient to
> reach out from link for these stats and make them available through
> bpf_link_info
>
> also there's no other way to get these data for some links
>
> like we could perhaps add some perf event specific interface to retrieve
> these stats for kprobes, because we have access to the perf event in user
> space, but that's not the case for kprobe_multi link, because there's no
> other way to reach the fprobe object
Fair enough. I guess this is a good stat to have for the bpf link.
More question about kprobe_multi: Shall we (or can we) collect "missed" for each
individual function we attach to?
Thanks,
Song
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 16:49 ` Song Liu
@ 2023-09-10 18:54 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-10 18:54 UTC (permalink / raw)
To: Song Liu
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Fri, Sep 08, 2023 at 09:49:17AM -0700, Song Liu wrote:
SNIP
> > the bpf link has access to its attach layer, like perf event for kprobe
> > in perf_link or fprobe for kprobe_multi link... so it's convenient to
> > reach out from link for these stats and make them available through
> > bpf_link_info
> >
> > also there's no other way to get these data for some links
> >
> > like we could perhaps add some perf event specific interface to retrieve
> > these stats for kprobes, because we have access to the perf event in user
> > space, but that's not the case for kprobe_multi link, because there's no
> > other way to reach the fprobe object
>
> Fair enough. I guess this is a good stat to have for the bpf link.
>
> More question about kprobe_multi: Shall we (or can we) collect "missed" for each
> individual function we attach to?
I think it's possible, but we'd need to keep/lookup stats for each
function/addr same way we do for cookies ... so it'd mean bsearch
on each missed path.. which might be not that bad, considering it's
error path instead of executing bpf program.. also it can be optional
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 11:43 ` Jiri Olsa
2023-09-08 16:49 ` Song Liu
@ 2023-09-08 23:22 ` Andrii Nakryiko
2023-09-08 23:32 ` Song Liu
2023-09-10 18:54 ` Jiri Olsa
1 sibling, 2 replies; 27+ messages in thread
From: Andrii Nakryiko @ 2023-09-08 23:22 UTC (permalink / raw)
To: Jiri Olsa
Cc: Song Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Fri, Sep 8, 2023 at 4:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> > On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Add missed value to kprobe attached through perf link info to
> > > hold the stats of missed kprobe handler execution.
> > >
> > > The kprobe's missed counter gets incremented when kprobe handler
> > > is not executed due to another kprobe running on the same cpu.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >
> > [...]
> >
> > The code looks good to me. But I have two thoughts on this (and 2/9).
> >
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index e5216420ec73..e824b0c50425 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > > __u32 name_len;
> > > __u32 offset; /* offset from func_name */
> > > __u64 addr;
> > > + __u64 missed;
> > > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > > struct {
> > > __aligned_u64 tp_name; /* in/out */
> >
> > 1) Shall we add missed for all bpf_link_info? Something like:
> >
> > diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> > index 5a39c7a13499..cf0b8b2a8b39 100644
> > --- i/include/uapi/linux/bpf.h
> > +++ w/include/uapi/linux/bpf.h
> > @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> > __u32 type;
> > __u32 id;
> > __u32 prog_id;
> > + __u64 missed;
> > union {
> > struct {
> > __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
>
> hm, there's lot of links under bpf_link_info, can't really tell if
> all could gather 'missed' data.. like I don't think we have any for
> standard perf event or perf tracepoint
even if missed for all link types would make sense, we can't add any
field before union, this would be a breaking change
>
> >
> > 2) "missed" doesn't seem to fit well with other information in
> > struct bpf_link_info. Other information there are more like stable-ish
> > information; while missed is a stat that changes over time. Given we
> > have prog_id in bpf_link_info, do we really need "missed" here?
>
> right, OTOH there's recursion_misses/run_time_ns/run_cnt in bpf_prog_info
>
> the bpf link has access to its attach layer, like perf event for kprobe
> in perf_link or fprobe for kprobe_multi link... so it's convenient to
> reach out from link for these stats and make them available through
> bpf_link_info
but what's confusing to me is that missed counter is per-program (at
least in your patch #1), but you report it on a link. But the same
BPF program can be attached multiple times through independent links.
So each link will report a shared misses count, which is quite
confusing.
Have you thought about counting misses per link instead of per
program? Is it possible?
>
> also there's no other way to get these data for some links
>
> like we could perhaps add some perf event specific interface to retrieve
> these stats for kprobes, because we have access to the perf event in user
> space, but that's not the case for kprobe_multi link, because there's no
> other way to reach the fprobe object
>
> jirka
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 23:22 ` Andrii Nakryiko
@ 2023-09-08 23:32 ` Song Liu
2023-09-08 23:44 ` Andrii Nakryiko
2023-09-10 18:54 ` Jiri Olsa
1 sibling, 1 reply; 27+ messages in thread
From: Song Liu @ 2023-09-08 23:32 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Fri, Sep 8, 2023 at 4:22 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Sep 8, 2023 at 4:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> > > On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Add missed value to kprobe attached through perf link info to
> > > > hold the stats of missed kprobe handler execution.
> > > >
> > > > The kprobe's missed counter gets incremented when kprobe handler
> > > > is not executed due to another kprobe running on the same cpu.
> > > >
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > >
> > > [...]
> > >
> > > The code looks good to me. But I have two thoughts on this (and 2/9).
> > >
> > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > > index e5216420ec73..e824b0c50425 100644
> > > > --- a/include/uapi/linux/bpf.h
> > > > +++ b/include/uapi/linux/bpf.h
> > > > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > > > __u32 name_len;
> > > > __u32 offset; /* offset from func_name */
> > > > __u64 addr;
> > > > + __u64 missed;
> > > > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > > > struct {
> > > > __aligned_u64 tp_name; /* in/out */
> > >
> > > 1) Shall we add missed for all bpf_link_info? Something like:
> > >
> > > diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> > > index 5a39c7a13499..cf0b8b2a8b39 100644
> > > --- i/include/uapi/linux/bpf.h
> > > +++ w/include/uapi/linux/bpf.h
> > > @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> > > __u32 type;
> > > __u32 id;
> > > __u32 prog_id;
> > > + __u64 missed;
> > > union {
> > > struct {
> > > __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
> >
> > hm, there's lot of links under bpf_link_info, can't really tell if
> > all could gather 'missed' data.. like I don't think we have any for
> > standard perf event or perf tracepoint
>
> even if missed for all link types would make sense, we can't add any
> field before union, this would be a breaking change
Right...
It is also tricky to add it to the union, right? We cannot tell whether
the kernel supports missed stats based on sizeof(struct bpf_link_info).
I guess this is also problematic?
Thanks,
Song
[...]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 23:32 ` Song Liu
@ 2023-09-08 23:44 ` Andrii Nakryiko
0 siblings, 0 replies; 27+ messages in thread
From: Andrii Nakryiko @ 2023-09-08 23:44 UTC (permalink / raw)
To: Song Liu
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Fri, Sep 8, 2023 at 4:33 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Sep 8, 2023 at 4:22 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Sep 8, 2023 at 4:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> > > > On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > Add missed value to kprobe attached through perf link info to
> > > > > hold the stats of missed kprobe handler execution.
> > > > >
> > > > > The kprobe's missed counter gets incremented when kprobe handler
> > > > > is not executed due to another kprobe running on the same cpu.
> > > > >
> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > >
> > > > [...]
> > > >
> > > > The code looks good to me. But I have two thoughts on this (and 2/9).
> > > >
> > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > > > index e5216420ec73..e824b0c50425 100644
> > > > > --- a/include/uapi/linux/bpf.h
> > > > > +++ b/include/uapi/linux/bpf.h
> > > > > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > > > > __u32 name_len;
> > > > > __u32 offset; /* offset from func_name */
> > > > > __u64 addr;
> > > > > + __u64 missed;
> > > > > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > > > > struct {
> > > > > __aligned_u64 tp_name; /* in/out */
> > > >
> > > > 1) Shall we add missed for all bpf_link_info? Something like:
> > > >
> > > > diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> > > > index 5a39c7a13499..cf0b8b2a8b39 100644
> > > > --- i/include/uapi/linux/bpf.h
> > > > +++ w/include/uapi/linux/bpf.h
> > > > @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> > > > __u32 type;
> > > > __u32 id;
> > > > __u32 prog_id;
> > > > + __u64 missed;
> > > > union {
> > > > struct {
> > > > __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
> > >
> > > hm, there's lot of links under bpf_link_info, can't really tell if
> > > all could gather 'missed' data.. like I don't think we have any for
> > > standard perf event or perf tracepoint
> >
> > even if missed for all link types would make sense, we can't add any
> > field before union, this would be a breaking change
>
> Right...
>
> It is also tricky to add it to the union, right? We cannot tell whether
> the kernel supports missed stats based on sizeof(struct bpf_link_info).
> I guess this is also problematic?
right, just checking size won't be reliable (it would be if missed is
added to largest substruct of a union). If it's important to know if
kernel reports missed, one would need to do a more proper feature
detection
>
> Thanks,
> Song
>
> [...]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf link info
2023-09-08 23:22 ` Andrii Nakryiko
2023-09-08 23:32 ` Song Liu
@ 2023-09-10 18:54 ` Jiri Olsa
1 sibling, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-10 18:54 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Song Liu, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao,
Daniel Xu
On Fri, Sep 08, 2023 at 04:22:05PM -0700, Andrii Nakryiko wrote:
> On Fri, Sep 8, 2023 at 4:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, Sep 07, 2023 at 11:40:46AM -0700, Song Liu wrote:
> > > On Thu, Sep 7, 2023 at 12:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Add missed value to kprobe attached through perf link info to
> > > > hold the stats of missed kprobe handler execution.
> > > >
> > > > The kprobe's missed counter gets incremented when kprobe handler
> > > > is not executed due to another kprobe running on the same cpu.
> > > >
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > >
> > > [...]
> > >
> > > The code looks good to me. But I have two thoughts on this (and 2/9).
> > >
> > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > > index e5216420ec73..e824b0c50425 100644
> > > > --- a/include/uapi/linux/bpf.h
> > > > +++ b/include/uapi/linux/bpf.h
> > > > @@ -6546,6 +6546,7 @@ struct bpf_link_info {
> > > > __u32 name_len;
> > > > __u32 offset; /* offset from func_name */
> > > > __u64 addr;
> > > > + __u64 missed;
> > > > } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> > > > struct {
> > > > __aligned_u64 tp_name; /* in/out */
> > >
> > > 1) Shall we add missed for all bpf_link_info? Something like:
> > >
> > > diff --git i/include/uapi/linux/bpf.h w/include/uapi/linux/bpf.h
> > > index 5a39c7a13499..cf0b8b2a8b39 100644
> > > --- i/include/uapi/linux/bpf.h
> > > +++ w/include/uapi/linux/bpf.h
> > > @@ -6465,6 +6465,7 @@ struct bpf_link_info {
> > > __u32 type;
> > > __u32 id;
> > > __u32 prog_id;
> > > + __u64 missed;
> > > union {
> > > struct {
> > > __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
> >
> > hm, there's lot of links under bpf_link_info, can't really tell if
> > all could gather 'missed' data.. like I don't think we have any for
> > standard perf event or perf tracepoint
>
> even if missed for all link types would make sense, we can't add any
> field before union, this would be a breaking change
>
> >
> > >
> > > 2) "missed" doesn't seem to fit well with other information in
> > > struct bpf_link_info. Other information there are more like stable-ish
> > > information; while missed is a stat that changes over time. Given we
> > > have prog_id in bpf_link_info, do we really need "missed" here?
> >
> > right, OTOH there's recursion_misses/run_time_ns/run_cnt in bpf_prog_info
> >
> > the bpf link has access to its attach layer, like perf event for kprobe
> > in perf_link or fprobe for kprobe_multi link... so it's convenient to
> > reach out from link for these stats and make them available through
> > bpf_link_info
>
> but what's confusing to me is that missed counter is per-program (at
> least in your patch #1), but you report it on a link. But the same
> BPF program can be attached multiple times through independent links.
> So each link will report a shared misses count, which is quite
> confusing.
>
> Have you thought about counting misses per link instead of per
> program? Is it possible?
I think recursion_misses makes sense for both program and link
currently we have recursion_misses per program which I think is
still useful even if the program is attached to multiple links
if the program is attached to multiple links it'd be useful to
have perf link stat as well
it is definitely possible for kprobe_multi link, which IIUC might
not be the main user here, because you can already attach program
to multiple functions
I guess perf_link would benefit more from this stats, but it
looks bit harder to add.. we'd need to add link pointer to
bpf_prog_array_item and add some extra logic, will check
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 4/9] bpf: Count missed stats in trace_call_bpf
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (2 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 3/9] bpf: Add missed value to kprobe perf " Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:46 ` Song Liu
2023-09-07 7:13 ` [PATCHv2 bpf-next 5/9] bpftool: Display missed count for kprobe_multi link Jiri Olsa
` (4 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
Increase misses stats in case bpf array execution is skipped
because of recursion check in trace_call_bpf.
Adding bpf_prog_inc_misses_counters that increase misses
counts for all bpf programs in bpf_prog_array.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/bpf.h | 16 ++++++++++++++++
kernel/trace/bpf_trace.c | 3 +++
2 files changed, 19 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 87eeb3a46a1d..abc18d6f2f2e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2911,6 +2911,22 @@ static inline int sock_map_bpf_prog_query(const union bpf_attr *attr,
#endif /* CONFIG_BPF_SYSCALL */
#endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
+static __always_inline void
+bpf_prog_inc_misses_counters(const struct bpf_prog_array *array)
+{
+ const struct bpf_prog_array_item *item;
+ struct bpf_prog *prog;
+
+ if (unlikely(!array))
+ return;
+
+ item = &array->items[0];
+ while ((prog = READ_ONCE(item->prog))) {
+ bpf_prog_inc_misses_counter(prog);
+ item++;
+ }
+}
+
#if defined(CONFIG_INET) && defined(CONFIG_BPF_SYSCALL)
void bpf_sk_reuseport_detach(struct sock *sk);
int bpf_fd_reuseport_array_lookup_elem(struct bpf_map *map, void *key,
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index a9d8634b503c..44f399b19af1 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -117,6 +117,9 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
* and don't send kprobe event into ring-buffer,
* so return zero here
*/
+ rcu_read_lock();
+ bpf_prog_inc_misses_counters(rcu_dereference(call->prog_array));
+ rcu_read_unlock();
ret = 0;
goto out;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 4/9] bpf: Count missed stats in trace_call_bpf
2023-09-07 7:13 ` [PATCHv2 bpf-next 4/9] bpf: Count missed stats in trace_call_bpf Jiri Olsa
@ 2023-09-07 18:46 ` Song Liu
0 siblings, 0 replies; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:46 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Increase misses stats in case bpf array execution is skipped
> because of recursion check in trace_call_bpf.
>
> Adding bpf_prog_inc_misses_counters that increase misses
> counts for all bpf programs in bpf_prog_array.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-and-tested-by: Song Liu <song@kernel.org>
> ---
> include/linux/bpf.h | 16 ++++++++++++++++
> kernel/trace/bpf_trace.c | 3 +++
> 2 files changed, 19 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 87eeb3a46a1d..abc18d6f2f2e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2911,6 +2911,22 @@ static inline int sock_map_bpf_prog_query(const union bpf_attr *attr,
> #endif /* CONFIG_BPF_SYSCALL */
> #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
>
> +static __always_inline void
> +bpf_prog_inc_misses_counters(const struct bpf_prog_array *array)
> +{
> + const struct bpf_prog_array_item *item;
> + struct bpf_prog *prog;
> +
> + if (unlikely(!array))
> + return;
> +
> + item = &array->items[0];
> + while ((prog = READ_ONCE(item->prog))) {
> + bpf_prog_inc_misses_counter(prog);
> + item++;
> + }
> +}
> +
> #if defined(CONFIG_INET) && defined(CONFIG_BPF_SYSCALL)
> void bpf_sk_reuseport_detach(struct sock *sk);
> int bpf_fd_reuseport_array_lookup_elem(struct bpf_map *map, void *key,
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index a9d8634b503c..44f399b19af1 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -117,6 +117,9 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
> * and don't send kprobe event into ring-buffer,
> * so return zero here
> */
> + rcu_read_lock();
> + bpf_prog_inc_misses_counters(rcu_dereference(call->prog_array));
> + rcu_read_unlock();
> ret = 0;
> goto out;
> }
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 5/9] bpftool: Display missed count for kprobe_multi link
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (3 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 4/9] bpf: Count missed stats in trace_call_bpf Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 6/9] bpftool: Display missed count for kprobe perf link Jiri Olsa
` (3 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Quentin Monnet, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao,
Daniel Xu
Adding 'missed' field to display missed counts for kprobes
attached by kprobe multi link, like:
# bpftool link
5: kprobe_multi prog 76
kprobe.multi func_cnt 1 missed 1
addr func [module]
ffffffffa039c030 fp3_test [fprobe_test]
# bpftool link -jp
[{
"id": 5,
"type": "kprobe_multi",
"prog_id": 76,
"retprobe": false,
"func_cnt": 1,
"missed": 1,
"funcs": [{
"addr": 18446744072102723632,
"func": "fp3_test",
"module": "fprobe_test"
}
]
}
]
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/bpf/bpftool/link.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 2e5c231e08ac..d15d74cd1bed 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -265,6 +265,7 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
jsonw_bool_field(json_wtr, "retprobe",
info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
+ jsonw_uint_field(json_wtr, "missed", info->kprobe_multi.missed);
jsonw_name(json_wtr, "funcs");
jsonw_start_array(json_wtr);
addrs = u64_to_ptr(info->kprobe_multi.addrs);
@@ -641,6 +642,8 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
else
printf("\n\tkprobe.multi ");
printf("func_cnt %u ", info->kprobe_multi.count);
+ if (info->kprobe_multi.missed)
+ printf("missed %llu ", info->kprobe_multi.missed);
addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
qsort(addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCHv2 bpf-next 6/9] bpftool: Display missed count for kprobe perf link
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (4 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 5/9] bpftool: Display missed count for kprobe_multi link Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe Jiri Olsa
` (2 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Quentin Monnet, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao,
Daniel Xu
Adding 'missed' field to display missed counts for kprobes
attached by perf event link, like:
# bpftool link
5: perf_event prog 82
kprobe ffffffff815203e0 ksys_write
6: perf_event prog 83
kprobe ffffffff811d1e50 scheduler_tick missed 682217
# bpftool link -jp
[{
"id": 5,
"type": "perf_event",
"prog_id": 82,
"retprobe": false,
"addr": 18446744071584220128,
"func": "ksys_write",
"offset": 0,
"missed": 0
},{
"id": 6,
"type": "perf_event",
"prog_id": 83,
"retprobe": false,
"addr": 18446744071580753488,
"func": "scheduler_tick",
"offset": 0,
"missed": 693469
}
]
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/bpf/bpftool/link.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index d15d74cd1bed..4b1407b05056 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -302,6 +302,7 @@ show_perf_event_kprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
jsonw_string_field(wtr, "func",
u64_to_ptr(info->perf_event.kprobe.func_name));
jsonw_uint_field(wtr, "offset", info->perf_event.kprobe.offset);
+ jsonw_uint_field(wtr, "missed", info->perf_event.kprobe.missed);
}
static void
@@ -686,6 +687,8 @@ static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
printf("%s", buf);
if (info->perf_event.kprobe.offset)
printf("+%#x", info->perf_event.kprobe.offset);
+ if (info->perf_event.kprobe.missed)
+ printf(" missed %llu", info->perf_event.kprobe.missed);
printf(" ");
}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (5 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 6/9] bpftool: Display missed count for kprobe perf link Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:52 ` Song Liu
2023-09-08 23:25 ` Andrii Nakryiko
2023-09-07 7:13 ` [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion " Jiri Olsa
2023-09-07 7:13 ` [PATCHv2 bpf-next 9/9] selftests/bpf: Add test for recursion counts of perf event link tracepoint Jiri Olsa
8 siblings, 2 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Hou Tao, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
Adding test that puts kprobe on bpf_fentry_test1 that calls
bpf_kfunc_common_test kfunc, which has also kprobe on.
The latter won't get triggered due to kprobe recursion check
and kprobe missed counter is incremented.
Acked-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 5 ++
.../bpf/bpf_testmod/bpf_testmod_kfunc.h | 2 +
.../testing/selftests/bpf/prog_tests/missed.c | 47 +++++++++++++++++++
.../selftests/bpf/progs/missed_kprobe.c | 30 ++++++++++++
4 files changed, 84 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/missed.c
create mode 100644 tools/testing/selftests/bpf/progs/missed_kprobe.c
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index cefc5dd72573..a5e246f7b202 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -138,6 +138,10 @@ __bpf_kfunc void bpf_iter_testmod_seq_destroy(struct bpf_iter_testmod_seq *it)
it->cnt = 0;
}
+__bpf_kfunc void bpf_kfunc_common_test(void)
+{
+}
+
struct bpf_testmod_btf_type_tag_1 {
int a;
};
@@ -343,6 +347,7 @@ BTF_SET8_START(bpf_testmod_common_kfunc_ids)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_kfunc_common_test)
BTF_SET8_END(bpf_testmod_common_kfunc_ids)
static const struct btf_kfunc_id_set bpf_testmod_common_kfunc_set = {
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
index f5c5b1375c24..7c664dd61059 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
@@ -104,4 +104,6 @@ void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p);
void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p);
void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
+
+void bpf_kfunc_common_test(void) __ksym;
#endif /* _BPF_TESTMOD_KFUNC_H */
diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
new file mode 100644
index 000000000000..fc674258c81f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/missed.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "missed_kprobe.skel.h"
+
+/*
+ * Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
+ * kfunc, which has also kprobe on. The latter won't get triggered due
+ * to kprobe recursion check and kprobe missed counter is incremented.
+ */
+static void test_missed_perf_kprobe(void)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ struct bpf_link_info info = {};
+ struct missed_kprobe *skel;
+ __u32 len = sizeof(info);
+ int err, prog_fd;
+
+ skel = missed_kprobe__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "missed_kprobe__open_and_load"))
+ goto cleanup;
+
+ err = missed_kprobe__attach(skel);
+ if (!ASSERT_OK(err, "missed_kprobe__attach"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.trigger);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ err = bpf_link_get_info_by_fd(bpf_link__fd(skel->links.test2), &info, &len);
+ if (!ASSERT_OK(err, "bpf_link_get_info_by_fd"))
+ goto cleanup;
+
+ ASSERT_EQ(info.type, BPF_LINK_TYPE_PERF_EVENT, "info.type");
+ ASSERT_EQ(info.perf_event.type, BPF_PERF_EVENT_KPROBE, "info.perf_event.type");
+ ASSERT_EQ(info.perf_event.kprobe.missed, 1, "info.perf_event.kprobe.missed");
+
+cleanup:
+ missed_kprobe__destroy(skel);
+}
+
+void serial_test_missed(void)
+{
+ if (test__start_subtest("perf_kprobe"))
+ test_missed_perf_kprobe();
+}
diff --git a/tools/testing/selftests/bpf/progs/missed_kprobe.c b/tools/testing/selftests/bpf/progs/missed_kprobe.c
new file mode 100644
index 000000000000..7f9ef701f5de
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/missed_kprobe.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod_kfunc.h"
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * No tests in here, just to trigger 'bpf_fentry_test*'
+ * through tracing test_run
+ */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(trigger)
+{
+ return 0;
+}
+
+SEC("kprobe/bpf_fentry_test1")
+int test1(struct pt_regs *ctx)
+{
+ bpf_kfunc_common_test();
+ return 0;
+}
+
+SEC("kprobe/bpf_kfunc_common_test")
+int test2(struct pt_regs *ctx)
+{
+ return 0;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe
2023-09-07 7:13 ` [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe Jiri Olsa
@ 2023-09-07 18:52 ` Song Liu
2023-09-08 23:25 ` Andrii Nakryiko
1 sibling, 0 replies; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test that puts kprobe on bpf_fentry_test1 that calls
> bpf_kfunc_common_test kfunc, which has also kprobe on.
>
> The latter won't get triggered due to kprobe recursion check
> and kprobe missed counter is incremented.
>
> Acked-by: Hou Tao <houtao1@huawei.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-and-tested-by: Song Liu <song@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe
2023-09-07 7:13 ` [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe Jiri Olsa
2023-09-07 18:52 ` Song Liu
@ 2023-09-08 23:25 ` Andrii Nakryiko
2023-09-10 18:54 ` Jiri Olsa
1 sibling, 1 reply; 27+ messages in thread
From: Andrii Nakryiko @ 2023-09-08 23:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test that puts kprobe on bpf_fentry_test1 that calls
> bpf_kfunc_common_test kfunc, which has also kprobe on.
>
> The latter won't get triggered due to kprobe recursion check
> and kprobe missed counter is incremented.
>
> Acked-by: Hou Tao <houtao1@huawei.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 5 ++
> .../bpf/bpf_testmod/bpf_testmod_kfunc.h | 2 +
> .../testing/selftests/bpf/prog_tests/missed.c | 47 +++++++++++++++++++
> .../selftests/bpf/progs/missed_kprobe.c | 30 ++++++++++++
> 4 files changed, 84 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/missed.c
> create mode 100644 tools/testing/selftests/bpf/progs/missed_kprobe.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index cefc5dd72573..a5e246f7b202 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -138,6 +138,10 @@ __bpf_kfunc void bpf_iter_testmod_seq_destroy(struct bpf_iter_testmod_seq *it)
> it->cnt = 0;
> }
>
> +__bpf_kfunc void bpf_kfunc_common_test(void)
> +{
> +}
> +
> struct bpf_testmod_btf_type_tag_1 {
> int a;
> };
> @@ -343,6 +347,7 @@ BTF_SET8_START(bpf_testmod_common_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
> +BTF_ID_FLAGS(func, bpf_kfunc_common_test)
> BTF_SET8_END(bpf_testmod_common_kfunc_ids)
>
> static const struct btf_kfunc_id_set bpf_testmod_common_kfunc_set = {
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> index f5c5b1375c24..7c664dd61059 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> @@ -104,4 +104,6 @@ void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p);
> void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p);
> void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
> void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
> +
> +void bpf_kfunc_common_test(void) __ksym;
> #endif /* _BPF_TESTMOD_KFUNC_H */
> diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
> new file mode 100644
> index 000000000000..fc674258c81f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/missed.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include "missed_kprobe.skel.h"
> +
> +/*
> + * Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
> + * kfunc, which has also kprobe on. The latter won't get triggered due
> + * to kprobe recursion check and kprobe missed counter is incremented.
> + */
> +static void test_missed_perf_kprobe(void)
> +{
> + LIBBPF_OPTS(bpf_test_run_opts, topts);
> + struct bpf_link_info info = {};
> + struct missed_kprobe *skel;
> + __u32 len = sizeof(info);
> + int err, prog_fd;
> +
> + skel = missed_kprobe__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "missed_kprobe__open_and_load"))
> + goto cleanup;
> +
> + err = missed_kprobe__attach(skel);
> + if (!ASSERT_OK(err, "missed_kprobe__attach"))
> + goto cleanup;
> +
> + prog_fd = bpf_program__fd(skel->progs.trigger);
> + err = bpf_prog_test_run_opts(prog_fd, &topts);
> + ASSERT_OK(err, "test_run");
> + ASSERT_EQ(topts.retval, 0, "test_run");
> +
> + err = bpf_link_get_info_by_fd(bpf_link__fd(skel->links.test2), &info, &len);
> + if (!ASSERT_OK(err, "bpf_link_get_info_by_fd"))
> + goto cleanup;
> +
> + ASSERT_EQ(info.type, BPF_LINK_TYPE_PERF_EVENT, "info.type");
> + ASSERT_EQ(info.perf_event.type, BPF_PERF_EVENT_KPROBE, "info.perf_event.type");
> + ASSERT_EQ(info.perf_event.kprobe.missed, 1, "info.perf_event.kprobe.missed");
> +
> +cleanup:
> + missed_kprobe__destroy(skel);
> +}
> +
> +void serial_test_missed(void)
why serial? if you check for kprobe.missed >= 1, it should be fine
even if some other test calls this testmod kfunc, right?
> +{
> + if (test__start_subtest("perf_kprobe"))
> + test_missed_perf_kprobe();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/missed_kprobe.c b/tools/testing/selftests/bpf/progs/missed_kprobe.c
> new file mode 100644
> index 000000000000..7f9ef701f5de
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/missed_kprobe.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod_kfunc.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +/*
> + * No tests in here, just to trigger 'bpf_fentry_test*'
> + * through tracing test_run
> + */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(trigger)
> +{
> + return 0;
> +}
> +
> +SEC("kprobe/bpf_fentry_test1")
> +int test1(struct pt_regs *ctx)
> +{
> + bpf_kfunc_common_test();
> + return 0;
> +}
> +
> +SEC("kprobe/bpf_kfunc_common_test")
> +int test2(struct pt_regs *ctx)
> +{
> + return 0;
> +}
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe
2023-09-08 23:25 ` Andrii Nakryiko
@ 2023-09-10 18:54 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-10 18:54 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Fri, Sep 08, 2023 at 04:25:30PM -0700, Andrii Nakryiko wrote:
SNIP
> > +/*
> > + * Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
> > + * kfunc, which has also kprobe on. The latter won't get triggered due
> > + * to kprobe recursion check and kprobe missed counter is incremented.
> > + */
> > +static void test_missed_perf_kprobe(void)
> > +{
> > + LIBBPF_OPTS(bpf_test_run_opts, topts);
> > + struct bpf_link_info info = {};
> > + struct missed_kprobe *skel;
> > + __u32 len = sizeof(info);
> > + int err, prog_fd;
> > +
> > + skel = missed_kprobe__open_and_load();
> > + if (!ASSERT_OK_PTR(skel, "missed_kprobe__open_and_load"))
> > + goto cleanup;
> > +
> > + err = missed_kprobe__attach(skel);
> > + if (!ASSERT_OK(err, "missed_kprobe__attach"))
> > + goto cleanup;
> > +
> > + prog_fd = bpf_program__fd(skel->progs.trigger);
> > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > + ASSERT_OK(err, "test_run");
> > + ASSERT_EQ(topts.retval, 0, "test_run");
> > +
> > + err = bpf_link_get_info_by_fd(bpf_link__fd(skel->links.test2), &info, &len);
> > + if (!ASSERT_OK(err, "bpf_link_get_info_by_fd"))
> > + goto cleanup;
> > +
> > + ASSERT_EQ(info.type, BPF_LINK_TYPE_PERF_EVENT, "info.type");
> > + ASSERT_EQ(info.perf_event.type, BPF_PERF_EVENT_KPROBE, "info.perf_event.type");
> > + ASSERT_EQ(info.perf_event.kprobe.missed, 1, "info.perf_event.kprobe.missed");
> > +
> > +cleanup:
> > + missed_kprobe__destroy(skel);
> > +}
> > +
> > +void serial_test_missed(void)
>
> why serial? if you check for kprobe.missed >= 1, it should be fine
> even if some other test calls this testmod kfunc, right?
hm, I think the reason for me was the testmod getting unloaded
in serial_test_bpf_mod_race test.. but it's serial, so I guess
we should be fine
thanks,
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion counts of perf event link kprobe
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (6 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 7/9] selftests/bpf: Add test for missed counts of perf event link kprobe Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 18:55 ` Song Liu
2023-09-07 7:13 ` [PATCHv2 bpf-next 9/9] selftests/bpf: Add test for recursion counts of perf event link tracepoint Jiri Olsa
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Hou Tao, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
Adding selftest that puts kprobe.multi on bpf_fentry_test1 that
calls bpf_kfunc_common_test kfunc which has 3 perf event kprobes
and 1 kprobe.multi attached.
Because fprobe (kprobe.multi attach layear) does not have strict
recursion check the kprobe's bpf_prog_active check is hit for test2-5.
Disabling this test for arm64, because there's no fprobe support yet.
Acked-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 +
.../testing/selftests/bpf/prog_tests/missed.c | 51 +++++++++++++++++++
.../bpf/progs/missed_kprobe_recursion.c | 48 +++++++++++++++++
3 files changed, 100 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
index 7f768d335698..3f2187c049db 100644
--- a/tools/testing/selftests/bpf/DENYLIST.aarch64
+++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
@@ -15,3 +15,4 @@ fexit_test/fexit_many_args # fexit_many_args:FAIL:fexit_ma
fill_link_info/kprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
fill_link_info/kretprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
fill_link_info/kprobe_multi_invalid_ubuff # bpf_program__attach_kprobe_multi_opts unexpected error: -95
+missed/kprobe_recursion # missed_kprobe_recursion__attach unexpected error: -95 (errno 95)
diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
index fc674258c81f..f10dc9232b3f 100644
--- a/tools/testing/selftests/bpf/prog_tests/missed.c
+++ b/tools/testing/selftests/bpf/prog_tests/missed.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
#include "missed_kprobe.skel.h"
+#include "missed_kprobe_recursion.skel.h"
/*
* Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
@@ -40,8 +41,58 @@ static void test_missed_perf_kprobe(void)
missed_kprobe__destroy(skel);
}
+static __u64 get_count(int fd)
+{
+ struct bpf_prog_info info = {};
+ __u32 len = sizeof(info);
+ int err;
+
+ err = bpf_prog_get_info_by_fd(fd, &info, &len);
+ if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd"))
+ return (__u64) -1;
+ return info.recursion_misses;
+}
+
+/*
+ * Putting kprobe.multi on bpf_fentry_test1 that calls bpf_kfunc_common_test
+ * kfunc which has 3 perf event kprobes and 1 kprobe.multi attached.
+ *
+ * Because fprobe (kprobe.multi attach layear) does not have strict recursion
+ * check the kprobe's bpf_prog_active check is hit for test2-5.
+ */
+static void test_missed_kprobe_recursion(void)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ struct missed_kprobe_recursion *skel;
+ int err, prog_fd;
+
+ skel = missed_kprobe_recursion__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "missed_kprobe_recursion__open_and_load"))
+ goto cleanup;
+
+ err = missed_kprobe_recursion__attach(skel);
+ if (!ASSERT_OK(err, "missed_kprobe_recursion__attach"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.trigger);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test1)), 0, "test1_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test2)), 1, "test2_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test3)), 1, "test3_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test4)), 1, "test4_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test5)), 1, "test5_recursion_misses");
+
+cleanup:
+ missed_kprobe_recursion__destroy(skel);
+}
+
void serial_test_missed(void)
{
if (test__start_subtest("perf_kprobe"))
test_missed_perf_kprobe();
+ if (test__start_subtest("kprobe_recursion"))
+ test_missed_kprobe_recursion();
}
diff --git a/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c b/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
new file mode 100644
index 000000000000..8ea71cbd6c45
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod_kfunc.h"
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * No tests in here, just to trigger 'bpf_fentry_test*'
+ * through tracing test_run
+ */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(trigger)
+{
+ return 0;
+}
+
+SEC("kprobe.multi/bpf_fentry_test1")
+int test1(struct pt_regs *ctx)
+{
+ bpf_kfunc_common_test();
+ return 0;
+}
+
+SEC("kprobe/bpf_kfunc_common_test")
+int test2(struct pt_regs *ctx)
+{
+ return 0;
+}
+
+SEC("kprobe/bpf_kfunc_common_test")
+int test3(struct pt_regs *ctx)
+{
+ return 0;
+}
+
+SEC("kprobe/bpf_kfunc_common_test")
+int test4(struct pt_regs *ctx)
+{
+ return 0;
+}
+
+SEC("kprobe.multi/bpf_kfunc_common_test")
+int test5(struct pt_regs *ctx)
+{
+ return 0;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion counts of perf event link kprobe
2023-09-07 7:13 ` [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion " Jiri Olsa
@ 2023-09-07 18:55 ` Song Liu
2023-09-14 7:56 ` Jiri Olsa
0 siblings, 1 reply; 27+ messages in thread
From: Song Liu @ 2023-09-07 18:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding selftest that puts kprobe.multi on bpf_fentry_test1 that
> calls bpf_kfunc_common_test kfunc which has 3 perf event kprobes
> and 1 kprobe.multi attached.
>
> Because fprobe (kprobe.multi attach layear) does not have strict
> recursion check the kprobe's bpf_prog_active check is hit for test2-5.
>
> Disabling this test for arm64, because there's no fprobe support yet.
>
> Acked-by: Hou Tao <houtao1@huawei.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-and-tested-by: Song Liu <song@kernel.org>
With on nitpick below.
> ---
> tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 +
> .../testing/selftests/bpf/prog_tests/missed.c | 51 +++++++++++++++++++
> .../bpf/progs/missed_kprobe_recursion.c | 48 +++++++++++++++++
> 3 files changed, 100 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
>
> diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
> index 7f768d335698..3f2187c049db 100644
> --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
> +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
> @@ -15,3 +15,4 @@ fexit_test/fexit_many_args # fexit_many_args:FAIL:fexit_ma
> fill_link_info/kprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> fill_link_info/kretprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> fill_link_info/kprobe_multi_invalid_ubuff # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> +missed/kprobe_recursion # missed_kprobe_recursion__attach unexpected error: -95 (errno 95)
> diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
> index fc674258c81f..f10dc9232b3f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/missed.c
> +++ b/tools/testing/selftests/bpf/prog_tests/missed.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <test_progs.h>
> #include "missed_kprobe.skel.h"
> +#include "missed_kprobe_recursion.skel.h"
>
> /*
> * Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
> @@ -40,8 +41,58 @@ static void test_missed_perf_kprobe(void)
> missed_kprobe__destroy(skel);
> }
>
> +static __u64 get_count(int fd)
nit: Probably rename it as get_missed_count() or get_missed().
> +{
> + struct bpf_prog_info info = {};
> + __u32 len = sizeof(info);
> + int err;
> +
> + err = bpf_prog_get_info_by_fd(fd, &info, &len);
> + if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd"))
> + return (__u64) -1;
> + return info.recursion_misses;
> +}
[...]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion counts of perf event link kprobe
2023-09-07 18:55 ` Song Liu
@ 2023-09-14 7:56 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2023-09-14 7:56 UTC (permalink / raw)
To: Song Liu
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Hou Tao,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Daniel Xu
On Thu, Sep 07, 2023 at 11:55:27AM -0700, Song Liu wrote:
> On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding selftest that puts kprobe.multi on bpf_fentry_test1 that
> > calls bpf_kfunc_common_test kfunc which has 3 perf event kprobes
> > and 1 kprobe.multi attached.
> >
> > Because fprobe (kprobe.multi attach layear) does not have strict
> > recursion check the kprobe's bpf_prog_active check is hit for test2-5.
> >
> > Disabling this test for arm64, because there's no fprobe support yet.
> >
> > Acked-by: Hou Tao <houtao1@huawei.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>
> Reviewed-and-tested-by: Song Liu <song@kernel.org>
>
> With on nitpick below.
>
> > ---
> > tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 +
> > .../testing/selftests/bpf/prog_tests/missed.c | 51 +++++++++++++++++++
> > .../bpf/progs/missed_kprobe_recursion.c | 48 +++++++++++++++++
> > 3 files changed, 100 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
> >
> > diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
> > index 7f768d335698..3f2187c049db 100644
> > --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
> > +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
> > @@ -15,3 +15,4 @@ fexit_test/fexit_many_args # fexit_many_args:FAIL:fexit_ma
> > fill_link_info/kprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> > fill_link_info/kretprobe_multi_link_info # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> > fill_link_info/kprobe_multi_invalid_ubuff # bpf_program__attach_kprobe_multi_opts unexpected error: -95
> > +missed/kprobe_recursion # missed_kprobe_recursion__attach unexpected error: -95 (errno 95)
> > diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
> > index fc674258c81f..f10dc9232b3f 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/missed.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/missed.c
> > @@ -1,6 +1,7 @@
> > // SPDX-License-Identifier: GPL-2.0
> > #include <test_progs.h>
> > #include "missed_kprobe.skel.h"
> > +#include "missed_kprobe_recursion.skel.h"
> >
> > /*
> > * Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
> > @@ -40,8 +41,58 @@ static void test_missed_perf_kprobe(void)
> > missed_kprobe__destroy(skel);
> > }
> >
> > +static __u64 get_count(int fd)
>
> nit: Probably rename it as get_missed_count() or get_missed().
right, I was thinking of that ;-) will change
thanks,
jirka
>
> > +{
> > + struct bpf_prog_info info = {};
> > + __u32 len = sizeof(info);
> > + int err;
> > +
> > + err = bpf_prog_get_info_by_fd(fd, &info, &len);
> > + if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd"))
> > + return (__u64) -1;
> > + return info.recursion_misses;
> > +}
> [...]
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv2 bpf-next 9/9] selftests/bpf: Add test for recursion counts of perf event link tracepoint
2023-09-07 7:13 [PATCHv2 bpf-next 0/9] bpf: Add missed stats for kprobes Jiri Olsa
` (7 preceding siblings ...)
2023-09-07 7:13 ` [PATCHv2 bpf-next 8/9] selftests/bpf: Add test for recursion " Jiri Olsa
@ 2023-09-07 7:13 ` Jiri Olsa
2023-09-07 19:00 ` Song Liu
8 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2023-09-07 7:13 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
Adding selftest that puts kprobe on bpf_fentry_test1 that calls bpf_printk
and invokes bpf_trace_printk tracepoint. The bpf_trace_printk tracepoint
has test[234] programs attached to it.
Because kprobe execution goes through bpf_prog_active check, programs
attached to the tracepoint will fail the recursion check and increment the
recursion_misses stats.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../testing/selftests/bpf/prog_tests/missed.c | 40 ++++++++++++++++++
.../selftests/bpf/progs/missed_tp_recursion.c | 41 +++++++++++++++++++
2 files changed, 81 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/missed_tp_recursion.c
diff --git a/tools/testing/selftests/bpf/prog_tests/missed.c b/tools/testing/selftests/bpf/prog_tests/missed.c
index f10dc9232b3f..ae12da2c1dab 100644
--- a/tools/testing/selftests/bpf/prog_tests/missed.c
+++ b/tools/testing/selftests/bpf/prog_tests/missed.c
@@ -2,6 +2,7 @@
#include <test_progs.h>
#include "missed_kprobe.skel.h"
#include "missed_kprobe_recursion.skel.h"
+#include "missed_tp_recursion.skel.h"
/*
* Putting kprobe on bpf_fentry_test1 that calls bpf_kfunc_common_test
@@ -89,10 +90,49 @@ static void test_missed_kprobe_recursion(void)
missed_kprobe_recursion__destroy(skel);
}
+/*
+ * Putting kprobe on bpf_fentry_test1 that calls bpf_printk and invokes
+ * bpf_trace_printk tracepoint. The bpf_trace_printk tracepoint has test[234]
+ * programs attached to it.
+ *
+ * Because kprobe execution goes through bpf_prog_active check, programs
+ * attached to the tracepoint will fail the recursion check and increment
+ * the recursion_misses stats.
+ */
+static void test_missed_tp_recursion(void)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, topts);
+ struct missed_tp_recursion *skel;
+ int err, prog_fd;
+
+ skel = missed_tp_recursion__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "missed_tp_recursion__open_and_load"))
+ goto cleanup;
+
+ err = missed_tp_recursion__attach(skel);
+ if (!ASSERT_OK(err, "missed_tp_recursion__attach"))
+ goto cleanup;
+
+ prog_fd = bpf_program__fd(skel->progs.trigger);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "test_run");
+ ASSERT_EQ(topts.retval, 0, "test_run");
+
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test1)), 0, "test1_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test2)), 1, "test2_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test3)), 1, "test3_recursion_misses");
+ ASSERT_EQ(get_count(bpf_program__fd(skel->progs.test4)), 1, "test4_recursion_misses");
+
+cleanup:
+ missed_tp_recursion__destroy(skel);
+}
+
void serial_test_missed(void)
{
if (test__start_subtest("perf_kprobe"))
test_missed_perf_kprobe();
if (test__start_subtest("kprobe_recursion"))
test_missed_kprobe_recursion();
+ if (test__start_subtest("tp_recursion"))
+ test_missed_tp_recursion();
}
diff --git a/tools/testing/selftests/bpf/progs/missed_tp_recursion.c b/tools/testing/selftests/bpf/progs/missed_tp_recursion.c
new file mode 100644
index 000000000000..762385f827c5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/missed_tp_recursion.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * No tests in here, just to trigger 'bpf_fentry_test*'
+ * through tracing test_run
+ */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(trigger)
+{
+ return 0;
+}
+
+SEC("kprobe/bpf_fentry_test1")
+int test1(struct pt_regs *ctx)
+{
+ bpf_printk("test");
+ return 0;
+}
+
+SEC("tp/bpf_trace/bpf_trace_printk")
+int test2(struct pt_regs *ctx)
+{
+ return 0;
+}
+
+SEC("tp/bpf_trace/bpf_trace_printk")
+int test3(struct pt_regs *ctx)
+{
+ return 0;
+}
+
+SEC("tp/bpf_trace/bpf_trace_printk")
+int test4(struct pt_regs *ctx)
+{
+ return 0;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHv2 bpf-next 9/9] selftests/bpf: Add test for recursion counts of perf event link tracepoint
2023-09-07 7:13 ` [PATCHv2 bpf-next 9/9] selftests/bpf: Add test for recursion counts of perf event link tracepoint Jiri Olsa
@ 2023-09-07 19:00 ` Song Liu
0 siblings, 0 replies; 27+ messages in thread
From: Song Liu @ 2023-09-07 19:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao, Daniel Xu
On Thu, Sep 7, 2023 at 12:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding selftest that puts kprobe on bpf_fentry_test1 that calls bpf_printk
> and invokes bpf_trace_printk tracepoint. The bpf_trace_printk tracepoint
> has test[234] programs attached to it.
>
> Because kprobe execution goes through bpf_prog_active check, programs
> attached to the tracepoint will fail the recursion check and increment the
> recursion_misses stats.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-and-tested-by: Song Liu <song@kernel.org>
[...]
^ permalink raw reply [flat|nested] 27+ messages in thread