Date:   Mon, 11 Feb 2019 11:47:13 +0000
From:   Jiong Wang <jiong.wang@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next 2/3] selftests: bpf: extend sub-register mode
 compilation to all bpf object files


> On 11 Feb 2019, at 04:04, Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:
> 
> On Fri, Feb 08, 2019 at 05:41:20PM +0000, Jiong Wang wrote:
>> At the moment, we only do extra sub-register mode compilation on bpf object
>> files used by "test_progs". These object files are really loaded and
>> executed.
>> 
>> This patch further extends sub-register mode compilation to all bpf object
>> files, even those without corresponding runtime tests. Because this could
>> help testing LLVM sub-register code-gen, kernel bpf selftest has much more
>> C testcases with reasonable size and complexity compared with LLVM
>> testsuite which only contains unit tests.
>> 
>> There were some file duplication inside BPF_OBJ_FILES_DUAL_COMPILE which
>> is removed now.
>> 
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>> Signed-off-by: Jiong Wang <jiong.wang@...ronome.com>
>> ---
>> tools/testing/selftests/bpf/Makefile | 21 ++++++++-------------
>> 1 file changed, 8 insertions(+), 13 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 383d2ff..70b2570 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -35,20 +35,15 @@ BPF_OBJ_FILES = \
>> 	sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
>> 	get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
>> 	test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o test_xdp_vlan.o \
>> -	xdp_dummy.o test_map_in_map.o test_spin_lock.o test_map_lock.o
>> -
>> -# Objects are built with default compilation flags and with sub-register
>> -# code-gen enabled.
>> -BPF_OBJ_FILES_DUAL_COMPILE = \
>> -	test_pkt_access.o test_pkt_access.o test_xdp.o test_adjust_tail.o \
>> -	test_l4lb.o test_l4lb_noinline.o test_xdp_noinline.o test_tcp_estats.o \
>> +	xdp_dummy.o test_map_in_map.o test_spin_lock.o test_map_lock.o \
>> +	test_pkt_access.o test_xdp.o test_adjust_tail.o test_l4lb.o \
>> +	test_l4lb_noinline.o test_xdp_noinline.o test_tcp_estats.o \
>> 	test_obj_id.o test_pkt_md_access.o test_tracepoint.o \
>> -	test_stacktrace_map.o test_stacktrace_map.o test_stacktrace_build_id.o \
>> -	test_stacktrace_build_id.o test_get_stack_rawtp.o \
>> -	test_get_stack_rawtp.o test_tracepoint.o test_sk_lookup_kern.o \
>> -	test_queue_map.o test_stack_map.o
>> +	test_stacktrace_map.o test_stacktrace_build_id.o \
>> +	test_get_stack_rawtp.o test_sk_lookup_kern.o test_queue_map.o \
>> +	test_stack_map.o
>> 
>> -TEST_GEN_FILES = $(BPF_OBJ_FILES) $(BPF_OBJ_FILES_DUAL_COMPILE)
>> +TEST_GEN_FILES = $(BPF_OBJ_FILES)
>> 
>> # Also test sub-register code-gen if LLVM + kernel both has eBPF v3 processor
>> # support which is the first version to contain both ALU32 and JMP32
>> @@ -58,7 +53,7 @@ SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \
>> 			$(LLC) -mattr=+alu32 -mcpu=probe 2>&1 | \
>> 			grep 'if w')
> 
> build and test servers can be different.
> Would it make sense to use -mcpu=v3 instead of -mcpu=probe ?

Have the same thoughts initially, but was worried the situation where doing native
bpf selftest, build/test on same machine and llvm is new but kernel is old.

I think remove the runtime kernel criteria make sense if for bpf selftest, separation
between build and test server are often, for example cross-compilation are used.

Will add a new patch to make this change.

> 
> Also while testing test_progs_32 fails like this:
> libbpf: failed to open ./bpf_flow.o: No such file or directory
> libbpf: failed to open ./test_spin_lock.o: No such file or directory
> test_spin_lock:bpf_prog_load errno 2
> 
> Do you see the same ?

Hmm, I haven’t see the same. This is expected to happen before this patch, but should
not after. Because bpf_flow and test_spin_lock were added after the initial JMP32 patch
set.

Noticed there is patch conflict between this patch and KaFai’s latest test_sock_fields
change. Perhaps this patch hasn’t been applied successfully when doing the test on your
env?

Regards,
Jiong

Powered by blists - more mailing lists