DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ABI: Add abi checking utility
@ 2015-01-30 21:16 Neil Horman
  2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Neil Horman @ 2015-01-30 21:16 UTC (permalink / raw)
  To: dev

There was a request for an abi validation utiltyfor the ongoing ABI stability
work.  As it turns out there is a abi compliance checker in development that
seems to be under active development and provides fairly detailed ABI compliance
reports.  Its not yet intellegent enough to understand symbol versioning, but it
does provide the ability to identify symbols which have changed between
releases, along with details of the change, and offers develoeprs the
opportunity to identify which symbols then need versioning and validation for a
given update via manaul testing.

This script automates the use of the compliance checker between two arbitrarily
specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
and run:

./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG

where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
suitable for passing as the T= variable in the make config command.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
 scripts/validate_abi.sh | 244 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 244 insertions(+)
 create mode 100755 scripts/validate_abi.sh

diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
new file mode 100755
index 0000000..9ba28da
--- /dev/null
+++ b/scripts/validate_abi.sh
@@ -0,0 +1,244 @@
+#!/bin/sh
+#   BSD LICENSE
+#
+#   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+TAG1=$1
+TAG2=$2
+TARGET=$3
+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+
+usage() {
+	echo "$0 <TAG1> <TAG2> <TARGET>"
+}
+
+log() {
+	local level=$1
+	shift
+	echo "$*"
+}
+
+validate_tags() {
+	git tag -l | grep -q "$TAG1"
+	if [ $? -ne 0 ]
+	then 
+		echo "$TAG1 is invalid"
+		return
+	fi
+	git tag -l | grep -q "$TAG2"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG2 is invalid"
+		return
+	fi
+}
+
+validate_args() {
+	if [ -z "$TAG1" ]
+	then
+		echo "Must Specify TAG1"
+		return
+	fi
+	if [ -z "$TAG2" ]
+	then	
+		echo "Must Specify TAG2"
+		return
+	fi
+	if [ -z "$TARGET" ]
+	then
+		echo "Must Specify a build target"
+	fi
+}
+
+
+cleanup_and_exit() {
+	rm -rf $ABI_DIR
+	exit $1
+}
+
+###########################################
+#START
+############################################
+
+#Save the current branch
+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+
+if [ -n "$VERBOSE" ]
+then
+	export VERBOSE=/dev/stdout
+else
+	export VERBOSE=/dev/null
+fi
+
+# Validate that we have all the arguments we need
+res=$(validate_args)
+if [ -n "$res" ]
+then
+	echo $res
+	usage
+	cleanup_and_exit 1
+fi
+
+# Make sure our tags exist
+res=$(validate_tags)
+if [ -n "$res" ]
+then
+	echo $res
+	cleanup_and_exit 1
+fi
+
+ABICHECK=`which abi-compliance-checker 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-compliance-checker utility"
+	cleanup_and_exit 1
+fi
+
+ABIDUMP=`which abi-dumper 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-dumper utility"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "We're going to check and make sure that applications built"
+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from tag $TAG2."
+log "INFO" ""
+
+# Check to make sure we have a clean tree
+git status | grep -q clean
+if [ $? -ne 0 ]
+then
+	log "WARN" "Working directory not clean, aborting"
+	cleanup_and_exit 1
+fi
+
+if [ ! -d ./.git ]
+then
+	log "WARN" "You must be in the root of the dpdk git tree"
+	log "WARN" "You are in $PWD"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "Checking out version $TAG1 of the dpdk"
+# Move to the old version of the tree
+git checkout $TAG1
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG1"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG1. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+# Move to the lib directory
+cd $TARGET/lib
+log "INFO" "COlLECTING ABI INFORMATION FOR $TAG1"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+done
+cd ../..
+
+# Now clean the tree, checkout the second tag, and rebuild
+git clean -f -d
+git reset --hard
+# Move to the new version of the tree
+log "INFO" "Checking out version $TAG2 of the dpdk"
+git checkout $TAG2
+
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG2"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG2. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+cd $TARGET/lib
+log "INFO" "COlLECTING ABI INFORMATION FOR $TAG2"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2 
+done
+cd ../..
+
+# Start comparison of ABI dumps
+for i in `ls $ABI_DIR/*-1.dump`
+do
+	NEWNAME=`basename $i`
+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
+
+	if [ ! -f $ABI_DIR/$OLDNAME ]
+	then
+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
+	fi
+
+	#compare the abi dumps
+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
+done
+
+git reset --hard
+git checkout $CURRENT_BRANCH
+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
+cleanup_and_exit 0
+
+
-- 
2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-01-30 21:16 [dpdk-dev] [PATCH] ABI: Add abi checking utility Neil Horman
@ 2015-02-02 18:18 ` Neil Horman
  2015-02-27 13:48   ` Neil Horman
  2015-03-03 22:18   ` Thomas Monjalon
  2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 27+ messages in thread
From: Neil Horman @ 2015-02-02 18:18 UTC (permalink / raw)
  To: dev

There was a request for an abi validation utiltyfor the ongoing ABI stability
work.  As it turns out there is a abi compliance checker in development that
seems to be under active development and provides fairly detailed ABI compliance
reports.  Its not yet intellegent enough to understand symbol versioning, but it
does provide the ability to identify symbols which have changed between
releases, along with details of the change, and offers develoeprs the
opportunity to identify which symbols then need versioning and validation for a
given update via manaul testing.

This script automates the use of the compliance checker between two arbitrarily
specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
and run:

./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG

where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
suitable for passing as the T= variable in the make config command.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Change Notes:

v2) Fixed some typos as requested by Thomas
---
 scripts/validate_abi.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 241 insertions(+)
 create mode 100755 scripts/validate_abi.sh

diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
new file mode 100755
index 0000000..31583df
--- /dev/null
+++ b/scripts/validate_abi.sh
@@ -0,0 +1,241 @@
+#!/bin/sh
+#   BSD LICENSE
+#
+#   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+TAG1=$1
+TAG2=$2
+TARGET=$3
+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+
+usage() {
+	echo "$0 <TAG1> <TAG2> <TARGET>"
+}
+
+log() {
+	local level=$1
+	shift
+	echo "$*"
+}
+
+validate_tags() {
+	git tag -l | grep -q "$TAG1"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG1 is invalid"
+		return
+	fi
+	git tag -l | grep -q "$TAG2"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG2 is invalid"
+		return
+	fi
+}
+
+validate_args() {
+	if [ -z "$TAG1" ]
+	then
+		echo "Must Specify TAG1"
+		return
+	fi
+	if [ -z "$TAG2" ]
+	then
+		echo "Must Specify TAG2"
+		return
+	fi
+	if [ -z "$TARGET" ]
+	then
+		echo "Must Specify a build target"
+	fi
+}
+
+
+cleanup_and_exit() {
+	rm -rf $ABI_DIR
+	exit $1
+}
+
+###########################################
+#START
+############################################
+
+#Save the current branch
+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+
+if [ -n "$VERBOSE" ]
+then
+	export VERBOSE=/dev/stdout
+else
+	export VERBOSE=/dev/null
+fi
+
+# Validate that we have all the arguments we need
+res=$(validate_args)
+if [ -n "$res" ]
+then
+	echo $res
+	usage
+	cleanup_and_exit 1
+fi
+
+# Make sure our tags exist
+res=$(validate_tags)
+if [ -n "$res" ]
+then
+	echo $res
+	cleanup_and_exit 1
+fi
+
+ABICHECK=`which abi-compliance-checker 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-compliance-checker utility"
+	cleanup_and_exit 1
+fi
+
+ABIDUMP=`which abi-dumper 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-dumper utility"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "We're going to check and make sure that applications built"
+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from tag $TAG2."
+log "INFO" ""
+
+# Check to make sure we have a clean tree
+git status | grep -q clean
+if [ $? -ne 0 ]
+then
+	log "WARN" "Working directory not clean, aborting"
+	cleanup_and_exit 1
+fi
+
+if [ ! -d ./.git ]
+then
+	log "WARN" "You must be in the root of the dpdk git tree"
+	log "WARN" "You are in $PWD"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "Checking out version $TAG1 of the dpdk"
+# Move to the old version of the tree
+git checkout $TAG1
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG1"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG1. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+# Move to the lib directory
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+done
+cd ../..
+
+# Now clean the tree, checkout the second tag, and rebuild
+git clean -f -d
+git reset --hard
+# Move to the new version of the tree
+log "INFO" "Checking out version $TAG2 of the dpdk"
+git checkout $TAG2
+
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG2"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG2. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
+done
+cd ../..
+
+# Start comparison of ABI dumps
+for i in `ls $ABI_DIR/*-1.dump`
+do
+	NEWNAME=`basename $i`
+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
+
+	if [ ! -f $ABI_DIR/$OLDNAME ]
+	then
+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
+	fi
+
+	#compare the abi dumps
+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
+done
+
+git reset --hard
+git checkout $CURRENT_BRANCH
+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
+cleanup_and_exit 0
+
+
-- 
2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
@ 2015-02-27 13:48   ` Neil Horman
  2015-02-27 13:55     ` Thomas Monjalon
  2015-03-03 22:18   ` Thomas Monjalon
  1 sibling, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-02-27 13:48 UTC (permalink / raw)
  To: dev

On Mon, Feb 02, 2015 at 01:18:26PM -0500, Neil Horman wrote:
> There was a request for an abi validation utiltyfor the ongoing ABI stability
> work.  As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI compliance
> reports.  Its not yet intellegent enough to understand symbol versioning, but it
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers develoeprs the
> opportunity to identify which symbols then need versioning and validation for a
> given update via manaul testing.
> 
> This script automates the use of the compliance checker between two arbitrarily
> specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> and run:
> 
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> 
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> Change Notes:
> 
> v2) Fixed some typos as requested by Thomas
> ---
>  scripts/validate_abi.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 241 insertions(+)
>  create mode 100755 scripts/validate_abi.sh
> 
> diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
> new file mode 100755
> index 0000000..31583df
> --- /dev/null
> +++ b/scripts/validate_abi.sh
> @@ -0,0 +1,241 @@
> +#!/bin/sh
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2015 Neil Horman. All rights reserved.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +TAG1=$1
> +TAG2=$2
> +TARGET=$3
> +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
> +
> +usage() {
> +	echo "$0 <TAG1> <TAG2> <TARGET>"
> +}
> +
> +log() {
> +	local level=$1
> +	shift
> +	echo "$*"
> +}
> +
> +validate_tags() {
> +	git tag -l | grep -q "$TAG1"
> +	if [ $? -ne 0 ]
> +	then
> +		echo "$TAG1 is invalid"
> +		return
> +	fi
> +	git tag -l | grep -q "$TAG2"
> +	if [ $? -ne 0 ]
> +	then
> +		echo "$TAG2 is invalid"
> +		return
> +	fi
> +}
> +
> +validate_args() {
> +	if [ -z "$TAG1" ]
> +	then
> +		echo "Must Specify TAG1"
> +		return
> +	fi
> +	if [ -z "$TAG2" ]
> +	then
> +		echo "Must Specify TAG2"
> +		return
> +	fi
> +	if [ -z "$TARGET" ]
> +	then
> +		echo "Must Specify a build target"
> +	fi
> +}
> +
> +
> +cleanup_and_exit() {
> +	rm -rf $ABI_DIR
> +	exit $1
> +}
> +
> +###########################################
> +#START
> +############################################
> +
> +#Save the current branch
> +CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
> +
> +if [ -n "$VERBOSE" ]
> +then
> +	export VERBOSE=/dev/stdout
> +else
> +	export VERBOSE=/dev/null
> +fi
> +
> +# Validate that we have all the arguments we need
> +res=$(validate_args)
> +if [ -n "$res" ]
> +then
> +	echo $res
> +	usage
> +	cleanup_and_exit 1
> +fi
> +
> +# Make sure our tags exist
> +res=$(validate_tags)
> +if [ -n "$res" ]
> +then
> +	echo $res
> +	cleanup_and_exit 1
> +fi
> +
> +ABICHECK=`which abi-compliance-checker 2>/dev/null`
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "Cant find abi-compliance-checker utility"
> +	cleanup_and_exit 1
> +fi
> +
> +ABIDUMP=`which abi-dumper 2>/dev/null`
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "Cant find abi-dumper utility"
> +	cleanup_and_exit 1
> +fi
> +
> +log "INFO" "We're going to check and make sure that applications built"
> +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> +log "INFO" "against DPDK DSOs built from tag $TAG2."
> +log "INFO" ""
> +
> +# Check to make sure we have a clean tree
> +git status | grep -q clean
> +if [ $? -ne 0 ]
> +then
> +	log "WARN" "Working directory not clean, aborting"
> +	cleanup_and_exit 1
> +fi
> +
> +if [ ! -d ./.git ]
> +then
> +	log "WARN" "You must be in the root of the dpdk git tree"
> +	log "WARN" "You are in $PWD"
> +	cleanup_and_exit 1
> +fi
> +
> +log "INFO" "Checking out version $TAG1 of the dpdk"
> +# Move to the old version of the tree
> +git checkout $TAG1
> +
> +# Make sure we configure SHARED libraries
> +# Also turn off IGB and KNI as those require kernel headers to build
> +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> +
> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g
> +
> +# Now configure the build
> +log "INFO" "Configuring DPDK $TAG1"
> +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> +
> +log "INFO" "Building DPDK $TAG1. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1
> +
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "THE BUILD FAILED.  ABORTING"
> +	cleanup_and_exit 1
> +fi
> +
> +# Move to the lib directory
> +cd $TARGET/lib
> +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
> +for i in `ls *.so`
> +do
> +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
> +done
> +cd ../..
> +
> +# Now clean the tree, checkout the second tag, and rebuild
> +git clean -f -d
> +git reset --hard
> +# Move to the new version of the tree
> +log "INFO" "Checking out version $TAG2 of the dpdk"
> +git checkout $TAG2
> +
> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g
> +
> +# Make sure we configure SHARED libraries
> +# Also turn off IGB and KNI as those require kernel headers to build
> +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> +
> +# Now configure the build
> +log "INFO" "Configuring DPDK $TAG2"
> +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> +
> +log "INFO" "Building DPDK $TAG2. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1
> +
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "THE BUILD FAILED.  ABORTING"
> +	cleanup_and_exit 1
> +fi
> +
> +cd $TARGET/lib
> +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
> +for i in `ls *.so`
> +do
> +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
> +done
> +cd ../..
> +
> +# Start comparison of ABI dumps
> +for i in `ls $ABI_DIR/*-1.dump`
> +do
> +	NEWNAME=`basename $i`
> +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> +
> +	if [ ! -f $ABI_DIR/$OLDNAME ]
> +	then
> +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> +	fi
> +
> +	#compare the abi dumps
> +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> +done
> +
> +git reset --hard
> +git checkout $CURRENT_BRANCH
> +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> +cleanup_and_exit 0
> +
> +
> -- 
> 2.1.0
> 
> 
Whats the disposition of this, I don't see it in the repository yet.

Neil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-02-27 13:48   ` Neil Horman
@ 2015-02-27 13:55     ` Thomas Monjalon
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-02-27 13:55 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi Neil,

2015-02-27 08:48, Neil Horman:
> On Mon, Feb 02, 2015 at 01:18:26PM -0500, Neil Horman wrote:
> > There was a request for an abi validation utiltyfor the ongoing ABI stability
> > work.  As it turns out there is a abi compliance checker in development that
> > seems to be under active development and provides fairly detailed ABI compliance
> > reports.  Its not yet intellegent enough to understand symbol versioning, but it
> > does provide the ability to identify symbols which have changed between
> > releases, along with details of the change, and offers develoeprs the
> > opportunity to identify which symbols then need versioning and validation for a
> > given update via manaul testing.
> > 
> > This script automates the use of the compliance checker between two arbitrarily
> > specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> > and run:
> > 
> > ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> > 
> > where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> > suitable for passing as the T= variable in the make config command.
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> Whats the disposition of this, I don't see it in the repository yet.

I plan to review it carefully during next week.
We can integrate this tool until end of March, that's why it was not the
highest priority.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
  2015-02-27 13:48   ` Neil Horman
@ 2015-03-03 22:18   ` Thomas Monjalon
  2015-03-04 11:49     ` Neil Horman
  1 sibling, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-03 22:18 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-02-02 13:18, Neil Horman:
> There was a request for an abi validation utiltyfor the ongoing ABI stability
> work.  As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI compliance
> reports.  Its not yet intellegent enough to understand symbol versioning, but it
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers develoeprs the
> opportunity to identify which symbols then need versioning and validation for a
> given update via manaul testing.

There's a lot of typos in this text. Please check.

> This script automates the use of the compliance checker between two arbitrarily
> specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> and run:
> 
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> 
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> Change Notes:
> 
> v2) Fixed some typos as requested by Thomas
> ---
>  scripts/validate_abi.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 241 insertions(+)
>  create mode 100755 scripts/validate_abi.sh
> 
> diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
> new file mode 100755
> index 0000000..31583df
> --- /dev/null
> +++ b/scripts/validate_abi.sh
> @@ -0,0 +1,241 @@
> +#!/bin/sh
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2015 Neil Horman. All rights reserved.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +TAG1=$1
> +TAG2=$2
> +TARGET=$3
> +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
> +
> +usage() {
> +	echo "$0 <TAG1> <TAG2> <TARGET>"
> +}
> +
> +log() {
> +	local level=$1

level is not used later?

> +	shift
> +	echo "$*"
> +}
> +
> +validate_tags() {
> +	git tag -l | grep -q "$TAG1"
> +	if [ $? -ne 0 ]
> +	then
> +		echo "$TAG1 is invalid"
> +		return
> +	fi
> +	git tag -l | grep -q "$TAG2"
> +	if [ $? -ne 0 ]
> +	then
> +		echo "$TAG2 is invalid"
> +		return
> +	fi
> +}
> +
> +validate_args() {
> +	if [ -z "$TAG1" ]
> +	then
> +		echo "Must Specify TAG1"
> +		return
> +	fi
> +	if [ -z "$TAG2" ]
> +	then
> +		echo "Must Specify TAG2"
> +		return
> +	fi
> +	if [ -z "$TARGET" ]
> +	then
> +		echo "Must Specify a build target"
> +	fi
> +}
> +
> +
> +cleanup_and_exit() {
> +	rm -rf $ABI_DIR
> +	exit $1
> +}

This function could be automatically invoked with trap.

> +###########################################
> +#START
> +############################################
> +
> +#Save the current branch
> +CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`

Will it work when not on any branch?

> +
> +if [ -n "$VERBOSE" ]
> +then
> +	export VERBOSE=/dev/stdout
> +else
> +	export VERBOSE=/dev/null
> +fi
> +
> +# Validate that we have all the arguments we need
> +res=$(validate_args)
> +if [ -n "$res" ]
> +then
> +	echo $res

Should be redirected to stderr >&2

> +	usage
> +	cleanup_and_exit 1
> +fi
> +
> +# Make sure our tags exist
> +res=$(validate_tags)
> +if [ -n "$res" ]
> +then
> +	echo $res
> +	cleanup_and_exit 1
> +fi
> +
> +ABICHECK=`which abi-compliance-checker 2>/dev/null`

Why not using the $() form like above?

I guess this is the tool:
	http://ispras.linuxbase.org/index.php/ABI_compliance_checker

> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "Cant find abi-compliance-checker utility"
> +	cleanup_and_exit 1
> +fi
> +
> +ABIDUMP=`which abi-dumper 2>/dev/null`
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "Cant find abi-dumper utility"
> +	cleanup_and_exit 1
> +fi
> +
> +log "INFO" "We're going to check and make sure that applications built"
> +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> +log "INFO" "against DPDK DSOs built from tag $TAG2."
> +log "INFO" ""
> +
> +# Check to make sure we have a clean tree
> +git status | grep -q clean
> +if [ $? -ne 0 ]
> +then

You may compact in one line:
if git status | grep -q clean ; then

> +	log "WARN" "Working directory not clean, aborting"
> +	cleanup_and_exit 1
> +fi
> +
> +if [ ! -d ./.git ]
> +then
> +	log "WARN" "You must be in the root of the dpdk git tree"
> +	log "WARN" "You are in $PWD"
> +	cleanup_and_exit 1
> +fi

Why not cd $(dirname $0)/.. instead of returning an error?

> +log "INFO" "Checking out version $TAG1 of the dpdk"
> +# Move to the old version of the tree
> +git checkout $TAG1
> +
> +# Make sure we configure SHARED libraries
> +# Also turn off IGB and KNI as those require kernel headers to build
> +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET

Why not tuning configuration after make config in .config file?

> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g

A comment is required (needed for abi-dumper?)

> +# Now configure the build
> +log "INFO" "Configuring DPDK $TAG1"
> +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> +
> +log "INFO" "Building DPDK $TAG1. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1

It would more efficient with a customizable -j option

> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "THE BUILD FAILED.  ABORTING"
> +	cleanup_and_exit 1
> +fi
> +
> +# Move to the lib directory
> +cd $TARGET/lib
> +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
> +for i in `ls *.so`

I think ls is useless.

> +do
> +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
> +done
> +cd ../..
> +
> +# Now clean the tree, checkout the second tag, and rebuild
> +git clean -f -d
> +git reset --hard
> +# Move to the new version of the tree
> +log "INFO" "Checking out version $TAG2 of the dpdk"
> +git checkout $TAG2
> +
> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g
> +
> +# Make sure we configure SHARED libraries
> +# Also turn off IGB and KNI as those require kernel headers to build
> +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> +
> +# Now configure the build
> +log "INFO" "Configuring DPDK $TAG2"
> +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> +
> +log "INFO" "Building DPDK $TAG2. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1
> +
> +if [ $? -ne 0 ]
> +then
> +	log "INFO" "THE BUILD FAILED.  ABORTING"
> +	cleanup_and_exit 1
> +fi
> +
> +cd $TARGET/lib
> +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
> +for i in `ls *.so`
> +do
> +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
> +done
> +cd ../..
> +
> +# Start comparison of ABI dumps
> +for i in `ls $ABI_DIR/*-1.dump`

Why ls?

> +do
> +	NEWNAME=`basename $i`
> +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> +
> +	if [ ! -f $ABI_DIR/$OLDNAME ]
> +	then
> +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> +	fi
> +
> +	#compare the abi dumps
> +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME

Do we need to do a visual check? I didn't try yet.

> +done
> +
> +git reset --hard
> +git checkout $CURRENT_BRANCH
> +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> +cleanup_and_exit 0

So you compare the ABI dumps.
Do we also need to run an app from TAG2 with libs from TAG1?

Thanks Neil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-03 22:18   ` Thomas Monjalon
@ 2015-03-04 11:49     ` Neil Horman
  2015-03-04 12:54       ` Thomas Monjalon
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-04 11:49 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> 2015-02-02 13:18, Neil Horman:
> > There was a request for an abi validation utiltyfor the ongoing ABI stability
> > work.  As it turns out there is a abi compliance checker in development that
> > seems to be under active development and provides fairly detailed ABI compliance
> > reports.  Its not yet intellegent enough to understand symbol versioning, but it
> > does provide the ability to identify symbols which have changed between
> > releases, along with details of the change, and offers develoeprs the
> > opportunity to identify which symbols then need versioning and validation for a
> > given update via manaul testing.
> 
> There's a lot of typos in this text. Please check.
> 
Three.  Theres 3 typos.  But sure, I'll fix them.

><snip>
> > +
> > +usage() {
> > +	echo "$0 <TAG1> <TAG2> <TARGET>"
> > +}
> > +
> > +log() {
> > +	local level=$1
> 
> level is not used later?
> 
Not yet, but you'll note all the log calls start with a log level to add
filtering.  I'd rather leave this here as it doesn't hurt anything and
effectively documents the paramter.

><snip>
> > +	shift
> > +	echo "$*"
> > +}
> > +
> > +validate_tags() {
> > +	git tag -l | grep -q "$TAG1"
> > +	if [ $? -ne 0 ]
> > +	then
> > +		echo "$TAG1 is invalid"
> > +		return
> > +	fi
> > +	git tag -l | grep -q "$TAG2"
> > +	if [ $? -ne 0 ]
> > +	then
> > +		echo "$TAG2 is invalid"
> > +		return
> > +	fi
> > +}
> > +
> > +validate_args() {
> > +	if [ -z "$TAG1" ]
> > +	then
> > +		echo "Must Specify TAG1"
> > +		return
> > +	fi
> > +	if [ -z "$TAG2" ]
> > +	then
> > +		echo "Must Specify TAG2"
> > +		return
> > +	fi
> > +	if [ -z "$TARGET" ]
> > +	then
> > +		echo "Must Specify a build target"
> > +	fi
> > +}
> > +
> > +
> > +cleanup_and_exit() {
> > +	rm -rf $ABI_DIR
> > +	exit $1
> > +}
> 
> This function could be automatically invoked with trap.
> 
Yes, I can add that.

> > +###########################################
> > +#START
> > +############################################
> > +
> > +#Save the current branch
> > +CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
> 
> Will it work when not on any branch?
> 
No it won't, and I honestly wasn't that worried about it, as people
don't/shouldn't make changes in detached head state.  I can add a check to
ensure you're on a branch though.

> > +
> > +if [ -n "$VERBOSE" ]
> > +then
> > +	export VERBOSE=/dev/stdout
> > +else
> > +	export VERBOSE=/dev/null
> > +fi
> > +
> > +# Validate that we have all the arguments we need
> > +res=$(validate_args)
> > +if [ -n "$res" ]
> > +then
> > +	echo $res
> 
> Should be redirected to stderr >&2
> 
Why? this is eactly what I intended.  All the other messages from log are
directed to stdout, so should this be.

> > +	usage
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +# Make sure our tags exist
> > +res=$(validate_tags)
> > +if [ -n "$res" ]
> > +then
> > +	echo $res
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +ABICHECK=`which abi-compliance-checker 2>/dev/null`
> 
> Why not using the $() form like above?
> 
I don't honestly recall, but I do remember fighting trying to get output from
that format for some reason, and just left this as it was, as it wasn't
particularly relevant.

> I guess this is the tool:
> 	http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> 
Correct.

> > +if [ $? -ne 0 ]
> > +then
> > +	log "INFO" "Cant find abi-compliance-checker utility"
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +ABIDUMP=`which abi-dumper 2>/dev/null`
> > +if [ $? -ne 0 ]
> > +then
> > +	log "INFO" "Cant find abi-dumper utility"
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +log "INFO" "We're going to check and make sure that applications built"
> > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > +log "INFO" ""
> > +
> > +# Check to make sure we have a clean tree
> > +git status | grep -q clean
> > +if [ $? -ne 0 ]
> > +then
> 
> You may compact in one line:
> if git status | grep -q clean ; then
> 
I explicitly do execution and error checking on separate lines as I think its
more clear.  You'll find this style consistent in the script.

> > +	log "WARN" "Working directory not clean, aborting"
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +if [ ! -d ./.git ]
> > +then
> > +	log "WARN" "You must be in the root of the dpdk git tree"
> > +	log "WARN" "You are in $PWD"
> > +	cleanup_and_exit 1
> > +fi
> 
> Why not cd $(dirname $0)/.. instead of returning an error?
> 
Why would that help in finding the base of the git tree.  Theres no guarantee
that you are in a subdirectory of a git tree.  I suppose we can try it
recursively until we hit /, but it seems just as easy and clear to tell the user
whats needed.

> > +log "INFO" "Checking out version $TAG1 of the dpdk"
> > +# Move to the old version of the tree
> > +git checkout $TAG1
> > +
> > +# Make sure we configure SHARED libraries
> > +# Also turn off IGB and KNI as those require kernel headers to build
> > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> 
> Why not tuning configuration after make config in .config file?
> 
Because this way we save a reconfig (from a developer viewpoint), you should run
make config again after changing configs, and so this way you save doing that.

> > +export EXTRA_CFLAGS=-g
> > +export EXTRA_LDFLAGS=-g
> 
> A comment is required (needed for abi-dumper?)
> 
Sure.

> > +# Now configure the build
> > +log "INFO" "Configuring DPDK $TAG1"
> > +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> > +
> > +log "INFO" "Building DPDK $TAG1. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> 
> It would more efficient with a customizable -j option
> 
I'm sure it would, I'll look at that in future enhancement. 

> > +if [ $? -ne 0 ]
> > +then
> > +	log "INFO" "THE BUILD FAILED.  ABORTING"
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +# Move to the lib directory
> > +cd $TARGET/lib
> > +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
> > +for i in `ls *.so`
> 
> I think ls is useless.
> 
Um, I don't?  Not sure what you're getting at here.

> > +do
> > +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
> > +done
> > +cd ../..
> > +
> > +# Now clean the tree, checkout the second tag, and rebuild
> > +git clean -f -d
> > +git reset --hard
> > +# Move to the new version of the tree
> > +log "INFO" "Checking out version $TAG2 of the dpdk"
> > +git checkout $TAG2
> > +
> > +export EXTRA_CFLAGS=-g
> > +export EXTRA_LDFLAGS=-g
> > +
> > +# Make sure we configure SHARED libraries
> > +# Also turn off IGB and KNI as those require kernel headers to build
> > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > +
> > +# Now configure the build
> > +log "INFO" "Configuring DPDK $TAG2"
> > +make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> > +
> > +log "INFO" "Building DPDK $TAG2. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> > +
> > +if [ $? -ne 0 ]
> > +then
> > +	log "INFO" "THE BUILD FAILED.  ABORTING"
> > +	cleanup_and_exit 1
> > +fi
> > +
> > +cd $TARGET/lib
> > +log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
> > +for i in `ls *.so`
> > +do
> > +	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
> > +done
> > +cd ../..
> > +
> > +# Start comparison of ABI dumps
> > +for i in `ls $ABI_DIR/*-1.dump`
> 
> Why ls?
> 
Because it preforms the needed action for what I want to do here. Not sure what
you're proposing

> > +do
> > +	NEWNAME=`basename $i`
> > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > +
> > +	if [ ! -f $ABI_DIR/$OLDNAME ]
> > +	then
> > +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> > +	fi
> > +
> > +	#compare the abi dumps
> > +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> 
> Do we need to do a visual check? I didn't try yet.
> 
Yes, it generates an html report of all the symbols exported in a build and
compares them with the alternate version.  That needs manual review.

> > +done
> > +
> > +git reset --hard
> > +git checkout $CURRENT_BRANCH
> > +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> > +cleanup_and_exit 0
> 
> So you compare the ABI dumps.
> Do we also need to run an app from TAG2 with libs from TAG1?
> 
I started down that path, but its not really that helpful, as all it will do is
refuse to run if there is a symbol missing from a later version.  While that
might be helpful, its no where near as through as the full report from the
compliance checker.

The bottom line is that real ABI compliance requires a developer to be aware of
the changes going into the code and how they affect binary layout. A simple
"does it still work" test isn't sufficient.

Neil
> Thanks Neil
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-04 11:49     ` Neil Horman
@ 2015-03-04 12:54       ` Thomas Monjalon
  2015-03-04 14:39         ` Neil Horman
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-04 12:54 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi Neil,

I remove parts that I agree and reply to those which deserve more discussion.

2015-03-04 06:49, Neil Horman:
> On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> > 2015-02-02 13:18, Neil Horman:
> > > +# Validate that we have all the arguments we need
> > > +res=$(validate_args)
> > > +if [ -n "$res" ]
> > > +then
> > > +	echo $res
> > 
> > Should be redirected to stderr >&2
> > 
> Why? this is eactly what I intended.  All the other messages from log are
> directed to stdout, so should this be.

I'm wondering if there's some normal output which could be redirected for
further processing, and some error output.
My comment was not only for this log but also for every error message.

> > I guess this is the tool:
> > 	http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> 
> Correct.

So maybe you should add this URL in the commit log.

> > > +log "INFO" "We're going to check and make sure that applications built"
> > > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > > +log "INFO" ""

> > > +if [ ! -d ./.git ]
> > > +then
> > > +	log "WARN" "You must be in the root of the dpdk git tree"
> > > +	log "WARN" "You are in $PWD"
> > > +	cleanup_and_exit 1
> > > +fi
> > 
> > Why not cd $(dirname $0)/.. instead of returning an error?
> 
> Why would that help in finding the base of the git tree.  Theres no guarantee
> that you are in a subdirectory of a git tree.  I suppose we can try it
> recursively until we hit /, but it seems just as easy and clear to tell the user
> whats needed.

No I'm saying that you could avoid this check by going into the right
directory from the beginning.
We know that the root dir is $(dirname $0)/.. because this script is in
scripts/ directory.

> > > +# Make sure we configure SHARED libraries
> > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > 
> > Why not tuning configuration after make config in .config file?
> > 
> Because this way we save a reconfig (from a developer viewpoint), you should run
> make config again after changing configs, and so this way you save doing that.

No, you run make config once and update .config file. That's the recommended
way to configure DPDK.
defconfig files are default configurations and should stay read-only.

> > > +for i in `ls *.so`
> > 
> > I think ls is useless.
> > 
> Um, I don't?  Not sure what you're getting at here.

I think "for i in *.so" should work.

> > > +	#compare the abi dumps
> > > +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> > 
> > Do we need to do a visual check? I didn't try yet.
> > 
> Yes, it generates an html report of all the symbols exported in a build and
> compares them with the alternate version.  That needs manual review.

OK I think it's important to explain in the commit log.

> > So you compare the ABI dumps.
> > Do we also need to run an app from TAG2 with libs from TAG1?
> 
> I started down that path, but its not really that helpful, as all it will do is
> refuse to run if there is a symbol missing from a later version.  While that
> might be helpful, its no where near as through as the full report from the
> compliance checker.
> 
> The bottom line is that real ABI compliance requires a developer to be aware of
> the changes going into the code and how they affect binary layout. A simple
> "does it still work" test isn't sufficient.

I hope we'll be able to integrate this kind of tool in an automated sanity
check in order to find obvious errors.

Thanks

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-04 12:54       ` Thomas Monjalon
@ 2015-03-04 14:39         ` Neil Horman
  2015-03-04 15:15           ` Thomas Monjalon
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-04 14:39 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Mar 04, 2015 at 01:54:49PM +0100, Thomas Monjalon wrote:
> Hi Neil,
> 
> I remove parts that I agree and reply to those which deserve more discussion.
> 
> 2015-03-04 06:49, Neil Horman:
> > On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> > > 2015-02-02 13:18, Neil Horman:
> > > > +# Validate that we have all the arguments we need
> > > > +res=$(validate_args)
> > > > +if [ -n "$res" ]
> > > > +then
> > > > +	echo $res
> > > 
> > > Should be redirected to stderr >&2
> > > 
> > Why? this is eactly what I intended.  All the other messages from log are
> > directed to stdout, so should this be.
> 
> I'm wondering if there's some normal output which could be redirected for
> further processing, and some error output.
> My comment was not only for this log but also for every error message.
> 

No, the report output is in html format and always to a file, so stdout isn't
used for any inline informational reporting.

> > > I guess this is the tool:
> > > 	http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> > 
> > Correct.
> 
> So maybe you should add this URL in the commit log.
> 
sure, fine.

> > > > +log "INFO" "We're going to check and make sure that applications built"
> > > > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > > > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > > > +log "INFO" ""
> 
> > > > +if [ ! -d ./.git ]
> > > > +then
> > > > +	log "WARN" "You must be in the root of the dpdk git tree"
> > > > +	log "WARN" "You are in $PWD"
> > > > +	cleanup_and_exit 1
> > > > +fi
> > > 
> > > Why not cd $(dirname $0)/.. instead of returning an error?
> > 
> > Why would that help in finding the base of the git tree.  Theres no guarantee
> > that you are in a subdirectory of a git tree.  I suppose we can try it
> > recursively until we hit /, but it seems just as easy and clear to tell the user
> > whats needed.
> 
> No I'm saying that you could avoid this check by going into the right
> directory from the beginning.
> We know that the root dir is $(dirname $0)/.. because this script is in
> scripts/ directory.
> 
That only helps if you start from the right directory.  If you run this command
from some other location, your solution just breaks.

> > > > +# Make sure we configure SHARED libraries
> > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > 
> > > Why not tuning configuration after make config in .config file?
> > > 
> > Because this way we save a reconfig (from a developer viewpoint), you should run
> > make config again after changing configs, and so this way you save doing that.
> 
> No, you run make config once and update .config file. That's the recommended
> way to configure DPDK.
> defconfig files are default configurations and should stay read-only.
They get overwritten when we do the git resets.  Its silly to modify your config
file after you run make config, in the event the make target has to re-read any
modified options and adjust dependent config files accordingly.  I understand
that doesn't happen now, but its common practice for every open source project
in existance.

> 
> > > > +for i in `ls *.so`
> > > 
> > > I think ls is useless.
> > > 
> > Um, I don't?  Not sure what you're getting at here.
> 
> I think "for i in *.so" should work.
> 
Then its irrelevant in my mind.  They both work equally well.


> > > > +	#compare the abi dumps
> > > > +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> > > 
> > > Do we need to do a visual check? I didn't try yet.
> > > 
> > Yes, it generates an html report of all the symbols exported in a build and
> > compares them with the alternate version.  That needs manual review.
> 
> OK I think it's important to explain in the commit log.
Ok.

> 
> > > So you compare the ABI dumps.
> > > Do we also need to run an app from TAG2 with libs from TAG1?
> > 
> > I started down that path, but its not really that helpful, as all it will do is
> > refuse to run if there is a symbol missing from a later version.  While that
> > might be helpful, its no where near as through as the full report from the
> > compliance checker.
> > 
> > The bottom line is that real ABI compliance requires a developer to be aware of
> > the changes going into the code and how they affect binary layout. A simple
> > "does it still work" test isn't sufficient.
> 
> I hope we'll be able to integrate this kind of tool in an automated sanity
> check in order to find obvious errors.
> 
> Thanks
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-04 14:39         ` Neil Horman
@ 2015-03-04 15:15           ` Thomas Monjalon
  2015-03-04 15:42             ` Neil Horman
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-04 15:15 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-03-04 09:39, Neil Horman:
> On Wed, Mar 04, 2015 at 01:54:49PM +0100, Thomas Monjalon wrote:
> > Hi Neil,
> > 
> > I remove parts that I agree and reply to those which deserve more discussion.
> > 
> > 2015-03-04 06:49, Neil Horman:
> > > On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> > > > 2015-02-02 13:18, Neil Horman:
> > > > > +# Validate that we have all the arguments we need
> > > > > +if [ ! -d ./.git ]
> > > > > +then
> > > > > +	log "WARN" "You must be in the root of the dpdk git tree"
> > > > > +	log "WARN" "You are in $PWD"
> > > > > +	cleanup_and_exit 1
> > > > > +fi
> > > > 
> > > > Why not cd $(dirname $0)/.. instead of returning an error?
> > > 
> > > Why would that help in finding the base of the git tree.  Theres no guarantee
> > > that you are in a subdirectory of a git tree.  I suppose we can try it
> > > recursively until we hit /, but it seems just as easy and clear to tell the user
> > > whats needed.
> > 
> > No I'm saying that you could avoid this check by going into the right
> > directory from the beginning.
> > We know that the root dir is $(dirname $0)/.. because this script is in
> > scripts/ directory.
> > 
> That only helps if you start from the right directory.  If you run this command
> from some other location, your solution just breaks.

Why it would break? $(dirname $0) is always reachable because you launched $0.
The only exception is for the case the PATH variable is used to find the DPDK
scripts/ directory (should not happen).

> > > > > +# Make sure we configure SHARED libraries
> > > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > > 
> > > > Why not tuning configuration after make config in .config file?
> > > > 
> > > Because this way we save a reconfig (from a developer viewpoint), you should run
> > > make config again after changing configs, and so this way you save doing that.
> > 
> > No, you run make config once and update .config file. That's the recommended
> > way to configure DPDK.
> > defconfig files are default configurations and should stay read-only.
> 
> They get overwritten when we do the git resets.  Its silly to modify your config
> file after you run make config, in the event the make target has to re-read any
> modified options and adjust dependent config files accordingly.  I understand
> that doesn't happen now, but its common practice for every open source project
> in existance.

I'm not sure to understand. Maybe an example would help.
By the way, your method works.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-04 15:15           ` Thomas Monjalon
@ 2015-03-04 15:42             ` Neil Horman
  2015-03-04 16:15               ` Thomas Monjalon
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-04 15:42 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Mar 04, 2015 at 04:15:18PM +0100, Thomas Monjalon wrote:
> 2015-03-04 09:39, Neil Horman:
> > On Wed, Mar 04, 2015 at 01:54:49PM +0100, Thomas Monjalon wrote:
> > > Hi Neil,
> > > 
> > > I remove parts that I agree and reply to those which deserve more discussion.
> > > 
> > > 2015-03-04 06:49, Neil Horman:
> > > > On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> > > > > 2015-02-02 13:18, Neil Horman:
> > > > > > +# Validate that we have all the arguments we need
> > > > > > +if [ ! -d ./.git ]
> > > > > > +then
> > > > > > +	log "WARN" "You must be in the root of the dpdk git tree"
> > > > > > +	log "WARN" "You are in $PWD"
> > > > > > +	cleanup_and_exit 1
> > > > > > +fi
> > > > > 
> > > > > Why not cd $(dirname $0)/.. instead of returning an error?
> > > > 
> > > > Why would that help in finding the base of the git tree.  Theres no guarantee
> > > > that you are in a subdirectory of a git tree.  I suppose we can try it
> > > > recursively until we hit /, but it seems just as easy and clear to tell the user
> > > > whats needed.
> > > 
> > > No I'm saying that you could avoid this check by going into the right
> > > directory from the beginning.
> > > We know that the root dir is $(dirname $0)/.. because this script is in
> > > scripts/ directory.
> > > 
> > That only helps if you start from the right directory.  If you run this command
> > from some other location, your solution just breaks.
> 
> Why it would break? $(dirname $0) is always reachable because you launched $0.
> The only exception is for the case the PATH variable is used to find the DPDK
> scripts/ directory (should not happen).
> 
Ah!  Sorry, misunderstood, for some reason I was conflating $0 with $PWD.  Yes,
this will work and I'll update it

> > > > > > +# Make sure we configure SHARED libraries
> > > > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > > > 
> > > > > Why not tuning configuration after make config in .config file?
> > > > > 
> > > > Because this way we save a reconfig (from a developer viewpoint), you should run
> > > > make config again after changing configs, and so this way you save doing that.
> > > 
> > > No, you run make config once and update .config file. That's the recommended
> > > way to configure DPDK.
> > > defconfig files are default configurations and should stay read-only.
> > 
> > They get overwritten when we do the git resets.  Its silly to modify your config
> > file after you run make config, in the event the make target has to re-read any
> > modified options and adjust dependent config files accordingly.  I understand
> > that doesn't happen now, but its common practice for every open source project
> > in existance.
> 
> I'm not sure to understand. Maybe an example would help.
> By the way, your method works.
For example, the linux kernel.  The .config file that is generated in the root
directory is converted to an autoconf.h in parallel with its generation, for
applications to key off of.  If you change something in .config, you need to run
make config again so that those changes are reflected into the other
auto-generated files.  Thats common practice.  So its counter intuitive to
assume that altering the generated .config file is automatically recognized by
the rest of the build, without a subsequent make config (be it explicit or and
implicit dependency of the make all target).

Neil


> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ABI: Add abi checking utility
  2015-03-04 15:42             ` Neil Horman
@ 2015-03-04 16:15               ` Thomas Monjalon
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-04 16:15 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-03-04 10:42, Neil Horman:
> On Wed, Mar 04, 2015 at 04:15:18PM +0100, Thomas Monjalon wrote:
> > 2015-03-04 09:39, Neil Horman:
> > > On Wed, Mar 04, 2015 at 01:54:49PM +0100, Thomas Monjalon wrote:
> > > > Hi Neil,
> > > > 
> > > > I remove parts that I agree and reply to those which deserve more discussion.
> > > > 
> > > > 2015-03-04 06:49, Neil Horman:
> > > > > On Tue, Mar 03, 2015 at 11:18:47PM +0100, Thomas Monjalon wrote:
> > > > > > 2015-02-02 13:18, Neil Horman:
> > > > > > > +# Make sure we configure SHARED libraries
> > > > > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > > > > 
> > > > > > Why not tuning configuration after make config in .config file?
> > > > > > 
> > > > > Because this way we save a reconfig (from a developer viewpoint), you should run
> > > > > make config again after changing configs, and so this way you save doing that.
> > > > 
> > > > No, you run make config once and update .config file. That's the recommended
> > > > way to configure DPDK.
> > > > defconfig files are default configurations and should stay read-only.
> > > 
> > > They get overwritten when we do the git resets.  Its silly to modify your config
> > > file after you run make config, in the event the make target has to re-read any
> > > modified options and adjust dependent config files accordingly.  I understand
> > > that doesn't happen now, but its common practice for every open source project
> > > in existance.
> > 
> > I'm not sure to understand. Maybe an example would help.
> > By the way, your method works.
> 
> For example, the linux kernel.  The .config file that is generated in the root
> directory is converted to an autoconf.h in parallel with its generation, for
> applications to key off of.  If you change something in .config, you need to run
> make config again so that those changes are reflected into the other
> auto-generated files.  Thats common practice.  So its counter intuitive to
> assume that altering the generated .config file is automatically recognized by
> the rest of the build, without a subsequent make config (be it explicit or and
> implicit dependency of the make all target).

OK thanks, now I better understand how you think about DPDK config.
Note that in Linux you are modifying .config, not the defconfig.
I'm not going to debate how it could be improved now but I think we shouldn't
dynamically modify defconfig files to avoid confusion about their purpose.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-01-30 21:16 [dpdk-dev] [PATCH] ABI: Add abi checking utility Neil Horman
  2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
@ 2015-03-04 16:26 ` Neil Horman
  2015-03-04 16:49   ` Thomas Monjalon
  2015-03-13 11:56   ` Kavanagh, Mark B
  2015-03-13 14:09 ` [dpdk-dev] [PATCH v4] " Neil Horman
  2015-03-17 18:08 ` [dpdk-dev] [PATCH v5] " Neil Horman
  3 siblings, 2 replies; 27+ messages in thread
From: Neil Horman @ 2015-03-04 16:26 UTC (permalink / raw)
  To: dev

There was a request for an abi validation utilty for the ongoing ABI stability
work.  As it turns out there is a abi compliance checker in development that
seems to be under active development and provides fairly detailed ABI compliance
reports.  Its not yet intellegent enough to understand symbol versioning, but it
does provide the ability to identify symbols which have changed between
releases, along with details of the change, and offers developers the
opportunity to identify which symbols then need versioning and validation for a
given update via manual testing.

This script automates the use of the compliance checker between two arbitrarily
specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
and run:

./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG

where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
suitable for passing as the T= variable in the make config command.

Note the upstream source for the abi compliance checker is here:
http://ispras.linuxbase.org/index.php/ABI_compliance_checker

It generates a report for each DSO built from the requested tags that developers
can review to find ABI compliance issues.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

---

Change Notes:

v2) Fixed some typos as requested by Thomas

v3) Fixed some additional typos Thomas requested
    Improved script to work from detached state
    Added some documentation to the changelog
    Added some comments to the scripts
---
 scripts/validate_abi.sh | 248 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 248 insertions(+)
 create mode 100755 scripts/validate_abi.sh

diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
new file mode 100755
index 0000000..899cf5f
--- /dev/null
+++ b/scripts/validate_abi.sh
@@ -0,0 +1,248 @@
+#!/bin/sh
+#   BSD LICENSE
+#
+#   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+TAG1=$1
+TAG2=$2
+TARGET=$3
+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+
+usage() {
+	echo "$0 <TAG1> <TAG2> <TARGET>"
+}
+
+log() {
+	local level=$1
+	shift
+	echo "$*"
+}
+
+validate_tags() {
+	git tag -l | grep -q "$TAG1"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG1 is invalid"
+		return
+	fi
+	git tag -l | grep -q "$TAG2"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG2 is invalid"
+		return
+	fi
+}
+
+validate_args() {
+	if [ -z "$TAG1" ]
+	then
+		echo "Must Specify TAG1"
+		return
+	fi
+	if [ -z "$TAG2" ]
+	then
+		echo "Must Specify TAG2"
+		return
+	fi
+	if [ -z "$TARGET" ]
+	then
+		echo "Must Specify a build target"
+	fi
+}
+
+
+cleanup_and_exit() {
+	rm -rf $ABI_DIR
+	exit $1
+}
+
+###########################################
+#START
+############################################
+
+#trap on ctrl-c to clean up
+trap cleanup_and_exit SIGINT
+
+#Save the current branch
+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+
+if [ -z "$CURRENT_BRANCH" ]
+then
+	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
+fi
+
+if [ -n "$VERBOSE" ]
+then
+	export VERBOSE=/dev/stdout
+else
+	export VERBOSE=/dev/null
+fi
+
+# Validate that we have all the arguments we need
+res=$(validate_args)
+if [ -n "$res" ]
+then
+	echo $res
+	usage
+	cleanup_and_exit 1
+fi
+
+# Make sure our tags exist
+res=$(validate_tags)
+if [ -n "$res" ]
+then
+	echo $res
+	cleanup_and_exit 1
+fi
+
+ABICHECK=`which abi-compliance-checker 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-compliance-checker utility"
+	cleanup_and_exit 1
+fi
+
+ABIDUMP=`which abi-dumper 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-dumper utility"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "We're going to check and make sure that applications built"
+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from tag $TAG2."
+log "INFO" ""
+
+# Check to make sure we have a clean tree
+git status | grep -q clean
+if [ $? -ne 0 ]
+then
+	log "WARN" "Working directory not clean, aborting"
+	cleanup_and_exit 1
+fi
+
+# Move to the root of the git tree
+cd $(dirname $0)/..
+
+log "INFO" "Checking out version $TAG1 of the dpdk"
+# Move to the old version of the tree
+git checkout $TAG1
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Checking abi compliance relies on using the dwarf information in
+# The shared objects.  Thats only included in the DSO's if we build
+# with -g
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG1"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG1. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+# Move to the lib directory
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+done
+cd ../..
+
+# Now clean the tree, checkout the second tag, and rebuild
+git clean -f -d
+git reset --hard
+# Move to the new version of the tree
+log "INFO" "Checking out version $TAG2 of the dpdk"
+git checkout $TAG2
+
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG2"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG2. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
+done
+cd ../..
+
+# Start comparison of ABI dumps
+for i in `ls $ABI_DIR/*-1.dump`
+do
+	NEWNAME=`basename $i`
+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
+
+	if [ ! -f $ABI_DIR/$OLDNAME ]
+	then
+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
+	fi
+
+	#compare the abi dumps
+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
+done
+
+git reset --hard
+git checkout $CURRENT_BRANCH
+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
+cleanup_and_exit 0
+
+
-- 
2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
@ 2015-03-04 16:49   ` Thomas Monjalon
  2015-03-05 16:57     ` Neil Horman
  2015-03-13 11:56   ` Kavanagh, Mark B
  1 sibling, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-04 16:49 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-03-04 11:26, Neil Horman:
> +#trap on ctrl-c to clean up
> +trap cleanup_and_exit SIGINT

I think INT is preffered over SIGINT.
You may also add QUIT and TERM.
With QUIT, you can replace cleanup_and_exit calls by a simple exit.

> +	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`

May be simpler "git log -1 --format=%H"

> +log "INFO" "We're going to check and make sure that applications built"
> +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> +log "INFO" "against DPDK DSOs built from tag $TAG2."

I think it may be removed as no app is run.

> +# Make sure we configure SHARED libraries
> +# Also turn off IGB and KNI as those require kernel headers to build
> +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET

So you prefer modifying defconfig instead of .config, right?
(you sent it while I was answering on v2)

> +# Checking abi compliance relies on using the dwarf information in
> +# The shared objects.  Thats only included in the DSO's if we build
> +# with -g
> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g
[...]
> +export EXTRA_CFLAGS=-g
> +export EXTRA_LDFLAGS=-g

Already exported.

> +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`

Could be OLDNAME=$(basename $i 1.dump)0.dump

> +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`

Could be LIBNAME=$(basename $i -ABI-1.dump)

Thanks

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-04 16:49   ` Thomas Monjalon
@ 2015-03-05 16:57     ` Neil Horman
  2015-03-11 19:36       ` Neil Horman
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-05 16:57 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Mar 04, 2015 at 05:49:50PM +0100, Thomas Monjalon wrote:
> 2015-03-04 11:26, Neil Horman:
> > +#trap on ctrl-c to clean up
> > +trap cleanup_and_exit SIGINT
> 
> I think INT is preffered over SIGINT.
> You may also add QUIT and TERM.
> With QUIT, you can replace cleanup_and_exit calls by a simple exit.
> 
> > +	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
> 
> May be simpler "git log -1 --format=%H"
> 
It might be, but the above is equivalent, and --format is a more recent git-log
feature.  Older versions still require --pretty=format

> > +log "INFO" "We're going to check and make sure that applications built"
> > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> 
> I think it may be removed as no app is run.
> 
The above doesn't indicate that an application will be run, only that the
purpose of this script is to ensure that older applications will still run,
which I think is appropriate.

> > +# Make sure we configure SHARED libraries
> > +# Also turn off IGB and KNI as those require kernel headers to build
> > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> 
> So you prefer modifying defconfig instead of .config, right?
> (you sent it while I was answering on v2)
> 
Yes, correct.

> > +# Checking abi compliance relies on using the dwarf information in
> > +# The shared objects.  Thats only included in the DSO's if we build
> > +# with -g
> > +export EXTRA_CFLAGS=-g
> > +export EXTRA_LDFLAGS=-g
> [...]
> > +export EXTRA_CFLAGS=-g
> > +export EXTRA_LDFLAGS=-g
> 
> Already exported.
> 
Yeah, I'll clean that up later.

> > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> 
> Could be OLDNAME=$(basename $i 1.dump)0.dump
> 
> > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> 
> Could be LIBNAME=$(basename $i -ABI-1.dump)
> 
It could be, but I prefer the clarity of the sed replacement.

Neil

> Thanks
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-05 16:57     ` Neil Horman
@ 2015-03-11 19:36       ` Neil Horman
  2015-03-13  8:51         ` Thomas Monjalon
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-11 19:36 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Thu, Mar 05, 2015 at 11:57:27AM -0500, Neil Horman wrote:
> On Wed, Mar 04, 2015 at 05:49:50PM +0100, Thomas Monjalon wrote:
> > 2015-03-04 11:26, Neil Horman:
> > > +#trap on ctrl-c to clean up
> > > +trap cleanup_and_exit SIGINT
> > 
> > I think INT is preffered over SIGINT.
> > You may also add QUIT and TERM.
> > With QUIT, you can replace cleanup_and_exit calls by a simple exit.
> > 
> > > +	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
> > 
> > May be simpler "git log -1 --format=%H"
> > 
> It might be, but the above is equivalent, and --format is a more recent git-log
> feature.  Older versions still require --pretty=format
> 
> > > +log "INFO" "We're going to check and make sure that applications built"
> > > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > 
> > I think it may be removed as no app is run.
> > 
> The above doesn't indicate that an application will be run, only that the
> purpose of this script is to ensure that older applications will still run,
> which I think is appropriate.
> 
> > > +# Make sure we configure SHARED libraries
> > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > 
> > So you prefer modifying defconfig instead of .config, right?
> > (you sent it while I was answering on v2)
> > 
> Yes, correct.
> 
> > > +# Checking abi compliance relies on using the dwarf information in
> > > +# The shared objects.  Thats only included in the DSO's if we build
> > > +# with -g
> > > +export EXTRA_CFLAGS=-g
> > > +export EXTRA_LDFLAGS=-g
> > [...]
> > > +export EXTRA_CFLAGS=-g
> > > +export EXTRA_LDFLAGS=-g
> > 
> > Already exported.
> > 
> Yeah, I'll clean that up later.
> 
> > > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > 
> > Could be OLDNAME=$(basename $i 1.dump)0.dump
> > 
> > > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > 
> > Could be LIBNAME=$(basename $i -ABI-1.dump)
> > 
> It could be, but I prefer the clarity of the sed replacement.
> 
> Neil
> 
> > Thanks
> > 
> > 
> 

Ping Thomas, is this going to make 2.0?

Thanks
Neil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-11 19:36       ` Neil Horman
@ 2015-03-13  8:51         ` Thomas Monjalon
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-13  8:51 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi Neil,

2015-03-11 15:36, Neil Horman:
> On Thu, Mar 05, 2015 at 11:57:27AM -0500, Neil Horman wrote:
> > On Wed, Mar 04, 2015 at 05:49:50PM +0100, Thomas Monjalon wrote:
> > > 2015-03-04 11:26, Neil Horman:
> > > > +#trap on ctrl-c to clean up
> > > > +trap cleanup_and_exit SIGINT
> > > 
> > > I think INT is preffered over SIGINT.
> > > You may also add QUIT and TERM.
> > > With QUIT, you can replace cleanup_and_exit calls by a simple exit.
> > > 
> > > > +	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
> > > 
> > > May be simpler "git log -1 --format=%H"
> > > 
> > It might be, but the above is equivalent, and --format is a more recent git-log
> > feature.  Older versions still require --pretty=format
> > 
> > > > +log "INFO" "We're going to check and make sure that applications built"
> > > > +log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> > > > +log "INFO" "against DPDK DSOs built from tag $TAG2."
> > > 
> > > I think it may be removed as no app is run.
> > > 
> > The above doesn't indicate that an application will be run, only that the
> > purpose of this script is to ensure that older applications will still run,
> > which I think is appropriate.
> > 
> > > > +# Make sure we configure SHARED libraries
> > > > +# Also turn off IGB and KNI as those require kernel headers to build
> > > > +sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> > > > +sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> > > 
> > > So you prefer modifying defconfig instead of .config, right?
> > > (you sent it while I was answering on v2)
> > > 
> > Yes, correct.
> > 
> > > > +# Checking abi compliance relies on using the dwarf information in
> > > > +# The shared objects.  Thats only included in the DSO's if we build
> > > > +# with -g
> > > > +export EXTRA_CFLAGS=-g
> > > > +export EXTRA_LDFLAGS=-g
> > > [...]
> > > > +export EXTRA_CFLAGS=-g
> > > > +export EXTRA_LDFLAGS=-g
> > > 
> > > Already exported.
> > > 
> > Yeah, I'll clean that up later.

OK, could you send a v4 please?

> > > > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > > 
> > > Could be OLDNAME=$(basename $i 1.dump)0.dump
> > > 
> > > > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > > 
> > > Could be LIBNAME=$(basename $i -ABI-1.dump)
> > > 
> > It could be, but I prefer the clarity of the sed replacement.
> > 
> > Neil
> > 
> > > Thanks
> > > 
> > > 
> > 
> 
> Ping Thomas, is this going to make 2.0?

Yes sure, waiting a v4.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
  2015-03-04 16:49   ` Thomas Monjalon
@ 2015-03-13 11:56   ` Kavanagh, Mark B
  2015-03-13 14:10     ` Neil Horman
  1 sibling, 1 reply; 27+ messages in thread
From: Kavanagh, Mark B @ 2015-03-13 11:56 UTC (permalink / raw)
  To: Neil Horman, dev



>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
>Sent: Wednesday, March 4, 2015 4:27 PM
>To: dev@dpdk.org
>Subject: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
>
>There was a request for an abi validation utilty for the ongoing ABI stability
>work.  As it turns out there is a abi compliance checker in development that
>seems to be under active development and provides fairly detailed ABI compliance
>reports.  Its not yet intellegent enough to understand symbol versioning, but it
>does provide the ability to identify symbols which have changed between
>releases, along with details of the change, and offers developers the
>opportunity to identify which symbols then need versioning and validation for a
>given update via manual testing.
>
>This script automates the use of the compliance checker between two arbitrarily
>specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
>and run:
>
>./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
>
>where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
>suitable for passing as the T= variable in the make config command.
>
>Note the upstream source for the abi compliance checker is here:
>http://ispras.linuxbase.org/index.php/ABI_compliance_checker
>
>It generates a report for each DSO built from the requested tags that developers
>can review to find ABI compliance issues.
>
>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
>---
>
>Change Notes:
>
>v2) Fixed some typos as requested by Thomas
>
>v3) Fixed some additional typos Thomas requested
>    Improved script to work from detached state
>    Added some documentation to the changelog
>    Added some comments to the scripts
>---
> scripts/validate_abi.sh | 248 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 248 insertions(+)
> create mode 100755 scripts/validate_abi.sh
>
>diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
>new file mode 100755
>index 0000000..899cf5f
>--- /dev/null
>+++ b/scripts/validate_abi.sh
>@@ -0,0 +1,248 @@
>+#!/bin/sh
>+#   BSD LICENSE
>+#
>+#   Copyright(c) 2015 Neil Horman. All rights reserved.
>+#   All rights reserved.
>+#
>+#   Redistribution and use in source and binary forms, with or without
>+#   modification, are permitted provided that the following conditions
>+#   are met:
>+#
>+#     * Redistributions of source code must retain the above copyright
>+#       notice, this list of conditions and the following disclaimer.
>+#     * Redistributions in binary form must reproduce the above copyright
>+#       notice, this list of conditions and the following disclaimer in
>+#       the documentation and/or other materials provided with the
>+#       distribution.
>+#
>+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+
>+TAG1=$1
>+TAG2=$2
>+TARGET=$3
>+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
>+
>+usage() {
>+	echo "$0 <TAG1> <TAG2> <TARGET>"
>+}
>+
>+log() {
>+	local level=$1
>+	shift
>+	echo "$*"
>+}
>+
>+validate_tags() {
>+	git tag -l | grep -q "$TAG1"
>+	if [ $? -ne 0 ]
>+	then
>+		echo "$TAG1 is invalid"
>+		return
>+	fi
>+	git tag -l | grep -q "$TAG2"
>+	if [ $? -ne 0 ]
>+	then
>+		echo "$TAG2 is invalid"
>+		return
>+	fi
>+}
>+
>+validate_args() {
>+	if [ -z "$TAG1" ]
>+	then
>+		echo "Must Specify TAG1"
>+		return
>+	fi
>+	if [ -z "$TAG2" ]
>+	then
>+		echo "Must Specify TAG2"
>+		return
>+	fi
>+	if [ -z "$TARGET" ]
>+	then
>+		echo "Must Specify a build target"
>+	fi
>+}
>+
>+
>+cleanup_and_exit() {
>+	rm -rf $ABI_DIR
>+	exit $1
>+}
>+
>+###########################################
>+#START
>+############################################
>+
>+#trap on ctrl-c to clean up
>+trap cleanup_and_exit SIGINT
>+
>+#Save the current branch
>+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
>+
>+if [ -z "$CURRENT_BRANCH" ]
>+then
>+	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
>+fi
>+
>+if [ -n "$VERBOSE" ]
>+then
>+	export VERBOSE=/dev/stdout
>+else
>+	export VERBOSE=/dev/null
>+fi
>+
>+# Validate that we have all the arguments we need
>+res=$(validate_args)
>+if [ -n "$res" ]
>+then
>+	echo $res
>+	usage
>+	cleanup_and_exit 1
>+fi
>+
>+# Make sure our tags exist
>+res=$(validate_tags)
>+if [ -n "$res" ]
>+then
>+	echo $res
>+	cleanup_and_exit 1
>+fi
>+
>+ABICHECK=`which abi-compliance-checker 2>/dev/null`
>+if [ $? -ne 0 ]
>+then
>+	log "INFO" "Cant find abi-compliance-checker utility"
>+	cleanup_and_exit 1
>+fi
>+
>+ABIDUMP=`which abi-dumper 2>/dev/null`
>+if [ $? -ne 0 ]
>+then
>+	log "INFO" "Cant find abi-dumper utility"
>+	cleanup_and_exit 1
>+fi
>+
>+log "INFO" "We're going to check and make sure that applications built"
>+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
>+log "INFO" "against DPDK DSOs built from tag $TAG2."
>+log "INFO" ""
>+
>+# Check to make sure we have a clean tree
>+git status | grep -q clean
>+if [ $? -ne 0 ]
>+then
>+	log "WARN" "Working directory not clean, aborting"
>+	cleanup_and_exit 1
>+fi
>+
>+# Move to the root of the git tree
>+cd $(dirname $0)/..
>+
>+log "INFO" "Checking out version $TAG1 of the dpdk"
>+# Move to the old version of the tree
>+git checkout $TAG1
>+
>+# Make sure we configure SHARED libraries
>+# Also turn off IGB and KNI as those require kernel headers to build
>+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
>+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
>+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
>+
>+# Checking abi compliance relies on using the dwarf information in
>+# The shared objects.  Thats only included in the DSO's if we build
>+# with -g
>+export EXTRA_CFLAGS=-g
>+export EXTRA_LDFLAGS=-g
>+
>+# Now configure the build
>+log "INFO" "Configuring DPDK $TAG1"
>+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
>+
>+log "INFO" "Building DPDK $TAG1. This might take a moment"
>+make O=$TARGET > $VERBOSE 2>&1
>+
>+if [ $? -ne 0 ]
>+then
>+	log "INFO" "THE BUILD FAILED.  ABORTING"

If the build fails while TAG1 is checked out, the user must check out their original local branch manually. I'd prefer it if the script checked out $CURRENT_BRANCH in the 'cleanup_and_exit' function. 

Same applies to TAG2, if the user CTRL-C's out of the script, and to any other command that might fail when a particular branch/tag is checked out (for example, the 'sed' commands fail when I run the script; however, they work when I run them on the command line - I'm investigating this currently).

>+	cleanup_and_exit 1
>+fi
>+
>+# Move to the lib directory
>+cd $TARGET/lib
>+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
>+for i in `ls *.so`
>+do
>+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
>+done
>+cd ../..
>+
>+# Now clean the tree, checkout the second tag, and rebuild
>+git clean -f -d
>+git reset --hard
>+# Move to the new version of the tree
>+log "INFO" "Checking out version $TAG2 of the dpdk"
>+git checkout $TAG2
>+
>+export EXTRA_CFLAGS=-g
>+export EXTRA_LDFLAGS=-g
>+
>+# Make sure we configure SHARED libraries
>+# Also turn off IGB and KNI as those require kernel headers to build
>+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
>+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
>+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
>+
>+# Now configure the build
>+log "INFO" "Configuring DPDK $TAG2"
>+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
>+
>+log "INFO" "Building DPDK $TAG2. This might take a moment"
>+make O=$TARGET > $VERBOSE 2>&1
>+
>+if [ $? -ne 0 ]
>+then
>+	log "INFO" "THE BUILD FAILED.  ABORTING"
>+	cleanup_and_exit 1
>+fi
>+
>+cd $TARGET/lib
>+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
>+for i in `ls *.so`
>+do
>+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
>+done
>+cd ../..
>+
>+# Start comparison of ABI dumps
>+for i in `ls $ABI_DIR/*-1.dump`
>+do
>+	NEWNAME=`basename $i`
>+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
>+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
>+
>+	if [ ! -f $ABI_DIR/$OLDNAME ]
>+	then
>+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
>+	fi
>+
>+	#compare the abi dumps
>+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
>+done
>+
>+git reset --hard
>+git checkout $CURRENT_BRANCH
>+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
>+cleanup_and_exit 0
>+
>+
>--
>2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v4] ABI: Add abi checking utility
  2015-01-30 21:16 [dpdk-dev] [PATCH] ABI: Add abi checking utility Neil Horman
  2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
  2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
@ 2015-03-13 14:09 ` Neil Horman
  2015-03-17 15:42   ` Thomas Monjalon
  2015-03-17 18:08 ` [dpdk-dev] [PATCH v5] " Neil Horman
  3 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-13 14:09 UTC (permalink / raw)
  To: dev

There was a request for an abi validation utilty for the ongoing ABI stability
work.  As it turns out there is a abi compliance checker in development that
seems to be under active development and provides fairly detailed ABI compliance
reports.  Its not yet intellegent enough to understand symbol versioning, but it
does provide the ability to identify symbols which have changed between
releases, along with details of the change, and offers developers the
opportunity to identify which symbols then need versioning and validation for a
given update via manual testing.

This script automates the use of the compliance checker between two arbitrarily
specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
and run:

./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG

where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
suitable for passing as the T= variable in the make config command.

Note the upstream source for the abi compliance checker is here:
http://ispras.linuxbase.org/index.php/ABI_compliance_checker

It generates a report for each DSO built from the requested tags that developers
can review to find ABI compliance issues.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

---

Change Notes:

v2) Fixed some typos as requested by Thomas

v3) Fixed some additional typos Thomas requested
    Improved script to work from detached state
    Added some documentation to the changelog
    Added some comments to the scripts

v4) Remove duplicate exports.
    Move restoration of starting branch/comit to cleanup_and_exit
---
 scripts/validate_abi.sh | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 245 insertions(+)
 create mode 100755 scripts/validate_abi.sh

diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
new file mode 100755
index 0000000..bdec431
--- /dev/null
+++ b/scripts/validate_abi.sh
@@ -0,0 +1,245 @@
+#!/bin/sh
+#   BSD LICENSE
+#
+#   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+TAG1=$1
+TAG2=$2
+TARGET=$3
+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+
+usage() {
+	echo "$0 <TAG1> <TAG2> <TARGET>"
+}
+
+log() {
+	local level=$1
+	shift
+	echo "$*"
+}
+
+validate_tags() {
+	git tag -l | grep -q "$TAG1"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG1 is invalid"
+		return
+	fi
+	git tag -l | grep -q "$TAG2"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG2 is invalid"
+		return
+	fi
+}
+
+validate_args() {
+	if [ -z "$TAG1" ]
+	then
+		echo "Must Specify TAG1"
+		return
+	fi
+	if [ -z "$TAG2" ]
+	then
+		echo "Must Specify TAG2"
+		return
+	fi
+	if [ -z "$TARGET" ]
+	then
+		echo "Must Specify a build target"
+	fi
+}
+
+
+cleanup_and_exit() {
+	rm -rf $ABI_DIR
+	exit $1
+	git checkout $CURRENT_BRANCH
+}
+
+###########################################
+#START
+############################################
+
+#trap on ctrl-c to clean up
+trap cleanup_and_exit SIGINT
+
+#Save the current branch
+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+
+if [ -z "$CURRENT_BRANCH" ]
+then
+	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
+fi
+
+if [ -n "$VERBOSE" ]
+then
+	export VERBOSE=/dev/stdout
+else
+	export VERBOSE=/dev/null
+fi
+
+# Validate that we have all the arguments we need
+res=$(validate_args)
+if [ -n "$res" ]
+then
+	echo $res
+	usage
+	cleanup_and_exit 1
+fi
+
+# Make sure our tags exist
+res=$(validate_tags)
+if [ -n "$res" ]
+then
+	echo $res
+	cleanup_and_exit 1
+fi
+
+ABICHECK=`which abi-compliance-checker 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-compliance-checker utility"
+	cleanup_and_exit 1
+fi
+
+ABIDUMP=`which abi-dumper 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-dumper utility"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "We're going to check and make sure that applications built"
+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from tag $TAG2."
+log "INFO" ""
+
+# Check to make sure we have a clean tree
+git status | grep -q clean
+if [ $? -ne 0 ]
+then
+	log "WARN" "Working directory not clean, aborting"
+	cleanup_and_exit 1
+fi
+
+# Move to the root of the git tree
+cd $(dirname $0)/..
+
+log "INFO" "Checking out version $TAG1 of the dpdk"
+# Move to the old version of the tree
+git checkout $TAG1
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Checking abi compliance relies on using the dwarf information in
+# The shared objects.  Thats only included in the DSO's if we build
+# with -g
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG1"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG1. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+# Move to the lib directory
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+done
+cd ../..
+
+# Now clean the tree, checkout the second tag, and rebuild
+git clean -f -d
+git reset --hard
+# Move to the new version of the tree
+log "INFO" "Checking out version $TAG2 of the dpdk"
+git checkout $TAG2
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG2"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG2. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
+done
+cd ../..
+
+# Start comparison of ABI dumps
+for i in `ls $ABI_DIR/*-1.dump`
+do
+	NEWNAME=`basename $i`
+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
+
+	if [ ! -f $ABI_DIR/$OLDNAME ]
+	then
+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
+	fi
+
+	#compare the abi dumps
+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
+done
+
+git reset --hard
+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
+cleanup_and_exit 0
+
+
-- 
2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-13 11:56   ` Kavanagh, Mark B
@ 2015-03-13 14:10     ` Neil Horman
  2015-03-13 14:25       ` Kavanagh, Mark B
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-13 14:10 UTC (permalink / raw)
  To: Kavanagh, Mark B; +Cc: dev

On Fri, Mar 13, 2015 at 11:56:59AM +0000, Kavanagh, Mark B wrote:
> 
> 
> >-----Original Message-----
> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> >Sent: Wednesday, March 4, 2015 4:27 PM
> >To: dev@dpdk.org
> >Subject: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
> >
> >There was a request for an abi validation utilty for the ongoing ABI stability
> >work.  As it turns out there is a abi compliance checker in development that
> >seems to be under active development and provides fairly detailed ABI compliance
> >reports.  Its not yet intellegent enough to understand symbol versioning, but it
> >does provide the ability to identify symbols which have changed between
> >releases, along with details of the change, and offers developers the
> >opportunity to identify which symbols then need versioning and validation for a
> >given update via manual testing.
> >
> >This script automates the use of the compliance checker between two arbitrarily
> >specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> >and run:
> >
> >./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> >
> >where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> >suitable for passing as the T= variable in the make config command.
> >
> >Note the upstream source for the abi compliance checker is here:
> >http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> >
> >It generates a report for each DSO built from the requested tags that developers
> >can review to find ABI compliance issues.
> >
> >Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >
> >---
> >
> >Change Notes:
> >
> >v2) Fixed some typos as requested by Thomas
> >
> >v3) Fixed some additional typos Thomas requested
> >    Improved script to work from detached state
> >    Added some documentation to the changelog
> >    Added some comments to the scripts
> >---
> > scripts/validate_abi.sh | 248 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 248 insertions(+)
> > create mode 100755 scripts/validate_abi.sh
> >
> >diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
> >new file mode 100755
> >index 0000000..899cf5f
> >--- /dev/null
> >+++ b/scripts/validate_abi.sh
> >@@ -0,0 +1,248 @@
> >+#!/bin/sh
> >+#   BSD LICENSE
> >+#
> >+#   Copyright(c) 2015 Neil Horman. All rights reserved.
> >+#   All rights reserved.
> >+#
> >+#   Redistribution and use in source and binary forms, with or without
> >+#   modification, are permitted provided that the following conditions
> >+#   are met:
> >+#
> >+#     * Redistributions of source code must retain the above copyright
> >+#       notice, this list of conditions and the following disclaimer.
> >+#     * Redistributions in binary form must reproduce the above copyright
> >+#       notice, this list of conditions and the following disclaimer in
> >+#       the documentation and/or other materials provided with the
> >+#       distribution.
> >+#
> >+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >+
> >+TAG1=$1
> >+TAG2=$2
> >+TARGET=$3
> >+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
> >+
> >+usage() {
> >+	echo "$0 <TAG1> <TAG2> <TARGET>"
> >+}
> >+
> >+log() {
> >+	local level=$1
> >+	shift
> >+	echo "$*"
> >+}
> >+
> >+validate_tags() {
> >+	git tag -l | grep -q "$TAG1"
> >+	if [ $? -ne 0 ]
> >+	then
> >+		echo "$TAG1 is invalid"
> >+		return
> >+	fi
> >+	git tag -l | grep -q "$TAG2"
> >+	if [ $? -ne 0 ]
> >+	then
> >+		echo "$TAG2 is invalid"
> >+		return
> >+	fi
> >+}
> >+
> >+validate_args() {
> >+	if [ -z "$TAG1" ]
> >+	then
> >+		echo "Must Specify TAG1"
> >+		return
> >+	fi
> >+	if [ -z "$TAG2" ]
> >+	then
> >+		echo "Must Specify TAG2"
> >+		return
> >+	fi
> >+	if [ -z "$TARGET" ]
> >+	then
> >+		echo "Must Specify a build target"
> >+	fi
> >+}
> >+
> >+
> >+cleanup_and_exit() {
> >+	rm -rf $ABI_DIR
> >+	exit $1
> >+}
> >+
> >+###########################################
> >+#START
> >+############################################
> >+
> >+#trap on ctrl-c to clean up
> >+trap cleanup_and_exit SIGINT
> >+
> >+#Save the current branch
> >+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
> >+
> >+if [ -z "$CURRENT_BRANCH" ]
> >+then
> >+	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
> >+fi
> >+
> >+if [ -n "$VERBOSE" ]
> >+then
> >+	export VERBOSE=/dev/stdout
> >+else
> >+	export VERBOSE=/dev/null
> >+fi
> >+
> >+# Validate that we have all the arguments we need
> >+res=$(validate_args)
> >+if [ -n "$res" ]
> >+then
> >+	echo $res
> >+	usage
> >+	cleanup_and_exit 1
> >+fi
> >+
> >+# Make sure our tags exist
> >+res=$(validate_tags)
> >+if [ -n "$res" ]
> >+then
> >+	echo $res
> >+	cleanup_and_exit 1
> >+fi
> >+
> >+ABICHECK=`which abi-compliance-checker 2>/dev/null`
> >+if [ $? -ne 0 ]
> >+then
> >+	log "INFO" "Cant find abi-compliance-checker utility"
> >+	cleanup_and_exit 1
> >+fi
> >+
> >+ABIDUMP=`which abi-dumper 2>/dev/null`
> >+if [ $? -ne 0 ]
> >+then
> >+	log "INFO" "Cant find abi-dumper utility"
> >+	cleanup_and_exit 1
> >+fi
> >+
> >+log "INFO" "We're going to check and make sure that applications built"
> >+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
> >+log "INFO" "against DPDK DSOs built from tag $TAG2."
> >+log "INFO" ""
> >+
> >+# Check to make sure we have a clean tree
> >+git status | grep -q clean
> >+if [ $? -ne 0 ]
> >+then
> >+	log "WARN" "Working directory not clean, aborting"
> >+	cleanup_and_exit 1
> >+fi
> >+
> >+# Move to the root of the git tree
> >+cd $(dirname $0)/..
> >+
> >+log "INFO" "Checking out version $TAG1 of the dpdk"
> >+# Move to the old version of the tree
> >+git checkout $TAG1
> >+
> >+# Make sure we configure SHARED libraries
> >+# Also turn off IGB and KNI as those require kernel headers to build
> >+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> >+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
> >+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> >+
> >+# Checking abi compliance relies on using the dwarf information in
> >+# The shared objects.  Thats only included in the DSO's if we build
> >+# with -g
> >+export EXTRA_CFLAGS=-g
> >+export EXTRA_LDFLAGS=-g
> >+
> >+# Now configure the build
> >+log "INFO" "Configuring DPDK $TAG1"
> >+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
> >+
> >+log "INFO" "Building DPDK $TAG1. This might take a moment"
> >+make O=$TARGET > $VERBOSE 2>&1
> >+
> >+if [ $? -ne 0 ]
> >+then
> >+	log "INFO" "THE BUILD FAILED.  ABORTING"
> 
> If the build fails while TAG1 is checked out, the user must check out their original local branch manually. I'd prefer it if the script checked out $CURRENT_BRANCH in the 'cleanup_and_exit' function. 
> 
Sure, its in V4.

> Same applies to TAG2, if the user CTRL-C's out of the script, and to any other command that might fail when a particular branch/tag is checked out (for example, the 'sed' commands fail when I run the script; however, they work when I run them on the command line - I'm investigating this currently).
> 
What does the log say?  Please post it here.  If it helps add a set -x to the
top of the script for additional verbosity.

Neil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-13 14:10     ` Neil Horman
@ 2015-03-13 14:25       ` Kavanagh, Mark B
  2015-03-13 14:58         ` Neil Horman
  0 siblings, 1 reply; 27+ messages in thread
From: Kavanagh, Mark B @ 2015-03-13 14:25 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

>On Fri, Mar 13, 2015 at 11:56:59AM +0000, Kavanagh, Mark B wrote:
>>
>>
>> >-----Original Message-----
>> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
>> >Sent: Wednesday, March 4, 2015 4:27 PM
>> >To: dev@dpdk.org
>> >Subject: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
>> >


(snip)

>> >+log "INFO" "Building DPDK $TAG1. This might take a moment"
>> >+make O=$TARGET > $VERBOSE 2>&1
>> >+
>> >+if [ $? -ne 0 ]
>> >+then
>> >+	log "INFO" "THE BUILD FAILED.  ABORTING"
>>
>> If the build fails while TAG1 is checked out, the user must check out their original
>local branch manually. I'd prefer it if the script checked out $CURRENT_BRANCH in the
>'cleanup_and_exit' function.
>>
>Sure, its in V4.

Cool.

>
>> Same applies to TAG2, if the user CTRL-C's out of the script, and to any other command
>that might fail when a particular branch/tag is checked out (for example, the 'sed'
>commands fail when I run the script; however, they work when I run them on the command
>line - I'm investigating this currently).
>>
>What does the log say?  Please post it here.  If it helps add a set -x to the
>top of the script for additional verbosity.
>

Hey Neil - this is the error, but it's not a problem with the script; presumably I'd cleaned my DPDK installation directory, so 'sed' couldn't find the defconfig file:
"sed: can't read config/defconfig_x86_64-ivshmem-linuxapp-gcc/: Not a directory"

Thanks,
Mark

>Neil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-13 14:25       ` Kavanagh, Mark B
@ 2015-03-13 14:58         ` Neil Horman
  2015-03-13 15:49           ` Kavanagh, Mark B
  0 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-13 14:58 UTC (permalink / raw)
  To: Kavanagh, Mark B; +Cc: dev

On Fri, Mar 13, 2015 at 02:25:17PM +0000, Kavanagh, Mark B wrote:
> >On Fri, Mar 13, 2015 at 11:56:59AM +0000, Kavanagh, Mark B wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> >> >Sent: Wednesday, March 4, 2015 4:27 PM
> >> >To: dev@dpdk.org
> >> >Subject: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
> >> >
> 
> 
> (snip)
> 
> >> >+log "INFO" "Building DPDK $TAG1. This might take a moment"
> >> >+make O=$TARGET > $VERBOSE 2>&1
> >> >+
> >> >+if [ $? -ne 0 ]
> >> >+then
> >> >+	log "INFO" "THE BUILD FAILED.  ABORTING"
> >>
> >> If the build fails while TAG1 is checked out, the user must check out their original
> >local branch manually. I'd prefer it if the script checked out $CURRENT_BRANCH in the
> >'cleanup_and_exit' function.
> >>
> >Sure, its in V4.
> 
> Cool.
> 
> >
> >> Same applies to TAG2, if the user CTRL-C's out of the script, and to any other command
> >that might fail when a particular branch/tag is checked out (for example, the 'sed'
> >commands fail when I run the script; however, they work when I run them on the command
> >line - I'm investigating this currently).
> >>
> >What does the log say?  Please post it here.  If it helps add a set -x to the
> >top of the script for additional verbosity.
> >
> 
> Hey Neil - this is the error, but it's not a problem with the script; presumably I'd cleaned my DPDK installation directory, so 'sed' couldn't find the defconfig file:
> "sed: can't read config/defconfig_x86_64-ivshmem-linuxapp-gcc/: Not a directory"
> 
Actually, it looks to me like you added a trailing "/" to the end of the third
argument on the script command line, so sed bombs when it tries to modify a
directory instead of a file.  Try specifying:
x86_64-ivshmem-linuxapp-gcc
instead of
x86_64-ivshmem-linuxapp-gcc/

Neil

> Thanks,
> Mark
> 
> >Neil
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
  2015-03-13 14:58         ` Neil Horman
@ 2015-03-13 15:49           ` Kavanagh, Mark B
  0 siblings, 0 replies; 27+ messages in thread
From: Kavanagh, Mark B @ 2015-03-13 15:49 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev



>-----Original Message-----
>From: Neil Horman [mailto:nhorman@tuxdriver.com]
>Sent: Friday, March 13, 2015 2:59 PM
>To: Kavanagh, Mark B
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
>
>On Fri, Mar 13, 2015 at 02:25:17PM +0000, Kavanagh, Mark B wrote:
>> >On Fri, Mar 13, 2015 at 11:56:59AM +0000, Kavanagh, Mark B wrote:
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
>> >> >Sent: Wednesday, March 4, 2015 4:27 PM
>> >> >To: dev@dpdk.org
>> >> >Subject: [dpdk-dev] [PATCH v3] ABI: Add abi checking utility
>> >> >
>>
>>
>> (snip)
>>
>> >> >+log "INFO" "Building DPDK $TAG1. This might take a moment"
>> >> >+make O=$TARGET > $VERBOSE 2>&1
>> >> >+
>> >> >+if [ $? -ne 0 ]
>> >> >+then
>> >> >+	log "INFO" "THE BUILD FAILED.  ABORTING"
>> >>
>> >> If the build fails while TAG1 is checked out, the user must check out their original
>> >local branch manually. I'd prefer it if the script checked out $CURRENT_BRANCH in the
>> >'cleanup_and_exit' function.
>> >>
>> >Sure, its in V4.
>>
>> Cool.
>>
>> >
>> >> Same applies to TAG2, if the user CTRL-C's out of the script, and to any other
>command
>> >that might fail when a particular branch/tag is checked out (for example, the 'sed'
>> >commands fail when I run the script; however, they work when I run them on the command
>> >line - I'm investigating this currently).
>> >>
>> >What does the log say?  Please post it here.  If it helps add a set -x to the
>> >top of the script for additional verbosity.
>> >
>>
>> Hey Neil - this is the error, but it's not a problem with the script; presumably I'd
>cleaned my DPDK installation directory, so 'sed' couldn't find the defconfig file:
>> "sed: can't read config/defconfig_x86_64-ivshmem-linuxapp-gcc/: Not a directory"
>>
>Actually, it looks to me like you added a trailing "/" to the end of the third
>argument on the script command line, so sed bombs when it tries to modify a
>directory instead of a file.  Try specifying:
>x86_64-ivshmem-linuxapp-gcc
>instead of
>x86_64-ivshmem-linuxapp-gcc/
>
>Neil

Nice catch - thanks!

>
>> Thanks,
>> Mark
>>
>> >Neil
>>
>>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v4] ABI: Add abi checking utility
  2015-03-13 14:09 ` [dpdk-dev] [PATCH v4] " Neil Horman
@ 2015-03-17 15:42   ` Thomas Monjalon
  2015-03-17 16:47     ` Thomas Monjalon
  2015-03-17 18:08     ` Neil Horman
  0 siblings, 2 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-17 15:42 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi Neil,

I tested this tool and I see few small improvements possible.

2015-03-13 10:09, Neil Horman:
> There was a request for an abi validation utilty for the ongoing ABI stability
                                            utility
> work.  As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI compliance
> reports.  Its not yet intellegent enough to understand symbol versioning, but it
                        intelligent
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers developers the
> opportunity to identify which symbols then need versioning and validation for a
> given update via manual testing.
> 
> This script automates the use of the compliance checker between two arbitrarily
> specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> and run:
> 
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> 
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
> 
> Note the upstream source for the abi compliance checker is here:
> http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> 
> It generates a report for each DSO built from the requested tags that developers
> can review to find ABI compliance issues.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> ---
> 
> Change Notes:
> 
> v2) Fixed some typos as requested by Thomas
> 
> v3) Fixed some additional typos Thomas requested
>     Improved script to work from detached state
>     Added some documentation to the changelog
>     Added some comments to the scripts
> 
> v4) Remove duplicate exports.
>     Move restoration of starting branch/comit to cleanup_and_exit
> ---
[...]
> +TAG1=$1
> +TAG2=$2
> +TARGET=$3
> +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`

+JOBS=$(grep -c '^processor' /proc/cpuinfo)

[...]
> +cleanup_and_exit() {
> +	rm -rf $ABI_DIR
> +	exit $1
> +	git checkout $CURRENT_BRANCH

Checkout is never done because of previous exit.

> +}
[...]
> +log "INFO" "Checking out version $TAG1 of the dpdk"
> +# Move to the old version of the tree
> +git checkout $TAG1

What about -q for quiet mode?

[...]
> +log "INFO" "Building DPDK $TAG1. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1

-j$JOBS would improve building time

[...]
> +# Move to the new version of the tree
> +log "INFO" "Checking out version $TAG2 of the dpdk"
> +git checkout $TAG2

-q ?

[...]
> +log "INFO" "Building DPDK $TAG2. This might take a moment"
> +make O=$TARGET > $VERBOSE 2>&1

-j ?

[...]
> +# Start comparison of ABI dumps
> +for i in `ls $ABI_DIR/*-1.dump`
> +do
> +	NEWNAME=`basename $i`
> +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> +
> +	if [ ! -f $ABI_DIR/$OLDNAME ]
> +	then
> +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> +	fi
> +
> +	#compare the abi dumps
> +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> +done

It would be more convenient to generate an HTML index giving access to every
reports for every DSOs.

> +
> +git reset --hard
> +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> +cleanup_and_exit 0

After reading the report, it's not clear what would be tolerated or not.
Should we forbid every defects?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v4] ABI: Add abi checking utility
  2015-03-17 15:42   ` Thomas Monjalon
@ 2015-03-17 16:47     ` Thomas Monjalon
  2015-03-17 18:08     ` Neil Horman
  1 sibling, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-17 16:47 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

More comments:
Please rename to validate-abi.sh (with an hyphen) to be more consistent with
other scripts.
Please add it in the MAINTAINERS file.

Thanks

2015-03-17 16:42, Thomas Monjalon:
> Hi Neil,
> 
> I tested this tool and I see few small improvements possible.
> 
> 2015-03-13 10:09, Neil Horman:
> > There was a request for an abi validation utilty for the ongoing ABI stability
>                                             utility
> > work.  As it turns out there is a abi compliance checker in development that
> > seems to be under active development and provides fairly detailed ABI compliance
> > reports.  Its not yet intellegent enough to understand symbol versioning, but it
>                         intelligent
> > does provide the ability to identify symbols which have changed between
> > releases, along with details of the change, and offers developers the
> > opportunity to identify which symbols then need versioning and validation for a
> > given update via manual testing.
> > 
> > This script automates the use of the compliance checker between two arbitrarily
> > specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> > and run:
> > 
> > ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> > 
> > where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> > suitable for passing as the T= variable in the make config command.
> > 
> > Note the upstream source for the abi compliance checker is here:
> > http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> > 
> > It generates a report for each DSO built from the requested tags that developers
> > can review to find ABI compliance issues.
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > ---
> > 
> > Change Notes:
> > 
> > v2) Fixed some typos as requested by Thomas
> > 
> > v3) Fixed some additional typos Thomas requested
> >     Improved script to work from detached state
> >     Added some documentation to the changelog
> >     Added some comments to the scripts
> > 
> > v4) Remove duplicate exports.
> >     Move restoration of starting branch/comit to cleanup_and_exit
> > ---
> [...]
> > +TAG1=$1
> > +TAG2=$2
> > +TARGET=$3
> > +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
> 
> +JOBS=$(grep -c '^processor' /proc/cpuinfo)
> 
> [...]
> > +cleanup_and_exit() {
> > +	rm -rf $ABI_DIR
> > +	exit $1
> > +	git checkout $CURRENT_BRANCH
> 
> Checkout is never done because of previous exit.
> 
> > +}
> [...]
> > +log "INFO" "Checking out version $TAG1 of the dpdk"
> > +# Move to the old version of the tree
> > +git checkout $TAG1
> 
> What about -q for quiet mode?
> 
> [...]
> > +log "INFO" "Building DPDK $TAG1. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> 
> -j$JOBS would improve building time
> 
> [...]
> > +# Move to the new version of the tree
> > +log "INFO" "Checking out version $TAG2 of the dpdk"
> > +git checkout $TAG2
> 
> -q ?
> 
> [...]
> > +log "INFO" "Building DPDK $TAG2. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> 
> -j ?
> 
> [...]
> > +# Start comparison of ABI dumps
> > +for i in `ls $ABI_DIR/*-1.dump`
> > +do
> > +	NEWNAME=`basename $i`
> > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > +
> > +	if [ ! -f $ABI_DIR/$OLDNAME ]
> > +	then
> > +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> > +	fi
> > +
> > +	#compare the abi dumps
> > +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> > +done
> 
> It would be more convenient to generate an HTML index giving access to every
> reports for every DSOs.
> 
> > +
> > +git reset --hard
> > +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> > +cleanup_and_exit 0
> 
> After reading the report, it's not clear what would be tolerated or not.
> Should we forbid every defects?
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v4] ABI: Add abi checking utility
  2015-03-17 15:42   ` Thomas Monjalon
  2015-03-17 16:47     ` Thomas Monjalon
@ 2015-03-17 18:08     ` Neil Horman
  1 sibling, 0 replies; 27+ messages in thread
From: Neil Horman @ 2015-03-17 18:08 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Mar 17, 2015 at 04:42:31PM +0100, Thomas Monjalon wrote:
> Hi Neil,
> 
> I tested this tool and I see few small improvements possible.
> 
I'll fix the bug you found, but I'm not going to go chasing every feature that
you happen to note.  Not saying they're not fine features, but I don't have time
to implement features that you happen to note might be nice to have, especially
not in time for 2.0. 

Regarding your question about report tolerance, Its not that kind of tool.  The
ABI checker simply calls a developers attention to symbols that have
inadvertently changed due to code or data structure modifications.  It is
incumbent on the developer to make a well informed decision about how to handle
those changes (via deprecation/versioning/etc), and to defend his/her reasoning.

Neil

> 2015-03-13 10:09, Neil Horman:
> > There was a request for an abi validation utilty for the ongoing ABI stability
>                                             utility
> > work.  As it turns out there is a abi compliance checker in development that
> > seems to be under active development and provides fairly detailed ABI compliance
> > reports.  Its not yet intellegent enough to understand symbol versioning, but it
>                         intelligent
> > does provide the ability to identify symbols which have changed between
> > releases, along with details of the change, and offers developers the
> > opportunity to identify which symbols then need versioning and validation for a
> > given update via manual testing.
> > 
> > This script automates the use of the compliance checker between two arbitrarily
> > specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> > and run:
> > 
> > ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> > 
> > where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> > suitable for passing as the T= variable in the make config command.
> > 
> > Note the upstream source for the abi compliance checker is here:
> > http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> > 
> > It generates a report for each DSO built from the requested tags that developers
> > can review to find ABI compliance issues.
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > ---
> > 
> > Change Notes:
> > 
> > v2) Fixed some typos as requested by Thomas
> > 
> > v3) Fixed some additional typos Thomas requested
> >     Improved script to work from detached state
> >     Added some documentation to the changelog
> >     Added some comments to the scripts
> > 
> > v4) Remove duplicate exports.
> >     Move restoration of starting branch/comit to cleanup_and_exit
> > ---
> [...]
> > +TAG1=$1
> > +TAG2=$2
> > +TARGET=$3
> > +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
> 
> +JOBS=$(grep -c '^processor' /proc/cpuinfo)
> 
> [...]
> > +cleanup_and_exit() {
> > +	rm -rf $ABI_DIR
> > +	exit $1
> > +	git checkout $CURRENT_BRANCH
> 
> Checkout is never done because of previous exit.
> 
> > +}
> [...]
> > +log "INFO" "Checking out version $TAG1 of the dpdk"
> > +# Move to the old version of the tree
> > +git checkout $TAG1
> 
> What about -q for quiet mode?
> 
> [...]
> > +log "INFO" "Building DPDK $TAG1. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> 
> -j$JOBS would improve building time
> 
> [...]
> > +# Move to the new version of the tree
> > +log "INFO" "Checking out version $TAG2 of the dpdk"
> > +git checkout $TAG2
> 
> -q ?
> 
> [...]
> > +log "INFO" "Building DPDK $TAG2. This might take a moment"
> > +make O=$TARGET > $VERBOSE 2>&1
> 
> -j ?
> 
> [...]
> > +# Start comparison of ABI dumps
> > +for i in `ls $ABI_DIR/*-1.dump`
> > +do
> > +	NEWNAME=`basename $i`
> > +	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
> > +	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
> > +
> > +	if [ ! -f $ABI_DIR/$OLDNAME ]
> > +	then
> > +		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
> > +	fi
> > +
> > +	#compare the abi dumps
> > +	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
> > +done
> 
> It would be more convenient to generate an HTML index giving access to every
> reports for every DSOs.
> 
> > +
> > +git reset --hard
> > +log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
> > +cleanup_and_exit 0
> 
> After reading the report, it's not clear what would be tolerated or not.
> Should we forbid every defects?
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v5] ABI: Add abi checking utility
  2015-01-30 21:16 [dpdk-dev] [PATCH] ABI: Add abi checking utility Neil Horman
                   ` (2 preceding siblings ...)
  2015-03-13 14:09 ` [dpdk-dev] [PATCH v4] " Neil Horman
@ 2015-03-17 18:08 ` Neil Horman
  2015-03-17 21:17   ` Thomas Monjalon
  3 siblings, 1 reply; 27+ messages in thread
From: Neil Horman @ 2015-03-17 18:08 UTC (permalink / raw)
  To: dev

There was a request for an abi validation utilty for the ongoing ABI stability
work.  As it turns out there is a abi compliance checker in development that
seems to be under active development and provides fairly detailed ABI compliance
reports.  Its not yet intellegent enough to understand symbol versioning, but it
does provide the ability to identify symbols which have changed between
releases, along with details of the change, and offers developers the
opportunity to identify which symbols then need versioning and validation for a
given update via manual testing.

This script automates the use of the compliance checker between two arbitrarily
specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
and run:

./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG

where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
suitable for passing as the T= variable in the make config command.

Note the upstream source for the abi compliance checker is here:
http://ispras.linuxbase.org/index.php/ABI_compliance_checker

It generates a report for each DSO built from the requested tags that developers
can review to find ABI compliance issues.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

---

Change Notes:

v2) Fixed some typos as requested by Thomas

v3) Fixed some additional typos Thomas requested
    Improved script to work from detached state
    Added some documentation to the changelog
    Added some comments to the scripts

v4) Remove duplicate exports.
    Move restoration of starting branch/comit to cleanup_and_exit

v5) Fixed exit cleanup
    Added MAINTAINERS entry
---
 MAINTAINERS             |   1 +
 scripts/validate-abi.sh | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 246 insertions(+)
 create mode 100755 scripts/validate-abi.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 07fdf5e..fa309ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -59,6 +59,7 @@ ABI versioning
 M: Neil Horman <nhorman@tuxdriver.com>
 F: lib/librte_compat/
 F: doc/guides/rel_notes/abi.rst
+F: scripts/validate-abi.sh
 
 Environment Abstraction Layer
 -----------------------------
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
new file mode 100755
index 0000000..369ea8a
--- /dev/null
+++ b/scripts/validate-abi.sh
@@ -0,0 +1,245 @@
+#!/bin/sh
+#   BSD LICENSE
+#
+#   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+TAG1=$1
+TAG2=$2
+TARGET=$3
+ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+
+usage() {
+	echo "$0 <TAG1> <TAG2> <TARGET>"
+}
+
+log() {
+	local level=$1
+	shift
+	echo "$*"
+}
+
+validate_tags() {
+	git tag -l | grep -q "$TAG1"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG1 is invalid"
+		return
+	fi
+	git tag -l | grep -q "$TAG2"
+	if [ $? -ne 0 ]
+	then
+		echo "$TAG2 is invalid"
+		return
+	fi
+}
+
+validate_args() {
+	if [ -z "$TAG1" ]
+	then
+		echo "Must Specify TAG1"
+		return
+	fi
+	if [ -z "$TAG2" ]
+	then
+		echo "Must Specify TAG2"
+		return
+	fi
+	if [ -z "$TARGET" ]
+	then
+		echo "Must Specify a build target"
+	fi
+}
+
+
+cleanup_and_exit() {
+	rm -rf $ABI_DIR
+	git checkout $CURRENT_BRANCH
+	exit $1
+}
+
+###########################################
+#START
+############################################
+
+#trap on ctrl-c to clean up
+trap cleanup_and_exit SIGINT
+
+#Save the current branch
+CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+
+if [ -z "$CURRENT_BRANCH" ]
+then
+	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
+fi
+
+if [ -n "$VERBOSE" ]
+then
+	export VERBOSE=/dev/stdout
+else
+	export VERBOSE=/dev/null
+fi
+
+# Validate that we have all the arguments we need
+res=$(validate_args)
+if [ -n "$res" ]
+then
+	echo $res
+	usage
+	cleanup_and_exit 1
+fi
+
+# Make sure our tags exist
+res=$(validate_tags)
+if [ -n "$res" ]
+then
+	echo $res
+	cleanup_and_exit 1
+fi
+
+ABICHECK=`which abi-compliance-checker 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-compliance-checker utility"
+	cleanup_and_exit 1
+fi
+
+ABIDUMP=`which abi-dumper 2>/dev/null`
+if [ $? -ne 0 ]
+then
+	log "INFO" "Cant find abi-dumper utility"
+	cleanup_and_exit 1
+fi
+
+log "INFO" "We're going to check and make sure that applications built"
+log "INFO" "against DPDK DSOs from tag $TAG1 will still run when executed"
+log "INFO" "against DPDK DSOs built from tag $TAG2."
+log "INFO" ""
+
+# Check to make sure we have a clean tree
+git status | grep -q clean
+if [ $? -ne 0 ]
+then
+	log "WARN" "Working directory not clean, aborting"
+	cleanup_and_exit 1
+fi
+
+# Move to the root of the git tree
+cd $(dirname $0)/..
+
+log "INFO" "Checking out version $TAG1 of the dpdk"
+# Move to the old version of the tree
+git checkout $TAG1
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Checking abi compliance relies on using the dwarf information in
+# The shared objects.  Thats only included in the DSO's if we build
+# with -g
+export EXTRA_CFLAGS=-g
+export EXTRA_LDFLAGS=-g
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG1"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG1. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+# Move to the lib directory
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $TAG1
+done
+cd ../..
+
+# Now clean the tree, checkout the second tag, and rebuild
+git clean -f -d
+git reset --hard
+# Move to the new version of the tree
+log "INFO" "Checking out version $TAG2 of the dpdk"
+git checkout $TAG2
+
+# Make sure we configure SHARED libraries
+# Also turn off IGB and KNI as those require kernel headers to build
+sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
+
+# Now configure the build
+log "INFO" "Configuring DPDK $TAG2"
+make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
+
+log "INFO" "Building DPDK $TAG2. This might take a moment"
+make O=$TARGET > $VERBOSE 2>&1
+
+if [ $? -ne 0 ]
+then
+	log "INFO" "THE BUILD FAILED.  ABORTING"
+	cleanup_and_exit 1
+fi
+
+cd $TARGET/lib
+log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
+for i in `ls *.so`
+do
+	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $TAG2
+done
+cd ../..
+
+# Start comparison of ABI dumps
+for i in `ls $ABI_DIR/*-1.dump`
+do
+	NEWNAME=`basename $i`
+	OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"`
+	LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"`
+
+	if [ ! -f $ABI_DIR/$OLDNAME ]
+	then
+		log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..."
+	fi
+
+	#compare the abi dumps
+	$ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME
+done
+
+git reset --hard
+log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
+cleanup_and_exit 0
+
+
-- 
2.1.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v5] ABI: Add abi checking utility
  2015-03-17 18:08 ` [dpdk-dev] [PATCH v5] " Neil Horman
@ 2015-03-17 21:17   ` Thomas Monjalon
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2015-03-17 21:17 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-03-17 14:08, Neil Horman:
> There was a request for an abi validation utilty for the ongoing ABI stability
> work.  As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI compliance
> reports.  Its not yet intellegent enough to understand symbol versioning, but it
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers developers the
> opportunity to identify which symbols then need versioning and validation for a
> given update via manual testing.
> 
> This script automates the use of the compliance checker between two arbitrarily
> specified tags within the dpdk tree.  To execute enter the $RTE_SDK directory
> and run:
> 
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
> 
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
> 
> Note the upstream source for the abi compliance checker is here:
> http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> 
> It generates a report for each DSO built from the requested tags that developers
> can review to find ABI compliance issues.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> 
> Change Notes:
> 
> v2) Fixed some typos as requested by Thomas
> 
> v3) Fixed some additional typos Thomas requested
>     Improved script to work from detached state
>     Added some documentation to the changelog
>     Added some comments to the scripts
> 
> v4) Remove duplicate exports.
>     Move restoration of starting branch/comit to cleanup_and_exit
> 
> v5) Fixed exit cleanup
>     Added MAINTAINERS entry

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2015-03-17 21:18 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-30 21:16 [dpdk-dev] [PATCH] ABI: Add abi checking utility Neil Horman
2015-02-02 18:18 ` [dpdk-dev] [PATCH v2] " Neil Horman
2015-02-27 13:48   ` Neil Horman
2015-02-27 13:55     ` Thomas Monjalon
2015-03-03 22:18   ` Thomas Monjalon
2015-03-04 11:49     ` Neil Horman
2015-03-04 12:54       ` Thomas Monjalon
2015-03-04 14:39         ` Neil Horman
2015-03-04 15:15           ` Thomas Monjalon
2015-03-04 15:42             ` Neil Horman
2015-03-04 16:15               ` Thomas Monjalon
2015-03-04 16:26 ` [dpdk-dev] [PATCH v3] " Neil Horman
2015-03-04 16:49   ` Thomas Monjalon
2015-03-05 16:57     ` Neil Horman
2015-03-11 19:36       ` Neil Horman
2015-03-13  8:51         ` Thomas Monjalon
2015-03-13 11:56   ` Kavanagh, Mark B
2015-03-13 14:10     ` Neil Horman
2015-03-13 14:25       ` Kavanagh, Mark B
2015-03-13 14:58         ` Neil Horman
2015-03-13 15:49           ` Kavanagh, Mark B
2015-03-13 14:09 ` [dpdk-dev] [PATCH v4] " Neil Horman
2015-03-17 15:42   ` Thomas Monjalon
2015-03-17 16:47     ` Thomas Monjalon
2015-03-17 18:08     ` Neil Horman
2015-03-17 18:08 ` [dpdk-dev] [PATCH v5] " Neil Horman
2015-03-17 21:17   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).