2024-03-15 14:31:39

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v4 6/6] selftests/bpf: add sleepable timer tests

bpf_experimental.h and ../bpf_testmod/bpf_testmod_kfunc.h are both
including vmlinux.h, which is not compatible with including time.h
or bpf_tcp_helpers.h.

So prevent vmlinux.h to be included, and override the few missing
types.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v4
---
tools/testing/selftests/bpf/bpf_experimental.h | 4 +
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 5 +
.../selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h | 1 +
tools/testing/selftests/bpf/prog_tests/timer.c | 1 +
tools/testing/selftests/bpf/progs/timer.c | 40 +++++++-
tools/testing/selftests/bpf/progs/timer_failure.c | 114 ++++++++++++++++++++-
6 files changed, 163 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index a5b9df38c162..79da06ca4136 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -459,4 +459,8 @@ extern int bpf_iter_css_new(struct bpf_iter_css *it,
extern struct cgroup_subsys_state *bpf_iter_css_next(struct bpf_iter_css *it) __weak __ksym;
extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym;

+extern int bpf_timer_set_sleepable_cb_impl(struct bpf_timer *timer,
+ int (callback_fn)(void *map, int *key, struct bpf_timer *timer), void *aux__ign) __ksym;
+#define bpf_timer_set_sleepable_cb(timer, cb) \
+ bpf_timer_set_sleepable_cb_impl(timer, cb, NULL)
#endif
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 39ad96a18123..eb2b78552ca2 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -494,6 +494,10 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
return arg;
}

+__bpf_kfunc void bpf_kfunc_call_test_sleepable(void)
+{
+}
+
BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
@@ -520,6 +524,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_sleepable, KF_SLEEPABLE)
BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)

static int bpf_testmod_ops_init(struct btf *btf)
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 7c664dd61059..ce5cd763561c 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
@@ -96,6 +96,7 @@ void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;

void bpf_kfunc_call_test_destructive(void) __ksym;
+void bpf_kfunc_call_test_sleepable(void) __ksym;

void bpf_kfunc_call_test_offset(struct prog_test_ref_kfunc *p);
struct prog_test_member *bpf_kfunc_call_memb_acquire(void);
diff --git a/tools/testing/selftests/bpf/prog_tests/timer.c b/tools/testing/selftests/bpf/prog_tests/timer.c
index d66687f1ee6a..48973c2e28c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/timer.c
+++ b/tools/testing/selftests/bpf/prog_tests/timer.c
@@ -61,6 +61,7 @@ static int timer(struct timer *timer_skel)

/* check that code paths completed */
ASSERT_EQ(timer_skel->bss->ok, 1 | 2 | 4, "ok");
+ ASSERT_EQ(timer_skel->bss->ok_sleepable, 1, "ok_sleepable");

