[PATCH bpf-next 0/2] selftests: bpftool: skip build tests if not in tree

[PATCH bpf-next 0/2] selftests: bpftool: skip build tests if not in tree

[Quentin Monnet]

The build test script for bpftool attempts to detect the toplevel path of
the kernel repository and attempts to build bpftool from there.

If it fails to find the correct directory, or if bpftool files are missing
for another reason (e.g. kselftests built on a first machine and copied
onto another, without bpftool sources), then it is preferable to skip the
tests entirely rather than dumping useless error messages.

The first patch moves the EXIT trap in the script lower down in the code,
to avoid tampering with return value on early exits at the beginning of the
script; then the second patch makes sure that we skip the build tests if
bpftool's Makefile is not found at its expected location.

Jakub Kicinski (1):
  selftests: bpftool: skip the build test if not in tree

Quentin Monnet (1):
  selftests: bpftool: set EXIT trap after usage function

 .../selftests/bpf/test_bpftool_build.sh       | 30 +++++++++++--------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.17.1

[PATCH bpf-next 1/2] selftests: bpftool: set EXIT trap after usage function

[Quentin Monnet]

The trap on EXIT is used to clean up any temporary directory left by the
build attempts. It is not needed when the user simply calls the script
with its --help option, and may not be needed either if we add checks
(e.g. on the availability of bpftool files) before the build attempts.

Let's move this trap and related variables lower down in the code, so
that we don't accidentally change the value returned from the script
on early exits at pre-checks.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../selftests/bpf/test_bpftool_build.sh       | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index 4ba5a34bff56..1fc6f6247f9b 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -1,18 +1,6 @@
 #!/bin/bash
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 
-ERROR=0
-TMPDIR=
-
-# If one build fails, continue but return non-0 on exit.
-return_value() {
-	if [ -d "$TMPDIR" ] ; then
-		rm -rf -- $TMPDIR
-	fi
-	exit $ERROR
-}
-trap return_value EXIT
-
 case $1 in
 	-h|--help)
 		echo -e "$0 [-j <n>]"
@@ -20,7 +8,7 @@ case $1 in
 		echo -e ""
 		echo -e "\tOptions:"
 		echo -e "\t\t-j <n>:\tPass -j flag to 'make'."
-		exit
+		exit 0
 		;;
 esac
 
@@ -33,6 +21,18 @@ SCRIPT_REL_DIR=$(dirname $SCRIPT_REL_PATH)
 KDIR_ROOT_DIR=$(realpath $PWD/$SCRIPT_REL_DIR/../../../../)
 cd $KDIR_ROOT_DIR
 
+ERROR=0
+TMPDIR=
+
+# If one build fails, continue but return non-0 on exit.
+return_value() {
+	if [ -d "$TMPDIR" ] ; then
+		rm -rf -- $TMPDIR
+	fi
+	exit $ERROR
+}
+trap return_value EXIT
+
 check() {
 	local dir=$(realpath $1)
 
-- 
2.17.1

[PATCH bpf-next 2/2] selftests: bpftool: skip the build test if not in tree

[Quentin Monnet]

From: Jakub Kicinski <jakub.kicinski@netronome.com>

If selftests are copied over to another machine/location
for execution the build test of bpftool will obviously
not work, since the sources are not copied.
Skip it if we can't find bpftool's Makefile.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_bpftool_build.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index 1fc6f6247f9b..ac349a5cea7e 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -20,6 +20,10 @@ SCRIPT_REL_PATH=$(realpath --relative-to=$PWD $0)
 SCRIPT_REL_DIR=$(dirname $SCRIPT_REL_PATH)
 KDIR_ROOT_DIR=$(realpath $PWD/$SCRIPT_REL_DIR/../../../../)
 cd $KDIR_ROOT_DIR
+if [ ! -e tools/bpf/bpftool/Makefile ]; then
+	echo -e "skip:    bpftool files not found!\n"
+	exit 0
+fi
 
 ERROR=0
 TMPDIR=
-- 
2.17.1

Re: [PATCH bpf-next 0/2] selftests: bpftool: skip build tests if not in tree

[Daniel Borkmann]

On Tue, Nov 19, 2019 at 10:50:08AM +0000, Quentin Monnet wrote:
> The build test script for bpftool attempts to detect the toplevel path of
> the kernel repository and attempts to build bpftool from there.
> 
> If it fails to find the correct directory, or if bpftool files are missing
> for another reason (e.g. kselftests built on a first machine and copied
> onto another, without bpftool sources), then it is preferable to skip the
> tests entirely rather than dumping useless error messages.
> 
> The first patch moves the EXIT trap in the script lower down in the code,
> to avoid tampering with return value on early exits at the beginning of the
> script; then the second patch makes sure that we skip the build tests if
> bpftool's Makefile is not found at its expected location.
> 
> Jakub Kicinski (1):
>   selftests: bpftool: skip the build test if not in tree
> 
> Quentin Monnet (1):
>   selftests: bpftool: set EXIT trap after usage function
> 
>  .../selftests/bpf/test_bpftool_build.sh       | 30 +++++++++++--------
>  1 file changed, 17 insertions(+), 13 deletions(-)

Applied, thanks!