> On 11 Apr 2019, at 15:32, Jiong Wang <jiong.wang@xxxxxxxxxxxxx> wrote: > > >> On 11 Apr 2019, at 04:19, Jakub Kicinski <jakub.kicinski@xxxxxxxxxxxxx> wrote: >> >> On Wed, 10 Apr 2019 20:50:25 +0100, Jiong Wang wrote: >>> Enable BPF_F_TEST_RND_HI32 for all existing bpf selftests or other >>> independent tests could involve quite a few changes to make sure all bpf >>> prog load places has BPF_F_TEST_RND_HI32 set. >>> >>> Given most of the tests are using libbpf, this patch introduces a new >>> global variable "libbpf_test_mode" into libbpf, once which is set, all bpf >>> prog load issued through libbpf will have BPF_F_TEST_RND_HI32 set >>> automatically, this could minimize changes required from testsuite. >>> >>> The other way might be introducing new load function like >>> "bpf_prog_test_load", which will set BPF_F_TEST_RND_HI32. But there are >>> several prog load APIs, and we need minor changes on some parameters. >>> >>> The global variable approach seems to be a proper first step for easy >>> testsuite porting. >>> >>> Signed-off-by: Jiong Wang <jiong.wang@xxxxxxxxxxxxx> >> >> Can we perhaps make it per-object by setting it after >> bpf_object__open() but before bpf_object__load(), or add >> it to struct bpf_object_open_attr? > > Not sure I followed the meaning correctly. My read is you mean it would > better if there is more accurate and fine control on this feature, especially > if prog_flags could become an API parameter during file/obj/prog load? > > As mentioned in the cover letter, I tried to implement bpf_prog_test_load > In parallel with existing bpf_prog_test_run, but then found there are > also similar APIs for bpf_object__open, and raw insn load “bpf_load_progams” > etc and they may involve extending bpf_prog_load_attr/bpf_load_program_attr > etc. I thought the change is a little bit heavy. I could go further if it > is thought to be the correct way or there is any other suggestion on nicely > passing “prog_flags” to low level bpf syscall in libbpf. > Would really appreciate more feedbacks on how to do the libbpf change for the next patch set re-spin. Whether this global variable approach is acceptable or we must find another way which enable high 32-bit randomization for all tests under test_verifier/test_progs with reasonable testsuite and libbpf change. I feel the global variable approach taken in this patch has done its job with minimum change, that is to offer a interface to enable high 32-bit randomization for all tests under test_verifier/test_progs to deliver equal stressfull tests on x86_64 platform and to expose bugs in the optimization, now given there is no regression, we could have confidence on the correctness of the optimization. If we want a more clean change in libbpf, it is more about how to offer clean API to pass "prog_flags" when load file/obj/raw_insn, it looks to me could be a separate patch set. However, for this set, we also need to make sure high 32-bit randomization *always enabled* for all test_verifier + test_progs so the new opt is tested in daily bpf selftest. Regards, Jiong
