[v2,1/2] devtools: restore null test

Message ID 20190717155202.1674-1-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] devtools: restore null test |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Thomas Monjalon July 17, 2019, 3:52 p.m. UTC
  This small testpmd test was not working for a long time
because of several changes in EAL and mempool.
The 3 main issues solved are:
	- Make --no-huge working by specifying an amount of memory
		to allocate in legacy mode, and disabling mlockall.
	- Load a mempool handler in shared library case.
	- Support meson

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/test-null.sh | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)
  

Comments

David Marchand July 29, 2019, 10:35 a.m. UTC | #1
On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> This small testpmd test was not working for a long time
> because of several changes in EAL and mempool.
> The 3 main issues solved are:
>         - Make --no-huge working by specifying an amount of memory
>                 to allocate in legacy mode, and disabling mlockall.
>         - Load a mempool handler in shared library case.
>         - Support meson
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  devtools/test-null.sh | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/devtools/test-null.sh b/devtools/test-null.sh
> index 61879e3e6..6928a6c15 100755
> --- a/devtools/test-null.sh
> +++ b/devtools/test-null.sh
> @@ -1,17 +1,26 @@
>  #! /bin/sh -e
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright 2015 6WIND S.A.
> +# Copyright 2019 Mellanox Technologies, Ltd
>
>  # Run a quick testpmd forwarding with null PMD without hugepage
>
>  build=${1:-build}
>  coremask=${2:-3} # default using cores 0 and 1
>
> -if grep -q SHARED_LIB=y $build/.config; then
> -       pmd='-d librte_pmd_null.so'
> +testpmd=$build/app/dpdk-testpmd
> +[ -f "$testpmd" ] || testpmd=$build/app/testpmd
> +if [ ! -f "$testpmd" ] ; then

You don't really care that testpmd is a file, prefer -e.


> +       echo 'ERROR: testpmd cannot be found' >&2
> +       exit 1
> +fi
> +
> +unset libs

You reference it later, I suppose you meant libs=



> +if ldd $testpmd | grep -q librte_ ; then
> +       libs='-d librte_mempool_ring.so -d librte_pmd_null.so'
>  fi
>
>  (sleep 1 && echo stop) |
> -$build/app/testpmd -c $coremask -n 1 --no-huge \
> -       $pmd --vdev net_null1 --vdev net_null2 -- \
> -       --total-num-mbufs=2048 -ia
> +$testpmd -c $coremask --no-huge -m 150 \
> +       $libs --vdev net_null1 --vdev net_null2 -- \
> +       --no-mlockall --total-num-mbufs=2048 -ia
> --
> 2.21.0
>


--
David Marchand
  
Thomas Monjalon July 29, 2019, 12:15 p.m. UTC | #2
29/07/2019 12:35, David Marchand:
> On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > --- a/devtools/test-null.sh
> > +++ b/devtools/test-null.sh
> > -if grep -q SHARED_LIB=y $build/.config; then
> > -       pmd='-d librte_pmd_null.so'
> > +testpmd=$build/app/dpdk-testpmd
> > +[ -f "$testpmd" ] || testpmd=$build/app/testpmd
> > +if [ ! -f "$testpmd" ] ; then
> 
> You don't really care that testpmd is a file, prefer -e.

Yes I care, I want to avoid finding a directory.

> > +       echo 'ERROR: testpmd cannot be found' >&2
> > +       exit 1
> > +fi
> > +
> > +unset libs
> 
> You reference it later, I suppose you meant libs=

I think "unset libs" is the standard syntax to initialize a variable
to an empty value.

> > +if ldd $testpmd | grep -q librte_ ; then
> > +       libs='-d librte_mempool_ring.so -d librte_pmd_null.so'
> >  fi
  
David Marchand July 29, 2019, 12:53 p.m. UTC | #3
On Mon, Jul 29, 2019 at 2:16 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 29/07/2019 12:35, David Marchand:
> > On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > --- a/devtools/test-null.sh
> > > +++ b/devtools/test-null.sh
> > > -if grep -q SHARED_LIB=y $build/.config; then
> > > -       pmd='-d librte_pmd_null.so'
> > > +testpmd=$build/app/dpdk-testpmd
> > > +[ -f "$testpmd" ] || testpmd=$build/app/testpmd
> > > +if [ ! -f "$testpmd" ] ; then
> >
> > You don't really care that testpmd is a file, prefer -e.
>
> Yes I care, I want to avoid finding a directory.

Ok, as long as we don't hit symbolic links.

>
> > > +       echo 'ERROR: testpmd cannot be found' >&2
> > > +       exit 1
> > > +fi
> > > +
> > > +unset libs
> >
> > You reference it later, I suppose you meant libs=
>
> I think "unset libs" is the standard syntax to initialize a variable
> to an empty value.

Nop, unset and initialising to empty are different, but the way you
use this works.


> > > +if ldd $testpmd | grep -q librte_ ; then
> > > +       libs='-d librte_mempool_ring.so -d librte_pmd_null.so'
> > >  fi
>
>
  

Patch

diff --git a/devtools/test-null.sh b/devtools/test-null.sh
index 61879e3e6..6928a6c15 100755
--- a/devtools/test-null.sh
+++ b/devtools/test-null.sh
@@ -1,17 +1,26 @@ 
 #! /bin/sh -e
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2015 6WIND S.A.
+# Copyright 2019 Mellanox Technologies, Ltd
 
 # Run a quick testpmd forwarding with null PMD without hugepage
 
 build=${1:-build}
 coremask=${2:-3} # default using cores 0 and 1
 
-if grep -q SHARED_LIB=y $build/.config; then
-	pmd='-d librte_pmd_null.so'
+testpmd=$build/app/dpdk-testpmd
+[ -f "$testpmd" ] || testpmd=$build/app/testpmd
+if [ ! -f "$testpmd" ] ; then
+	echo 'ERROR: testpmd cannot be found' >&2
+	exit 1
+fi
+
+unset libs
+if ldd $testpmd | grep -q librte_ ; then
+	libs='-d librte_mempool_ring.so -d librte_pmd_null.so'
 fi
 
 (sleep 1 && echo stop) |
-$build/app/testpmd -c $coremask -n 1 --no-huge \
-	$pmd --vdev net_null1 --vdev net_null2 -- \
-	--total-num-mbufs=2048 -ia
+$testpmd -c $coremask --no-huge -m 150 \
+	$libs --vdev net_null1 --vdev net_null2 -- \
+	--no-mlockall --total-num-mbufs=2048 -ia