On Thu, Feb 09, 2023 at 09:41:43AM -0800, Dave Marchevsky wrote:
> This patch adds selftests exercising the logic changed/added in the
> previous patches in the series. A variety of successful and unsuccessful
> rbtree usages are validated:
>
> Success:
> * Add some nodes, let map_value bpf_rbtree_root destructor clean them
> up
> * Add some nodes, remove one using the non-owning ref leftover by
> successful rbtree_add() call
> * Add some nodes, remove one using the non-owning ref returned by
> rbtree_first() call
>
> Failure:
> * BTF where bpf_rb_root owns bpf_list_node should fail to load
> * BTF where node of type X is added to tree containing nodes of type Y
> should fail to load
> * No calling rbtree api functions in 'less' callback for rbtree_add
> * No releasing lock in 'less' callback for rbtree_add
> * No removing a node which hasn't been added to any tree
> * No adding a node which has already been added to a tree
> * No escaping of non-owning references past their lock's
> critical section
> * No escaping of non-owning references past other invalidation points
> (rbtree_remove)
>
> These tests mostly focus on rbtree-specific additions, but some of the
> failure cases revalidate scenarios common to both linked_list and rbtree
> which are covered in the former's tests. Better to be a bit redundant in
> case linked_list and rbtree semantics deviate over time.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> ---
> .../testing/selftests/bpf/prog_tests/rbtree.c | 184 +++++++++++
> tools/testing/selftests/bpf/progs/rbtree.c | 176 +++++++++++
> .../progs/rbtree_btf_fail__add_wrong_type.c | 52 +++
> .../progs/rbtree_btf_fail__wrong_node_type.c | 49 +++
> .../testing/selftests/bpf/progs/rbtree_fail.c | 296 ++++++++++++++++++
> 5 files changed, 757 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/rbtree.c
> create mode 100644 tools/testing/selftests/bpf/progs/rbtree.c
> create mode 100644 tools/testing/selftests/bpf/progs/rbtree_btf_fail__add_wrong_type.c
> create mode 100644 tools/testing/selftests/bpf/progs/rbtree_btf_fail__wrong_node_type.c
> create mode 100644 tools/testing/selftests/bpf/progs/rbtree_fail.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/rbtree.c b/tools/testing/selftests/bpf/prog_tests/rbtree.c
> new file mode 100644
> index 000000000000..733db8d79a2d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rbtree.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
> +
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +
> +#include "rbtree.skel.h"
> +#include "rbtree_fail.skel.h"
> +#include "rbtree_btf_fail__wrong_node_type.skel.h"
> +#include "rbtree_btf_fail__add_wrong_type.skel.h"
> +
> +static char log_buf[1024 * 1024];
> +
> +static struct {
> + const char *prog_name;
> + const char *err_msg;
> +} rbtree_fail_tests[] = {
> + {"rbtree_api_nolock_add", "bpf_spin_lock at off=16 must be held for bpf_rb_root"},
> + {"rbtree_api_nolock_remove", "bpf_spin_lock at off=16 must be held for bpf_rb_root"},
> + {"rbtree_api_nolock_first", "bpf_spin_lock at off=16 must be held for bpf_rb_root"},
> +
> + /* Specific failure string for these three isn't very important, but it shouldn't be
> + * possible to call rbtree api func from within add() callback
> + */
> + {"rbtree_api_add_bad_cb_bad_fn_call_add", "allocated object must be referenced"},
> + {"rbtree_api_add_bad_cb_bad_fn_call_remove", "rbtree_remove not allowed in rbtree cb"},
> + {"rbtree_api_add_bad_cb_bad_fn_call_first_unlock_after",
> + "can't spin_{lock,unlock} in rbtree cb"},
> +
> + {"rbtree_api_remove_unadded_node", "rbtree_remove node input must be non-owning ref"},
> + {"rbtree_api_add_to_multiple_trees", "allocated object must be referenced"},
> + {"rbtree_api_add_release_unlock_escape", "arg#1 expected pointer to allocated object"},
> + {"rbtree_api_first_release_unlock_escape", "arg#1 expected pointer to allocated object"},
> + {"rbtree_api_remove_no_drop", "Unreleased reference id=2 alloc_insn=11"},
> + {"rbtree_api_release_aliasing", "arg#1 expected pointer to allocated object"},
Please convert the fail tests to RUN_TESTS() and __failure __msg() framework.
The success tests can stay as-is, since the runner checks the return values
and we don't have this feature yet in RUN_TESTS().
We probably should add such feature, but that's orthogonal and not a blocker for this set.
