From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests Date: Thu, 1 Feb 2018 11:59:29 +0100 Message-ID: <8e21a7e5-c356-cfe8-c56e-aac45ed56afa@iogearbox.net> References: <151707400217.12604.5522514180508858186.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: acme@redhat.com, joe@ovn.org, jakub.kicinski@netronome.com, eric@regit.org To: Jesper Dangaard Brouer , netdev@vger.kernel.org, Daniel Borkmann , Alexei Starovoitov , wangnan0@huawei.com Return-path: Received: from www62.your-server.de ([213.133.104.62]:59588 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751761AbeBAK7b (ORCPT ); Thu, 1 Feb 2018 05:59:31 -0500 In-Reply-To: <151707400217.12604.5522514180508858186.stgit@firesoul> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 > > -- >