[bpf-next,1/3] selftests: bpf: add trivial JSET test

We seem to have no JSET instruction test, and LLVM does not
generate it at all, so let's add a simple hand-coded test
to make sure JIT implementations are correct.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/testing/selftests/bpf/test_progs.c | 71 ++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

On 12/13/2018 04:41 AM, Jakub Kicinski wrote:
> We seem to have no JSET instruction test, and LLVM does not
> generate it at all, so let's add a simple hand-coded test
> to make sure JIT implementations are correct.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 71 ++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 126fc624290d..d9b7197fdf94 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1882,6 +1882,76 @@ static void test_queue_stack_map(int type)
>  	bpf_object__close(obj);
>  }
>  
> +static void test_jset(void)
> +{
> +	/* LLVM does not seem to support JSET-like instructions so by hand.. */
> +	static const struct bpf_insn insns[] = {
> +		/* r0 = 0 */
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		/* prep for direct packet access via r2 */
> +		BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
> +			    offsetof(struct __sk_buff, data)),
> +		BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
> +			    offsetof(struct __sk_buff, data_end)),
> +		BPF_MOV64_REG(BPF_REG_4, BPF_REG_2),
> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, 8),
> +		BPF_JMP_REG(BPF_JLE, BPF_REG_4, BPF_REG_3, 1),
> +		BPF_EXIT_INSN(),
> +
> +		BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_2, 0),
> +
> +		/* reg, bit 63 or bit 0 set, taken */
> +		BPF_LD_IMM64(BPF_REG_8, 0x8000000000000001),
> +		BPF_JMP_REG(BPF_JSET, BPF_REG_7, BPF_REG_8, 1),
> +		BPF_EXIT_INSN(),
> +
> +		/* reg, bit 62, not taken */
> +		BPF_LD_IMM64(BPF_REG_8, 0x4000000000000000),
> +		BPF_JMP_REG(BPF_JSET, BPF_REG_7, BPF_REG_8, 1),
> +		BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> +		BPF_EXIT_INSN(),
> +
> +		/* imm, any bit set, taken */
> +		BPF_JMP_IMM(BPF_JSET, BPF_REG_7, -1, 1),
> +		BPF_EXIT_INSN(),
> +
> +		/* imm, bit 31 set, taken */
> +		BPF_JMP_IMM(BPF_JSET, BPF_REG_7, 0x80000000, 1),
> +		BPF_EXIT_INSN(),
> +
> +		/* all good - return r0 == 2 */
> +		BPF_MOV64_IMM(BPF_REG_0, 2),
> +		BPF_EXIT_INSN(),
> +	};
> +	__u32 duration = 0, retval;
> +	__u64 data[8] = {};
> +	int err, prog_fd;
> +
> +	prog_fd = bpf_load_program(BPF_PROG_TYPE_SCHED_CLS, insns,
> +				   ARRAY_SIZE(insns), "GPL", 0, NULL, 0);
> +	if (CHECK(prog_fd < 0, "jset", "load fd %d errno %d\n", prog_fd, errno))
> +		return;
> +
> +#define TEST(val, name, res)						\
> +	do {								\
> +		data[0] = (val);					\
> +		err = bpf_prog_test_run(prog_fd, 1, data, sizeof(data),	\
> +					NULL, NULL, &retval, &duration); \
> +		CHECK(err || retval != (res), (name),			\
> +		      "err %d errno %d retval %d duration %d\n",	\
> +		      err, errno, retval, duration);			\
> +	} while (0)
> +
> +	TEST((1ULL << 63) | (1U << 31) | (1U << 0), "bit63+31+0", 2);
> +	TEST((1ULL << 63) | (1U << 31), "bit63+31", 2);
> +	TEST((1ULL << 31) | (1U << 0), "bit31+0", 2);
> +	TEST((__u32)-1, "u32", 2);
> +	TEST(~0x4000000000000000ULL, "~bit62", 2);
> +	TEST(0, "zero", 0);
> +	TEST(~0ULL, "all", 0);
> +#undef TEST

Could we rather extend the test_verifier infrastructure in order to be able to
define data input for bpf_prog_test_run()? I think this would be very useful
for future tests there as well and avoid having to duplicate or split functionality
into test_progs.c instead.

> +}
> +
>  int main(void)
>  {
>  	srand(time(NULL));
> @@ -1909,6 +1979,7 @@ int main(void)
>  	test_reference_tracking();
>  	test_queue_stack_map(QUEUE);
>  	test_queue_stack_map(STACK);
> +	test_jset();
>  
>  	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
>  	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;

Thanks,
Daniel

On Thu, 13 Dec 2018 16:17:37 +0100, Daniel Borkmann wrote:
> Could we rather extend the test_verifier infrastructure in order to
> be able to define data input for bpf_prog_test_run()? I think this
> would be very useful for future tests there as well and avoid having
> to duplicate or split functionality into test_progs.c instead.

No strong feelings but if it's called test_verifier, and the sample puts
no stress on the verifier it feels weird to put it there..  But okay, I
will respin.. at some point :)

On 12/13/2018 07:52 PM, Jakub Kicinski wrote:
> On Thu, 13 Dec 2018 16:17:37 +0100, Daniel Borkmann wrote:
>> Could we rather extend the test_verifier infrastructure in order to
>> be able to define data input for bpf_prog_test_run()? I think this
>> would be very useful for future tests there as well and avoid having
>> to duplicate or split functionality into test_progs.c instead.
> 
> No strong feelings but if it's called test_verifier, and the sample puts
> no stress on the verifier it feels weird to put it there..  But okay, I
> will respin.. at some point :)

Well, test_verifier is running every program that passes the verifier
via bpf_prog_test_run() anyway to increase test coverage also for JIT
and runtime rather than just plain verification itself. In that sense
it includes testing what verifier has been rewritten, for example. I
just noticed we already have this feature via 93731ef086ce ("bpf:
migrate ebpf ld_abs/ld_ind tests to test_verifier"). ;-) So just adding
the test code there should suffice.