[dpdk-dev] [PATCH 0/3] travis: enhancements for build (plus a meson fix)

[dpdk-dev] [PATCH 0/3] travis: enhancements for build (plus a meson fix)

[Aaron Conole]

This series improves the travis integration a bit, and adds the ability for
the meson system to auto-detect the number of cores.

Aaron Conole (3):
  travis: enable ccache
  travis: add a distinguisher to the 'extra' builds
  app/test/meson: auto detect number of cores

 .travis.yml          | 19 +++++++++----------
 app/test/meson.build | 24 +++++++++++++++++++++---
 2 files changed, 30 insertions(+), 13 deletions(-)

-- 
2.19.1

[dpdk-dev] [PATCH 1/3] travis: enable ccache

[Aaron Conole]

We try to make the planet happy travis builds dpdk.

Also, the 'sudo' flag was recently deprecated.  Drop it.

Suggested-by: Luca Boccassi <bluca@debian.org>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 3045eada5..72f8671b1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,4 +1,5 @@
 language: c
+cache: ccache
 compiler:
   - gcc
   - clang
@@ -24,8 +25,6 @@ extra_packages: &extra_packages
 
 before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
 
-sudo: false
-
 env:
   - DEF_LIB="static"
   - DEF_LIB="shared"
-- 
2.19.1

Re: [dpdk-dev] [PATCH 1/3] travis: enable ccache

[David Marchand]

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

> We try to make the planet happy travis builds dpdk.
>
> Also, the 'sudo' flag was recently deprecated.  Drop it.
>
> Suggested-by: Luca Boccassi <bluca@debian.org>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  .travis.yml | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 3045eada5..72f8671b1 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,4 +1,5 @@
>  language: c
> +cache: ccache
>  compiler:
>    - gcc
>    - clang
> @@ -24,8 +25,6 @@ extra_packages: &extra_packages
>
>  before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
>
> -sudo: false
> -
>  env:
>    - DEF_LIB="static"
>    - DEF_LIB="shared"
> --
> 2.19.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/3] travis: enable ccache

[Luca Boccassi]

On Thu, 2019-04-11 at 15:52 -0400, Aaron Conole wrote:
> We try to make the planet happy travis builds dpdk.
> 
> Also, the 'sudo' flag was recently deprecated.  Drop it.
> 
> Suggested-by: Luca Boccassi <
> bluca@debian.org
> >
> Signed-off-by: Aaron Conole <
> aconole@redhat.com
> >
> ---
>  .travis.yml | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 3045eada5..72f8671b1 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,4 +1,5 @@
>  language: c
> +cache: ccache
>  compiler:
>    - gcc
>    - clang
> @@ -24,8 +25,6 @@ extra_packages: &extra_packages
>  
>  before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
>  
> -sudo: false
> -
>  env:
>    - DEF_LIB="static"
>    - DEF_LIB="shared"
> 

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

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH 2/3] travis: add a distinguisher to the 'extra' builds

[Aaron Conole]

This helps in two ways:
  1. When looking at travis page for dpdk, it's a visual distinction

  2. For ccache support, the build IDs include the 'env', so we get unique
     cache data.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 .travis.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 72f8671b1..7b167fa64 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -45,49 +45,49 @@ matrix:
       apt:
         packages:
           - *aarch64_packages
-  - env: DEF_LIB="static"
+  - env: DEF_LIB="static" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared"
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static"
+  - env: DEF_LIB="static" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared"
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
-- 
2.19.1

Re: [dpdk-dev] [PATCH 2/3] travis: add a distinguisher to the 'extra' builds

[David Marchand]

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

> This helps in two ways:
>   1. When looking at travis page for dpdk, it's a visual distinction
>
>   2. For ccache support, the build IDs include the 'env', so we get unique
>      cache data.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  .travis.yml | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 72f8671b1..7b167fa64 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -45,49 +45,49 @@ matrix:
>        apt:
>          packages:
>            - *aarch64_packages
> -  - env: DEF_LIB="static"
> +  - env: DEF_LIB="static" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared"
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static"
> +  - env: DEF_LIB="static" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared"
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
> --
> 2.19.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH 2/3] travis: add a distinguisher to the 'extra' builds

[Luca Boccassi]

On Thu, 2019-04-11 at 15:52 -0400, Aaron Conole wrote:
> This helps in two ways:
>   1. When looking at travis page for dpdk, it's a visual distinction
> 
>   2. For ccache support, the build IDs include the 'env', so we get
> unique
>      cache data.
> 
> Signed-off-by: Aaron Conole <
> aconole@redhat.com
> >
> ---
>  .travis.yml | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 72f8671b1..7b167fa64 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -45,49 +45,49 @@ matrix:
>        apt:
>          packages:
>            - *aarch64_packages
> -  - env: DEF_LIB="static"
> +  - env: DEF_LIB="static" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared"
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> EXTRA_PACKAGES=1
>      compiler: gcc
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static"
> +  - env: DEF_LIB="static" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared"
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
>        apt:
> 

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

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH 3/3] app/test/meson: auto detect number of cores

[Aaron Conole]

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
+	num_cores = run_command('lscpu', '-p=cpu').stdout().strip().split('\n')[-1]
+	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

Re: [dpdk-dev] [PATCH 3/3] app/test/meson: auto detect number of cores

[David Marchand]

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>


-- 
David Marchand

Re: [dpdk-dev] [PATCH 3/3] app/test/meson: auto detect number of cores

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH 3/3] app/test/meson: auto detect number of cores

[Luca Boccassi]

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>

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH v2 0/3] travis: enhancements for build (plus a meson fix)

[Aaron Conole]

This series improves the travis integration a bit, and adds the ability for
the meson system to auto-detect the number of cores.

v1->v2 (all patch 3/3 changes):
* Fix a spelling mistake
* Add support for FreeBSD
* Include a default fallback
* Use a more robust core-mask argument source (rather than lscpu)

Aaron Conole (3):
  travis: enable ccache
  travis: add a distinguisher to the 'extra' builds
  app/test/meson: auto detect number of cores

 .travis.yml          | 19 +++++++++----------
 app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 13 deletions(-)

-- 
2.19.1

[dpdk-dev] [PATCH v2 1/3] travis: enable ccache

[Aaron Conole]

We try to make the planet happy travis builds dpdk.

Also, the 'sudo' flag was recently deprecated.  Drop it.

Suggested-by: Luca Boccassi <bluca@debian.org>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 3045eada5..72f8671b1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,4 +1,5 @@
 language: c
+cache: ccache
 compiler:
   - gcc
   - clang
@@ -24,8 +25,6 @@ extra_packages: &extra_packages
 
 before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
 
-sudo: false
-
 env:
   - DEF_LIB="static"
   - DEF_LIB="shared"
-- 
2.19.1

[dpdk-dev] [PATCH v2 2/3] travis: add a distinguisher to the 'extra' builds

[Aaron Conole]

This helps in two ways:
  1. When looking at travis page for dpdk, it's a visual distinction

  2. For ccache support, the build IDs include the 'env', so we get unique
     cache data.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
 .travis.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 72f8671b1..7b167fa64 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -45,49 +45,49 @@ matrix:
       apt:
         packages:
           - *aarch64_packages
-  - env: DEF_LIB="static"
+  - env: DEF_LIB="static" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared"
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static"
+  - env: DEF_LIB="static" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared"
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
         packages:
           - *extra_packages
-  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
+  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: clang
     addons:
       apt:
-- 
2.19.1

[dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores

[Aaron Conole]

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>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
v2:
* Fix a spelling mistake
* Add support for FreeBSD
* Include a default fallback
* Use a more robust core-mask argument source (rather than lscpu)

Conflicts with http://patches.dpdk.org/patch/50850/

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

diff --git a/app/test/meson.build b/app/test/meson.build
index 867cc5863..5e056eb59 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -344,17 +344,43 @@ if get_option('tests')
 	timeout_seconds = 600
 	timeout_seconds_fast = 10
 
+	# Retrieve the number of CPU cores, defaulting to 4.
+	num_cores = '0-3'
+	if host_machine.system() == 'linux'
+		num_cores = run_command('cat',
+					'/sys/devices/system/cpu/present'
+				       ).stdout().strip()
+	elif host_machine.system() == 'freebsd'
+		snum_cores = run_command('/sbin/sysctl', '-n',
+					 'hw.ncpu').stdout().strip()
+		inum_cores = snum_cores.to_int() - 1
+                num_cores = '0-@0@'.format(inum_cores)
+	endif
+
+	num_cores_arg = '-l ' + 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 +389,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 +398,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 +407,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

Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores

[Bruce Richardson]

On Fri, Apr 12, 2019 at 12:21:41PM -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>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Luca Boccassi <bluca@debian.org>
> ---
> v2:
> * Fix a spelling mistake
> * Add support for FreeBSD
> * Include a default fallback
> * Use a more robust core-mask argument source (rather than lscpu)
> 
> Conflicts with http://patches.dpdk.org/patch/50850/
> 
>  app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 867cc5863..5e056eb59 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -344,17 +344,43 @@ if get_option('tests')
>  	timeout_seconds = 600
>  	timeout_seconds_fast = 10
>  
> +	# Retrieve the number of CPU cores, defaulting to 4.
> +	num_cores = '0-3'
> +	if host_machine.system() == 'linux'
> +		num_cores = run_command('cat',
> +					'/sys/devices/system/cpu/present'
> +				       ).stdout().strip()
> +	elif host_machine.system() == 'freebsd'
> +		snum_cores = run_command('/sbin/sysctl', '-n',
> +					 'hw.ncpu').stdout().strip()
> +		inum_cores = snum_cores.to_int() - 1
> +                num_cores = '0-@0@'.format(inum_cores)
> +	endif
> +
> +	num_cores_arg = '-l ' + num_cores
> +
> +	test_args = [num_cores_arg, '-n 4']

This -n 4 parameter can be dropped. Four is the default setting IIRC.
I also wonder are the parameters coming through to the app correctly,
generally meson does not work well with parameters with spaces in them -
I'd expect the "-l" and the num_cores values to be separated in the array.
I also think num_cores_arg value could be dropped too.

If it works though, I'm ok to keep as-is though.

>  	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

While this is needed now, I think in the medium term we should have the
"file-prefix" flag being a warning rather than a hard-error on FreeBSD.
[i.e. keep this, but we should fix it in 19.08 to be shorter]

/Bruce

Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Apr 12, 2019 at 12:21:41PM -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>
>> Reviewed-by: David Marchand <david.marchand@redhat.com>
>> Acked-by: Luca Boccassi <bluca@debian.org>
>> ---
>> v2:
>> * Fix a spelling mistake
>> * Add support for FreeBSD
>> * Include a default fallback
>> * Use a more robust core-mask argument source (rather than lscpu)
>> 
>> Conflicts with http://patches.dpdk.org/patch/50850/
>> 
>>  app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)
>> 
>> diff --git a/app/test/meson.build b/app/test/meson.build
>> index 867cc5863..5e056eb59 100644
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> @@ -344,17 +344,43 @@ if get_option('tests')
>>  	timeout_seconds = 600
>>  	timeout_seconds_fast = 10
>>  
>> +	# Retrieve the number of CPU cores, defaulting to 4.
>> +	num_cores = '0-3'
>> +	if host_machine.system() == 'linux'
>> +		num_cores = run_command('cat',
>> +					'/sys/devices/system/cpu/present'
>> +				       ).stdout().strip()
>> +	elif host_machine.system() == 'freebsd'
>> +		snum_cores = run_command('/sbin/sysctl', '-n',
>> +					 'hw.ncpu').stdout().strip()
>> +		inum_cores = snum_cores.to_int() - 1
>> +                num_cores = '0-@0@'.format(inum_cores)
>> +	endif
>> +
>> +	num_cores_arg = '-l ' + num_cores
>> +
>> +	test_args = [num_cores_arg, '-n 4']
>
> This -n 4 parameter can be dropped. Four is the default setting IIRC.

For another patch.  I thought about doing it with this one, but I'd
rather keep the changes a little bit traceable.  If you think I should
resubmit with it dropped, I will.

> I also wonder are the parameters coming through to the app correctly,
> generally meson does not work well with parameters with spaces in them -
> I'd expect the "-l" and the num_cores values to be separated in the array.
> I also think num_cores_arg value could be dropped too.
>
> If it works though, I'm ok to keep as-is though.

It does work.  Actually, I needed to change because on some of the VM
setups (including the one used by Travis) the '-c f' arg errors because
it wants 4 cores, and only 2 exist.  Either way, this patch doesn't
change the spacing being passed with args :)

>>  	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
>
> While this is needed now, I think in the medium term we should have the
> "file-prefix" flag being a warning rather than a hard-error on FreeBSD.
> [i.e. keep this, but we should fix it in 19.08 to be shorter]

Agreed.  Probably a good cleanup in the future.

> /Bruce

Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores

[Bruce Richardson]

On Fri, Apr 12, 2019 at 02:21:41PM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > On Fri, Apr 12, 2019 at 12:21:41PM -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>
> >> Reviewed-by: David Marchand <david.marchand@redhat.com>
> >> Acked-by: Luca Boccassi <bluca@debian.org>
> >> ---
> >> v2:
> >> * Fix a spelling mistake
> >> * Add support for FreeBSD
> >> * Include a default fallback
> >> * Use a more robust core-mask argument source (rather than lscpu)
> >> 
> >> Conflicts with http://patches.dpdk.org/patch/50850/
> >> 
> >>  app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
> >>  1 file changed, 32 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/app/test/meson.build b/app/test/meson.build
> >> index 867cc5863..5e056eb59 100644
> >> --- a/app/test/meson.build
> >> +++ b/app/test/meson.build
> >> @@ -344,17 +344,43 @@ if get_option('tests')
> >>  	timeout_seconds = 600
> >>  	timeout_seconds_fast = 10
> >>  
> >> +	# Retrieve the number of CPU cores, defaulting to 4.
> >> +	num_cores = '0-3'
> >> +	if host_machine.system() == 'linux'
> >> +		num_cores = run_command('cat',
> >> +					'/sys/devices/system/cpu/present'
> >> +				       ).stdout().strip()
> >> +	elif host_machine.system() == 'freebsd'
> >> +		snum_cores = run_command('/sbin/sysctl', '-n',
> >> +					 'hw.ncpu').stdout().strip()
> >> +		inum_cores = snum_cores.to_int() - 1
> >> +                num_cores = '0-@0@'.format(inum_cores)
> >> +	endif
> >> +
> >> +	num_cores_arg = '-l ' + num_cores
> >> +
> >> +	test_args = [num_cores_arg, '-n 4']
> >
> > This -n 4 parameter can be dropped. Four is the default setting IIRC.
> 
> For another patch.  I thought about doing it with this one, but I'd
> rather keep the changes a little bit traceable.  If you think I should
> resubmit with it dropped, I will.
> 
> > I also wonder are the parameters coming through to the app correctly,
> > generally meson does not work well with parameters with spaces in them -
> > I'd expect the "-l" and the num_cores values to be separated in the array.
> > I also think num_cores_arg value could be dropped too.
> >
> > If it works though, I'm ok to keep as-is though.
> 
> It does work.  Actually, I needed to change because on some of the VM
> setups (including the one used by Travis) the '-c f' arg errors because
> it wants 4 cores, and only 2 exist.  Either way, this patch doesn't
> change the spacing being passed with args :)
> 
> >>  	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
> >
> > While this is needed now, I think in the medium term we should have the
> > "file-prefix" flag being a warning rather than a hard-error on FreeBSD.
> > [i.e. keep this, but we should fix it in 19.08 to be shorter]
> 
> Agreed.  Probably a good cleanup in the future.
> 

Fair responses to all comments.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v2 0/3] travis: enhancements for build (plus a meson fix)

[Thomas Monjalon]

12/04/2019 18:21, Aaron Conole:
> This series improves the travis integration a bit, and adds the ability for
> the meson system to auto-detect the number of cores.
> 
> v1->v2 (all patch 3/3 changes):
> * Fix a spelling mistake
> * Add support for FreeBSD
> * Include a default fallback
> * Use a more robust core-mask argument source (rather than lscpu)
> 
> Aaron Conole (3):
>   travis: enable ccache
>   travis: add a distinguisher to the 'extra' builds
>   app/test/meson: auto detect number of cores

Applied, thanks

For info, we can also cache the build directory with
	cache:
	  directories:
	    - path/to/meson/build/dir
See https://docs.travis-ci.com/user/caching/#arbitrary-directories.