[v1,bpf-next,6/9] selftests/bpf: Modify linked_list tests to work with macro-ified inserts

Commit Message

Dave Marchevsky April 10, 2023, 7:07 p.m. UTC

The linked_list tests use macros and function pointers to reduce code
duplication. Earlier in the series, bpf_list_push_{front,back} were
modified to be macros, expanding to invoke actual kfuncs
bpf_list_push_{front,back}_impl. Due to this change, a code snippet
like:

  void (*p)(void *, void *) = (void *)&bpf_list_##op;
  p(hexpr, nexpr);

meant to do bpf_list_push_{front,back}(hexpr, nexpr), will no longer
work as it's no longer valid to do &bpf_list_push_{front,back} since
they're no longer functions.

This patch fixes issues of this type, along with two other minor changes
- one improvement and one fix - both related to the node argument to
list_push_{front,back}.

  * The fix: migration of list_push tests away from (void *, void *)
    func ptr uncovered that some tests were incorrectly passing pointer
    to node, not pointer to struct bpf_list_node within the node. This
    patch fixes such issues (CHECK(..., f) -> CHECK(..., &f->node))

  * The improvement: In linked_list tests, the struct foo type has two
    list_node fields: node and node2, at byte offsets 0 and 40 within
    the struct, respectively. Currently node is used in ~all tests
    involving struct foo and lists. The verifier needs to do some work
    to account for the offset of bpf_list_node within the node type, so
    using node2 instead of node exercises that logic more in the tests.
    This patch migrates linked_list tests to use node2 instead of node.

