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

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

[Quentin Monnet]

libbpf is now able to load successfully test_l4lb_noinline.o and
samples/bpf/tracex3_kern.o, so we can uncomment related tests from
test_libbpf.c and remove the associated "TODO"s.

It is also trivial to fix test_xdp_noinline.o so that it provides a
version and can be loaded. Fix it and uncomment this test as well.

For the record, the error message obtainted with tracex3_kern.o was
fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
objects containing .eh_frames")

I have not been abled to reproduce the "libbpf: incorrect bpf_call
opcode" error for test_l4lb_noinline.o, even with the version of libbpf
present at the time when test_libbpf.sh and test_libbpf_open.c were
created.

Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_libbpf.sh  | 12 +++---------
 tools/testing/selftests/bpf/test_xdp_meta.c |  2 ++
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
index 156d89f1edcc..a426f28163a5 100755
--- 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
 
 # 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

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

[Jesper Dangaard Brouer]

On Sat, 20 Oct 2018 23:00:24 +0100
Quentin Monnet <quentin.monnet@netronome.com> wrote:

> libbpf is now able to load successfully test_l4lb_noinline.o and
> samples/bpf/tracex3_kern.o, so we can uncomment related tests from
> test_libbpf.c and remove the associated "TODO"s.

Thanks for working on this, comments below.

> It is also trivial to fix test_xdp_noinline.o so that it provides a
> version and can be loaded. Fix it and uncomment this test as well.
> 
> For the record, the error message obtainted with tracex3_kern.o was
> fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
> objects containing .eh_frames")
> 
> I have not been abled to reproduce the "libbpf: incorrect bpf_call
> opcode" error for test_l4lb_noinline.o, even with the version of libbpf
> present at the time when test_libbpf.sh and test_libbpf_open.c were
> created.
> 
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  tools/testing/selftests/bpf/test_libbpf.sh  | 12 +++---------
>  tools/testing/selftests/bpf/test_xdp_meta.c |  2 ++
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
> index 156d89f1edcc..a426f28163a5 100755
> --- 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)

>  # 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)
>  {



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

[Quentin Monnet]

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

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

[Jesper Dangaard Brouer]

On Sun, 21 Oct 2018 16:37:08 +0100
Quentin Monnet <quentin.monnet@netronome.com> wrote:

> 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?

I think it makes sense to have a test program compiled without the
"-target-bpf", as that will happen for users.  And I guess we can add
some more specific test that are related to "-target-bpf".

> Or should tests for libbpf have a directory of their own,
> with another Makefile?

Hmm, I'm not sure about that idea.

I did plan by naming the test "libbpf_open_file", what we add more
libbpf_ prefixed tests to the test_libbpf.sh script, which should
cover more aspects of the _base_ libbpf functionality.

> 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).

Well, yes.  I was hinting if we should relax the version requirement
for e.g. XDP BPF progs.

> ---
>  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)
>  {



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

[Quentin Monnet]

2018-10-21 23:04 UTC+0200 ~ Jesper Dangaard Brouer <brouer@redhat.com>
> On Sun, 21 Oct 2018 16:37:08 +0100
> Quentin Monnet <quentin.monnet@netronome.com> wrote:
> 
>> 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?
> 
> I think it makes sense to have a test program compiled without the
> "-target-bpf", as that will happen for users.  And I guess we can add
> some more specific test that are related to "-target-bpf".

Alright, I can repost my second version that takes a test out of the
default target for building BPF programs, after the merge window.

>> Or should tests for libbpf have a directory of their own,
>> with another Makefile?
> 
> Hmm, I'm not sure about that idea.
> 
> I did plan by naming the test "libbpf_open_file", what we add more
> libbpf_ prefixed tests to the test_libbpf.sh script, which should
> cover more aspects of the _base_ libbpf functionality.
> 
>> 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).
> 
> Well, yes.  I was hinting if we should relax the version requirement
> for e.g. XDP BPF progs.

This is already the case. What happens for this test is that we never
tell libbpf that this program is XDP, we just ask it to open the ELF
file and the whole time libbpf treats it as a program of type
BPF_PROG_TYPE_UNSPEC. So we can fix the BPF source (by adding a version)
or we can fix test_libbpf_open.c (to tell libbpf this is XDP), but I
don't believe there is anything to add to libbpf in that regard. I think
we could simply remove the test on test_xdp_meta.o from test_libbpf.h,
actually. What is you opinion?

Thanks,
Quentin

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

[Jesper Dangaard Brouer]

On Mon, 22 Oct 2018 11:00:27 +0100
Quentin Monnet <quentin.monnet@netronome.com> wrote:

> 2018-10-21 23:04 UTC+0200 ~ Jesper Dangaard Brouer <brouer@redhat.com>
> > On Sun, 21 Oct 2018 16:37:08 +0100
> > Quentin Monnet <quentin.monnet@netronome.com> wrote:
> >   
> >> 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?  
> > 
> > I think it makes sense to have a test program compiled without the
> > "-target-bpf", as that will happen for users.  And I guess we can add
> > some more specific test that are related to "-target-bpf".  
> 
> Alright, I can repost my second version that takes a test out of the
> default target for building BPF programs, after the merge window.
> 

Okay, guess there is no rush in getting this in now, and we can wait
for after the merge window.


> >> Or should tests for libbpf have a directory of their own,
> >> with another Makefile?  
> > 
> > Hmm, I'm not sure about that idea.
> > 
> > I did plan by naming the test "libbpf_open_file", what we add more
> > libbpf_ prefixed tests to the test_libbpf.sh script, which should
> > cover more aspects of the _base_ libbpf functionality.
> >   
> >> 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).  
> > 
> > Well, yes.  I was hinting if we should relax the version requirement
> > for e.g. XDP BPF progs.  
> 
> This is already the case. What happens for this test is that we never
> tell libbpf that this program is XDP, we just ask it to open the ELF
> file and the whole time libbpf treats it as a program of type
> BPF_PROG_TYPE_UNSPEC. So we can fix the BPF source (by adding a version)
> or we can fix test_libbpf_open.c (to tell libbpf this is XDP), but I
> don't believe there is anything to add to libbpf in that regard. I think
> we could simply remove the test on test_xdp_meta.o from test_libbpf.h,
> actually. What is you opinion?

Yes, we should likely just drop the test then.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

[Quentin Monnet]

libbpf is now able to load successfully test_l4lb_noinline.o and
samples/bpf/tracex3_kern.o.

For the test_l4lb_noinline, uncomment related tests from test_libbpf.c
and remove the associated "TODO".

For tracex3_kern.o, instead of loading a program from samples/bpf/ that
might not have been compiled at this stage, try loading a program from
BPF selftests. Since this test case is about loading a program compiled
without the "-target bpf" flag, change the Makefile to compile one
program accordingly (instead of passing the flag for compiling all
programs).

Regarding test_xdp_noinline.o: in its current shape the program fails to
load because it provides no version section, but the loader needs one.
The test was added to make sure that libbpf could load XDP programs even
if they do not provide a version number in a dedicated section. But
libbpf is already capable of doing that: in our case loading fails
because the loader does not know that this is an XDP program (it does
not need to, since it does not attach the program). So trying to load
test_xdp_noinline.o does not bring much here: just delete this subtest.

For the record, the error message obtained with tracex3_kern.o was
fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
objects containing .eh_frames")

I have not been abled to reproduce the "libbpf: incorrect bpf_call
opcode" error for test_l4lb_noinline.o, even with the version of libbpf
present at the time when test_libbpf.sh and test_libbpf_open.c were
created.

RFC -> v1:
- Compile test_xdp without the "-target bpf" flag, and try to load it
  instead of ../../samples/bpf/tracex3_kern.o.
