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: Sun, 21 Oct 2018 23:04:38 +0200 Message-ID: <20181021230438.10481e7f@redhat.com> References: <20181021115729.2095d2a0@redhat.com> <1540136228-11360-1-git-send-email-quentin.monnet@netronome.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]:33310 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727095AbeJVFUY (ORCPT ); Mon, 22 Oct 2018 01:20:24 -0400 In-Reply-To: <1540136228-11360-1-git-send-email-quentin.monnet@netronome.com> Sender: netdev-owner@vger.kernel.org List-ID: 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". > 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. > --- > tools/testing/selftests/bpf/Makefile | 10 ++++++++++ > tools/testing/selftests/bpf/test_libbpf.sh | 14 +++++--------- > tools/testing/selftests/bpf/test_xdp_meta.c | 2 ++ > 3 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index e39dfb4e7970..ecd79b7fb107 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -135,6 +135,16 @@ endif > endif > endif > > +# Have one program compiled without "-target bpf" to test whether libbpf loads > +# it successfully > +$(OUTPUT)/test_xdp.o: test_xdp.c > + $(CLANG) $(CLANG_FLAGS) \ > + -O2 -emit-llvm -c $< -o - | \ > + $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@ > +ifeq ($(DWARF2BTF),y) > + $(BTF_PAHOLE) -J $@ > +endif > + > $(OUTPUT)/%.o: %.c > $(CLANG) $(CLANG_FLAGS) \ > -O2 -target bpf -emit-llvm -c $< -o - | \ > diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh > index 156d89f1edcc..b45962a44243 100755 > --- a/tools/testing/selftests/bpf/test_libbpf.sh > +++ b/tools/testing/selftests/bpf/test_libbpf.sh > @@ -33,17 +33,13 @@ 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 > +# Load a program with BPF-to-BPF calls > +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 > +# Load a program compiled without the "-target bpf" flag > +libbpf_open_file test_xdp.o > > # Success > exit 0 > diff --git a/tools/testing/selftests/bpf/test_xdp_meta.c b/tools/testing/selftests/bpf/test_xdp_meta.c > index 8d0182650653..2f42de66e2bb 100644 > --- a/tools/testing/selftests/bpf/test_xdp_meta.c > +++ b/tools/testing/selftests/bpf/test_xdp_meta.c > @@ -8,6 +8,8 @@ > #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1) > #define ctx_ptr(ctx, mem) (void *)(unsigned long)ctx->mem > > +int _version SEC("version") = 1; > + > SEC("t") > int ing_cls(struct __sk_buff *ctx) > { -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer