Checks
Commit Message
For normal developers, those checks are disabled.
Enabling them requires a configuration that will trigger the ABI dumps
generation as part of the existing devtools/test-build.sh and
devtools/test-meson-builds.sh scripts.
Those checks are enabled in the CI for the default meson options on x86
and aarch64 so that proposed patches are validated via our CI robot.
A cache of the ABI is stored in travis jobs to avoid rebuilding too
often.
Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
breaking the ABI in a future release.
Explicit suppression rules have been added on internal structures
exposed to crypto drivers as the current ABI policy does not apply to
them.
This could be improved in the future by carefully splitting the headers
content with application and driver "users" in mind.
We currently have issues reported for librte_crypto recent changes for
which suppression rules have been added too.
Mellanox glue libraries are explicitly skipped as they are not part of
the application ABI.
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
Changelog since v2:
- forced -g / buildtype=debugoptimised in the test scripts so that
we can check ABI in existing environments,
- little update on the documentation,
Changelog since v1:
- reworked the scripts so that the build test scripts clone and build the
reference automatically. A developer only needs to set one variable to
enable the checks,
- meson builds are done with debug so that abidiff can inspect the
structures,
- abidiff checks only public types by looking at installed headers,
- abidiff has some issue when comparing a dump with a .so built with clang
so all diff are now done with dump files only,
- suppression rules have been added to waive warnings on exposed internal
types,
- an abi breakage has been reported on changes in cryptodev.
For now, suppression rules have been put in place to let the CI run,
---
.ci/linux-build.sh | 23 +++++++++++
.travis.yml | 20 +++++++++-
MAINTAINERS | 2 +
devtools/check-abi.sh | 59 +++++++++++++++++++++++++++++
devtools/dpdk.abignore | 21 ++++++++++
devtools/gen-abi.sh | 26 +++++++++++++
devtools/test-build.sh | 51 ++++++++++++++++++++++---
devtools/test-meson-builds.sh | 37 +++++++++++++++++-
doc/guides/contributing/patches.rst | 15 ++++++++
9 files changed, 246 insertions(+), 8 deletions(-)
create mode 100755 devtools/check-abi.sh
create mode 100644 devtools/dpdk.abignore
create mode 100755 devtools/gen-abi.sh
Comments
30/01/2020 17:00, David Marchand:
> Enabling them requires a configuration that will trigger the ABI dumps
> generation as part of the existing devtools/test-build.sh and
> devtools/test-meson-builds.sh scripts.
[...]
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> if [ -f "$builddir/build.ninja" ] ; then
> + # for existing environments, force debugoptimized so that ABI
> + # checks can run
> + $MESON configure --buildtype=debugoptimized $builddir
This is forcing meson to re-run each time, even if the buildtype is
already "debugoptimized".
Please query meson configuration to avoid useless re-run:
$MESON configure $builddir | awk '$1=="buildtype" {print $2}'
30/01/2020 17:00, David Marchand:
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> +refsrcdir=$(mktemp -d -t dpdk-${DPDK_ABI_REF_VERSION:-}.XXX)
Instead of a temporary source directory,
could it be inside $DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION ?
I feel it would more "hackable" for debugging of the process.
[..]
> + if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> + DPDK_ABI_REF_DIR=${DPDK_ABI_REF_DIR:-reference}
> + abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION/$targetdir
> + if [ ! -d $abirefdir ]; then
> + # clone current sources
> + if [ ! -d $refsrcdir/.git ]; then
> + git clone --local --no-hardlinks \
> + --single-branch \
> + -b $DPDK_ABI_REF_VERSION \
> + $srcdir $refsrcdir
> + fi
> +
> + rm -rf $refsrcdir/build
> + config $refsrcdir $refsrcdir/build $*
> + compile $refsrcdir/build $abirefdir
> + $srcdir/devtools/gen-abi.sh $abirefdir
> + fi
On Thu, Jan 30, 2020 at 11:33 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 30/01/2020 17:00, David Marchand:
> > Enabling them requires a configuration that will trigger the ABI dumps
> > generation as part of the existing devtools/test-build.sh and
> > devtools/test-meson-builds.sh scripts.
> [...]
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > if [ -f "$builddir/build.ninja" ] ; then
> > + # for existing environments, force debugoptimized so that ABI
> > + # checks can run
> > + $MESON configure --buildtype=debugoptimized $builddir
>
> This is forcing meson to re-run each time, even if the buildtype is
> already "debugoptimized".
> Please query meson configuration to avoid useless re-run:
>
> $MESON configure $builddir | awk '$1=="buildtype" {print $2}'
>
>
Good catch, will fix.
@@ -30,6 +30,7 @@ fi
OPTS="$OPTS --default-library=$DEF_LIB"
OPTS="$OPTS --prefix=/usr -Dlibdir=lib"
+OPTS="$OPTS --buildtype=debugoptimized"
meson build --werror -Dexamples=all $OPTS
ninja -C build
DESTDIR=$(pwd)/install ninja -C build install
@@ -40,6 +41,28 @@ if [ "$AARCH64" != "1" ]; then
unset LD_LIBRARY_PATH
fi
+if [ "$ABI_CHECKS" = "1" ]; then
+ REF_GIT_REPO=${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
+ REF_GIT_TAG=${REF_GIT_TAG:-v19.11}
+
+ if [ "$(cat reference/VERSION 2>/dev/null)" != "$REF_GIT_TAG" ]; then
+ rm -rf reference
+ fi
+
+ if [ ! -d reference ]; then
+ refsrcdir=$(readlink -f $(pwd)/../dpdk-$REF_GIT_TAG)
+ git clone --single-branch -b $REF_GIT_TAG $REF_GIT_REPO $refsrcdir
+ meson --werror $OPTS $refsrcdir $refsrcdir/build
+ ninja -C $refsrcdir/build
+ DESTDIR=$(pwd)/reference ninja -C $refsrcdir/build install
+ devtools/gen-abi.sh reference
+ echo $REF_GIT_TAG > reference/VERSION
+ fi
+
+ devtools/gen-abi.sh install
+ devtools/check-abi.sh reference install ${ABI_CHECKS_WARN_ONLY:-}
+fi
+
if [ "$RUN_TESTS" = "1" ]; then
sudo meson test -C build --suite fast-tests -t 3
fi
@@ -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
@@ -151,5 +154,18 @@ matrix:
packages:
- *required_packages
- *doc_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *extra_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+ arch: arm64
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *extra_packages
script: ./.ci/${TRAVIS_OS_NAME}-build.sh
@@ -144,8 +144,10 @@ M: Neil Horman <nhorman@tuxdriver.com>
F: lib/librte_eal/common/include/rte_compat.h
F: lib/librte_eal/common/include/rte_function_versioning.h
F: doc/guides/rel_notes/deprecation.rst
+F: devtools/check-abi.sh
F: devtools/check-abi-version.sh
F: devtools/check-symbol-change.sh
+F: devtools/gen-abi.sh
F: devtools/update-abi.sh
F: devtools/update_version_map_abi.py
F: devtools/validate-abi.sh
new file mode 100755
@@ -0,0 +1,59 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ] && [ $# != 3 ]; then
+ echo "Usage: $0 refdir newdir [warnonly]"
+ exit 1
+fi
+
+refdir=$1
+newdir=$2
+warnonly=${3:-}
+ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore --no-added-syms"
+
+if [ ! -d $refdir ]; then
+ echo "Error: reference directory '$refdir' does not exist."
+ exit 1
+fi
+incdir=$(find $refdir -type d -a -name include)
+if [ -z "$incdir" ] || [ ! -e "$incdir" ]; then
+ echo "WARNING: could not identify a include directory for $refdir, expect false positives..."
+else
+ ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir1 $incdir"
+fi
+
+if [ ! -d $newdir ]; then
+ echo "Error: directory to check '$newdir' does not exist."
+ exit 1
+fi
+incdir2=$(find $newdir -type d -a -name include)
+if [ -z "$incdir2" ] || [ ! -e "$incdir2" ]; then
+ echo "WARNING: could not identify a include directory for $newdir, expect false positives..."
+else
+ ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir2 $incdir2"
+fi
+
+error=
+for dump in $(find $refdir -name "*.dump"); do
+ name=$(basename $dump)
+ # skip glue drivers, example librte_pmd_mlx5_glue.dump
+ # We can't rely on a suppression rule for now:
+ # https://sourceware.org/bugzilla/show_bug.cgi?id=25480
+ if [ "$name" != "${name%%_glue.dump}" ]; then
+ echo "Skipping ${dump}..."
+ continue
+ fi
+ dump2=$(find $newdir -name $name)
+ if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
+ echo "Error: can't find $name in $newdir"
+ error=1
+ continue
+ fi
+ if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
+ echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
+ error=1
+ fi
+done
+
+[ -z "$error" ] || [ -n "$warnonly" ]
new file mode 100644
@@ -0,0 +1,21 @@
+[suppress_function]
+ symbol_version = EXPERIMENTAL
+[suppress_variable]
+ symbol_version = EXPERIMENTAL
+
+; Explicit ignore for driver-only ABI
+[suppress_type]
+ name = rte_cryptodev_ops
+; Ignore this enum update as it is part of an experimental API
+[suppress_type]
+ type_kind = enum
+ name = rte_crypto_asym_xform_type
+ changed_enumerators = RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
+
+; FIXME
+[suppress_type]
+ type_kind = enum
+ name = rte_crypto_aead_algorithm
+ changed_enumerators = RTE_CRYPTO_AEAD_LIST_END
+[suppress_variable]
+ name = rte_crypto_aead_algorithm_strings
new file mode 100755
@@ -0,0 +1,26 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 1 ]; then
+ echo "Usage: $0 installdir"
+ exit 1
+fi
+
+installdir=$1
+if [ ! -d $installdir ]; then
+ echo "Error: install directory '$installdir' does not exist."
+ exit 1
+fi
+
+dumpdir=$installdir/dump
+rm -rf $dumpdir
+mkdir -p $dumpdir
+for f in $(find $installdir -name "*.so.*"); do
+ if test -L $f; then
+ continue
+ fi
+
+ libname=$(basename $f)
+ abidw --out-file $dumpdir/${libname%.so*}.dump $f
+done
@@ -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,10 +65,12 @@ print_help () {
[ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
J=$DPDK_MAKE_JOBS
+refsrcdir=$(mktemp -d -t dpdk-${DPDK_ABI_REF_VERSION:-}.XXX)
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
short=false
unset verbose
-maxerr=-Wfatal-errors
+# for ABI checks, we need debuginfo
+test_cflags="-Wfatal-errors -g"
while getopts hj:sv ARG ; do
case $ARG in
j ) J=$OPTARG ;;
@@ -91,13 +94,14 @@ on_exit ()
[ "$DPDK_NOTIFY" != notify-send ] || \
notify-send -u low --icon=dialog-error 'DPDK build' 'failed'
fi
+ rm -rf $refsrcdir
}
# catch manual interrupt to ignore notification
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,14 +237,14 @@ 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
config $dir $target $options
echo "================== Build $conf"
- ${MAKE} -j$J EXTRA_CFLAGS="$maxerr $DPDK_DEP_CFLAGS" \
+ ${MAKE} -j$J EXTRA_CFLAGS="$test_cflags $DPDK_DEP_CFLAGS" \
EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose O=$dir
! $short || break
export RTE_TARGET=$target
@@ -253,6 +257,43 @@ for conf in $configs ; do
EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose \
O=$(readlink -f $dir)/examples
unset RTE_TARGET
+ if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+ DPDK_ABI_REF_DIR=${DPDK_ABI_REF_DIR:-reference}
+ abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION/$conf
+ if [ ! -d $abirefdir ]; then
+ # clone current sources
+ if [ ! -d $refsrcdir/.git ]; then
+ git clone --local --no-hardlinks \
+ --single-branch \
+ -b $DPDK_ABI_REF_VERSION \
+ $(pwd) $refsrcdir
+ fi
+
+ cd $refsrcdir
+
+ rm -rf build
+ config build $target $options
+
+ echo -n "================== Build $conf "
+ echo "($DPDK_ABI_REF_VERSION)"
+ ${MAKE} -j$J \
+ EXTRA_CFLAGS="$test_cflags $DPDK_DEP_CFLAGS" \
+ EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose \
+ O=build
+ ! $short || break
+ export RTE_TARGET=$target
+ ${MAKE} install O=build DESTDIR=$abirefdir \
+ prefix=
+ $devtools_dir/gen-abi.sh $abirefdir
+
+ # back to current workdir
+ cd $devtools_dir/..
+ fi
+
+ echo "================== Check ABI $conf"
+ $devtools_dir/gen-abi.sh $dir/install
+ $devtools_dir/check-abi.sh $abirefdir $dir/install
+ fi
echo "################## $conf done."
unset dir
done
@@ -16,8 +16,15 @@ srcdir=$(dirname $(readlink -f $0))/..
MESON=${MESON:-meson}
use_shared="--default-library=shared"
+refsrcdir=$(mktemp -d -t dpdk-${DPDK_ABI_REF_VERSION:-}.XXX)
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+on_exit ()
+{
+ rm -rf $refsrcdir
+}
+trap on_exit EXIT
+
if command -v gmake >/dev/null 2>&1 ; then
MAKE=gmake
else
@@ -64,9 +71,14 @@ config () # <dir> <builddir> <meson options>
builddir=$1
shift
if [ -f "$builddir/build.ninja" ] ; then
+ # for existing environments, force debugoptimized so that ABI
+ # checks can run
+ $MESON configure --buildtype=debugoptimized $builddir
return
fi
- options="--werror -Dexamples=all -Dlibdir=lib"
+ options=
+ options="$options --werror -Dexamples=all -Dlibdir=lib"
+ options="$options --buildtype=debugoptimized"
for option in $DPDK_MESON_OPTIONS ; do
options="$options -D$option"
done
@@ -107,6 +119,29 @@ build () # <directory> <target compiler> <meson options>
config $srcdir $builds_dir/$targetdir $*
compile $builds_dir/$targetdir \
$(readlink -f $builds_dir/$targetdir/install)
+ if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+ DPDK_ABI_REF_DIR=${DPDK_ABI_REF_DIR:-reference}
+ abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION/$targetdir
+ if [ ! -d $abirefdir ]; then
+ # clone current sources
+ if [ ! -d $refsrcdir/.git ]; then
+ git clone --local --no-hardlinks \
+ --single-branch \
+ -b $DPDK_ABI_REF_VERSION \
+ $srcdir $refsrcdir
+ fi
+
+ rm -rf $refsrcdir/build
+ config $refsrcdir $refsrcdir/build $*
+ compile $refsrcdir/build $abirefdir
+ $srcdir/devtools/gen-abi.sh $abirefdir
+ fi
+
+ $srcdir/devtools/gen-abi.sh \
+ $(readlink -f $builds_dir/$targetdir/install)
+ $srcdir/devtools/check-abi.sh $abirefdir \
+ $(readlink -f $builds_dir/$targetdir/install)
+ fi
}
if [ "$1" = "-vv" ] ; then
@@ -513,6 +513,21 @@ 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
+--------------------------
+
+By default, ABI compatibility checks are disabled.
+
+To enable them, a reference version must be selected via the environment
+variable ``DPDK_ABI_REF_VERSION``.
+
+The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts
+then build this reference version in a temporary directory and store the
+results in a subfolder of the current working directory.
+The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go
+to a different location.
+
+
Sending Patches
---------------