Re: [dpdk-dev] [PATCH v2 2/2] ci: add test suite run without hugepage

[Ruifeng Wang <Ruifeng.Wang@arm.com>]

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Thursday, March 12, 2020 15:13
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Aaron Conole
> <aconole@redhat.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd
> <nd@arm.com>
> Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Hi,
> 
> What if we just split the test names into fast_hugepage_test_name and
> fast_nohuge_test_names and create two suites (fast-suite and fast-nohuge-
> suite) from that? That seems like a simpler version of one list with booleans
> indicating whether a test is a huge/nohuge test, though it would require dev
> to add tests into the proper list. We could then run the full suite in x86 jobs
> and the nogue suite in arm jobs (and users could use either suite when they
> have/don't have hugepages configured).
> 
> The other option would be to detect hugepages at runtime and skip the tests
> in the full suite when there aren't hugepages on the system. Possibly the
> best option, but where should the check be? Before testcase setup (i.e. if
> there aren't hugepages, skip the testcase right away)?
> 
I'm on this direction. Detect hugepages at build time and generate fast_tests suite with applicable cases.
nr_hugepages is checked before testcase setup.

Will send out patches for review soon.

Thanks.
/Ruifeng

> Thoughts?
> Thanks,
> Juraj
> 
> -----Original Message-----
> From: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Sent: Saturday, March 7, 2020 3:36 PM
> To: Aaron Conole <aconole@redhat.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> 
> > -----Original Message-----
> > From: Aaron Conole <aconole@redhat.com>
> > Sent: Friday, March 6, 2020 23:57
> > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> <Gavin.Hu@arm.com>;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >
> > Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >
> > >> -----Original Message-----
> > >> From: Aaron Conole <aconole@redhat.com>
> > >> Sent: Thursday, March 5, 2020 22:37
> > >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> > <Gavin.Hu@arm.com>;
> > >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> > >>
> > >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> > >>
> > >> >> -----Original Message-----
> > >> >> From: Aaron Conole <aconole@redhat.com>
> > >> >> Sent: Thursday, March 5, 2020 01:31
> > >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> > >> <Gavin.Hu@arm.com>;
> > >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without
> > >> >> hugepage
> > >> >>
> > >> >> Ruifeng Wang <ruifeng.wang@arm.com> writes:
> > >> >>
> > >> >> > This test suite is derived from fast-tests suite. Cases in
> > >> >> > this suite are run with '--no-huge' flag.
> > >> >> >
> > >> >> > The suite aims to cover as many as possible test cases out of
> > >> >> > the fast-tests suites in the environments without huge pages
> > >> >> > support, like containers.
> > >> >> >
> > >> >> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > >> >> > Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> > >> >> > ---
> > >> >>
> > >> >> I like this much more.  Few comments.
> > >> >>
> > >> >> >  .travis.yml          | 10 +++++--
> > >> >> >  app/test/meson.build | 71
> > >> >> >  ++++++++++++++++++++++++++++++++++++++++++++
> > >> >>
> > >> >> You should update doc/guides/prog_guide/meson_ut.rst to include
> > >> >> some detail about the new tests suite.
> > >> >>
> > >> > Thanks. Will update document in next version.
> > >> >
> > >> >> >  2 files changed, 79 insertions(+), 2 deletions(-)
> > >> >> >
> > >> >> > diff --git a/.travis.yml b/.travis.yml index
> > >> >> > b64a81bd0..eed1d96db
> > >> >> > 100644
> > >> >> > --- a/.travis.yml
> > >> >> > +++ b/.travis.yml
> > >> >> > @@ -40,7 +40,7 @@ jobs:
> > >> >> >    - env: DEF_LIB="static"
> > >> >> >      arch: amd64
> > >> >> >      compiler: gcc
> > >> >> > -  - env: DEF_LIB="shared" RUN_TESTS=1
> > >> >> > +  - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests
> > >> >> nohuge-tests"
> > >> >> >      arch: amd64
> > >> >> >      compiler: gcc
> > >> >> >    - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -63,7 +63,7 @@
> jobs:
> > >> >> >    - env: DEF_LIB="static"
> > >> >> >      arch: amd64
> > >> >> >      compiler: clang
> > >> >> > -  - env: DEF_LIB="shared" RUN_TESTS=1
> > >> >> > +  - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests
> > >> >> nohuge-tests"
> > >> >> >      arch: amd64
> > >> >> >      compiler: clang
> > >> >> >    - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -101,6 +101,9 @@
> > jobs:
> > >> >> >    - env: DEF_LIB="static"
> > >> >> >      arch: arm64
> > >> >> >      compiler: gcc
> > >> >> > +  - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-
> > tests"
> > >> >> > +    arch: arm64
> > >> >> > +    compiler: gcc
> > >> >> >    - env: DEF_LIB="shared" BUILD_DOCS=1
> > >> >> >      arch: arm64
> > >> >> >      compiler: gcc
> > >> >> > @@ -124,3 +127,6 @@ jobs:
> > >> >> >    - env: DEF_LIB="shared"
> > >> >> >      arch: arm64
> > >> >> >      compiler: clang
> > >> >> > +  - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-
> > tests"
> > >> >> > +    arch: arm64
> > >> >> > +    compiler: clang
> > >> >> > diff --git a/app/test/meson.build b/app/test/meson.build index
> > >> >> > 0a2ce710f..162a1a76f 100644
> > >> >> > --- a/app/test/meson.build
> > >> >> > +++ b/app/test/meson.build
> > >> >> > @@ -237,6 +237,60 @@ fast_test_names = [
> > >> >> >          'thash_autotest',
> > >> >> >  ]
> > >> >>
> > >> >> Shouldn't we also trim the list of fast-tests?  Otherwise, these
> > >> >> tests will run twice.
> > >> >>
> > >> > I think you mean to have exclusive lists for fast-tests and nohuge-tests.
> > >>
> > >> That's what I was thinking.
> > >>
> > >> > Overlapped cases will run twice if both test suites are opted in.
> > >> > But the two runs are not the same, one runs with hugepage and the
> > >> > other runs in no-huge mode.
> > >>
> > >> Is it really so different between huge and no-huge?  Most of the
> > >> libraries won't care - they call the rte_**alloc functions, and it
> > >> returns blocks of memory.  Maybe I am simplifying it too much.
> > >>
> > >> > If fast-tests list is splited,  we will need to always run
> > >> > multiple suites to cover all fast tests.
> > >> > We can keep x86 to run only fast-tests suite to avoid extra test
> > >> > runs if they are not necessary. Thoughts?
> > >>
> > >> I guess since most DPDK usage will be with hugepages, we should
> > >> prefer to test with it.  I don't care too much about the color of
> > >> this particular shed.  If you want to do it that way, it's okay by
> > >> me
> > >> - it gives us the coverage, and doesn't duplicate tests between
> > >> those
> > environments.
> > >>
> > >> BUT it means when we add a new test to the suite, we need to
> > remember
> > >> to add it in two places - fast_test and nohuge_test.  That almost
> > >> guarantees we will miss tests (because we accidentally don't add it
> > >> to nohuge).  Maybe there's another way, like keep a list of all the
> > >> tests and some information on whether the test needs hugepages to
> > >> run.  Then if there are no hugepages available, we can write that
> > >> we SKIP
> > the tests that don't support huge pages.
> > >> In that way, we don't need two different lists - and if there are
> > >> hugepages, we will run all the test cases.
> > >> WDYT?
> > >>
> > > Yes. Agree with you that having duplicate tests in suites is error prone.
> >
> > Cool!
> >
> > > IIUC, cases in a suite is determined at build time, as well as
> > > command
> > options to run cases.
> > > This implies hugepage availability needs to be detected at build
> > > time if we want to run only suitable cases in suite in an
> > > environment. It could be
> > something we don't want.
> > >
> > > I'll trim fast-tests in next version to remove duplicated cases.
> >
> > I think it might be better to make the array something like (just a
> > psuedo- code example):
> >
> >    # psuedo-code to check for hugepages
> >    has_hugepages = check_for_hugepages()
> >
> >    ...
> >
> >    fast_test_names = [
> >      ['acl_autotest', true],
> >      ['alarm_autotest', true],
> >      ['atomic_autotest', true],
> >      ...
> >
> > Then in the code:
> >
> >    foreach arg : fast_test_names
> >       ....
> >       if not arg[1]
> >          test(arg[0], ...)
> >       if has_hugepages and arg[1]
> >          test(arg[0], )
> >
> > Does it make sense?  Do you see a problem?
> >
> Yes, this will keep tests in a single suite.
> 
> Actually, I thought about this approach, but had no idea on
> check_for_hugepages().
> Value of "/proc/sys/vm/nr_hugepages" may be not reliable. Hugepage could
> be allocated at run time after project building.
> I can try hugepage allocating to detect, but it looks inelegant.
> Any suggestions?
> 
> Thank you.
> /Ruifeng
> > > Thank you.
> > >
> > >> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> > >> >>
> > >> >> > +nohuge_test_names = [
> > >> >> > +        'byteorder_autotest',
> > >> >> > +        'cmdline_autotest',
> > >> >> > +        'common_autotest',
> > >> >> > +        'cpuflags_autotest',
> > >> >> > +        'cycles_autotest',
> > >> >> > +        'debug_autotest',
> > >> >> > +        'eal_flags_n_opt_autotest',
> > >> >> > +        'eal_flags_no_huge_autotest',
> > >> >> > +        'eal_flags_vdev_opt_autotest',
> > >> >> > +        'eal_fs_autotest',
> > >> >> > +        'errno_autotest',
> > >> >> > +        'event_ring_autotest',
> > >> >> > +        'fib_autotest',
> > >> >> > +        'fib6_autotest',
> > >> >> > +        'interrupt_autotest',
> > >> >> > +        'logs_autotest',
> > >> >> > +        'lpm_autotest',
> > >> >> > +        'lpm6_autotest',
> > >> >> > +        'memcpy_autotest',
> > >> >> > +        'meter_autotest',
> > >> >> > +        'per_lcore_autotest',
> > >> >> > +        'prefetch_autotest',
> > >> >> > +        'rcu_qsbr_autotest',
> > >> >> > +        'red_autotest',
> > >> >> > +        'rib_autotest',
> > >> >> > +        'rib6_autotest',
> > >> >> > +        'ring_autotest',
> > >> >> > +        'rwlock_rda_autotest',
> > >> >> > +        'rwlock_rds_wrm_autotest',
> > >> >> > +        'rwlock_rde_wro_autotest',
> > >> >> > +        'sched_autotest',
> > >> >> > +        'spinlock_autotest',
> > >> >> > +        'string_autotest',
> > >> >> > +        'tailq_autotest',
> > >> >> > +        'user_delay_us',
> > >> >> > +        'version_autotest',
> > >> >> > +        'crc_autotest',
> > >> >> > +        'delay_us_sleep_autotest',
> > >> >> > +        'eventdev_common_autotest',
> > >> >> > +        'fbarray_autotest',
> > >> >> > +        'ipsec_autotest',
> > >> >> > +        'kni_autotest',
> > >> >> > +        'kvargs_autotest',
> > >> >> > +        'member_autotest',
> > >> >> > +        'metrics_autotest',
> > >> >> > +        'power_cpufreq_autotest',
> > >> >> > +        'power_autotest',
> > >> >> > +        'power_kvm_vm_autotest',
> > >> >> > +        'reorder_autotest',
> > >> >> > +        'service_autotest',
> > >> >> > +        'thash_autotest',
> > >> >> > +]
> > >> >> > +
> > >> >> >  perf_test_names = [
> > >> >> >          'ring_perf_autotest',
> > >> >> >          'mempool_perf_autotest', @@ -341,6 +395,10 @@ if
> > >> >> > dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> > >> >> >  	fast_test_names += 'latencystats_autotest'
> > >> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
> > >> >> >  	fast_test_names += 'pdump_autotest'
> > >> >> > +	nohuge_test_names += 'ring_pmd_autotest'
> > >> >> > +	nohuge_test_names += 'bitratestats_autotest'
> > >> >> > +	nohuge_test_names += 'latencystats_autotest'
> > >> >> > +	nohuge_test_names += 'pdump_autotest'
> > >> >> >  endif
> > >> >> >
> > >> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
> > >> foreach
> > >> >> > arg : fast_test_names
> > >> >> >  	endif
> > >> >> >  endforeach
> > >> >> >
> > >> >> > +foreach arg : nohuge_test_names
> > >> >> > +	if host_machine.system() == 'linux'
> > >> >> > +		test(arg, dpdk_test,
> > >> >> > +			  env : ['DPDK_TEST=' + arg],
> > >> >> > +			  args : test_args +
> > >> >> > +				 ['--no-huge'] + ['-m 1024'] +
> > >> >> > +				 ['--file-prefix=@0@'.format(arg)],
> > >> >> > +		timeout : timeout_seconds_fast,
> > >> >> > +		is_parallel : false,
> > >> >> > +		suite : 'nohuge-tests')
> > >> >> > +	endif
> > >> >> > +endforeach
> > >> >> > +
> > >> >> >  foreach arg : perf_test_names
> > >> >> >  	test(arg, dpdk_test,
> > >> >> >  	env : ['DPDK_TEST=' + arg],