Signed-off-by: Dave Marchevsky <[e-mail]>
---
 .../selftests/bpf/prog_tests/linked_list.c    |  6 +-
 .../testing/selftests/bpf/progs/linked_list.c | 34 +++----
 .../testing/selftests/bpf/progs/linked_list.h |  4 +-
 .../selftests/bpf/progs/linked_list_fail.c    | 96 ++++++++++---------
 4 files changed, 73 insertions(+), 67 deletions(-)

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
index 0ed8132ce1c3..872e4bd500fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
+++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
@@ -84,11 +84,11 @@  static struct {
 	{ "double_push_back", "arg#1 expected pointer to allocated object" },
 	{ "no_node_value_type", "bpf_list_node not found at offset=0" },
 	{ "incorrect_value_type",
-	  "operation on bpf_list_head expects arg#1 bpf_list_node at offset=0 in struct foo, "
+	  "operation on bpf_list_head expects arg#1 bpf_list_node at offset=40 in struct foo, "
 	  "but arg is at offset=0 in struct bar" },
 	{ "incorrect_node_var_off", "variable ptr_ access var_off=(0x0; 0xffffffff) disallowed" },
-	{ "incorrect_node_off1", "bpf_list_node not found at offset=1" },
-	{ "incorrect_node_off2", "arg#1 offset=40, but expected bpf_list_node at offset=0 in struct foo" },
+	{ "incorrect_node_off1", "bpf_list_node not found at offset=41" },
+	{ "incorrect_node_off2", "arg#1 offset=0, but expected bpf_list_node at offset=40 in struct foo" },
 	{ "no_head_type", "bpf_list_head not found at offset=0" },
 	{ "incorrect_head_var_off1", "R1 doesn't have constant offset" },
 	{ "incorrect_head_var_off2", "variable ptr_ access var_off=(0x0; 0xffffffff) disallowed" },
diff --git a/tools/testing/selftests/bpf/progs/linked_list.c b/tools/testing/selftests/bpf/progs/linked_list.c
index 53ded51a3abb..57440a554304 100644
--- a/tools/testing/selftests/bpf/progs/linked_list.c
+++ b/tools/testing/selftests/bpf/progs/linked_list.c
@@ -25,7 +25,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 	n = bpf_list_pop_front(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		bpf_obj_drop(f);
 		return 3;
 	}
@@ -34,7 +34,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 	n = bpf_list_pop_back(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		bpf_obj_drop(f);
 		return 4;
 	}
@@ -42,7 +42,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 
 	bpf_spin_lock(lock);
 	f->data = 42;
-	bpf_list_push_front(head, &f->node);
+	bpf_list_push_front(head, &f->node2);
 	bpf_spin_unlock(lock);
 	if (leave_in_map)
 		return 0;
@@ -51,7 +51,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 	bpf_spin_unlock(lock);
 	if (!n)
 		return 5;
-	f = container_of(n, struct foo, node);
+	f = container_of(n, struct foo, node2);
 	if (f->data != 42) {
 		bpf_obj_drop(f);
 		return 6;
@@ -59,14 +59,14 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 
 	bpf_spin_lock(lock);
 	f->data = 13;
-	bpf_list_push_front(head, &f->node);
+	bpf_list_push_front(head, &f->node2);
 	bpf_spin_unlock(lock);
 	bpf_spin_lock(lock);
 	n = bpf_list_pop_front(head);
 	bpf_spin_unlock(lock);
 	if (!n)
 		return 7;
-	f = container_of(n, struct foo, node);
+	f = container_of(n, struct foo, node2);
 	if (f->data != 13) {
 		bpf_obj_drop(f);
 		return 8;
@@ -77,7 +77,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 	n = bpf_list_pop_front(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		return 9;
 	}
 
@@ -85,7 +85,7 @@  int list_push_pop(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool l
 	n = bpf_list_pop_back(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		return 10;
 	}
 	return 0;
@@ -119,8 +119,8 @@  int list_push_pop_multiple(struct bpf_spin_lock *lock, struct bpf_list_head *hea
 		f[i + 1]->data = i + 1;
 
 		bpf_spin_lock(lock);
-		bpf_list_push_front(head, &f[i]->node);
-		bpf_list_push_front(head, &f[i + 1]->node);
+		bpf_list_push_front(head, &f[i]->node2);
+		bpf_list_push_front(head, &f[i + 1]->node2);
 		bpf_spin_unlock(lock);
 	}
 
@@ -130,13 +130,13 @@  int list_push_pop_multiple(struct bpf_spin_lock *lock, struct bpf_list_head *hea
 		bpf_spin_unlock(lock);
 		if (!n)
 			return 3;
-		pf = container_of(n, struct foo, node);
+		pf = container_of(n, struct foo, node2);
 		if (pf->data != (ARRAY_SIZE(f) - i - 1)) {
 			bpf_obj_drop(pf);
 			return 4;
 		}
 		bpf_spin_lock(lock);
-		bpf_list_push_back(head, &pf->node);
+		bpf_list_push_back(head, &pf->node2);
 		bpf_spin_unlock(lock);
 	}
 
@@ -149,7 +149,7 @@  int list_push_pop_multiple(struct bpf_spin_lock *lock, struct bpf_list_head *hea
 		bpf_spin_unlock(lock);
 		if (!n)
 			return 5;
-		pf = container_of(n, struct foo, node);
+		pf = container_of(n, struct foo, node2);
 		if (pf->data != i) {
 			bpf_obj_drop(pf);
 			return 6;
@@ -160,7 +160,7 @@  int list_push_pop_multiple(struct bpf_spin_lock *lock, struct bpf_list_head *hea
 	n = bpf_list_pop_back(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		return 7;
 	}
 
@@ -168,7 +168,7 @@  int list_push_pop_multiple(struct bpf_spin_lock *lock, struct bpf_list_head *hea
 	n = bpf_list_pop_front(head);
 	bpf_spin_unlock(lock);
 	if (n) {
-		bpf_obj_drop(container_of(n, struct foo, node));
+		bpf_obj_drop(container_of(n, struct foo, node2));
 		return 8;
 	}
 	return 0;
@@ -199,7 +199,7 @@  int list_in_list(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool le
 
 	bpf_spin_lock(lock);
 	f->data = 42;
-	bpf_list_push_front(head, &f->node);
+	bpf_list_push_front(head, &f->node2);
 	bpf_spin_unlock(lock);
 
 	if (leave_in_map)
@@ -210,7 +210,7 @@  int list_in_list(struct bpf_spin_lock *lock, struct bpf_list_head *head, bool le
 	bpf_spin_unlock(lock);
 	if (!n)
 		return 4;
-	f = container_of(n, struct foo, node);
+	f = container_of(n, struct foo, node2);
 	if (f->data != 42) {
 		bpf_obj_drop(f);
 		return 5;
diff --git a/tools/testing/selftests/bpf/progs/linked_list.h b/tools/testing/selftests/bpf/progs/linked_list.h
index 3fb2412552fc..c0f3609a7ffa 100644
--- a/tools/testing/selftests/bpf/progs/linked_list.h
+++ b/tools/testing/selftests/bpf/progs/linked_list.h
@@ -22,7 +22,7 @@  struct foo {
 struct map_value {
 	struct bpf_spin_lock lock;
 	int data;
-	struct bpf_list_head head __contains(foo, node);
+	struct bpf_list_head head __contains(foo, node2);
 };
 
 struct array_map {
@@ -50,7 +50,7 @@  struct {
 #define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
 
 private(A) struct bpf_spin_lock glock;
-private(A) struct bpf_list_head ghead __contains(foo, node);
+private(A) struct bpf_list_head ghead __contains(foo, node2);
 private(B) struct bpf_spin_lock glock2;
 
 #endif
diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c
index 41978b46f58e..f4c63daba229 100644
--- a/tools/testing/selftests/bpf/progs/linked_list_fail.c
+++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c
@@ -73,22 +73,21 @@  CHECK(inner_map, pop_back, &iv->head);
 	int test##_missing_lock_##op(void *ctx)				\
 	{								\
 		INIT;							\
-		void (*p)(void *, void *) = (void *)&bpf_list_##op;	\
-		p(hexpr, nexpr);					\
+		bpf_list_##op(hexpr, nexpr);				\
 		return 0;						\
 	}
 
-CHECK(kptr, push_front, &f->head, b);
-CHECK(kptr, push_back, &f->head, b);
+CHECK(kptr, push_front, &f->head, &b->node);
+CHECK(kptr, push_back, &f->head, &b->node);
 
-CHECK(global, push_front, &ghead, f);
-CHECK(global, push_back, &ghead, f);
+CHECK(global, push_front, &ghead, &f->node2);
+CHECK(global, push_back, &ghead, &f->node2);
 
-CHECK(map, push_front, &v->head, f);
-CHECK(map, push_back, &v->head, f);
+CHECK(map, push_front, &v->head, &f->node2);
+CHECK(map, push_back, &v->head, &f->node2);
 
-CHECK(inner_map, push_front, &iv->head, f);
-CHECK(inner_map, push_back, &iv->head, f);
+CHECK(inner_map, push_front, &iv->head, &f->node2);
+CHECK(inner_map, push_back, &iv->head, &f->node2);
 
 #undef CHECK
 
@@ -135,32 +134,31 @@  CHECK_OP(pop_back);
 	int test##_incorrect_lock_##op(void *ctx)			\
 	{								\
 		INIT;							\
-		void (*p)(void *, void*) = (void *)&bpf_list_##op;	\
 		bpf_spin_lock(lexpr);					\
-		p(hexpr, nexpr);					\
+		bpf_list_##op(hexpr, nexpr);				\
 		return 0;						\
 	}
 
 #define CHECK_OP(op)							\
-	CHECK(kptr_kptr, op, &f1->lock, &f2->head, b);			\
-	CHECK(kptr_global, op, &f1->lock, &ghead, f);			\
-	CHECK(kptr_map, op, &f1->lock, &v->head, f);			\
-	CHECK(kptr_inner_map, op, &f1->lock, &iv->head, f);		\
+	CHECK(kptr_kptr, op, &f1->lock, &f2->head, &b->node);		\
+	CHECK(kptr_global, op, &f1->lock, &ghead, &f->node2);		\
+	CHECK(kptr_map, op, &f1->lock, &v->head, &f->node2);		\
+	CHECK(kptr_inner_map, op, &f1->lock, &iv->head, &f->node2);	\
 									\
-	CHECK(global_global, op, &glock2, &ghead, f);			\
-	CHECK(global_kptr, op, &glock, &f1->head, b);			\
-	CHECK(global_map, op, &glock, &v->head, f);			\
-	CHECK(global_inner_map, op, &glock, &iv->head, f);		\
+	CHECK(global_global, op, &glock2, &ghead, &f->node2);		\
+	CHECK(global_kptr, op, &glock, &f1->head, &b->node);		\
+	CHECK(global_map, op, &glock, &v->head, &f->node2);		\
+	CHECK(global_inner_map, op, &glock, &iv->head, &f->node2);	\
 									\
-	CHECK(map_map, op, &v->lock, &v2->head, f);			\
-	CHECK(map_kptr, op, &v->lock, &f2->head, b);			\
-	CHECK(map_global, op, &v->lock, &ghead, f);			\
-	CHECK(map_inner_map, op, &v->lock, &iv->head, f);		\
+	CHECK(map_map, op, &v->lock, &v2->head, &f->node2);		\
+	CHECK(map_kptr, op, &v->lock, &f2->head, &b->node);		\
+	CHECK(map_global, op, &v->lock, &ghead, &f->node2);		\
+	CHECK(map_inner_map, op, &v->lock, &iv->head, &f->node2);	\
 									\
-	CHECK(inner_map_inner_map, op, &iv->lock, &iv2->head, f);	\
-	CHECK(inner_map_kptr, op, &iv->lock, &f2->head, b);		\
-	CHECK(inner_map_global, op, &iv->lock, &ghead, f);		\
-	CHECK(inner_map_map, op, &iv->lock, &v->head, f);
+	CHECK(inner_map_inner_map, op, &iv->lock, &iv2->head, &f->node2);\
+	CHECK(inner_map_kptr, op, &iv->lock, &f2->head, &b->node);	\
+	CHECK(inner_map_global, op, &iv->lock, &ghead, &f->node2);	\
+	CHECK(inner_map_map, op, &iv->lock, &v->head, &f->node2);
 
 CHECK_OP(push_front);
 CHECK_OP(push_back);
@@ -340,7 +338,7 @@  int direct_read_node(void *ctx)
 	f = bpf_obj_new(typeof(*f));
 	if (!f)
 		return 0;
-	return *(int *)&f->node;
+	return *(int *)&f->node2;
 }
 
 SEC("?tc")
@@ -351,12 +349,12 @@  int direct_write_node(void *ctx)
 	f = bpf_obj_new(typeof(*f));
 	if (!f)
 		return 0;
-	*(int *)&f->node = 0;
+	*(int *)&f->node2 = 0;
 	return 0;
 }
 
 static __always_inline
-int use_after_unlock(void (*op)(void *head, void *node))
+int use_after_unlock(bool push_front)
 {
 	struct foo *f;
 
@@ -365,7 +363,10 @@  int use_after_unlock(void (*op)(void *head, void *node))
 		return 0;
 	bpf_spin_lock(&glock);
 	f->data = 42;
-	op(&ghead, &f->node);
+	if (push_front)
+		bpf_list_push_front(&ghead, &f->node2);
+	else
+		bpf_list_push_back(&ghead, &f->node2);
 	bpf_spin_unlock(&glock);
 
 	return f->data;
@@ -374,17 +375,17 @@  int use_after_unlock(void (*op)(void *head, void *node))
 SEC("?tc")
 int use_after_unlock_push_front(void *ctx)
 {
-	return use_after_unlock((void *)bpf_list_push_front);
+	return use_after_unlock(true);
 }
 
 SEC("?tc")
 int use_after_unlock_push_back(void *ctx)
 {
-	return use_after_unlock((void *)bpf_list_push_back);
+	return use_after_unlock(false);
 }
 
 static __always_inline
-int list_double_add(void (*op)(void *head, void *node))
+int list_double_add(bool push_front)
 {
 	struct foo *f;
 
@@ -392,8 +393,13 @@  int list_double_add(void (*op)(void *head, void *node))
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	op(&ghead, &f->node);
-	op(&ghead, &f->node);
+	if (push_front) {
+		bpf_list_push_front(&ghead, &f->node2);
+		bpf_list_push_front(&ghead, &f->node2);
+	} else {
+		bpf_list_push_back(&ghead, &f->node2);
+		bpf_list_push_back(&ghead, &f->node2);
+	}
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -402,13 +408,13 @@  int list_double_add(void (*op)(void *head, void *node))
 SEC("?tc")
 int double_push_front(void *ctx)
 {
-	return list_double_add((void *)bpf_list_push_front);
+	return list_double_add(true);
 }
 
 SEC("?tc")
 int double_push_back(void *ctx)
 {
-	return list_double_add((void *)bpf_list_push_back);
+	return list_double_add(false);
 }
 
 SEC("?tc")
@@ -450,7 +456,7 @@  int incorrect_node_var_off(struct __sk_buff *ctx)
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	bpf_list_push_front(&ghead, (void *)&f->node + ctx->protocol);
+	bpf_list_push_front(&ghead, (void *)&f->node2 + ctx->protocol);
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -465,7 +471,7 @@  int incorrect_node_off1(void *ctx)
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	bpf_list_push_front(&ghead, (void *)&f->node + 1);
+	bpf_list_push_front(&ghead, (void *)&f->node2 + 1);
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -480,7 +486,7 @@  int incorrect_node_off2(void *ctx)
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	bpf_list_push_front(&ghead, &f->node2);
+	bpf_list_push_front(&ghead, &f->node);
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -510,7 +516,7 @@  int incorrect_head_var_off1(struct __sk_buff *ctx)
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	bpf_list_push_front((void *)&ghead + ctx->protocol, &f->node);
+	bpf_list_push_front((void *)&ghead + ctx->protocol, &f->node2);
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -525,7 +531,7 @@  int incorrect_head_var_off2(struct __sk_buff *ctx)
 	if (!f)
 		return 0;
 	bpf_spin_lock(&glock);
-	bpf_list_push_front((void *)&f->head + ctx->protocol, &f->node);
+	bpf_list_push_front((void *)&f->head + ctx->protocol, &f->node2);
 	bpf_spin_unlock(&glock);
 
 	return 0;
@@ -563,7 +569,7 @@  int incorrect_head_off2(void *ctx)
 		return 0;
 
 	bpf_spin_lock(&glock);
-	bpf_list_push_front((void *)&ghead + 1, &f->node);
+	bpf_list_push_front((void *)&ghead + 1, &f->node2);
 	bpf_spin_unlock(&glock);
 
 	return 0;