[v2,2/2] devtools: allow misc options in null test

Message ID 20190717155202.1674-2-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

Commit Message

Thomas Monjalon July 17, 2019, 3:52 p.m. UTC
  In order to ease basic testing with customized options,
EAL and testpmd options can be added as third and fourth arguments
of the "null PMD" script.

Also, the first argument becomes more flexible by accepting
the testpmd path as an alternative to the build directory.

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

Comments

David Marchand July 29, 2019, 10:36 a.m. UTC | #1
On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> In order to ease basic testing with customized options,
> EAL and testpmd options can be added as third and fourth arguments
> of the "null PMD" script.
>
> Also, the first argument becomes more flexible by accepting
> the testpmd path as an alternative to the build directory.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  devtools/test-null.sh | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/devtools/test-null.sh b/devtools/test-null.sh
> index 6928a6c15..69eddc359 100755
> --- a/devtools/test-null.sh
> +++ b/devtools/test-null.sh
> @@ -5,10 +5,13 @@
>
>  # Run a quick testpmd forwarding with null PMD without hugepage
>
> -build=${1:-build}
> +build=${1:-build} # first argument can be the build directory
> +testpmd=$1 # or first argument can be the testpmd path
>  coremask=${2:-3} # default using cores 0 and 1
> +eal_options=$3
> +testpmd_options=$4
>
> -testpmd=$build/app/dpdk-testpmd
> +[ -f "$testpmd" ] || testpmd=$build/app/dpdk-testpmd
>  [ -f "$testpmd" ] || testpmd=$build/app/testpmd
>  if [ ! -f "$testpmd" ] ; then
>         echo 'ERROR: testpmd cannot be found' >&2
> @@ -22,5 +25,5 @@ fi
>
>  (sleep 1 && echo stop) |
>  $testpmd -c $coremask --no-huge -m 150 \
> -       $libs --vdev net_null1 --vdev net_null2 -- \
> -       --no-mlockall --total-num-mbufs=2048 -ia
> +       $libs --vdev net_null1 --vdev net_null2 $eal_options -- \
> +       --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
> --
> 2.21.0
>

Not sure I would have split this from the first patch.

Out of curiosity, what are the cases where you need to set options?


--
David Marchand
  
Thomas Monjalon July 29, 2019, 12:19 p.m. UTC | #2
29/07/2019 12:36, David Marchand:
> On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > In order to ease basic testing with customized options,
> > EAL and testpmd options can be added as third and fourth arguments
> > of the "null PMD" script.
> >
> > Also, the first argument becomes more flexible by accepting
> > the testpmd path as an alternative to the build directory.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Not sure I would have split this from the first patch.

Patches have different intents, different explanations.

> Out of curiosity, what are the cases where you need to set options?

It allows to test different builds with a specific option,
and see which one is failing.
For instance, it was useful when debugging a failure with -w option recently.
  
David Marchand July 29, 2019, 12:55 p.m. UTC | #3
On Mon, Jul 29, 2019 at 2:19 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 29/07/2019 12:36, David Marchand:
> > On Wed, Jul 17, 2019 at 5:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > In order to ease basic testing with customized options,
> > > EAL and testpmd options can be added as third and fourth arguments
> > > of the "null PMD" script.
> > >
> > > Also, the first argument becomes more flexible by accepting
> > > the testpmd path as an alternative to the build directory.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >
> > Not sure I would have split this from the first patch.
>
> Patches have different intents, different explanations.

You could have split the discovery of the testpmd binary out of the
first patch (the fix on meson support, as you named it).
Then merged it with this part that accepts both a directory and a direct path.

We are in a dark area there, and well the patches are not that
critical, so go as you like.

> > Out of curiosity, what are the cases where you need to set options?
>
> It allows to test different builds with a specific option,
> and see which one is failing.
> For instance, it was useful when debugging a failure with -w option recently.
>

Ok.
  

Patch

diff --git a/devtools/test-null.sh b/devtools/test-null.sh
index 6928a6c15..69eddc359 100755
--- a/devtools/test-null.sh
+++ b/devtools/test-null.sh
@@ -5,10 +5,13 @@ 
 
 # Run a quick testpmd forwarding with null PMD without hugepage
 
-build=${1:-build}
+build=${1:-build} # first argument can be the build directory
+testpmd=$1 # or first argument can be the testpmd path
 coremask=${2:-3} # default using cores 0 and 1
+eal_options=$3
+testpmd_options=$4
 
-testpmd=$build/app/dpdk-testpmd
+[ -f "$testpmd" ] || testpmd=$build/app/dpdk-testpmd
 [ -f "$testpmd" ] || testpmd=$build/app/testpmd
 if [ ! -f "$testpmd" ] ; then
 	echo 'ERROR: testpmd cannot be found' >&2
@@ -22,5 +25,5 @@  fi
 
 (sleep 1 && echo stop) |
 $testpmd -c $coremask --no-huge -m 150 \
-	$libs --vdev net_null1 --vdev net_null2 -- \
-	--no-mlockall --total-num-mbufs=2048 -ia
+	$libs --vdev net_null1 --vdev net_null2 $eal_options -- \
+	--no-mlockall --total-num-mbufs=2048 $testpmd_options -ia