On Tue, 2023-03-28 at 15:24 -0700, Andrii Nakryiko wrote:
[...]
>
> > # Simplistic tests (14 files)
> >
> > Some tests are just simplistic and it is not clear if moving those to inline
> > assembly really makes sense, for example, here is `basic_call.c`:
> >
> > {
> > "invalid call insn1",
> > .insns = {
> > BPF_RAW_INSN(BPF_JMP | BPF_CALL | BPF_X, 0, 0, 0, 0),
> > BPF_EXIT_INSN(),
> > },
> > .errstr = "unknown opcode 8d",
> > .result = REJECT,
> > },
> >
>
> For tests like this we can have a simple ELF parser/loader that
> doesn't use bpf_object__open() functionality. It's not too hard to
> just find all the FUNC ELF symbols and fetch corresponding raw
> instructions. Assumption here is that we can take those assembly
> instructions as is, of course. If there are some map references and
> such, this won't work.
Custom elf parser/loader is interesting.
However, also consider how such tests look in assembly:
SEC("socket")
__description("invalid call insn1")
__failure __msg("unknown opcode 8d")
__failure_unpriv
__naked void invalid_call_insn1(void)
{
asm volatile (" \
.8byte %[raw_insn]; \
exit; \
" :
: __imm_insn(raw_insn, BPF_RAW_INSN(BPF_JMP | BPF_CALL | BPF_X, 0, 0, 0, 0))
: __clobber_all);
}
I'd say that original is better.
Do you want to get rid of ./test_verifier binary?
If so, we can move such tests under ./test_progs w/o converting to
inline assembly.
[...]
>
> > # Pseudo-call instructions (9 files)
> >
> > An object file might contain several BPF programs plus some functions used from
> > different programs. In order to load a program from such file, `libbpf` creates
> > a buffer and copies the program and all functions called from this program into
> > that buffer. For each visited pseudo-call instruction `libbpf` requires it to
> > point to a valid function described in ELF header.
> >
> > However, this is not how `verifier/*.c` tests are written, for example here is a
> > translated fragment from `verifier/loops1.c`:
> >
> > SEC("tracepoint")
> > __description("bounded recursion")
> > __failure __msg("back-edge")
> > __naked void bounded_recursion(void)
> > {
> > asm volatile (" \
> > r1 = 0; \
> > call l0_%=; \
> > exit; \
> > l0_%=: r1 += 1; \
> > r0 = r1; \
> > if r1 < 4 goto l1_%=; \
> > exit; \
> > l1_%=: call l0_%=; \
> > exit; \
> > " ::: __clobber_all);
> > }
> >
> > There are several possibilities here:
> > - split such tests into several functions during migration;
> > - add a special flag for `libbpf` asking to allow such calls;
> > - Andrii also suggested to try using `.section` directives inside inline
> > assembly block.
> >
> > This requires further investigation, I'll discuss it with Andrii some time later
> > this week or on Monday.
>
> So I did try this a few weeks ago, and yes, you can make this work
> with assembly directives. Except that DWARF (and thus .BTF and
> .BTF.ext) information won't be emitted, as it is emitted very
> painfully and manually by C compiler as explicit assembly directives.
> But we might work around that by clearing .BTF and .BTF.ext
> information for such object files, perhaps. So tentatively this should
> be doable.
Could you please share an example?
[...]
> > # `.fill_helper` (5 files)
> >
> > Programs for some tests are generated programmatically by specifying
> > `.fill_helper` function in the test description, e.g. `verifier/scale.c`:
> >
> > {
> > "scale: scale test 1",
> > .insns = { },
> > .data = { },
> > .fill_helper = bpf_fill_scale,
> > .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> > .result = ACCEPT,
> > .retval = 1,
> > },
> >
> > Such tests cannot be migrated
> > (but sometimes these are not the only tests in the file).
>
> We can just write these as explicitly programmatically generated
> programs, probably. There are just a few of these, shouldn't be a big
> deal.
You mean move the generating function from test_verifier.c to some
test under prog_tests/whatever.c, right?
> > # libbpf does not like some junk code (3 files)
> >
> > `libbpf` (and bpftool) reject some junk instructions intentionally encoded in
> > the tests, e.g. empty program from `verifier/basic.c`:
> >
> > SEC("socket")
> > __description("empty prog")
> > __failure __msg("last insn is not an exit or jmp")
> > __failure_unpriv
> > __naked void empty_prog(void)
> > {
>
> even if you add some random "r0 = 0" instruction? It won't change the
> meaning of the test, but should work with libbpf.
Random instruction should work.
>
> > asm volatile ("" ::: __clobber_all);
> > }
> >
> > # Small log buffer (2 files)
> >
> > Currently `test_loader.c` uses 1Mb log buffer, while `test_verifier.c` uses 16Mb
> > log buffer. There are a few tests (like in `verifier/bounds.c`) that exit with
> > `-ESPC` for 1Mb buffer.
> >
> > I can either bump log buffer size for `test_loader.c` or wait until Andrii's
> > rotating log implementation lands.
>
> Just bump to 16MB, no need to wait on anything.
Will do.
[...]
