-----Original Message-----
From: Richardson, Bruce <bruce.richard...@intel.com>
Sent: Friday 20 December 2019 15:32
To: David Marchand <david.march...@redhat.com>; dev@dpdk.org
Cc: tho...@monjalon.net; Laatz, Kevin <kevin.la...@intel.com>;
acon...@redhat.com; nhor...@tuxdriver.com; Michael Santana
<maicolgabr...@hotmail.com>; Mcnamara, John <john.mcnam...@intel.com>;
Kovacevic, Marko <marko.kovace...@intel.com>; Kinsella, Ray
<ray.kinse...@intel.com>
Subject: RE: [PATCH] add ABI checks
-----Original Message-----
From: David Marchand <david.march...@redhat.com>
Sent: Friday, December 20, 2019 3:21 PM
To: dev@dpdk.org
Cc: tho...@monjalon.net; Richardson, Bruce
<bruce.richard...@intel.com>; Laatz, Kevin <kevin.la...@intel.com>;
acon...@redhat.com; nhor...@tuxdriver.com; Michael Santana
<maicolgabr...@hotmail.com>; Mcnamara, John
<john.mcnam...@intel.com>;
Kovacevic, Marko <marko.kovace...@intel.com>
Subject: [PATCH] add ABI checks
Starting from Kevin and Bruce idea of using libabigail, here is an
alternate approach to implement ABI checks.
By default, those checks are disabled and enabling them requires a
manual step that generates the ABI dumps on a reference version for a
set of configurations.
Those checks are enabled in the CI by default for the default meson
options on x86 and aarch64 so that proposed patches are validated.
A cache of the ABI is stored in travis jobs.
Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
breaking the ABI in a future release.
For advanced developers and maintainers, the contributing guide
details the higher level scripts that are quite close to the existing
devtools scripts.
Signed-off-by: David Marchand <david.march...@redhat.com>
---
.ci/linux-build.sh | 43 +++++++++++++++++++++++++++
.travis.yml | 20 +++++++++++--
devtools/check-abi-dump.sh | 46
+++++++++++++++++++++++++++++
devtools/check-abi-reference.sh | 27 +++++++++++++++++
devtools/dpdk.abignore | 2 ++
devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
devtools/gen-abi-reference.sh | 24 +++++++++++++++
devtools/test-build.sh | 13 ++++++--
devtools/test-meson-builds.sh | 6 ++++
doc/guides/contributing/patches.rst | 25 ++++++++++++++++
10 files changed, 230 insertions(+), 5 deletions(-) create mode
100755 devtools/check-abi-dump.sh create mode 100755
devtools/check-abi- reference.sh create mode 100644
devtools/dpdk.abignore create mode
100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
reference.sh
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
ccc3a7ccd..345dba264 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -30,8 +30,51 @@ fi
OPTS="$OPTS --default-library=$DEF_LIB"
meson build --werror -Dexamples=all $OPTS
+
+if [ "$ABI_CHECKS" = "1" ]; then
+ git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
+ git fetch --tags ref ${REF_GIT_BRANCH:-master}
+
+ head=$(git describe --all)
+ tag=$(git describe --abbrev=0)
+
+ if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
+ rm -rf reference
+ fi
+
+ if [ ! -d reference ]; then
+ gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
+ cp -a devtools/gen-abi-dump.sh $gen_abi_dump
+
+ git checkout -qf $tag
+ ninja -C build
+ $gen_abi_dump build reference
+
+ if [ "$AARCH64" != "1" ]; then
+ mkdir -p reference/app
+ cp -a build/app/dpdk-testpmd reference/app/
+
+ export
LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+ devtools/test-null.sh reference/app/dpdk-testpmd
+ unset LD_LIBRARY_PATH
+ fi
+ echo $tag > reference/VERSION
+
+ git checkout -qf $head
+ fi
+fi
+
ninja -C build
+if [ "$ABI_CHECKS" = "1" ]; then
+ devtools/check-abi-dump.sh build reference
${ABI_CHECKS_WARN_ONLY:-}
+ if [ "$AARCH64" != "1" ]; then
+ export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+ devtools/test-null.sh reference/app/dpdk-testpmd
+ unset LD_LIBRARY_PATH
+ fi
+fi
+
if [ "$AARCH64" != "1" ]; then
devtools/test-null.sh
fi
diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,8 @@
language: c
-cache: ccache
+cache:
+ ccache: true
+ directories:
+ - reference
compiler:
- gcc
- clang
@@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
extra_packages: &extra_packages
- *required_packages
- - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
+ - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
+ abigail-tools]
build_32b_packages: &build_32b_packages
- *required_packages
@@ -59,6 +62,13 @@ matrix:
apt:
packages:
- *aarch64_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *aarch64_packages
+ - *extra_packages
- env: DEF_LIB="static" EXTRA_PACKAGES=1
compiler: gcc
addons:
@@ -72,6 +82,12 @@ matrix:
packages:
- *extra_packages
- *doc_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *extra_packages
- env: DEF_LIB="static" OPTS="-Denable_kmods=false"
EXTRA_PACKAGES=1
compiler: gcc
addons:
diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
new file mode 100755 index 000000000..f48a2ae7e
--- /dev/null
+++ b/devtools/check-abi-dump.sh
@@ -0,0 +1,46 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
Hat,
+Inc.
+
+if [ $# != 2 ] && [ $# != 3 ]; then
+ echo "Usage: $0 builddir dumpdir [warnonly]"
+ exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+warnonly=${3:-}
+if [ ! -d $builddir ]; then
+ echo "Error: build directory '$builddir' does not exist."
+ exit 1
+fi
+if [ ! -d $dumpdir ]; then
+ echo "Error: dump directory '$dumpdir' does not exist."
+ exit 1
+fi
+
+ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
+for dump in $(find $dumpdir -name "*.dump"); do
+ libname=$(basename $dump)
+ libname=${libname%.dump}
+ result=
+ for f in $(find $builddir -name "$libname.so.*"); do
+ if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+ continue
+ fi
+ result=found
+
+ if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
+ if [ -z "$warnonly" ]; then
+ echo "Error: ABI issue reported for $dump, $f"
+ exit 1
+ fi
+ echo "Warning: ABI issue reported for $dump, $f"
+ fi
+ break
+ done
+ if [ "$result" != "found" ]; then
+ echo "Error: can't find a library for dump file $dump"
+ exit 1
+ fi
+done
diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
reference.sh new file mode 100755 index 000000000..7addb094e
--- /dev/null
+++ b/devtools/check-abi-reference.sh
@@ -0,0 +1,27 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
Hat,
+Inc.
+
+devtools_dir=$(dirname $(readlink -f $0)) .
+$devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+
+for dir in $abi_ref_build_dir/*; do
+ if [ "$dir" = "$abi_ref_build_dir" ]; then
+ exit 1
+ fi
+ if [ ! -d $dir/dump ]; then
+ echo "Skipping $dir"
+ continue
+ fi
+ target=$(basename $dir)
+ if [ -d $builds_dir/$target/install ]; then
+ libdir=$builds_dir/$target/install
+ else
+ libdir=$builds_dir/$target
+ fi
+ echo "Checking ABI between $libdir and $dir/dump"
+ $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file
mode
100644 index 000000000..b866b7f26
--- /dev/null
+++ b/devtools/dpdk.abignore
@@ -0,0 +1,2 @@
+[suppress_function]
+ symbol_version = EXPERIMENTAL
diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new
file mode 100755 index 000000000..4e38d751f
--- /dev/null
+++ b/devtools/gen-abi-dump.sh
@@ -0,0 +1,29 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
Hat,
+Inc.
+
+if [ $# != 2 ]; then
+ echo "Usage: $0 builddir dumpdir"
+ exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+if [ ! -d $builddir ]; then
+ echo "Error: build directory '$builddir' does not exist."
+ exit 1
+fi
+if [ -d $dumpdir ]; then
+ echo "Error: dump directory '$dumpdir' already exists."
+ exit 1
+fi
+
+mkdir -p $dumpdir
+for f in $(find $builddir -name "*.so.*"); do
+ if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+ continue
+ fi
+
+ libname=$(basename $f)
+ abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
diff --git a/devtools/gen-abi-reference.sh
b/devtools/gen-abi-reference.sh new file mode 100755 index
000000000..f41d7fadc
--- /dev/null
+++ b/devtools/gen-abi-reference.sh
@@ -0,0 +1,24 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
Hat,
+Inc.
+
+devtools_dir=$(dirname $(readlink -f $0)) .
+$devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+for dir in $abi_ref_build_dir/*; do
+ if [ "$dir" = "$abi_ref_build_dir" ]; then
+ exit 1
+ fi
+ if [ -d $dir/dump ]; then
+ echo "Skipping $dir"
+ continue
+ fi
+ if [ -d $dir/install ]; then
+ libdir=$dir/install
+ else
+ libdir=$dir
+ fi
+ echo "Dumping libraries from $libdir in $dir/dump"
+ $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
52305fbb8..8cb5b56fb 100755
--- a/devtools/test-build.sh
+++ b/devtools/test-build.sh
@@ -30,7 +30,8 @@ default_path=$PATH
# - LIBSSO_SNOW3G_PATH
# - LIBSSO_KASUMI_PATH
# - LIBSSO_ZUC_PATH
-. $(dirname $(readlink -f $0))/load-devel-config
+devtools_dir=$(dirname $(readlink -f $0)) .
+$devtools_dir/load-devel-config
print_usage () {
echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
...]]"
@@ -64,6 +65,7 @@ print_help () {
[ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
J=$DPDK_MAKE_JOBS
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
short=false
unset verbose
@@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
#
notify result on exit trap on_exit EXIT
-cd $(dirname $(readlink -f $0))/..
+cd $devtools_dir/..
reset_env ()
{
@@ -233,7 +235,7 @@ for conf in $configs ; do
# reload config with DPDK_TARGET set
DPDK_TARGET=$target
reset_env
- . $(dirname $(readlink -f $0))/load-devel-config
+ . $devtools_dir/load-devel-config
options=$(echo $conf | sed 's,[^~+]*,,')
dir=$builds_dir/$conf
@@ -246,6 +248,11 @@ for conf in $configs ; do
export RTE_TARGET=$target
rm -rf $dir/install
${MAKE} install O=$dir DESTDIR=$dir/install prefix=
+ if [ -d $abi_ref_build_dir/$conf/dump ]; then
+ echo "================== Check ABI $conf"
+ $devtools_dir/check-abi-dump.sh $dir/install \
+ $abi_ref_build_dir/$conf/dump
+ fi
echo "================== Build examples for $conf"
export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
diff --git a/devtools/test-meson-builds.sh
b/devtools/test-meson-builds.sh index 688567714..aaefa38a2 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
MESON=${MESON:-meson}
use_shared="--default-library=shared"
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build
() # <directory> <target compiler> <meson options>
echo "$ninja_cmd -C $builddir"
$ninja_cmd -C $builddir
fi
+
+ if [ -d $abi_ref_build_dir/$1/dump ]; then
+ $srcdir/devtools/check-abi-dump.sh $builddir
+ $abi_ref_build_dir/$1/dump
+ fi
}
if [ "$1" = "-vv" ] ; then
diff --git a/doc/guides/contributing/patches.rst
b/doc/guides/contributing/patches.rst
index 0686450e4..de3dff145 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -513,6 +513,31 @@ in a single subfolder called "__builds" created
in the current directory.
Setting ``DPDK_BUILD_TEST_DIR`` to an absolute directory path e.g.
``/tmp`` is also supported.
+Checking ABI compatibility
+--------------------------
+
+The first thing is to build reference binaries for the latest
release
+your patches are built on top of.
+
+Either you are in a git tree and an easy way to identify this is to
run::
+
+ git checkout $(git describe --abbrev=0)
+
+Or you use a tarball and you extract the sources in a director of
+your
choice.
+
+Next is building those sources, refer to the previous paragraph.
+You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
+occur in this directory.
+
+Finally, the ABI dump files are generated with the
+``devtools/gen-abi-reference.sh`` script. This script will look for
+builds in the current sub directory ``reference``. But you can set
+the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
location.
+
+Once done, you can check your current binaries ABI with this
+reference with the ``devtools/check-abi-reference.sh`` script.
+
+
Sending Patches
---------------
--
2.23.0
I still very much dislike forcing the user to generate his own
reference version to compare the ABI against. These should be archived
and the user should just be able to pull them down via git or http or
otherwise. Two reasons for this:
1. Less error prone, since there is no chance of the user having an
incorrect build for whatever reason.
