From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [PATCH bpf-next] selftests/bpf: enable (uncomment) all tests in test_libbpf.sh Date: Mon, 22 Oct 2018 19:08:53 +0200 Message-ID: <20181022190853.4f4b1f89@redhat.com> References: <20181021115729.2095d2a0@redhat.com> <1540136228-11360-1-git-send-email-quentin.monnet@netronome.com> <20181021230438.10481e7f@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Alexei Starovoitov , Daniel Borkmann , netdev@vger.kernel.org, oss-drivers@netronome.com, brouer@redhat.com To: Quentin Monnet Return-path: Received: from mx1.redhat.com ([209.132.183.28]:39302 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728236AbeJWB2U (ORCPT ); Mon, 22 Oct 2018 21:28:20 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Mon, 22 Oct 2018 11:00:27 +0100 Quentin Monnet wrote: > 2018-10-21 23:04 UTC+0200 ~ Jesper Dangaard Brouer > > On Sun, 21 Oct 2018 16:37:08 +0100 > > Quentin Monnet wrote: > > > >> 2018-10-21 11:57 UTC+0200 ~ Jesper Dangaard Brouer > >>> On Sat, 20 Oct 2018 23:00:24 +0100 > >>> Quentin Monnet 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