Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests
On 02/04/2018 10:19 AM, Jesper Dangaard Brouer wrote: > On Thu, 1 Feb 2018 18:56:14 +0100 > Daniel Borkmann wrote: > >> Don't get me wrong, I'm not at all against such tool or test, I think >> it's a great idea and needed. I just think that tools/lib/bpf/ is not >> the right place to put it into lib directory. Right now, as you say, >> it's a mixture of example code on how to use the lib, and tool at the >> same time to dump/test load an object file with libbpf. > > Okay, to avoid polluting the directory of the library with test/samples > programs, bow for your suggestion of moving the file to the selftests > directory. > > I'm at FOSDEM now, and I cannot send a V2 patch right now... If you > need this in fast (due merge timing), you can make the change yourself > and apply it... else I'll send a V2 on Tuesday. Okay, then lets go with Tuesday since currently bpf pull-request is still pending, thus this fix would go out with the next one anyway. Thanks, Daniel
Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests
On Thu, 1 Feb 2018 18:56:14 +0100 Daniel Borkmann wrote: > Don't get me wrong, I'm not at all against such tool or test, I think > it's a great idea and needed. I just think that tools/lib/bpf/ is not > the right place to put it into lib directory. Right now, as you say, > it's a mixture of example code on how to use the lib, and tool at the > same time to dump/test load an object file with libbpf. Okay, to avoid polluting the directory of the library with test/samples programs, bow for your suggestion of moving the file to the selftests directory. I'm at FOSDEM now, and I cannot send a V2 patch right now... If you need this in fast (due merge timing), you can make the change yourself and apply it... else I'll send a V2 on Tuesday. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests
On 02/01/2018 03:41 PM, Jesper Dangaard Brouer wrote:
> On Thu, 1 Feb 2018 11:59:29 +0100
> Daniel Borkmann wrote:
>
>> Hi Jesper,
>>
>> On 01/27/2018 06:26 PM, Jesper Dangaard Brouer wrote:
>>> While playing with using libbpf for the Suricata project, we had
>>> issues LLVM >= 4.0.1 generating ELF files that could not be loaded
>>> with libbpf (tools/lib/bpf/).
>>>
>>> During the troubleshooting phase, I wrote a test program and improved
>>> the debugging output in libbpf. I turned this into a selftests
>>> program, and it also serves as a code example for libbpf in itself.
>>>
>>> I discovered that there are at least three ELF load issues with
>>> libbpf. I left them as TODO comments in (tools/testing/selftests/bpf)
>>> test_libbpf.sh. I've only fixed the load issue with eh_frames. We can
>>> work on the other issues later.
>>
>> The series looks great, and given the fix, we should get this into bpf.
>>
>> Only one small request, could you move the test_libbpf_open.c into the
>> BPF selftests as well? We have test_libbpf.sh there which is the only
>> one making use of test_libbpf_open.c, so I think it fits much better if
>> we put them both into selftests. Otherwise rest is fine, thanks!
>
> I'm happy that you noticed, but I will argue that the location of
> test_libbpf_open.c is the right place.
>
> I deliberately placed test_libbpf_open.c in tools/lib/bpf/ which is
> together with the library that it uses, because it serves as an example
> of howto use the library libbpf.
>
> Plus, it is functional on its own. When people on the mailing list
> report issues with libbpf, we can ask them to run the tool on their
> bpf-elf objfile and quickly figure out that is wrong.
Don't get me wrong, I'm not at all against such tool or test, I think
it's a great idea and needed. I just think that tools/lib/bpf/ is not
the right place to put it into lib directory. Right now, as you say,
it's a mixture of example code on how to use the lib, and tool at the
same time to dump/test load an object file with libbpf.
I think there are a couple of options depending on the main use case:
sample, pure test case, or tool. I think the latter would be the most
fitting and useful in fact. Often when having loader issues (like the
one you ran into in your last patch), then 'readelf -a' helps to check
out sections and their correlations, such a tool could display it more
user friendly, leaving out the unrelated bits, and could do a dry-load
as well at the same time.
Then lets integrate such an object dump into bpftool, and run it from
the BPF selftests. This would be most beneficial, imo. bpftool already
uses libbpf as a loader and it could provide a new subcommand for the
dump e.g. 'bpftool prog dump object FILE' to debug the contents of it.
At the same time we could also have a dry-run for loading the object,
and given 49a086c201a9 ("bpftool: implement prog load command"), we're
basically there already. It could be a 'bpftool probe OBJ' that would
dump the verifier log unconditionally and in case of success just
closes the prog and exits again. I think this would bring the most
benefit for users to have this functionality integrated in bpftool.
Could you change the few bits to integrate it there? That would be
really great.
> $ ./test_libbpf_open --debug ../../../samples/bpf/tracex1_kern.o
> Open BPF ELF-file with libbpf: ../../../samples/bpf/tracex1_kern.o
> [debug] libbpf: loading ../../../samples/bpf/tracex1_kern.o
> [debug] libbpf: section(1) .strtab, size 119, link 0, flags 0, type=3
> [debug] libbpf: skip section(1) .strtab
> [debug] libbpf: section(2) .text, size 0, link 0, flags 6, type=1
> [debug] libbpf: skip section(2) .text
> [debug] libbpf: section(3) kprobe/__netif_receive_skb_core, size 352, link 0,
> flags 6, type=1
> [debug] libbpf: found program kprobe/__netif_receive_skb_core
> [debug] libbpf: section(4) .rodata.str1.1, size 15, link 0, flags 32, type=1
> [debug] libbpf: skip section(4) .rodata.str1.1
> [debug] libbpf: section(5) license, size 4, link 0, flags 3, type=1
> [debug] libbpf: license of ../../../samples/bpf/tracex1_kern.o is GPL
> [debug] libbpf: section(6) version, size 4, link 0, flags 3, type=1
> [debug] libbpf: kernel version of ../../../samples/bpf/tracex1_kern.o is 40f00
> [debug] libbpf: section(7) .eh_frame, size 40, link 0, flags 2, type=1
> [debug] libbpf: skip section(7) .eh_frame
> [debug] libbpf: section(8) .rel.eh_frame, size 16, link 9, flags 0, type=9
> [debug] libbpf: section(9) .symtab, size 144, link 1, flags 0, type=2
> [warning] libbpf: relocation failed: no section(7)
> Unable to load eBPF objects in file '../../../samples/bpf/tracex1_kern.o':
> Relocation failed
[...]
>> +# 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
>>
>> The kernel version is only required for tracing programs, although
>> it's just fine as well to put a dummy 'int _version SEC("versio
Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests
On Thu, 1 Feb 2018 11:59:29 +0100
Daniel Borkmann wrote:
> Hi Jesper,
>
> On 01/27/2018 06:26 PM, Jesper Dangaard Brouer wrote:
> > While playing with using libbpf for the Suricata project, we had
> > issues LLVM >= 4.0.1 generating ELF files that could not be loaded
> > with libbpf (tools/lib/bpf/).
> >
> > During the troubleshooting phase, I wrote a test program and improved
> > the debugging output in libbpf. I turned this into a selftests
> > program, and it also serves as a code example for libbpf in itself.
> >
> > I discovered that there are at least three ELF load issues with
> > libbpf. I left them as TODO comments in (tools/testing/selftests/bpf)
> > test_libbpf.sh. I've only fixed the load issue with eh_frames. We can
> > work on the other issues later.
>
> The series looks great, and given the fix, we should get this into bpf.
>
> Only one small request, could you move the test_libbpf_open.c into the
> BPF selftests as well? We have test_libbpf.sh there which is the only
> one making use of test_libbpf_open.c, so I think it fits much better if
> we put them both into selftests. Otherwise rest is fine, thanks!
I'm happy that you noticed, but I will argue that the location of
test_libbpf_open.c is the right place.
I deliberately placed test_libbpf_open.c in tools/lib/bpf/ which is
together with the library that it uses, because it serves as an example
of howto use the library libbpf.
Plus, it is functional on its own. When people on the mailing list
report issues with libbpf, we can ask them to run the tool on their
bpf-elf objfile and quickly figure out that is wrong.
$ ./test_libbpf_open --debug ../../../samples/bpf/tracex1_kern.o
Open BPF ELF-file with libbpf: ../../../samples/bpf/tracex1_kern.o
[debug] libbpf: loading ../../../samples/bpf/tracex1_kern.o
[debug] libbpf: section(1) .strtab, size 119, link 0, flags 0, type=3
[debug] libbpf: skip section(1) .strtab
[debug] libbpf: section(2) .text, size 0, link 0, flags 6, type=1
[debug] libbpf: skip section(2) .text
[debug] libbpf: section(3) kprobe/__netif_receive_skb_core, size 352, link 0,
flags 6, type=1
[debug] libbpf: found program kprobe/__netif_receive_skb_core
[debug] libbpf: section(4) .rodata.str1.1, size 15, link 0, flags 32, type=1
[debug] libbpf: skip section(4) .rodata.str1.1
[debug] libbpf: section(5) license, size 4, link 0, flags 3, type=1
[debug] libbpf: license of ../../../samples/bpf/tracex1_kern.o is GPL
[debug] libbpf: section(6) version, size 4, link 0, flags 3, type=1
[debug] libbpf: kernel version of ../../../samples/bpf/tracex1_kern.o is 40f00
[debug] libbpf: section(7) .eh_frame, size 40, link 0, flags 2, type=1
[debug] libbpf: skip section(7) .eh_frame
[debug] libbpf: section(8) .rel.eh_frame, size 16, link 9, flags 0, type=9
[debug] libbpf: section(9) .symtab, size 144, link 1, flags 0, type=2
[warning] libbpf: relocation failed: no section(7)
Unable to load eBPF objects in file '../../../samples/bpf/tracex1_kern.o':
Relocation failed
If the ELF objfile is non-broken, it will in "non-quiet" mode also list
the programs and maps found in the file:
$ ./test_libbpf_open ../../testing/selftests/bpf/test_xdp.o
Open BPF ELF-file with libbpf: ../../testing/selftests/bpf/test_xdp.o
Prog (count:1) section_name: xdp_tx_iptunnel
Map (count:1) name: rxcnt
Map (count:2) name: vip2tnl
Close BPF ELF-file with libbpf: ../../testing/selftests/bpf/test_xdp.o
> Regarding the TODO comments:
>
> +# TODO: fix libbpf to load noinline functions
> +# [warning] libbpf: incorrect bpf_call opcode
> +#libbpf_open_file test_l4lb_noinline.o
>
> Right, so this would require a newer llvm version that supports calls.
> Maybe there could be a fallback to turn the noinline into __always_inline
> for older llvms with dumping a warning to the user that this case cannot
> properly be tested due to old llvm version.
Okay, good to know... guess it will be a while before we can enable
this test then.
> +# 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
>
> The kernel version is only required for tracing programs, although
> it's just fine as well to put a dummy 'int _version SEC("version") = 1;'
> here. The test_xdp_meta.sh uses iproute2 for loading into the kernel,
> but no objections to add a version section there.
A lot of bpf-prog does not seems to set the version section, and other
loaders seems to ignore this ... maybe we should remove this
requirement from libbpf?
> +# TODO: fix libbpf to handle .eh_frame
> +# [warning] libbpf: relocation failed: no section(10)
> +#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
>
> This is resolved in your last patch then, right? So the two could just
> be swapped and this one uncommented, although it would require that
> tracex3_kern.o has been built earlier.
Yes, after my patch, we can enable this test, but as you write this
would require creating a Makefile build depend
Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests
Hi Jesper,
On 01/27/2018 06:26 PM, Jesper Dangaard Brouer wrote:
> While playing with using libbpf for the Suricata project, we had
> issues LLVM >= 4.0.1 generating ELF files that could not be loaded
> with libbpf (tools/lib/bpf/).
>
> During the troubleshooting phase, I wrote a test program and improved
> the debugging output in libbpf. I turned this into a selftests
> program, and it also serves as a code example for libbpf in itself.
>
> I discovered that there are at least three ELF load issues with
> libbpf. I left them as TODO comments in (tools/testing/selftests/bpf)
> test_libbpf.sh. I've only fixed the load issue with eh_frames. We can
> work on the other issues later.
The series looks great, and given the fix, we should get this into bpf.
Only one small request, could you move the test_libbpf_open.c into the
BPF selftests as well? We have test_libbpf.sh there which is the only
one making use of test_libbpf_open.c, so I think it fits much better if
we put them both into selftests. Otherwise rest is fine, thanks!
Regarding the TODO comments:
+# TODO: fix libbpf to load noinline functions
+# [warning] libbpf: incorrect bpf_call opcode
+#libbpf_open_file test_l4lb_noinline.o
Right, so this would require a newer llvm version that supports calls.
Maybe there could be a fallback to turn the noinline into __always_inline
for older llvms with dumping a warning to the user that this case cannot
properly be tested due to old llvm version.
+# 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
The kernel version is only required for tracing programs, although
it's just fine as well to put a dummy 'int _version SEC("version") = 1;'
here. The test_xdp_meta.sh uses iproute2 for loading into the kernel,
but no objections to add a version section there.
+# TODO: fix libbpf to handle .eh_frame
+# [warning] libbpf: relocation failed: no section(10)
+#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o
This is resolved in your last patch then, right? So the two could just
be swapped and this one uncommented, although it would require that
tracex3_kern.o has been built earlier.
Thanks,
Daniel
> Jesper Dangaard Brouer (5):
> bpf: Sync kernel ABI header with tooling header for bpf_common.h
> tools/libbpf: improve the pr_debug statements to contain section numbers
> tools/libbpf: add test program for loading BPF ELF files
> selftests/bpf: add selftest that use test_libbpf_open
> tools/libbpf: handle issues with bpf ELF objects containing .eh_frames
>
>
> tools/testing/selftests/bpf/Makefile |9 +-
> tools/testing/selftests/bpf/test_libbpf.sh | 45
>
> 2 files changed, 53 insertions(+), 1 deletion(-)
> create mode 100755 tools/testing/selftests/bpf/test_libbpf.sh
>
> --
>
[bpf-next PATCH 0/5] tools/libbpf improvements and selftests
While playing with using libbpf for the Suricata project, we had issues LLVM >= 4.0.1 generating ELF files that could not be loaded with libbpf (tools/lib/bpf/). During the troubleshooting phase, I wrote a test program and improved the debugging output in libbpf. I turned this into a selftests program, and it also serves as a code example for libbpf in itself. I discovered that there are at least three ELF load issues with libbpf. I left them as TODO comments in (tools/testing/selftests/bpf) test_libbpf.sh. I've only fixed the load issue with eh_frames. We can work on the other issues later. --- Jesper Dangaard Brouer (5): bpf: Sync kernel ABI header with tooling header for bpf_common.h tools/libbpf: improve the pr_debug statements to contain section numbers tools/libbpf: add test program for loading BPF ELF files selftests/bpf: add selftest that use test_libbpf_open tools/libbpf: handle issues with bpf ELF objects containing .eh_frames tools/testing/selftests/bpf/Makefile |9 +- tools/testing/selftests/bpf/test_libbpf.sh | 45 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/bpf/test_libbpf.sh --
