[bpf-next,v4,6/6] selftests/bpf: add sleepable timer tests

Commit Message

Benjamin Tissoires March 15, 2024, 2:29 p.m. UTC

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 <[e-mail]>

---

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

Comments

Eduard Zingerman March 19, 2024, 12:14 a.m. UTC | #1

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 <[e-mail]>

[...]

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

Benjamin Tissoires March 21, 2024, 3:45 p.m. UTC | #2

On Tue, Mar 19, 2024 at 1:20 AM Eduard Zingerman <[e-mail]> 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 <[e-mail]>
>
> [...]
>
> > @@ -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

Patch

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