On Mon, Apr 10, 2023 at 12:07:53PM -0700, Dave Marchevsky wrote:
> Test refcounted local kptr functionality added in previous patches in
> the series.
>
> Usecases which pass verification:
>
> * Add refcounted local kptr to both tree and list. Then, read and -
> possibly, depending on test variant - delete from tree, then list.
> * Also test doing read-and-maybe-delete in opposite order
> * Stash a refcounted local kptr in a map_value, then add it to a
> rbtree. Read from both, possibly deleting after tree read.
> * Add refcounted local kptr to both tree and list. Then, try reading and
> deleting twice from one of the collections.
> * bpf_refcount_acquire of just-added non-owning ref should work, as
> should bpf_refcount_acquire of owning ref just out of bpf_obj_new
>
> Usecases which fail verification:
>
> * The simple successful bpf_refcount_acquire cases from above should
> both fail to verify if the newly-acquired owning ref is not dropped
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> ---
> .../bpf/prog_tests/refcounted_kptr.c | 18 +
> .../selftests/bpf/progs/refcounted_kptr.c | 410 ++++++++++++++++++
> .../bpf/progs/refcounted_kptr_fail.c | 72 +++
> 3 files changed, 500 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c
> create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr.c
> create mode 100644 tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c
> new file mode 100644
> index 000000000000..2ab23832062d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +
> +#include "refcounted_kptr.skel.h"
> +#include "refcounted_kptr_fail.skel.h"
> +
> +void test_refcounted_kptr(void)
> +{
> + RUN_TESTS(refcounted_kptr);
> +}
> +
> +void test_refcounted_kptr_fail(void)
> +{
> + RUN_TESTS(refcounted_kptr_fail);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> new file mode 100644
> index 000000000000..b444e4cb07fb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_core_read.h>
> +#include "bpf_misc.h"
> +#include "bpf_experimental.h"
> +
> +struct node_data {
> + long key;
> + long list_data;
> + struct bpf_rb_node r;
> + struct bpf_list_node l;
> + struct bpf_refcount ref;
> +};
> +
> +struct map_value {
> + struct node_data __kptr *node;
> +};
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __type(key, int);
> + __type(value, struct map_value);
> + __uint(max_entries, 1);
> +} stashed_nodes SEC(".maps");
> +
> +struct node_acquire {
> + long key;
> + long data;
> + struct bpf_rb_node node;
> + struct bpf_refcount refcount;
> +};
> +
> +#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
> +private(A) struct bpf_spin_lock lock;
> +private(A) struct bpf_rb_root root __contains(node_data, r);
> +private(A) struct bpf_list_head head __contains(node_data, l);
> +
> +private(B) struct bpf_spin_lock alock;
> +private(B) struct bpf_rb_root aroot __contains(node_acquire, node);
> +
> +static bool less(struct bpf_rb_node *node_a, const struct bpf_rb_node *node_b)
> +{
> + struct node_data *a;
> + struct node_data *b;
> +
> + a = container_of(node_a, struct node_data, r);
> + b = container_of(node_b, struct node_data, r);
> +
> + return a->key < b->key;
> +}
> +
> +static bool less_a(struct bpf_rb_node *a, const struct bpf_rb_node *b)
> +{
> + struct node_acquire *node_a;
> + struct node_acquire *node_b;
> +
> + node_a = container_of(a, struct node_acquire, node);
> + node_b = container_of(b, struct node_acquire, node);
> +
> + return node_a->key < node_b->key;
> +}
> +
> +static __always_inline
> +long __insert_in_tree_and_list(struct bpf_list_head *head,
> + struct bpf_rb_root *root,
> + struct bpf_spin_lock *lock)
> +{
> + struct node_data *n, *m;
> +
> + n = bpf_obj_new(typeof(*n));
> + if (!n)
> + return -1;
> +
> + m = bpf_refcount_acquire(n);
> + m->key = 123;
> + m->list_data = 456;
> +
> + bpf_spin_lock(lock);
> + if (bpf_rbtree_add(root, &n->r, less)) {
> + /* Failure to insert - unexpected */
> + bpf_spin_unlock(lock);
> + bpf_obj_drop(m);
> + return -2;
> + }
> + bpf_spin_unlock(lock);
> +
> + bpf_spin_lock(lock);
> + if (bpf_list_push_front(head, &m->l)) {
> + /* Failure to insert - unexpected */
> + bpf_spin_unlock(lock);
> + return -3;
> + }
> + bpf_spin_unlock(lock);
> + return 0;
> +}
> +
> +static __always_inline
> +long __stash_map_insert_tree(int idx, int val, struct bpf_rb_root *root,
> + struct bpf_spin_lock *lock)
> +{
> + struct map_value *mapval;
> + struct node_data *n, *m;
> +
> + mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
> + if (!mapval)
> + return -1;
> +
> + n = bpf_obj_new(typeof(*n));
> + if (!n)
> + return -2;
> +
> + n->key = val;
> + m = bpf_refcount_acquire(n);
> +
> + n = bpf_kptr_xchg(&mapval->node, n);
> + if (n) {
> + bpf_obj_drop(n);
> + bpf_obj_drop(m);
> + return -3;
> + }
> +
> + bpf_spin_lock(lock);
> + if (bpf_rbtree_add(root, &m->r, less)) {
> + /* Failure to insert - unexpected */
> + bpf_spin_unlock(lock);
> + return -4;
> + }
> + bpf_spin_unlock(lock);
> + return 0;
> +}
> +
> +static __always_inline
> +long __read_from_tree(struct bpf_rb_root *root, struct bpf_spin_lock *lock,
> + bool remove_from_tree)
> +{
> + struct bpf_rb_node *rb;
> + struct node_data *n;
> + long res = -99;
> +
> + bpf_spin_lock(lock);
> +
> + rb = bpf_rbtree_first(root);
> + if (!rb) {
> + bpf_spin_unlock(lock);
> + return -1;
> + }
> +
> + n = container_of(rb, struct node_data, r);
> + res = n->key;
> +
> + if (!remove_from_tree) {
> + bpf_spin_unlock(lock);
> + return res;
> + }
> +
> + rb = bpf_rbtree_remove(root, rb);
> + bpf_spin_unlock(lock);
> + if (!rb) {
> + return -2;
> + }
> + n = container_of(rb, struct node_data, r);
> + bpf_obj_drop(n);
> + return res;
> +}
> +
> +static __always_inline
> +long __read_from_list(struct bpf_list_head *head, struct bpf_spin_lock *lock,
> + bool remove_from_list)
> +{
> + struct bpf_list_node *l;
> + struct node_data *n;
> + long res = -99;
> +
> + bpf_spin_lock(lock);
> +
> + l = bpf_list_pop_front(head);
> + if (!l) {
> + bpf_spin_unlock(lock);
> + return -1;
> + }
> +
> + n = container_of(l, struct node_data, l);
> + res = n->list_data;
> +
> + if (!remove_from_list) {
> + if (bpf_list_push_back(head, &n->l)) {
> + bpf_spin_unlock(lock);
> + return -2;
> + }
> + }
> +
> + bpf_spin_unlock(lock);
> +
> + if (remove_from_list)
> + bpf_obj_drop(n);
> + return res;
> +}
> +
> +static __always_inline
Why __always_inline in this 5 helpers?
Will it pass the verifier if __always_inline is replaced with noinline?
> +long __read_from_unstash(int idx)
