| Message ID | 20190627091450.78550-1-iii@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] selftests/bpf: do not ignore clang failures | expand |
On Thu, Jun 27, 2019 at 2:15 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > When compiling an eBPF prog fails, make still returns 0, because > failing clang command's output is piped to llc and therefore its > exit status is ignored. > > This patch uses bash's pipefail option to fail the build when clang > fails, and also make's .DELETE_ON_ERROR target to get rid of partial > BPF bytecode files. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/testing/selftests/bpf/Makefile | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index f2dbe2043067..2316fa2d5b3b 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > > +SHELL := /bin/bash I am not sure whether it is ok to require bash. I don't see such requirements in other Makefile's under tools/. Can we enable some fall back when bash is not present? Thanks, Song
> Am 28.06.2019 um 22:35 schrieb Song Liu <liu.song.a23@gmail.com>: > > On Thu, Jun 27, 2019 at 2:15 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: >> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile >> index f2dbe2043067..2316fa2d5b3b 100644 >> --- a/tools/testing/selftests/bpf/Makefile >> +++ b/tools/testing/selftests/bpf/Makefile >> @@ -1,5 +1,6 @@ >> # SPDX-License-Identifier: GPL-2.0 >> >> +SHELL := /bin/bash > > I am not sure whether it is ok to require bash. I don't see such requirements in > other Makefile's under tools/. > > Can we enable some fall back when bash is not present? > > Thanks, > Song I think checking for bash presence would unnecessarily complicate things. What do you think about having separate targets for clang-generated bitcode? Best regards, Ilya
On Mon, Jul 1, 2019 at 1:56 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > > Am 28.06.2019 um 22:35 schrieb Song Liu <liu.song.a23@gmail.com>: > > > > On Thu, Jun 27, 2019 at 2:15 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > >> > >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > >> index f2dbe2043067..2316fa2d5b3b 100644 > >> --- a/tools/testing/selftests/bpf/Makefile > >> +++ b/tools/testing/selftests/bpf/Makefile > >> @@ -1,5 +1,6 @@ > >> # SPDX-License-Identifier: GPL-2.0 > >> > >> +SHELL := /bin/bash > > > > I am not sure whether it is ok to require bash. I don't see such requirements in > > other Makefile's under tools/. > > > > Can we enable some fall back when bash is not present? > > > > Thanks, > > Song > > I think checking for bash presence would unnecessarily complicate > things. What do you think about having separate targets for > clang-generated bitcode? Do we still need clang | llc pipeline with new clang? Could the same be achieved with single clang invocation? That would solve the problem of not detecting pipeline failures. But either way, I think .DELETE_ON_ERROR is worth adding nevertheless. > > Best regards, > Ilya
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index f2dbe2043067..2316fa2d5b3b 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +SHELL := /bin/bash LIBDIR := ../../../lib BPFDIR := $(LIBDIR)/bpf APIDIR := ../../../include/uapi @@ -191,7 +192,7 @@ $(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c $(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR) \ $(ALU32_BUILD_DIR)/test_progs_32 - $(CLANG) $(CLANG_FLAGS) \ + set -o pipefail; $(CLANG) $(CLANG_FLAGS) \ -O2 -target bpf -emit-llvm -c $< -o - | \ $(LLC) -march=bpf -mattr=+alu32 -mcpu=$(CPU) $(LLC_FLAGS) \ -filetype=obj -o $@ @@ -203,7 +204,7 @@ endif # Have one program compiled without "-target bpf" to test whether libbpf loads # it successfully $(OUTPUT)/test_xdp.o: progs/test_xdp.c - $(CLANG) $(CLANG_FLAGS) \ + set -o pipefail; $(CLANG) $(CLANG_FLAGS) \ -O2 -emit-llvm -c $< -o - | \ $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@ ifeq ($(DWARF2BTF),y) @@ -211,7 +212,7 @@ ifeq ($(DWARF2BTF),y) endif $(OUTPUT)/%.o: progs/%.c - $(CLANG) $(CLANG_FLAGS) \ + set -o pipefail; $(CLANG) $(CLANG_FLAGS) \ -O2 -target bpf -emit-llvm -c $< -o - | \ $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@ ifeq ($(DWARF2BTF),y) @@ -283,3 +284,5 @@ $(OUTPUT)/verifier/tests.h: $(VERIFIER_TESTS_DIR) $(VERIFIER_TEST_FILES) EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) \ $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \ feature + +.DELETE_ON_ERROR:
When compiling an eBPF prog fails, make still returns 0, because failing clang command's output is piped to llc and therefore its exit status is ignored. This patch uses bash's pipefail option to fail the build when clang fails, and also make's .DELETE_ON_ERROR target to get rid of partial BPF bytecode files. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/Makefile | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)