[3/3] app/test/meson: auto detect number of cores

  The arguments being passed will cause failures on laptops that have,
for instance, 2 cores only.  Most of the tests don't require more
than a single core.  Some require multiple cores (but those tests
should be modified to 'SKIP' when the correct number of cores
aren't available).

The unit test results shouldn't be impacted by this change, but it
allows for a future enhancement to pass flags such as '--no-huge'.

Also include a fix to a reported issue with running on FreeBSD.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
Conflicts with http://patches.dpdk.org/patch/50850/

 app/test/meson.build | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)
  

On Thu, Apr 11, 2019 at 9:52 PM Aaron Conole <aconole@redhat.com> wrote:

> The arguments being passed will cause failures on laptops that have,
> for instance, 2 cores only.  Most of the tests don't require more
> than a single core.  Some require multiple cores (but those tests
> should be modified to 'SKIP' when the correct number of cores
> aren't available).
>
> The unit test results shouldn't be impacted by this change, but it
> allows for a future enhancement to pass flags such as '--no-huge'.
>
> Also include a fix to a reported issue with running on FreeBSD.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> Conflicts with http://patches.dpdk.org/patch/50850/
>
>  app/test/meson.build | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 867cc5863..1010bfbc8 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -344,17 +344,32 @@ if get_option('tests')
>         timeout_seconds = 600
>         timeout_seconds_fast = 10
>
> +       # Retreive the number of CPU cores
>

nit: Retrieve

Little concern here on the approach of getting the max available cpu index.
If we have non contiguous cpus (let's say hyper threading is disabled),
this won't work.
But we can just assume this won't happen for non regression setups (vms).


+       num_cores = run_command('lscpu',
> '-p=cpu').stdout().strip().split('\n')[-1]
>

lscpu is a linux command afaik.

Maybe for FreeBSD:
root@freebsd-10:~ # sysctl dev.cpu |cut -d . -f 3 |sort |tail -1
3

Not sure if FreeBSD ensures that the keys names/objects won't change
accross the versions.


+       num_cores_arg = '-l 0-' + num_cores
>
+
> +       test_args = [num_cores_arg, '-n 4']
>         foreach arg : fast_parallel_test_names
> -               test(arg, dpdk_test,
> -                       env : ['DPDK_TEST=' + arg],
> -                       args : ['-c f','-n 4', '--file-prefix=@0@
> '.format(arg)],
> +               if host_machine.system() == 'linux'
> +                       test(arg, dpdk_test,
> +                                 env : ['DPDK_TEST=' + arg],
> +                                 args : test_args +
> +                                        ['--file-prefix=@0@
> '.format(arg)],
> +                       timeout : timeout_seconds_fast,
> +                       suite : 'fast-tests')
> +               else
> +                       test(arg, dpdk_test,
> +                               env : ['DPDK_TEST=' + arg],
> +                               args : test_args,
>                         timeout : timeout_seconds_fast,
>                         suite : 'fast-tests')
> +               endif
>         endforeach
>
>         foreach arg : fast_non_parallel_test_names
>                 test(arg, dpdk_test,
>                         env : ['DPDK_TEST=' + arg],
> +                       args : test_args,
>                         timeout : timeout_seconds_fast,
>                         is_parallel : false,
>                         suite : 'fast-tests')
> @@ -363,6 +378,7 @@ if get_option('tests')
>         foreach arg : perf_test_names
>                 test(arg, dpdk_test,
>                 env : ['DPDK_TEST=' + arg],
> +               args : test_args,
>                 timeout : timeout_seconds,
>                 is_parallel : false,
>                 suite : 'perf-tests')
> @@ -371,6 +387,7 @@ if get_option('tests')
>         foreach arg : driver_test_names
>                 test(arg, dpdk_test,
>                         env : ['DPDK_TEST=' + arg],
> +                       args : test_args,
>                         timeout : timeout_seconds,
>                         is_parallel : false,
>                         suite : 'driver-tests')
> @@ -379,6 +396,7 @@ if get_option('tests')
>         foreach arg : dump_test_names
>                 test(arg, dpdk_test,
>                         env : ['DPDK_TEST=' + arg],
> +                       args : test_args,
>                         timeout : timeout_seconds,
>                         is_parallel : false,
>                         suite : 'debug-tests')
> --
> 2.19.1
>

The rest looks good to me.
Reviewed-by: David Marchand <david.marchand@redhat.com>
  

On Fri, Apr 12, 2019 at 09:46:17AM +0200, David Marchand wrote:
>    On Thu, Apr 11, 2019 at 9:52 PM Aaron Conole <[1]aconole@redhat.com>
>    wrote:
> 
>      The arguments being passed will cause failures on laptops that have,
>      for instance, 2 cores only.  Most of the tests don't require more
>      than a single core.  Some require multiple cores (but those tests
>      should be modified to 'SKIP' when the correct number of cores
>      aren't available).
>      The unit test results shouldn't be impacted by this change, but it
>      allows for a future enhancement to pass flags such as '--no-huge'.
>      Also include a fix to a reported issue with running on FreeBSD.
>      Signed-off-by: Aaron Conole <[2]aconole@redhat.com>
>      ---
>      Conflicts with [3]http://patches.dpdk.org/patch/50850/
>       app/test/meson.build | 24 +++++++++++++++++++++---
>       1 file changed, 21 insertions(+), 3 deletions(-)
>      diff --git a/app/test/meson.build b/app/test/meson.build
>      index 867cc5863..1010bfbc8 100644
>      --- a/app/test/meson.build
>      +++ b/app/test/meson.build
>      @@ -344,17 +344,32 @@ if get_option('tests')
>              timeout_seconds = 600
>              timeout_seconds_fast = 10
>      +       # Retreive the number of CPU cores
> 
>    nit: Retrieve
>    Little concern here on the approach of getting the max available cpu
>    index.
>    If we have non contiguous cpus (let's say hyper threading is disabled),
>    this won't work.
>    But we can just assume this won't happen for non regression setups
>    (vms).
> 
>      +       num_cores = run_command('lscpu',
>      '-p=cpu').stdout().strip().split('\n')[-1]
> 
>    lscpu is a linux command afaik.
>    Maybe for FreeBSD:
>    root@freebsd-10:~ # sysctl dev.cpu |cut -d . -f 3 |sort |tail -1
>    3
>    Not sure if FreeBSD ensures that the keys names/objects won't change
>    accross the versions.
> 

Very similar thoughs/concerns here. I think doing this per OS is probably
safer in the long term - even though lscpu is available for FreeBSD as an
extra package.

My suggestion: for FreeBSD, get the number of CPUs using
	"/sbin/sysctl -n hw.ncpu"
For Linux, rather than using a "0-N" range, use output from 
"cat /sys/devices/system/cpu/present", which will ensure that it works
even in the non-contiguous CPU numbering case. [Though I suspect we get
non-contiguous numbers only in the case a core or two has been hotplugged
out, I think - disabling hyperthreading won't leave gaps, just fewer cores]

/Bruce
  

On Thu, 2019-04-11 at 15:52 -0400, Aaron Conole wrote:
> The arguments being passed will cause failures on laptops that have,
> for instance, 2 cores only.  Most of the tests don't require more
> than a single core.  Some require multiple cores (but those tests
> should be modified to 'SKIP' when the correct number of cores
> aren't available).
> 
> The unit test results shouldn't be impacted by this change, but it
> allows for a future enhancement to pass flags such as '--no-huge'.
> 
> Also include a fix to a reported issue with running on FreeBSD.
> 
> Signed-off-by: Aaron Conole <
> aconole@redhat.com
> >
> ---
> Conflicts with 
> http://patches.dpdk.org/patch/50850/
> 
> 
>  app/test/meson.build | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 

With the provision that the changes suggested by Bruce and David are
applied,

Acked-by: Luca Boccassi <bluca@debian.org>