prog_fd = bpf_program__fd(timer_skel->progs.race);
for (i = 0; i < NUM_THR; i++) {
diff --git a/tools/testing/selftests/bpf/progs/timer.c b/tools/testing/selftests/bpf/progs/timer.c
index f615da97df26..6b19254c5b75 100644
--- a/tools/testing/selftests/bpf/progs/timer.c
+++ b/tools/testing/selftests/bpf/progs/timer.c
@@ -6,6 +6,14 @@
#include <bpf/bpf_helpers.h>
#include "bpf_tcp_helpers.h"

+#define __VMLINUX_H__
+#define u32 __u32
+#define u64 __u64
+#include "bpf_experimental.h"
+struct prog_test_member1;
+#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#undef __VMLINUX_H__
+
char _license[] SEC("license") = "GPL";
struct hmap_elem {
int counter;
@@ -34,7 +42,7 @@ struct elem {

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
- __uint(max_entries, 2);
+ __uint(max_entries, 3);
__type(key, int);
__type(value, struct elem);
} array SEC(".maps");
@@ -62,6 +70,7 @@ __u64 callback_check = 52;
__u64 callback2_check = 52;
__u64 pinned_callback_check;
__s32 pinned_cpu;
+__u32 ok_sleepable;

#define ARRAY 1
#define HTAB 2
@@ -422,3 +431,32 @@ int race(void *ctx)

return 0;
}
+
+/* callback for sleepable timer */
+static int timer_cb_sleepable(void *map, int *key, struct bpf_timer *timer)
+{
+ bpf_kfunc_call_test_sleepable();
+ ok_sleepable |= 1;
+ return 0;
+}
+
+SEC("fentry/bpf_fentry_test6")
+int BPF_PROG2(test6, int, a)
+{
+ int key = 2;
+ struct bpf_timer *timer;
+
+ bpf_printk("test6");
+
+ timer = bpf_map_lookup_elem(&array, &key);
+ if (timer) {
+ if (bpf_timer_init(timer, &array, CLOCK_MONOTONIC) != 0)
+ err |= 32768;
+ bpf_timer_set_sleepable_cb(timer, timer_cb_sleepable);
+ bpf_timer_start(timer, 0, BPF_F_TIMER_SLEEPABLE);
+ } else {
+ err |= 65536;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/timer_failure.c b/tools/testing/selftests/bpf/progs/timer_failure.c
index 0996c2486f05..72942a90189b 100644
--- a/tools/testing/selftests/bpf/progs/timer_failure.c
+++ b/tools/testing/selftests/bpf/progs/timer_failure.c
@@ -8,6 +8,14 @@
#include "bpf_misc.h"
#include "bpf_tcp_helpers.h"

+#define __VMLINUX_H__
+#define u32 __u32
+#define u64 __u64
+#include "bpf_experimental.h"
+struct prog_test_member1;
+#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#undef __VMLINUX_H__
+
char _license[] SEC("license") = "GPL";

struct elem {
@@ -16,7 +24,7 @@ struct elem {

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
- __uint(max_entries, 1);
+ __uint(max_entries, 2);
__type(key, int);
__type(value, struct elem);
} timer_map SEC(".maps");
@@ -66,3 +74,107 @@ long BPF_PROG2(test_bad_ret, int, a)

return 0;
}
+
+/* callback for sleepable timer */
+static int timer_cb_sleepable(void *map, int *key, struct bpf_timer *timer)
+{
+ bpf_kfunc_call_test_sleepable();
+ return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+__log_level(2)
+__failure
+/* check that bpf_timer_set_callback() can not be called with a
+ * sleepable callback
+ */
+__msg("mark_precise: frame0: regs=r0 stack= before")
+__msg(": (85) call bpf_kfunc_call_test_sleepable#") /* anchor message */
+__msg("program must be sleepable to call sleepable kfunc bpf_kfunc_call_test_sleepable")
+int BPF_PROG2(test_non_sleepable_sleepable_callback, int, a)
+{
+ int key = 0;
+ struct bpf_timer *timer;
+
+ timer = bpf_map_lookup_elem(&timer_map, &key);
+ if (timer) {
+ bpf_timer_init(timer, &timer_map, CLOCK_MONOTONIC);
+ bpf_timer_set_callback(timer, timer_cb_sleepable);
+ bpf_timer_start(timer, 0, BPF_F_TIMER_SLEEPABLE);
+ }
+
+ return 0;
+}
+
+SEC("tc")
+/* check that calling bpf_timer_start() without BPF_F_TIMER_SLEEPABLE on a sleepable
+ * callback is returning -EINVAL
+ */
+__retval(-22)
+long test_call_sleepable_missing_flag(void *ctx)
+{
+ int key = 0;
+ struct bpf_timer *timer;
+
+ timer = bpf_map_lookup_elem(&timer_map, &key);
+ if (!timer)
+ return 1;
+
+ if (bpf_timer_init(timer, &timer_map, CLOCK_MONOTONIC))
+ return 2;
+
+ if (bpf_timer_set_sleepable_cb(timer, timer_cb_sleepable))
+ return 3;
+
+ return bpf_timer_start(timer, 0, 0);
+}
+
+SEC("tc")
+/* check that calling bpf_timer_start() without BPF_F_TIMER_SLEEPABLE on a sleepable
+ * callback is returning -EINVAL
+ */
+__retval(-22)
+long test_call_sleepable_delay(void *ctx)
+{
+ int key = 1;
+ struct bpf_timer *timer;
+
+ timer = bpf_map_lookup_elem(&timer_map, &key);
+ if (!timer)
+ return 1;
+
+ if (bpf_timer_init(timer, &timer_map, CLOCK_MONOTONIC))
+ return 2;
+
+ if (bpf_timer_set_sleepable_cb(timer, timer_cb_sleepable))
+ return 3;
+
+ return bpf_timer_start(timer, 1, BPF_F_TIMER_SLEEPABLE);
+}
+
+SEC("tc")
+__log_level(2)
+__failure
+/* check that the first argument of bpf_timer_set_callback()
+ * is a correct bpf_timer pointer.
+ */
+__msg("mark_precise: frame0: regs=r1 stack= before")
+__msg(": (85) call bpf_timer_set_sleepable_cb_impl#") /* anchor message */
+__msg("arg#0 doesn't point to a map value")
+long test_wrong_pointer(void *ctx)
+{
+ int key = 0;
+ struct bpf_timer *timer;
+
+ timer = bpf_map_lookup_elem(&timer_map, &key);
+ if (!timer)
+ return 1;
+
+ if (bpf_timer_init(timer, &timer_map, CLOCK_MONOTONIC))
+ return 2;
+
+ if (bpf_timer_set_sleepable_cb((void *)&timer, timer_cb_sleepable))
+ return 3;
+
+ return -22;
+}

--
2.44.0



2024-03-19 00:15:13

by Eduard Zingerman

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 6/6] selftests/bpf: add sleepable timer tests

On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
> bpf_experimental.h and ../bpf_testmod/bpf_testmod_kfunc.h are both
> including vmlinux.h, which is not compatible with including time.h
> or bpf_tcp_helpers.h.
>
> So prevent vmlinux.h to be included, and override the few missing
> types.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

[...]

> @@ -6,6 +6,14 @@
> #include <bpf/bpf_helpers.h>
> #include "bpf_tcp_helpers.h"
>
> +#define __VMLINUX_H__
> +#define u32 __u32
> +#define u64 __u64
> +#include "bpf_experimental.h"
> +struct prog_test_member1;
> +#include "../bpf_testmod/bpf_testmod_kfunc.h"
> +#undef __VMLINUX_H__

Tbh, this looks very ugly.
Would it be possible to create a new tests file sleepable_timer.c
and include bpf_experimental.h there, skipping time.h?
It appears that for the new tests the only necessary definition from
time.h is CLOCK_MONOTONIC.


2024-03-21 15:47:18

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 6/6] selftests/bpf: add sleepable timer tests

On Tue, Mar 19, 2024 at 1:20 AM Eduard Zingerman <[email protected]> wrote:
>
> On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
> > bpf_experimental.h and ../bpf_testmod/bpf_testmod_kfunc.h are both
> > including vmlinux.h, which is not compatible with including time.h
> > or bpf_tcp_helpers.h.
> >
> > So prevent vmlinux.h to be included, and override the few missing
> > types.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
>
> [...]
>
> > @@ -6,6 +6,14 @@
> > #include <bpf/bpf_helpers.h>
> > #include "bpf_tcp_helpers.h"
> >
> > +#define __VMLINUX_H__
> > +#define u32 __u32
> > +#define u64 __u64
> > +#include "bpf_experimental.h"
> > +struct prog_test_member1;
> > +#include "../bpf_testmod/bpf_testmod_kfunc.h"
> > +#undef __VMLINUX_H__
>
> Tbh, this looks very ugly.

heh :)

> Would it be possible to create a new tests file sleepable_timer.c
> and include bpf_experimental.h there, skipping time.h?

Sounds like an option, yeah :)

> It appears that for the new tests the only necessary definition from
> time.h is CLOCK_MONOTONIC.
>

Yeah, that #define should be the only one missing.

I'll try to come up with better tests in v5 :)

Cheers,
Benjamin