Re: [PATCH bpf-next] selftests/bpf: enable (uncomment) all tests in test_libbpf.sh

[Quentin Monnet <quentin.monnet@netronome.com>]

2018-10-21 11:57 UTC+0200 ~ Jesper Dangaard Brouer <brouer@redhat.com>
> On Sat, 20 Oct 2018 23:00:24 +0100
> Quentin Monnet <quentin.monnet@netronome.com> wrote:
> 

[...]

>> --- a/tools/testing/selftests/bpf/test_libbpf.sh
>> +++ b/tools/testing/selftests/bpf/test_libbpf.sh
>> @@ -33,17 +33,11 @@ trap exit_handler 0 2 3 6 9
>>   
>>   libbpf_open_file test_l4lb.o
>>   
>> -# TODO: fix libbpf to load noinline functions
>> -# [warning] libbpf: incorrect bpf_call opcode
>> -#libbpf_open_file test_l4lb_noinline.o
>> +libbpf_open_file test_l4lb_noinline.o
>>   
>> -# TODO: fix test_xdp_meta.c to load with libbpf
>> -# [warning] libbpf: test_xdp_meta.o doesn't provide kernel version
>> -#libbpf_open_file test_xdp_meta.o
>> +libbpf_open_file test_xdp_meta.o
>>   
>> -# TODO: fix libbpf to handle .eh_frame
>> -# [warning] libbpf: relocation failed: no section(10)
>> -#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
>> +libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
> 
> I don't like the ../../../../samples/bpf/ reference (even-through I
> added this TODO), as the kselftests AFAIK support installing the
> selftests and then this tests will fail.
> Maybe we can find another example kern.o file?
> (which isn't compiled with -target bpf)

Hi Jesper, yeah maybe making the test rely on something from samples/bpf
instead of just the selftests/bpf directory is not a good idea. But
there is no program compiled without the "-target-bpf" in that
directory. What we could do is explicitly compile one without the flag
in the Makefile, as in the patch below, but I am not sure that doing so
is acceptable? Or should tests for libbpf have a directory of their own,
with another Makefile?

Another question regarding the test with test_xdp_meta.o: does the fix I
suggested (setting a version in the .C file) makes sense, or did you
leave this test for testing someday that libbpf would be able to open
even programs that do not set a version (in which case this is still not
the case if program type is not provided, and in fact my fix ruins
everything? :s).

Thanks,
Quentin

---
 tools/testing/selftests/bpf/Makefile        | 10 ++++++++++
 tools/testing/selftests/bpf/test_libbpf.sh  | 14 +++++---------
 tools/testing/selftests/bpf/test_xdp_meta.c |  2 ++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e39dfb4e7970..ecd79b7fb107 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -135,6 +135,16 @@ endif
 endif
 endif
 
+# Have one program compiled without "-target bpf" to test whether libbpf loads
+# it successfully
+$(OUTPUT)/test_xdp.o: test_xdp.c
+	$(CLANG) $(CLANG_FLAGS) \
+		-O2 -emit-llvm -c $< -o - | \
+	$(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
+ifeq ($(DWARF2BTF),y)
+	$(BTF_PAHOLE) -J $@
+endif
+
 $(OUTPUT)/%.o: %.c
 	$(CLANG) $(CLANG_FLAGS) \
 		 -O2 -target bpf -emit-llvm -c $< -o - |      \
diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
index 156d89f1edcc..b45962a44243 100755
--- a/tools/testing/selftests/bpf/test_libbpf.sh
+++ b/tools/testing/selftests/bpf/test_libbpf.sh
@@ -33,17 +33,13 @@ trap exit_handler 0 2 3 6 9
 
 libbpf_open_file test_l4lb.o
 
-# TODO: fix libbpf to load noinline functions
-# [warning] libbpf: incorrect bpf_call opcode
-#libbpf_open_file test_l4lb_noinline.o
+# Load a program with BPF-to-BPF calls
+libbpf_open_file test_l4lb_noinline.o
 
-# TODO: fix test_xdp_meta.c to load with libbpf
-# [warning] libbpf: test_xdp_meta.o doesn't provide kernel version
-#libbpf_open_file test_xdp_meta.o
+libbpf_open_file test_xdp_meta.o
 
-# TODO: fix libbpf to handle .eh_frame
-# [warning] libbpf: relocation failed: no section(10)
-#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
+# Load a program compiled without the "-target bpf" flag
+libbpf_open_file test_xdp.o
 
 # Success
 exit 0
diff --git a/tools/testing/selftests/bpf/test_xdp_meta.c b/tools/testing/selftests/bpf/test_xdp_meta.c
index 8d0182650653..2f42de66e2bb 100644
--- a/tools/testing/selftests/bpf/test_xdp_meta.c
+++ b/tools/testing/selftests/bpf/test_xdp_meta.c
@@ -8,6 +8,8 @@
 #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1)
 #define ctx_ptr(ctx, mem) (void *)(unsigned long)ctx->mem
 
+int _version SEC("version") = 1;
+
 SEC("t")
 int ing_cls(struct __sk_buff *ctx)
 {
-- 
2.7.4