Re: [PATCH bpf-next 00/43] First set of verifier/*.c migrated to inline assembly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eduard,

On 3/25/23 3:54 AM, Eduard Zingerman wrote:
This is a follow up for RFC [1]. It migrates a first batch of 38
verifier/*.c tests to inline assembly and use of ./test_progs for
actual execution. The migration is done by a python script (see [2]).

Each migrated verifier/xxx.c file is mapped to progs/verifier_xxx.c
plus an entry in the prog_tests/verifier.c. One patch per each file.

A few patches at the beginning of the patch-set extend test_loader
with necessary functionality, mainly:
- support for tests execution in unprivileged mode;
- support for test runs for test programs.

Migrated tests could be selected for execution using the following filter:

   ./test_progs -a verifier_*
An example of the migrated test:

   SEC("xdp")
   __description("XDP pkt read, pkt_data' > pkt_end, corner case, good access")
   __success __retval(0) __flag(BPF_F_ANY_ALIGNMENT)
   __naked void end_corner_case_good_access_1(void)
   {
           asm volatile ("                                 \
           r2 = *(u32*)(r1 + %[xdp_md_data]);              \
           r3 = *(u32*)(r1 + %[xdp_md_data_end]);          \
           r1 = r2;                                        \
           r1 += 8;                                        \
           if r1 > r3 goto l0_%=;                          \
           r0 = *(u64*)(r1 - 8);                           \
   l0_%=:  r0 = 0;                                         \
           exit;                                           \
   "       :
           : __imm_const(xdp_md_data, offsetof(struct xdp_md, data)),
             __imm_const(xdp_md_data_end, offsetof(struct xdp_md, data_end))
           : __clobber_all);
   }

Changes compared to RFC:
- test_loader.c is extended to support test program runs;
- capabilities handling now matches behavior of test_verifier;
- BPF_ST_MEM instructions are automatically replaced by BPF_STX_MEM
   instructions to overcome current clang limitations;
- tests styling updates according to RFC feedback;
- 38 migrated files are included instead of 1.

I used the following means for testing:
- migration tool itself has a set of self-tests;
- migrated tests are passing;
- manually compared each old/new file side-by-side.

While doing side-by-side comparison I've noted a few defects in the
original tests:
- and.c:
   - One of the jump targets is off by one;
   - BPF_ST_MEM wrong OFF/IMM ordering;
- array_access.c:
   - BPF_ST_MEM wrong OFF/IMM ordering;
- value_or_null.c:
   - BPF_ST_MEM wrong OFF/IMM ordering.

These defects would be addressed separately.

The cover letter describes what this series does, but not /why/ we need it. Would be
useful in future to also have the rationale in here. The migration of the verifier
tests look ok, and probably simplifies things to some degree, it certainly makes the
tests more readable. Is the goal to eventually get rid of test_verifier altogether?
I don't think we fully can do that, e.g. what about verifier testing of invalid insn
encodings or things like invalid jumps into the middle of double insns, invalid call
offsets, etc?

[1] RFC
     https://lore.kernel.org/bpf/20230123145148.2791939-1-eddyz87@xxxxxxxxx/
[2] Migration tool
     https://github.com/eddyz87/verifier-tests-migrator

Thanks,
Daniel



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux