Hi Andrii,
On Fri, Dec 01, 2023 at 03:52:25PM -0800, Andrii Nakryiko wrote:
On Fri, Dec 1, 2023 at 12:24 PM Daniel Xu dxu@dxuuu.xyz wrote:
Add some tests that exercise BPF_CORE_WRITE_BITFIELD() macro. Since some non-trivial bit fiddling is going on, make sure various edge cases (such as adjacent bitfields and bitfields at the edge of structs) are exercised.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
.../selftests/bpf/prog_tests/verifier.c | 2 + .../bpf/progs/verifier_bitfield_write.c | 100 ++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/verifier_bitfield_write.c
LGTM, but I'm not sure why we need all those __failure_unpriv, see below. Regardless:
Acked-by: Andrii Nakryiko andrii@kernel.org
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index 5cfa7a6316b6..67b4948865a3 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -6,6 +6,7 @@ #include "verifier_and.skel.h" #include "verifier_array_access.skel.h" #include "verifier_basic_stack.skel.h" +#include "verifier_bitfield_write.skel.h" #include "verifier_bounds.skel.h" #include "verifier_bounds_deduction.skel.h" #include "verifier_bounds_deduction_non_const.skel.h" @@ -115,6 +116,7 @@ static void run_tests_aux(const char *skel_name,
void test_verifier_and(void) { RUN(verifier_and); } void test_verifier_basic_stack(void) { RUN(verifier_basic_stack); } +void test_verifier_bitfield_write(void) { RUN(verifier_bitfield_write); } void test_verifier_bounds(void) { RUN(verifier_bounds); } void test_verifier_bounds_deduction(void) { RUN(verifier_bounds_deduction); } void test_verifier_bounds_deduction_non_const(void) { RUN(verifier_bounds_deduction_non_const); } diff --git a/tools/testing/selftests/bpf/progs/verifier_bitfield_write.c b/tools/testing/selftests/bpf/progs/verifier_bitfield_write.c new file mode 100644 index 000000000000..8fe355a19ba6 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_bitfield_write.c @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h> +#include <stdint.h>
+#include <bpf/bpf_helpers.h> +#include <bpf/bpf_core_read.h>
+#include "bpf_misc.h"
+struct core_reloc_bitfields {
/* unsigned bitfields */uint8_t ub1: 1;uint8_t ub2: 2;uint32_t ub7: 7;/* signed bitfields */int8_t sb4: 4;int32_t sb20: 20;/* non-bitfields */uint32_t u32;int32_t s32;+} __attribute__((preserve_access_index));
+SEC("tc") +__description("single CO-RE bitfield roundtrip") +__btf_path("btf__core_reloc_bitfields.bpf.o") +__success __failure_unpriv
do we want __failure_unpriv at all? Is this failure related to *bitfield* logic at all?
Oh, I pre-emptively added it. From the docs, I thought __failure_unpriv meant "don't try to load this as an unprivileged used cuz it'll fail". And since I used the tc hook, I figured it'd fail.
Removing the annotation doesn't seem to do anything bad so I'll drop it for v4.
[...]
Thanks, Daniel