DPDK patches and discussions
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@redhat.com>
To: Ruifeng Wang <Ruifeng.Wang@arm.com>
Cc: "maicolgabriel\@hotmail.com" <maicolgabriel@hotmail.com>,
	"bruce.richardson\@intel.com" <bruce.richardson@intel.com>,
	"dev\@dpdk.org" <dev@dpdk.org>,
	"david.marchand\@redhat.com" <david.marchand@redhat.com>,
	Gavin Hu <Gavin.Hu@arm.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	"juraj.linkes\@pantheon.tech" <juraj.linkes@pantheon.tech>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v2 2/2] ci: add test suite run without hugepage
Date: Fri, 06 Mar 2020 10:56:56 -0500	[thread overview]
Message-ID: <f7two7xmq7r.fsf@dhcp-25.97.bos.redhat.com> (raw)
In-Reply-To: <AM0PR08MB3986344EB482C47998782E159EE30@AM0PR08MB3986.eurprd08.prod.outlook.com> (Ruifeng Wang's message of "Fri, 6 Mar 2020 08:09:29 +0000")

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?

> 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],


  reply	other threads:[~2020-03-06 15:57 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-25  7:32 [dpdk-dev] [PATCH v1] " Ruifeng Wang
2020-02-25  9:14 ` David Marchand
2020-02-25 14:36   ` Aaron Conole
2020-02-25 15:39     ` Ruifeng Wang
2020-02-25 15:56       ` Aaron Conole
2020-02-26  2:47         ` Ruifeng Wang
2020-02-25 15:23   ` Ruifeng Wang
2020-02-28  4:19 ` [dpdk-dev] [PATCH v2 0/2] no-huge test suite Ruifeng Wang
2020-02-28  4:19   ` [dpdk-dev] [PATCH v2 1/2] ci: allow multiple test suites to run in one job Ruifeng Wang
2020-02-28  4:19   ` [dpdk-dev] [PATCH v2 2/2] ci: add test suite run without hugepage Ruifeng Wang
2020-03-04 17:31     ` Aaron Conole
2020-03-05  3:41       ` Ruifeng Wang
2020-03-05 14:36         ` Aaron Conole
2020-03-06  8:09           ` Ruifeng Wang
2020-03-06 15:56             ` Aaron Conole [this message]
2020-03-06 16:05               ` David Marchand
2020-03-06 16:16                 ` Aaron Conole
2020-03-06 16:33                   ` Bruce Richardson
2020-03-07 14:36               ` Ruifeng Wang
2020-03-12  7:13                 ` Juraj Linkeš
2020-03-12  8:33                   ` Ruifeng Wang
2020-03-13  8:16 ` [dpdk-dev] [PATCH v3 0/4] no-huge unit test Ruifeng Wang
2020-03-13  8:16   ` [dpdk-dev] [PATCH v3 1/4] test: enable tests to run in no-huge mode Ruifeng Wang
2020-03-17 12:49     ` Aaron Conole
2020-03-17 19:15     ` Wang, Yipeng1
2020-03-13  8:16   ` [dpdk-dev] [PATCH v3 2/4] ci: generate fast-tests suite base on hugepage availability Ruifeng Wang
2020-03-17 12:50     ` Aaron Conole
2020-03-23  8:56     ` David Marchand
2020-03-23  9:32       ` David Marchand
2020-03-23 15:50         ` Ruifeng Wang
2020-03-28 16:17           ` David Marchand
2020-03-30 16:03             ` Ruifeng Wang
2020-04-10 12:25               ` David Marchand
2020-03-23 15:36       ` Ruifeng Wang
2020-03-13  8:16   ` [dpdk-dev] [PATCH v3 3/4] ci: proceed with verification without hugepage Ruifeng Wang
2020-03-17 12:50     ` Aaron Conole
2020-03-13  8:16   ` [dpdk-dev] [PATCH v3 4/4] ci: enable unit test for aarch64 Ruifeng Wang
2020-03-17 12:50     ` Aaron Conole
2020-03-13 12:48   ` [dpdk-dev] [PATCH v3 0/4] no-huge unit test Aaron Conole
2020-03-13 13:04     ` Aaron Conole
2020-03-13 15:54       ` David Marchand
2020-03-16  8:27         ` Ruifeng Wang
2020-03-16 13:25           ` Aaron Conole
2020-03-16 14:13         ` Aaron Conole
2020-03-16 14:41           ` Bruce Richardson
2020-03-17 17:02   ` dwilder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f7two7xmq7r.fsf@dhcp-25.97.bos.redhat.com \
    --to=aconole@redhat.com \
    --cc=Gavin.Hu@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=maicolgabriel@hotmail.com \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).