diff mbox series

[bpf-next,2/3] selftests: bpf: extend sub-register mode compilation to all bpf object files

Message ID 1549647681-13818-3-git-send-email-jiong.wang@netronome.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series selftests: bpf: improve bpf object file rules | expand

Commit Message

Jiong Wang Feb. 8, 2019, 5:41 p.m. UTC
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@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
 tools/testing/selftests/bpf/Makefile | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

Comments

Alexei Starovoitov Feb. 11, 2019, 4:04 a.m. UTC | #1
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@netronome.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.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 ?

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 ?
Jiong Wang Feb. 11, 2019, 11:47 a.m. UTC | #2
> On 11 Feb 2019, at 04:04, Alexei Starovoitov <alexei.starovoitov@gmail.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@netronome.com>
>> Signed-off-by: Jiong Wang <jiong.wang@netronome.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
diff mbox series

Patch

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')
 ifneq ($(SUBREG_CODEGEN),)
-TEST_GEN_FILES += $(patsubst %.o,alu32/%.o, $(BPF_OBJ_FILES_DUAL_COMPILE))
+TEST_GEN_FILES += $(patsubst %.o,alu32/%.o, $(BPF_OBJ_FILES))
 endif
 
 # Order correspond to 'make run_tests' order