> -----Original Message----- > From: Juraj Linkeš <juraj.lin...@pantheon.tech> > Sent: Thursday, March 12, 2020 15:13 > To: Ruifeng Wang <ruifeng.w...@arm.com>; Aaron Conole > <acon...@redhat.com> > Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; dev@dpdk.org; > david.march...@redhat.com; Gavin Hu <gavin...@arm.com>; Honnappa > Nagarahalli <honnappa.nagaraha...@arm.com>; nd <n...@arm.com>; nd > <n...@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.w...@arm.com> > Sent: Saturday, March 7, 2020 3:36 PM > To: Aaron Conole <acon...@redhat.com> > Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; dev@dpdk.org; > david.march...@redhat.com; Gavin Hu <gavin...@arm.com>; Honnappa > Nagarahalli <honnappa.nagaraha...@arm.com>; Juraj Linkeš > <juraj.lin...@pantheon.tech>; nd <n...@arm.com>; nd <n...@arm.com> > Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage > > > > -----Original Message----- > > From: Aaron Conole <acon...@redhat.com> > > Sent: Friday, March 6, 2020 23:57 > > To: Ruifeng Wang <ruifeng.w...@arm.com> > > Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; > > dev@dpdk.org; david.march...@redhat.com; Gavin Hu > <gavin...@arm.com>; > > Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; > > juraj.lin...@pantheon.tech; nd <n...@arm.com> > > Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage > > > > Ruifeng Wang <ruifeng.w...@arm.com> writes: > > > > >> -----Original Message----- > > >> From: Aaron Conole <acon...@redhat.com> > > >> Sent: Thursday, March 5, 2020 22:37 > > >> To: Ruifeng Wang <ruifeng.w...@arm.com> > > >> Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; > > >> dev@dpdk.org; david.march...@redhat.com; Gavin Hu > > <gavin...@arm.com>; > > >> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; > > >> juraj.lin...@pantheon.tech; nd <n...@arm.com> > > >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage > > >> > > >> Ruifeng Wang <ruifeng.w...@arm.com> writes: > > >> > > >> >> -----Original Message----- > > >> >> From: Aaron Conole <acon...@redhat.com> > > >> >> Sent: Thursday, March 5, 2020 01:31 > > >> >> To: Ruifeng Wang <ruifeng.w...@arm.com> > > >> >> Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; > > >> >> dev@dpdk.org; david.march...@redhat.com; Gavin Hu > > >> <gavin...@arm.com>; > > >> >> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; > > >> >> juraj.lin...@pantheon.tech; nd <n...@arm.com> > > >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without > > >> >> hugepage > > >> >> > > >> >> Ruifeng Wang <ruifeng.w...@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.w...@arm.com> > > >> >> > Reviewed-by: Gavin Hu <gavin...@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],