On Mon, 22 Oct 2018 11:00:27 +0100 Quentin Monnet <quentin.mon...@netronome.com> wrote:
> 2018-10-21 23:04 UTC+0200 ~ Jesper Dangaard Brouer <bro...@redhat.com> > > On Sun, 21 Oct 2018 16:37:08 +0100 > > Quentin Monnet <quentin.mon...@netronome.com> wrote: > > > >> 2018-10-21 11:57 UTC+0200 ~ Jesper Dangaard Brouer <bro...@redhat.com> > >>> On Sat, 20 Oct 2018 23:00:24 +0100 > >>> Quentin Monnet <quentin.mon...@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