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