bpf: Add missed stats for kprobes

Adding support to gather missed stats for kprobe_multi programs due to bpf_prog_active protection.

Signed-off-by Jiri Olsa <[email protected]>

---
 kernel/trace/bpf_trace.c | 1 +
 1 file changed, 1 insertion(+)

kernel/trace/bpf_trace.c +1 -1

index a7264b2c17ad..279a3d370812 100644
@@ -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

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 <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 include/uapi/linux/bpf.h       | 1 +
 kernel/trace/bpf_trace.c       | 1 +
 tools/include/uapi/linux/bpf.h | 1 +
 3 files changed, 3 insertions(+)

include/uapi/linux/bpf.h +1 -0

index 73b155e52204..e5216420ec73 100644
@@ -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 */

kernel/trace/bpf_trace.c +1 -0

index 279a3d370812..aec52938c703 100644
@@ -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;

tools/include/uapi/linux/bpf.h +1 -1

index 73b155e52204..e5216420ec73 100644
@@ -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

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 <[email protected]>

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

include/uapi/linux/bpf.h +0 -0

index 5a39c7a13499..cf0b8b2a8b39 100644

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?

Quoted 29 lines · Song Liu

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

include/uapi/linux/bpf.h struct bpf_link_info { · line 6471

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index 5a39c7a13499..cf0b8b2a8b39 100644
@@ -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

Quoted 6 lines

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

Quoted 11 lines

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?

Quoted 29 lines · Song Liu

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

include/uapi/linux/bpf.h struct bpf_link_info { · line 6471

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index 5a39c7a13499..cf0b8b2a8b39 100644
@@ -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

Quoted 6 lines

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

Quoted 31 lines · Jiri Olsa

Quoted 29 lines · Song Liu

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

Quoted 16 lines

include/uapi/linux/bpf.h struct bpf_link_info { · line 6471

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index 5a39c7a13499..cf0b8b2a8b39 100644
@@ -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

Quoted 50 lines · Andrii Nakryiko

Quoted 47 lines · Jiri Olsa

Quoted 34 lines · Song Liu

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

include/uapi/linux/bpf.h +0 -0

index 5a39c7a13499..cf0b8b2a8b39 100644

include/uapi/linux/bpf.h struct bpf_link_info { · line 6471

Quoted 7 lines

include/uapi/linux/bpf.h +1 -0

@@ -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

Quoted 2 lines

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

[...]

Quoted 31 lines · Jiri Olsa

Quoted 29 lines · Song Liu

Quoted 8 lines · Jiri Olsa

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 <[email protected]>

[...]

The code looks good to me. But I have two thoughts on this (and 2/9).

include/uapi/linux/bpf.h struct bpf_link_info { · line 6552

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index e5216420ec73..e824b0c50425 100644
@@ -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:

Quoted 16 lines

include/uapi/linux/bpf.h struct bpf_link_info { · line 6471

Quoted 12 lines

include/uapi/linux/bpf.h +1 -0

index 5a39c7a13499..cf0b8b2a8b39 100644
@@ -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

Quoted 7 lines

Quoted 6 lines

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?

Quoted 6 lines

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?

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 <[email protected]>

---
 include/linux/bpf.h      | 16 ++++++++++++++++
 kernel/trace/bpf_trace.c |  3 +++
 2 files changed, 19 insertions(+)

include/linux/bpf.h +16 -0

index 87eeb3a46a1d..abc18d6f2f2e 100644
@@ -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,

kernel/trace/bpf_trace.c +3 -1

index a9d8634b503c..44f399b19af1 100644
@@ -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

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 <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

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 <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 .../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

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +5 -0

index cefc5dd72573..a5e246f7b202 100644
@@ -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 = {

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h +2 -0

index f5c5b1375c24..7c664dd61059 100644
@@ -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 */

tools/testing/selftests/bpf/prog_tests/missed.c new file +33 -0

new file mode 100644
index 000000000000..fc674258c81f
@@ -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;

tools/testing/selftests/bpf/prog_tests/missed.c new file +4 -0

+{
+       if (test__start_subtest("perf_kprobe"))
+               test_missed_perf_kprobe();
+}

tools/testing/selftests/bpf/progs/missed_kprobe.c new file +30 -1

new file mode 100644
index 000000000000..7f9ef701f5de
@@ -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

Quoted 34 lines

tools/testing/selftests/bpf/prog_tests/missed.c new file +34 -0

+/*
+ * 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");
+

tools/testing/selftests/bpf/prog_tests/missed.c line 43

Quoted 5 lines

tools/testing/selftests/bpf/prog_tests/missed.c new file +5 -0

+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?

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 <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 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

tools/testing/selftests/bpf/DENYLIST.aarch64 +1 -0

index 7f768d335698..3f2187c049db 100644
@@ -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)

tools/testing/selftests/bpf/prog_tests/missed.c +1 -0

index fc674258c81f..f10dc9232b3f 100644
@@ -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

tools/testing/selftests/bpf/prog_tests/missed.c +10 -0

+{
+       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;
+}

Quoted 5 lines · Jiri Olsa

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.

Quoted 7 lines

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 <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Reviewed-and-tested-by Song Liu <[email protected]>

With on nitpick below.

Quoted 32 lines

---
 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

tools/testing/selftests/bpf/DENYLIST.aarch64 +1 -0

index 7f768d335698..3f2187c049db 100644
@@ -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)

tools/testing/selftests/bpf/prog_tests/missed.c +1 -0

index fc674258c81f..f10dc9232b3f 100644
@@ -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);
 }
 

tools/testing/selftests/bpf/prog_tests/missed.c static void test_missed_perf_kprobe(void) · line 44

Quoted 1 line

tools/testing/selftests/bpf/prog_tests/missed.c +1 -0

+static __u64 get_count(int fd)

nit: Probably rename it as get_missed_count() or get_missed().

Quoted 11 lines

tools/testing/selftests/bpf/prog_tests/missed.c +10 -0

+{
+       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;
+}

[...]

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 <[email protected]>