- Delete test_xdp_noinline.o subtest.

Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/testing/selftests/bpf/Makefile       | 10 ++++++++++
 tools/testing/selftests/bpf/test_libbpf.sh | 14 ++++----------
 2 files changed, 14 insertions(+), 10 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..2989b2e2d856 100755
--- 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
+# 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
-
-# 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
-- 
2.7.4

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

[Jesper Dangaard Brouer]

On Wed,  7 Nov 2018 12:28:45 +0000
Quentin Monnet <quentin.monnet@netronome.com> wrote:

> libbpf is now able to load successfully test_l4lb_noinline.o and
> samples/bpf/tracex3_kern.o.
> 
> For the test_l4lb_noinline, uncomment related tests from test_libbpf.c
> and remove the associated "TODO".
> 
> For tracex3_kern.o, instead of loading a program from samples/bpf/ that
> might not have been compiled at this stage, try loading a program from
> BPF selftests. Since this test case is about loading a program compiled
> without the "-target bpf" flag, change the Makefile to compile one
> program accordingly (instead of passing the flag for compiling all
> programs).
> 
> Regarding test_xdp_noinline.o: in its current shape the program fails to
> load because it provides no version section, but the loader needs one.
> The test was added to make sure that libbpf could load XDP programs even
> if they do not provide a version number in a dedicated section. But
> libbpf is already capable of doing that: in our case loading fails
> because the loader does not know that this is an XDP program (it does
> not need to, since it does not attach the program). So trying to load
> test_xdp_noinline.o does not bring much here: just delete this subtest.
> 
> For the record, the error message obtained with tracex3_kern.o was
> fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
> objects containing .eh_frames")
> 
> I have not been abled to reproduce the "libbpf: incorrect bpf_call
> opcode" error for test_l4lb_noinline.o, even with the version of libbpf
> present at the time when test_libbpf.sh and test_libbpf_open.c were
> created.
> 
> RFC -> v1:
> - Compile test_xdp without the "-target bpf" flag, and try to load it
>   instead of ../../samples/bpf/tracex3_kern.o.
> - Delete test_xdp_noinline.o subtest.
> 
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

Thanks for following up Quentin :-)
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

[Daniel Borkmann]

On 11/07/2018 01:28 PM, Quentin Monnet wrote:
> libbpf is now able to load successfully test_l4lb_noinline.o and
> samples/bpf/tracex3_kern.o.
> 
> For the test_l4lb_noinline, uncomment related tests from test_libbpf.c
> and remove the associated "TODO".
> 
> For tracex3_kern.o, instead of loading a program from samples/bpf/ that
> might not have been compiled at this stage, try loading a program from
> BPF selftests. Since this test case is about loading a program compiled
> without the "-target bpf" flag, change the Makefile to compile one
> program accordingly (instead of passing the flag for compiling all
> programs).
> 
> Regarding test_xdp_noinline.o: in its current shape the program fails to
> load because it provides no version section, but the loader needs one.
> The test was added to make sure that libbpf could load XDP programs even
> if they do not provide a version number in a dedicated section. But
> libbpf is already capable of doing that: in our case loading fails
> because the loader does not know that this is an XDP program (it does
> not need to, since it does not attach the program). So trying to load
> test_xdp_noinline.o does not bring much here: just delete this subtest.
> 
> For the record, the error message obtained with tracex3_kern.o was
> fixed by commit e3d91b0ca523 ("tools/libbpf: handle issues with bpf ELF
> objects containing .eh_frames")
> 
> I have not been abled to reproduce the "libbpf: incorrect bpf_call
> opcode" error for test_l4lb_noinline.o, even with the version of libbpf
> present at the time when test_libbpf.sh and test_libbpf_open.c were
> created.
> 
> RFC -> v1:
> - Compile test_xdp without the "-target bpf" flag, and try to load it
>   instead of ../../samples/bpf/tracex3_kern.o.
> - Delete test_xdp_noinline.o subtest.
> 
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied to bpf-next, thanks!