Re: [PATCH bpf-next 2/2] selftests/bpf: Add more tests for loading insn arrays with offsets

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 26/04/01 11:38PM, Mykyta Yatsenko wrote:
> 
> 
> On 4/1/26 5:15 PM, Anton Protopopov wrote:
> > A typical series of load instructions for a gotox looks like
> > 
> >     r1 = &map + offset1
> >     r1 += offset2
> >     r1 = *(r1 + offset3)
> >     gotox r1
> > 
> > Here offset3 is, normally, equal to zero; but this is not guaranteed.
> > Extend selftests with tests for non-zero offset3 and, while here, also
> > add tests for negative offsets (the offset1 and the sum of three offsets
> > still have to be non-negative).
> > 
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
> >   .../selftests/bpf/prog_tests/bpf_gotox.c      | 114 +++++++++++-------
> >   1 file changed, 73 insertions(+), 41 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> > index 75b0cf2467ab..594adf698fdb 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> > @@ -317,7 +317,7 @@ static void check_ldimm64_off_load(struct bpf_gotox *skel __always_unused)
> >   static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns,
> >   					   __u32 insn_cnt,
> > -					   __u32 off1, __u32 off2)
> > +					   int off1, int off2, int off3)
> >   {
> >   	const __u32 values[] = {5, 7, 9, 11, 13, 15};
> >   	const __u32 max_entries = ARRAY_SIZE(values);
> > @@ -349,16 +349,46 @@ static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns,
> >   	/* r1 += off2 */
> >   	insns[2].imm = off2;
> > +	/* r1 = *(r1 + off3) */
> > +	insns[3].off = off3;
> > +
> >   	ret = prog_load(insns, insn_cnt);
> >   	close(map_fd);
> >   	return ret;
> >   }
> > -static void reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2)
> > +static void
> > +allow_offsets(struct bpf_insn *insns, __u32 insn_cnt, int off1, int off2, int off3)
> > +{
> > +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> > +	int prog_fd, err;
> > +	char s[128] = "";
> > +
> > +	prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2, off3);
> > +	snprintf(s, sizeof(s), "__check_ldimm64_gotox_prog_load(%u,%u,%u)", off1, off2, off3);
> 
> here offsets are int, but printed with %u.

Yes, thanks, fixed.

> > +	if (!ASSERT_GE(prog_fd, 0, s))
> > +		return;
> > +
> > +	err = bpf_prog_test_run_opts(prog_fd, &topts);
> > +	if (!ASSERT_OK(err, "test_run_opts err")) {
> > +		close(prog_fd);
> > +		return;
> > +	}
> > +
> > +	if (!ASSERT_EQ(topts.retval, (off1 + off2 + off3) / 8, "test_run_opts retval")) {
> > +		close(prog_fd);
> > +		return;
> > +	}
> > +
> > +	close(prog_fd);
> > +}
> > +
> > +static void
> > +reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2, __u32 off3)
> 
> If offsets can be negative, should these arguments be int rather than __u32,
> also matching reject_offsets?

Fixed.

> [...]