| Message ID | 20191202131847.30837-7-jolsa@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | perf/bpftool: Allow to link libbpf dynamically | expand |
Thanks Jiri ! A few comments inline. 2019-12-02 14:18 UTC+0100 ~ Jiri Olsa <jolsa@kernel.org> > Adding new test to test_bpftool_build.sh script to > test the dynamic linkage of libbpf for bpftool: > > $ ./test_bpftool_build.sh > [SNIP] > > ... with dynamic libbpf > > $PWD: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool > command: make -s -C ../../build/feature clean >/dev/null > command: make -s -C ../../lib/bpf clean >/dev/null > command: make -s -C ../../lib/bpf prefix=/tmp/tmp.fG8O2Ps8ER install_lib install_headers >/dev/null > Parsed description of 117 helper function(s) > command: make -s clean >/dev/null > command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/tmp.fG8O2Ps8ER >/dev/null > binary: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool/bpftool > binary: linked with libbpf > > The test installs libbpf into temp directory > and links bpftool dynamically with it. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > .../selftests/bpf/test_bpftool_build.sh | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh > index ac349a5cea7e..e4a6a0520f8e 100755 > --- a/tools/testing/selftests/bpf/test_bpftool_build.sh > +++ b/tools/testing/selftests/bpf/test_bpftool_build.sh > @@ -85,6 +85,55 @@ make_with_tmpdir() { > echo > } > > +# Assumes current directory is tools/bpf/bpftool > +make_with_dynamic_libbpf() { > + TMPDIR=$(mktemp -d) > + echo -e "\$PWD: $PWD" > + > + # It might be needed to clean build tree first because features > + # framework does not detect the change properly > + echo -e "command: make -s -C ../../build/feature clean >/dev/null" (So far I did not echo the "make clean" commands, I printed only the ones used to build bpftool. But that's your call.) > + make $J -s -C ../../build/feature clean >/dev/null > + if [ $? -ne 0 ] ; then > + ERROR=1 > + fi > + echo -e "command: make -s -C ../../lib/bpf clean >/dev/null" > + make $J -s -C ../../lib/bpf clean >/dev/null > + if [ $? -ne 0 ] ; then > + ERROR=1 > + fi > + > + # Now install libbpf into TMPDIR > + echo -e "command: make -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null" > + make $J -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null > + if [ $? -ne 0 ] ; then > + ERROR=1 > + fi > + > + # And final bpftool build (with clean first) with libbpf dynamic link > + echo -e "command: make -s clean >/dev/null" > + if [ $? -ne 0 ] ; then > + ERROR=1 > + fi I do not believe you need to "make clean" here, this should have been done by the previous test in that dir earlier in the script (cd tools/bpf/bpftool; make_and_clean) > + echo -e "command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null" > + make $J -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null > + if [ $? -ne 0 ] ; then > + ERROR=1 > + fi > + > + check . > + ldd bpftool | grep -q libbpf.so > + if [ $? -ne 0 ] ; then (Or "if ldd bpftool | grep -q libbpf.so ; then") > + printf "FAILURE: Did not find libbpf linked\n" Please also set $(ERROR) here. (Also, stick to echo rather than mixing with printf? I can't remember why I used one over echo in the check() function, that was probably not on purpose.) > + else > + echo "binary: linked with libbpf" > + fi > + make -s -C ../../lib/bpf clean > + make -s clean > + rm -rf -- $TMPDIR We probably want to clean features too? We tried to check that libbpf was available, but with a very specific $(LIBBPF_DIR), which was temporary and no longer exist. So better to reset features to a clean state? > + echo > +} > + > echo "Trying to build bpftool" > echo -e "... through kbuild\n" > > @@ -145,3 +194,7 @@ make_and_clean > make_with_tmpdir OUTPUT > > make_with_tmpdir O > + > +echo -e "... with dynamic libbpf\n" > + > +make_with_dynamic_libbpf > Thanks, Quentin
diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh index ac349a5cea7e..e4a6a0520f8e 100755 --- a/tools/testing/selftests/bpf/test_bpftool_build.sh +++ b/tools/testing/selftests/bpf/test_bpftool_build.sh @@ -85,6 +85,55 @@ make_with_tmpdir() { echo } +# Assumes current directory is tools/bpf/bpftool +make_with_dynamic_libbpf() { + TMPDIR=$(mktemp -d) + echo -e "\$PWD: $PWD" + + # It might be needed to clean build tree first because features + # framework does not detect the change properly + echo -e "command: make -s -C ../../build/feature clean >/dev/null" + make $J -s -C ../../build/feature clean >/dev/null + if [ $? -ne 0 ] ; then + ERROR=1 + fi + echo -e "command: make -s -C ../../lib/bpf clean >/dev/null" + make $J -s -C ../../lib/bpf clean >/dev/null + if [ $? -ne 0 ] ; then + ERROR=1 + fi + + # Now install libbpf into TMPDIR + echo -e "command: make -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null" + make $J -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null + if [ $? -ne 0 ] ; then + ERROR=1 + fi + + # And final bpftool build (with clean first) with libbpf dynamic link + echo -e "command: make -s clean >/dev/null" + if [ $? -ne 0 ] ; then + ERROR=1 + fi + echo -e "command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null" + make $J -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null + if [ $? -ne 0 ] ; then + ERROR=1 + fi + + check . + ldd bpftool | grep -q libbpf.so + if [ $? -ne 0 ] ; then + printf "FAILURE: Did not find libbpf linked\n" + else + echo "binary: linked with libbpf" + fi + make -s -C ../../lib/bpf clean + make -s clean + rm -rf -- $TMPDIR + echo +} + echo "Trying to build bpftool" echo -e "... through kbuild\n" @@ -145,3 +194,7 @@ make_and_clean make_with_tmpdir OUTPUT make_with_tmpdir O + +echo -e "... with dynamic libbpf\n" + +make_with_dynamic_libbpf
Adding new test to test_bpftool_build.sh script to test the dynamic linkage of libbpf for bpftool: $ ./test_bpftool_build.sh [SNIP] ... with dynamic libbpf $PWD: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool command: make -s -C ../../build/feature clean >/dev/null command: make -s -C ../../lib/bpf clean >/dev/null command: make -s -C ../../lib/bpf prefix=/tmp/tmp.fG8O2Ps8ER install_lib install_headers >/dev/null Parsed description of 117 helper function(s) command: make -s clean >/dev/null command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/tmp.fG8O2Ps8ER >/dev/null binary: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool/bpftool binary: linked with libbpf The test installs libbpf into temp directory and links bpftool dynamically with it. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- .../selftests/bpf/test_bpftool_build.sh | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+)