From: Dave Marchevsky <davemarchevsky@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@kernel.org>,
Kernel Team <kernel-team@fb.com>,
Dave Marchevsky <davemarchevsky@fb.com>
Subject: [PATCH v2 bpf-next 6/9] selftests/bpf: Modify linked_list tests to work with macro-ified inserts
Date: Sat, 15 Apr 2023 13:18:08 -0700 [thread overview]
Message-ID: <20230415201811.343116-7-davemarchevsky@fb.com> (raw)
In-Reply-To: <20230415201811.343116-1-davemarchevsky@fb.com>
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 <davemarchevsky@fb.com>
---
.../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(-)
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;
--
2.34.1
next prev parent reply other threads:[~2023-04-15 20:18 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-15 20:18 [PATCH v2 bpf-next 0/9] Shared ownership for local kptrs Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 1/9] bpf: Remove btf_field_offs, use btf_record's fields instead Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 2/9] bpf: Introduce opaque bpf_refcount struct and add btf_record plumbing Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 3/9] bpf: Support refcounted local kptrs in existing semantics Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 4/9] bpf: Add bpf_refcount_acquire kfunc Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 5/9] bpf: Migrate bpf_rbtree_add and bpf_list_push_{front,back} to possibly fail Dave Marchevsky
2023-04-16 1:11 ` Alexei Starovoitov
2023-04-17 18:08 ` Dave Marchevsky
2023-04-15 20:18 ` Dave Marchevsky [this message]
2023-04-15 20:18 ` [PATCH v2 bpf-next 7/9] bpf: Migrate bpf_rbtree_remove " Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 8/9] bpf: Centralize btf_field-specific initialization logic Dave Marchevsky
2023-04-15 20:18 ` [PATCH v2 bpf-next 9/9] selftests/bpf: Add refcounted_kptr tests Dave Marchevsky
2023-04-21 22:17 ` Kumar Kartikeya Dwivedi
2023-04-21 23:49 ` Alexei Starovoitov
2023-04-22 2:06 ` Kumar Kartikeya Dwivedi
2023-04-22 2:18 ` Alexei Starovoitov
2023-04-25 6:53 ` Dave Marchevsky
2023-04-16 0:50 ` [PATCH v2 bpf-next 0/9] Shared ownership for local kptrs patchwork-bot+netdevbpf
2023-04-22 2:03 ` Kumar Kartikeya Dwivedi
2023-04-22 3:21 ` Alexei Starovoitov
2023-04-22 18:42 ` Kumar Kartikeya Dwivedi
2023-04-22 21:25 ` Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230415201811.343116-7-davemarchevsky@fb.com \
--to=davemarchevsky@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox