[dpdk-dev] [PATCH 00/14] Unit tests fixes for CI

[dpdk-dev] [PATCH 00/14] Unit tests fixes for CI

[David Marchand]

This is a joint effort to make the unit tests ready for CI.
The first 8 patches are fixes that I had accumulated.
Then the second part of the series focuses on skipping tests when some
requirements are not fulfilled so that we can start them in a restrained
environment like Travis virtual machines that gives us two cores and does
not have specific hw devices.

We are still not ready for enabling those tests in Travis.
At least, the following issues remain:
- some fixes on librte_acl have not been merged yet [1],
- the tests on --file-prefix are still ko, and have been isolated in a
  test that we could disable while waiting for the fixes,
- rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
  than 10s,
- librte_table unit test crashes on ipv6 [2],
- the "perf" tests are taking way too long for my taste,
- the shared build unit tests all fail when depending on mempool since
  the mempool drivers are not loaded,

1: http://patchwork.dpdk.org/project/dpdk/list/?series=4242
2: https://bugs.dpdk.org/show_bug.cgi?id=285

Comments/reviews welcome!

-- 
David Marchand

David Marchand (11):
  test/bonding: add missing sources for link bonding RSS
  test/crypto: move tests to the driver specific list
  test/eventdev: move tests to the driver specific list
  test/hash: fix off-by-one check on core count
  test/hash: clean remaining trace of scaling autotest
  test/latencystats: fix stack smashing
  test/stack: fix lock-free test name
  test/eal: set memory channel config only in dedicated test
  test/eal: set core mask/list config only in dedicated test
  test: do not start tests in parallel
  test: skip tests when missing requirements

Dharmik Thakkar (1):
  test/hash: rectify slaveid to point to valid cores

Michael Santana (2):
  test/eal: check number of cores before running subtests
  test/eal: make the test pass again

 app/test/autotest.py                |   2 +-
 app/test/autotest_data.py           |  10 +-
 app/test/meson.build                |  67 ++++------
 app/test/test.c                     |  24 ++--
 app/test/test_compressdev.c         |   4 +-
 app/test/test_cryptodev.c           |   4 +-
 app/test/test_distributor.c         |   4 +-
 app/test/test_distributor_perf.c    |   4 +-
 app/test/test_eal_flags.c           | 252 +++++++++++++++++++-----------------
 app/test/test_event_timer_adapter.c |   5 +-
 app/test/test_eventdev.c            |   2 +
 app/test/test_func_reentrancy.c     |   6 +-
 app/test/test_hash_multiwriter.c    |   7 +-
 app/test/test_hash_readwrite.c      |   7 +-
 app/test/test_hash_readwrite_lf.c   |  34 +++--
 app/test/test_ipsec.c               |   4 +-
 app/test/test_latencystats.c        |  18 ---
 app/test/test_mbuf.c                |  13 +-
 app/test/test_rcu_qsbr.c            |  10 +-
 app/test/test_rcu_qsbr_perf.c       |   9 +-
 app/test/test_service_cores.c       |  14 ++
 app/test/test_stack.c               |   8 +-
 app/test/test_timer.c               |  10 +-
 app/test/test_timer_secondary.c     |  10 +-
 24 files changed, 272 insertions(+), 256 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 01/14] test/bonding: add missing sources for link bonding RSS

[David Marchand]

Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test/meson.build b/app/test/meson.build
index 83391ce..faf16ee 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -58,6 +58,7 @@ test_sources = files('commands.c',
 	'test_latencystats.c',
 	'test_link_bonding.c',
 	'test_link_bonding_mode4.c',
+	'test_link_bonding_rssconf.c',
 	'test_logs.c',
 	'test_lpm.c',
 	'test_lpm6.c',
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 01/14] test/bonding: add missing sources for link bonding RSS

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 02/14] test/crypto: move tests to the driver specific list

[David Marchand]

For consistency, put all specific crypto driver tests in the dedicated
list (in alphabetic order).

While at it:
- remove dead reference to cryptodev_sw_mrvl_autotest (renamed as
  cryptodev_sw_mvsam_autotest),
- call the crypto scheduler test only when built,

Fixes: 9eabcb682493 ("test: update autotest list")
Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/autotest_data.py |  4 ++--
 app/test/meson.build      | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 0f2c9a7..6cf7eca 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -393,8 +393,8 @@
         "Report":  None,
     },
     {
-        "Name":    "Cryptodev sw mrvl autotest",
-        "Command": "cryptodev_sw_mrvl_autotest",
+        "Name":    "Cryptodev sw mvsam autotest",
+        "Command": "cryptodev_sw_mvsam_autotest",
         "Func":    default_autotest,
         "Report":  None,
     },
diff --git a/app/test/meson.build b/app/test/meson.build
index faf16ee..d428e02 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -196,10 +196,7 @@ fast_parallel_test_names = [
 # All test cases in fast_non_parallel_test_names list are non-parallel
 fast_non_parallel_test_names = [
         'bitratestats_autotest',
-        'cryptodev_sw_armv8_autotest',
         'crc_autotest',
-        'cryptodev_openssl_asym_autotest',
-        'cryptodev_sw_mvsam_autotest',
         'delay_us_sleep_autotest',
         'distributor_autotest',
         'eventdev_common_autotest',
@@ -255,21 +252,22 @@ perf_test_names = [
 
 # All test cases in driver_test_names list are non-parallel
 driver_test_names = [
-        'link_bonding_autotest',
-        'link_bonding_mode4_autotest',
-        'link_bonding_rssconf_autotest',
-        'cryptodev_sw_mrvl_autotest',
-        'cryptodev_dpaa2_sec_autotest',
-        'cryptodev_dpaa_sec_autotest',
-        'cryptodev_qat_autotest',
         'cryptodev_aesni_mb_autotest',
-        'cryptodev_openssl_autotest',
-        'cryptodev_scheduler_autotest',
         'cryptodev_aesni_gcm_autotest',
+        'cryptodev_dpaa_sec_autotest',
+        'cryptodev_dpaa2_sec_autotest',
         'cryptodev_null_autotest',
-        'cryptodev_sw_snow3g_autotest',
+        'cryptodev_openssl_autotest',
+        'cryptodev_openssl_asym_autotest',
+        'cryptodev_qat_autotest',
+        'cryptodev_sw_armv8_autotest',
         'cryptodev_sw_kasumi_autotest',
+        'cryptodev_sw_mvsam_autotest',
+        'cryptodev_sw_snow3g_autotest',
         'cryptodev_sw_zuc_autotest',
+        'link_bonding_autotest',
+        'link_bonding_mode4_autotest',
+        'link_bonding_rssconf_autotest',
 ]
 
 # All test cases in dump_test_names list are non-parallel
@@ -325,6 +323,10 @@ if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
 	endif
 endif
 
+if dpdk_conf.has('RTE_LIBRTE_PMD_CRYPTO_SCHEDULER')
+	driver_test_names += 'cryptodev_scheduler_autotest'
+endif
+
 foreach d:test_deps
 	def_lib = get_option('default_library')
 	test_dep_objs += get_variable(def_lib + '_rte_' + d)
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 02/14] test/crypto: move tests to the driver specific list

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> For consistency, put all specific crypto driver tests in the dedicated
> list (in alphabetic order).
>
> While at it:
> - remove dead reference to cryptodev_sw_mrvl_autotest (renamed as
>   cryptodev_sw_mvsam_autotest),
> - call the crypto scheduler test only when built,
>
> Fixes: 9eabcb682493 ("test: update autotest list")
> Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 03/14] test/eventdev: move tests to the driver specific list

[David Marchand]

Same treatment than crypto tests, move the eventdev drivers tests in the
driver list.

While at it:
- eventdev_octeontx_autotest has been renamed as
  eventdev_selftest_octeontx,
- eventdev_sw_autotest has been renamed as
  eventdev_selftest_sw,

Fixes: 50fb749a3972 ("event/octeontx: move test to driver")
Fixes: 85fb515b7318 ("event/sw: move test to driver")
Fixes: 123d67c73b06 ("test/event: register selftests")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index d428e02..eb3df24 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -200,8 +200,6 @@ fast_non_parallel_test_names = [
         'delay_us_sleep_autotest',
         'distributor_autotest',
         'eventdev_common_autotest',
-        'eventdev_octeontx_autotest',
-        'eventdev_sw_autotest',
         'fbarray_autotest',
         'hash_readwrite_autotest',
         'hash_readwrite_lf_autotest',
@@ -237,7 +235,6 @@ perf_test_names = [
         'timer_racecond_autotest',
         'efd_autotest',
         'hash_functions_autotest',
-        'eventdev_selftest_sw',
         'member_perf_autotest',
         'efd_perf_autotest',
         'lpm6_perf_autotest',
@@ -265,6 +262,8 @@ driver_test_names = [
         'cryptodev_sw_mvsam_autotest',
         'cryptodev_sw_snow3g_autotest',
         'cryptodev_sw_zuc_autotest',
+        'eventdev_selftest_octeontx',
+        'eventdev_selftest_sw',
         'link_bonding_autotest',
         'link_bonding_mode4_autotest',
         'link_bonding_rssconf_autotest',
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 03/14] test/eventdev: move tests to the driver specific list

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Same treatment than crypto tests, move the eventdev drivers tests in the
> driver list.
>
> While at it:
> - eventdev_octeontx_autotest has been renamed as
>   eventdev_selftest_octeontx,
> - eventdev_sw_autotest has been renamed as
>   eventdev_selftest_sw,
>
> Fixes: 50fb749a3972 ("event/octeontx: move test to driver")
> Fixes: 85fb515b7318 ("event/sw: move test to driver")
> Fixes: 123d67c73b06 ("test/event: register selftests")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 04/14] test/hash: fix off-by-one check on core count

[David Marchand]

This subtest wants to start rwc_core_cnt[n] reader threads, while the
master core is waiting for them to report.

Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_hash_readwrite_lf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 343a338..f9f233a 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -939,7 +939,7 @@ struct {
 		}
 		for (n = 0; n < NUM_TEST; n++) {
 			unsigned int tot_lcore = rte_lcore_count();
-			if (tot_lcore < rwc_core_cnt[n])
+			if (tot_lcore < rwc_core_cnt[n] + 1)
 				goto finish;
 
 			printf("\nNumber of readers: %u\n", rwc_core_cnt[n]);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 04/14] test/hash: fix off-by-one check on core count

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> This subtest wants to start rwc_core_cnt[n] reader threads, while the
> master core is waiting for them to report.
>
> Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

Re: [dpdk-dev] [PATCH 04/14] test/hash: fix off-by-one check on core count

[Wang, Yipeng1]

>-----Original Message-----
>From: David Marchand [mailto:david.marchand@redhat.com]
>Sent: Tuesday, June 4, 2019 2:00 AM
>To: dev@dpdk.org
>Cc: thomas@monjalon.net; aconole@redhat.com; msantana@redhat.com; stable@dpdk.org; Wang, Yipeng1
><yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; De Lara
>Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Subject: [PATCH 04/14] test/hash: fix off-by-one check on core count
>
>This subtest wants to start rwc_core_cnt[n] reader threads, while the
>master core is waiting for them to report.
>
[Wang, Yipeng] May add a sentence to make the commit message more explicit:
The code does not consider the master core in the core
count calculation. This commit fixes this issue.


>Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
>Cc: stable@dpdk.org
>
>Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Yipeng Wang <yipeng1.wang@intel.com>

[dpdk-dev] [PATCH 05/14] test/hash: rectify slaveid to point to valid cores

[David Marchand]

From: Dharmik Thakkar <dharmik.thakkar@arm.com>

This patch rectifies slave_id to point to valid core indexes rather than
core ranks in read-write lock-free concurrency test.

It also replaces a 'for' loop with RTE_LCORE_FOREACH API.

Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
Cc: stable@dpdk.org

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_hash_readwrite_lf.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index f9f233a..5644361 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -126,11 +126,9 @@ struct {
 	uint32_t i = 0;
 	uint16_t core_id;
 	uint32_t max_cores = rte_lcore_count();
-	for (core_id = 0; core_id < RTE_MAX_LCORE && i < max_cores; core_id++) {
-		if (rte_lcore_is_enabled(core_id)) {
-			enabled_core_ids[i] = core_id;
-			i++;
-		}
+	RTE_LCORE_FOREACH(core_id) {
+		enabled_core_ids[i] = core_id;
+		i++;
 	}
 
 	if (i != max_cores) {
@@ -738,7 +736,7 @@ struct {
 							enabled_core_ids[i]);
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -810,7 +808,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -886,7 +884,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -962,7 +960,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1037,7 +1035,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1132,12 +1130,12 @@ struct {
 				for (i = rwc_core_cnt[n] + 1;
 				     i <= rwc_core_cnt[m] + rwc_core_cnt[n];
 				     i++)
-					rte_eal_wait_lcore(i);
+					rte_eal_wait_lcore(enabled_core_ids[i]);
 
 				writer_done = 1;
 
 				for (i = 1; i <= rwc_core_cnt[n]; i++)
-					if (rte_eal_wait_lcore(i) < 0)
+					if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 						goto err;
 
 				unsigned long long cycles_per_lookup =
@@ -1221,7 +1219,7 @@ struct {
 			writer_done = 1;
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 05/14] test/hash: rectify slaveid to point to valid cores

[Wang, Yipeng1]

>-----Original Message-----
>From: David Marchand [mailto:david.marchand@redhat.com]
>Sent: Tuesday, June 4, 2019 2:00 AM
>To: dev@dpdk.org
>Cc: thomas@monjalon.net; aconole@redhat.com; msantana@redhat.com; Dharmik Thakkar <dharmik.thakkar@arm.com>;
>stable@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Subject: [PATCH 05/14] test/hash: rectify slaveid to point to valid cores
>
>From: Dharmik Thakkar <dharmik.thakkar@arm.com>
>
>This patch rectifies slave_id to point to valid core indexes rather than
>core ranks in read-write lock-free concurrency test.
>
>It also replaces a 'for' loop with RTE_LCORE_FOREACH API.
>
>Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
>Cc: stable@dpdk.org
>
>Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>Signed-off-by: David Marchand <david.marchand@redhat.com>
 
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>

[dpdk-dev] [PATCH 06/14] test/hash: clean remaining trace of scaling autotest

[David Marchand]

Fixes: 3c518ca41ffa ("test/hash: remove hash scaling unit test")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index eb3df24..cd46289 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -203,7 +203,6 @@ fast_non_parallel_test_names = [
         'fbarray_autotest',
         'hash_readwrite_autotest',
         'hash_readwrite_lf_autotest',
-        'hash_scaling_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 06/14] test/hash: clean remaining trace of scaling autotest

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Fixes: 3c518ca41ffa ("test/hash: remove hash scaling unit test")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 07/14] test/latencystats: fix stack smashing

[David Marchand]

Caught in one Travis run:
 + ------------------------------------------------------- +
 + Test Suite : Latency Stats Unit Test Suite
 + ------------------------------------------------------- +
 + TestCase [ 0] : test_latency_init succeeded
 + TestCase [ 1] : test_latency_update succeeded
[snip]
 + TestCase [1601724781] : test_latencystats_get_names succeeded
[snip]
 + Tests Failed :      1601790830

htonl(1601724781) -> "m", "a", "x", "_"
htonl(1601790830) -> "n", "c", "y", "_"

Looks like someone went too far.

The test passes a bigger size than the array it passes along.

Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_latencystats.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 039c508..8dd794b 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -69,13 +69,10 @@ static int test_latencystats_get_names(void)
 	int ret = 0, i = 0;
 	int size = 0;
 	struct rte_metric_name names[NUM_STATS];
-	struct rte_metric_name wrongnames[NUM_STATS - 2];
 
 	size_t m_size = sizeof(struct rte_metric_name);
 	for (i = 0; i < NUM_STATS; i++)
 		memset(&names[i], 0, m_size);
-	for (i = 0; i < NUM_STATS - 2; i++)
-		memset(&wrongnames[i], 0, m_size);
 
 	/* Success Test: Valid names and size */
 	size = NUM_STATS;
@@ -99,10 +96,6 @@ static int test_latencystats_get_names(void)
 	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get the metrics count,"
 		    "Actual: %d Expected: %d", ret, NUM_STATS);
 
-	/* Failure Test: Invalid names (array size lesser than size) */
-	size = NUM_STATS + 1;
-	ret = rte_latencystats_get_names(wrongnames, size);
-	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get metrics names");
 	return TEST_SUCCESS;
 }
 
@@ -112,13 +105,10 @@ static int test_latencystats_get(void)
 	int ret = 0, i = 0;
 	int size = 0;
 	struct rte_metric_value values[NUM_STATS];
-	struct rte_metric_value wrongvalues[NUM_STATS - 2];
 
 	size_t v_size = sizeof(struct rte_metric_value);
 	for (i = 0; i < NUM_STATS; i++)
 		memset(&values[i], 0, v_size);
-	for (i = 0; i < NUM_STATS - 2; i++)
-		memset(&wrongvalues[i], 0, v_size);
 
 	/* Success Test: Valid values and valid size */
 	size = NUM_STATS;
@@ -137,14 +127,6 @@ static int test_latencystats_get(void)
 	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get the stats count,"
 		    "Actual: %d Expected: %d", ret, NUM_STATS);
 
-	/* Failure Test: Invalid values(array size lesser than size)
-	 * and invalid size
-	 */
-	size = NUM_STATS + 2;
-	ret = rte_latencystats_get(wrongvalues, size);
-	TEST_ASSERT(ret == NUM_STATS, "Test Failed to get latency metrics"
-			" values");
-
 	return TEST_SUCCESS;
 }
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 07/14] test/latencystats: fix stack smashing

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Caught in one Travis run:
>  + ------------------------------------------------------- +
>  + Test Suite : Latency Stats Unit Test Suite
>  + ------------------------------------------------------- +
>  + TestCase [ 0] : test_latency_init succeeded
>  + TestCase [ 1] : test_latency_update succeeded
> [snip]
>  + TestCase [1601724781] : test_latencystats_get_names succeeded
> [snip]
>  + Tests Failed :      1601790830
>
> htonl(1601724781) -> "m", "a", "x", "_"
> htonl(1601790830) -> "n", "c", "y", "_"
>
> Looks like someone went too far.
>
> The test passes a bigger size than the array it passes along.
>
> Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 08/14] test/stack: fix lock-free test name

[David Marchand]

Fixes: 0420378bbfc4 ("test/stack: check lock-free implementation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index cd46289..3c9706a 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -184,7 +184,7 @@ fast_parallel_test_names = [
         'sched_autotest',
         'spinlock_autotest',
         'stack_autotest',
-        'stack_nb_autotest',
+        'stack_lf_autotest',
         'string_autotest',
         'table_autotest',
         'tailq_autotest',
@@ -243,7 +243,7 @@ perf_test_names = [
         'ring_pmd_perf_autotest',
         'pmd_perf_autotest',
         'stack_perf_autotest',
-        'stack_nb_perf_autotest',
+        'stack_lf_perf_autotest',
 ]
 
 # All test cases in driver_test_names list are non-parallel
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 08/14] test/stack: fix lock-free test name

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Fixes: 0420378bbfc4 ("test/stack: check lock-free implementation")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

[dpdk-dev] [PATCH 09/14] test/eal: set memory channel config only in dedicated test

[David Marchand]

The -n option is an optimisation configuration option that defaults to 0.
Such a default value makes the mempool library distributes objects as if
there was 4 memory channels, so -n 4 is the same as the default behavior.

This parameter was mandatory a long time ago, but has been optional for
a while. We check that setting this value works fine in its own test.
Remove it everywhere else.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/autotest.py      |   2 +-
 app/test/meson.build      |   2 +-
 app/test/test_eal_flags.c | 191 ++++++++++++++++++++++++----------------------
 3 files changed, 101 insertions(+), 94 deletions(-)

diff --git a/app/test/autotest.py b/app/test/autotest.py
index 46c469e..b42f488 100644
--- a/app/test/autotest.py
+++ b/app/test/autotest.py
@@ -32,7 +32,7 @@ def usage():
     else:
         test_whitelist = testlist
 
-cmdline = "%s -c f -n 4" % (sys.argv[1])
+cmdline = "%s -c f" % (sys.argv[1])
 
 print(cmdline)
 
diff --git a/app/test/meson.build b/app/test/meson.build
index 3c9706a..7ad3684 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -365,7 +365,7 @@ if get_option('tests')
 
 	num_cores_arg = '-l ' + num_cores
 
-	test_args = [num_cores_arg, '-n 4']
+	test_args = [num_cores_arg]
 	foreach arg : fast_parallel_test_names
 		if host_machine.system() == 'linux'
 			test(arg, dpdk_test,
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 9112c96..e82e56a 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -245,25 +245,25 @@ enum hugepage_action {
 #endif
 
 	const char *wlinval[][11] = {
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "error", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:error:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0.1error", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "error0:0:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0.1.2", "", ""},
 	};
 	/* Test with valid whitelist option */
-	const char *wlval1[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval1[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "00FF:09:0B.3"};
-	const char *wlval2[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval2[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "09:0B.3", pci_whitelist, "0a:0b.1"};
-	const char *wlval3[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval3[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "09:0B.3,type=test",
 			pci_whitelist, "08:00.1,type=normal",
 	};
@@ -311,15 +311,16 @@ enum hugepage_action {
 #endif
 
 	const char *blinval[][9] = {
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "error"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:error:0.1"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0.1error"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "error0:0:0.1"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0.1.2"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "error"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:error:0.1"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1error"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "error0:0:0.1"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1.2"},
 	};
 	/* Test with valid blacklist option */
-	const char *blval[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "FF:09:0B.3"};
+	const char *blval[] = {prgname, prefix, mp_flag, "-c", "1",
+			       "-b", "FF:09:0B.3"};
 
 	int i;
 
@@ -354,17 +355,17 @@ enum hugepage_action {
 #endif
 
 	/* Test with invalid vdev option */
-	const char *vdevinval[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevinval[] = {prgname, prefix, no_huge,
 				"-c", "1", vdev, "eth_dummy"};
 
 	/* Test with valid vdev option */
-	const char *vdevval1[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval1[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0"};
 
-	const char *vdevval2[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval2[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0,args=test"};
 
-	const char *vdevval3[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval3[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0,nodeaction=r1:0:CREATE"};
 
 	if (launch_proc(vdevinval) == 0) {
@@ -412,13 +413,13 @@ enum hugepage_action {
 #endif
 
 	const char *rinval[][9] = {
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "error"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "0"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "-1"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "17"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "error"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "0"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "-1"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "17"},
 	};
 	/* Test with valid blacklist option */
-	const char *rval[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "16"};
+	const char *rval[] = {prgname, prefix, mp_flag, "-c", "1", "-r", "16"};
 
 	int i;
 
@@ -456,74 +457,74 @@ enum hugepage_action {
 #endif
 
 	/* -c flag but no coremask value */
-	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
+	const char *argv1[] = { prgname, prefix, mp_flag, "-c"};
 	/* No -c, -l or --lcores flag at all */
-	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
+	const char *argv2[] = { prgname, prefix, mp_flag};
 	/* bad coremask value */
 	const char *argv3[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-c", "error" };
+				"-c", "error" };
 	/* sanity check of tests - valid coremask value */
 	const char *argv4[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-c", "1" };
+				"-c", "1" };
 	/* -l flag but no corelist value */
 	const char *argv5[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l"};
+				"-l"};
 	const char *argv6[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", " " };
+				"-l", " " };
 	/* bad corelist values */
 	const char *argv7[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "error" };
+				"-l", "error" };
 	const char *argv8[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1-" };
+				"-l", "1-" };
 	const char *argv9[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1," };
+				"-l", "1," };
 	const char *argv10[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "-l", "1#2" };
+				 "-l", "1#2" };
 	/* core number is negative value */
 	const char * const argv11[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "-5" };
+				"-l", "-5" };
 	const char * const argv12[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "-5-7" };
+				"-l", "-5-7" };
 	/* core number is maximum value */
 	const char * const argv13[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+				"-l", RTE_STR(RTE_MAX_LCORE) };
 	const char * const argv14[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
+				"-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 	/* sanity check test - valid corelist value */
 	const char * const argv15[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "-l", "1-2,3" };
+				 "-l", "1-2,3" };
 
 	/* --lcores flag but no lcores value */
 	const char * const argv16[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores" };
+				 "--lcores" };
 	const char * const argv17[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", " " };
+				 "--lcores", " " };
 	/* bad lcores value */
 	const char * const argv18[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "1-3-5" };
+				 "--lcores", "1-3-5" };
 	const char * const argv19[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "0-1,,2" };
+				 "--lcores", "0-1,,2" };
 	const char * const argv20[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "0-,1" };
+				 "--lcores", "0-,1" };
 	const char * const argv21[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(0-,2-4)" };
+				 "--lcores", "(0-,2-4)" };
 	const char * const argv22[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(-1,2)" };
+				 "--lcores", "(-1,2)" };
 	const char * const argv23[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(2-4)@(2-4-6)" };
+				 "--lcores", "(2-4)@(2-4-6)" };
 	const char * const argv24[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(a,2)" };
+				 "--lcores", "(a,2)" };
 	const char * const argv25[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "1-3@(1,3)" };
+				 "--lcores", "1-3@(1,3)" };
 	const char * const argv26[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "3@((1,3)" };
+				 "--lcores", "3@((1,3)" };
 	const char * const argv27[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(4-7)=(1,3)" };
+				 "--lcores", "(4-7)=(1,3)" };
 	const char * const argv28[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "[4-7]@(1,3)" };
+				 "--lcores", "[4-7]@(1,3)" };
 	/* sanity check of tests - valid lcores value */
 	const char * const argv29[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores",
+				 "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
 
 	if (launch_proc(argv2) != 0) {
@@ -606,16 +607,22 @@ enum hugepage_action {
 #endif
 
 	/* --master-lcore flag but no value */
-	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore"};
+	const char *argv1[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore"};
 	/* --master-lcore flag with invalid value */
-	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "-1"};
-	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "X"};
+	const char *argv2[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "-1"};
+	const char *argv3[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "X"};
 	/* master lcore not in coremask */
-	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "2"};
+	const char *argv4[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "2"};
 	/* valid value */
-	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "1"};
+	const char *argv5[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "1"};
 	/* valid value set before coremask */
-	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "1", "--master-lcore", "1", "-c", "3"};
+	const char *argv6[] = { prgname, prefix, mp_flag,
+				"--master-lcore", "1", "-c", "3"};
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -703,9 +710,9 @@ enum hugepage_action {
 #endif
 
 	/* With --no-hpet */
-	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1", "-n", "2"};
+	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1"};
 	/* Without --no-hpet */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1", "-n", "2"};
+	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1"};
 
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-hpet flag\n");
@@ -734,16 +741,16 @@ enum hugepage_action {
 #endif
 
 	/* With --no-huge */
-	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2"};
+	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1"};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1",
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
-	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1",
 			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
-	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1",
 			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
@@ -844,17 +851,17 @@ enum hugepage_action {
 	/* With invalid --syslog */
 	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
-	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv6[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix, no_huge};
 
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv7[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv8[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv9[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
@@ -862,23 +869,23 @@ enum hugepage_action {
 
 	/* try running with base-virtaddr param */
 	const char *argv11[] = {prgname, "--file-prefix=virtaddr",
-			"-c", "1", "-n", "2", "--base-virtaddr=0x12345678"};
+			"-c", "1", "--base-virtaddr=0x12345678"};
 
 	/* try running with --vfio-intr INTx flag */
 	const char *argv12[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=legacy"};
+			"-c", "1", "--vfio-intr=legacy"};
 
 	/* try running with --vfio-intr MSI flag */
 	const char *argv13[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=msi"};
+			"-c", "1", "--vfio-intr=msi"};
 
 	/* try running with --vfio-intr MSI-X flag */
 	const char *argv14[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=msix"};
+			"-c", "1", "--vfio-intr=msix"};
 
 	/* try running with --vfio-intr invalid flag */
 	const char *argv15[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=invalid"};
+			"-c", "1", "--vfio-intr=invalid"};
 
 	/* run all tests also applicable to FreeBSD first */
 
@@ -989,25 +996,25 @@ enum hugepage_action {
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
-			"--file-prefix=" memtest };
+	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest };
 
 	/* primary process with memtest1 and default mem mode */
-	const char *argv1[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv1[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
 
 	/* primary process with memtest1 and legacy mem mode */
-	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv2[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and legacy mem mode */
-	const char *argv3[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv3[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and default mem mode */
-	const char *argv4[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv4[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
 
 	/* check if files for current prefix are present */
@@ -1153,38 +1160,38 @@ enum hugepage_action {
 
 	/* valid -m flag and mp flag */
 	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
-			"-n", "2", "-m", DEFAULT_MEM_SIZE};
+			"-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
-	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv1[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* valid (zero) --socket-mem flag */
-	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv2[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
 
 	/* invalid (incomplete) --socket-mem flag */
-	const char *argv3[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv3[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,"};
 
 	/* invalid (mixed with invalid data) --socket-mem flag */
-	const char *argv4[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv4[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred"};
 
 	/* invalid (with numeric value as last character) --socket-mem flag */
-	const char *argv5[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv5[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred0"};
 
 	/* invalid (with empty socket) --socket-mem flag */
-	const char *argv6[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv6[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,,2"};
 
 	/* invalid (null) --socket-mem flag */
-	const char *argv7[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv7[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem="};
 
 	/* valid --socket-mem specified together with -m flag */
-	const char *argv8[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv8[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
@@ -1236,11 +1243,11 @@ enum hugepage_action {
 	}
 
 	/* invalid --socket-mem flag (with extra socket) */
-	const char *argv9[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv9[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, invalid_socket_mem};
 
 	/* valid --socket-mem flag */
-	const char *argv10[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv10[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, valid_socket_mem};
 
 	if (launch_proc(argv0) != 0) {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 09/14] test/eal: set memory channel config only in dedicated test

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> The -n option is an optimisation configuration option that defaults to 0.
> Such a default value makes the mempool library distributes objects as if
> there was 4 memory channels, so -n 4 is the same as the default behavior.
>
> This parameter was mandatory a long time ago, but has been optional for
> a while. We check that setting this value works fine in its own test.
> Remove it everywhere else.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

Re: [dpdk-dev] [PATCH 09/14] test/eal: set memory channel config only in dedicated test

[Burakov, Anatoly]

On 04-Jun-19 9:59 AM, David Marchand wrote:
> The -n option is an optimisation configuration option that defaults to 0.
> Such a default value makes the mempool library distributes objects as if
> there was 4 memory channels, so -n 4 is the same as the default behavior.
> 
> This parameter was mandatory a long time ago, but has been optional for
> a while. We check that setting this value works fine in its own test.
> Remove it everywhere else.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

[dpdk-dev] [PATCH 10/14] test/eal: set core mask/list config only in dedicated test

[David Marchand]

Setting a coremask was mandatory a long time ago but has been optional
for a while.
The checks on PCI whitelist/blacklist, vdev, memory rank, memory channel,
HPET, memory size and other miscs options have no requirement wrt cores.

Let's remove those coremasks so that we only care about it in the
dedicated checks.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_eal_flags.c | 141 ++++++++++++++++++++++++----------------------
 1 file changed, 73 insertions(+), 68 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index e82e56a..5e11b9f 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -245,25 +245,25 @@ enum hugepage_action {
 #endif
 
 	const char *wlinval[][11] = {
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "error", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:error:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0.1error", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "error0:0:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0.1.2", "", ""},
 	};
 	/* Test with valid whitelist option */
-	const char *wlval1[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval1[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "00FF:09:0B.3"};
-	const char *wlval2[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval2[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "09:0B.3", pci_whitelist, "0a:0b.1"};
-	const char *wlval3[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval3[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "09:0B.3,type=test",
 			pci_whitelist, "08:00.1,type=normal",
 	};
@@ -311,15 +311,15 @@ enum hugepage_action {
 #endif
 
 	const char *blinval[][9] = {
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "error"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:error:0.1"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1error"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "error0:0:0.1"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1.2"},
+		{prgname, prefix, mp_flag, "-b", "error"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0"},
+		{prgname, prefix, mp_flag, "-b", "0:error:0.1"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0.1error"},
+		{prgname, prefix, mp_flag, "-b", "error0:0:0.1"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0.1.2"},
 	};
 	/* Test with valid blacklist option */
-	const char *blval[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *blval[] = {prgname, prefix, mp_flag,
 			       "-b", "FF:09:0B.3"};
 
 	int i;
@@ -356,17 +356,17 @@ enum hugepage_action {
 
 	/* Test with invalid vdev option */
 	const char *vdevinval[] = {prgname, prefix, no_huge,
-				"-c", "1", vdev, "eth_dummy"};
+				vdev, "eth_dummy"};
 
 	/* Test with valid vdev option */
 	const char *vdevval1[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0"};
+	vdev, "net_ring0"};
 
 	const char *vdevval2[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0,args=test"};
+	vdev, "net_ring0,args=test"};
 
 	const char *vdevval3[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0,nodeaction=r1:0:CREATE"};
+	vdev, "net_ring0,nodeaction=r1:0:CREATE"};
 
 	if (launch_proc(vdevinval) == 0) {
 		printf("Error - process did run ok with invalid "
@@ -413,13 +413,13 @@ enum hugepage_action {
 #endif
 
 	const char *rinval[][9] = {
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "error"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "0"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "-1"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "17"},
+			{prgname, prefix, mp_flag, "-r", "error"},
+			{prgname, prefix, mp_flag, "-r", "0"},
+			{prgname, prefix, mp_flag, "-r", "-1"},
+			{prgname, prefix, mp_flag, "-r", "17"},
 	};
 	/* Test with valid blacklist option */
-	const char *rval[] = {prgname, prefix, mp_flag, "-c", "1", "-r", "16"};
+	const char *rval[] = {prgname, prefix, mp_flag, "-r", "16"};
 
 	int i;
 
@@ -661,15 +661,19 @@ enum hugepage_action {
 #endif
 
 	/* -n flag but no value */
-	const char *argv1[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n"};
+	const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
+				"-n"};
 	/* bad numeric value */
-	const char *argv2[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "e" };
+	const char *argv2[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "e" };
 	/* zero is invalid */
-	const char *argv3[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "0" };
+	const char *argv3[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "0" };
 	/* sanity test - check with good value */
-	const char *argv4[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "2" };
+	const char *argv4[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "2" };
 	/* sanity test - check with no -n flag */
-	const char *argv5[] = { prgname, prefix, no_huge, no_shconf, "-c", "1"};
+	const char *argv5[] = { prgname, prefix, no_huge, no_shconf};
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -710,9 +714,9 @@ enum hugepage_action {
 #endif
 
 	/* With --no-hpet */
-	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1"};
+	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
 	/* Without --no-hpet */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1"};
+	const char *argv2[] = {prgname, prefix, mp_flag};
 
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-hpet flag\n");
@@ -741,16 +745,16 @@ enum hugepage_action {
 #endif
 
 	/* With --no-huge */
-	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1"};
+	const char *argv1[] = {prgname, prefix, no_huge};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv2[] = {prgname, prefix, no_huge,
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
-	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv3[] = {prgname, prefix, no_huge,
 			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
-	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv4[] = {prgname, prefix, no_huge,
 			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
@@ -837,55 +841,56 @@ enum hugepage_action {
 	 * No further testing of output done.
 	 */
 	/* sanity check - failure with invalid option */
-	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "1", "--invalid-opt"};
+	const char *argv0[] = {prgname, prefix, mp_flag, "--invalid-opt"};
 
 	/* With --no-pci */
-	const char *argv1[] = {prgname, prefix, mp_flag, "-c", "1", "--no-pci"};
+	const char *argv1[] = {prgname, prefix, mp_flag, "--no-pci"};
 	/* With -v */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1", "-v"};
+	const char *argv2[] = {prgname, prefix, mp_flag, "-v"};
 	/* With valid --syslog */
-	const char *argv3[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *argv3[] = {prgname, prefix, mp_flag,
 			"--syslog", "syslog"};
 	/* With empty --syslog (should fail) */
-	const char *argv4[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog"};
+	const char *argv4[] = {prgname, prefix, mp_flag, "--syslog"};
 	/* With invalid --syslog */
-	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
+	const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
-	const char *argv6[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv6[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix, no_huge};
 
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv7[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv8[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv9[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
-	const char *argv10[] = {prgname, prefix, mp_flag, "-c", "1", "--huge-dir", "invalid"};
+	const char *argv10[] = {prgname, prefix, mp_flag,
+			"--huge-dir", "invalid"};
 
 	/* try running with base-virtaddr param */
 	const char *argv11[] = {prgname, "--file-prefix=virtaddr",
-			"-c", "1", "--base-virtaddr=0x12345678"};
+			"--base-virtaddr=0x12345678"};
 
 	/* try running with --vfio-intr INTx flag */
 	const char *argv12[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=legacy"};
+			"--vfio-intr=legacy"};
 
 	/* try running with --vfio-intr MSI flag */
 	const char *argv13[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=msi"};
+			"--vfio-intr=msi"};
 
 	/* try running with --vfio-intr MSI-X flag */
 	const char *argv14[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=msix"};
+			"--vfio-intr=msix"};
 
 	/* try running with --vfio-intr invalid flag */
 	const char *argv15[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=invalid"};
+			"--vfio-intr=invalid"};
 
 	/* run all tests also applicable to FreeBSD first */
 
@@ -996,25 +1001,25 @@ enum hugepage_action {
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-m",
+	const char *argv0[] = {prgname, mp_flag, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest };
 
 	/* primary process with memtest1 and default mem mode */
-	const char *argv1[] = {prgname, "-c", "1", "-m",
+	const char *argv1[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
 
 	/* primary process with memtest1 and legacy mem mode */
-	const char *argv2[] = {prgname, "-c", "1", "-m",
+	const char *argv2[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and legacy mem mode */
-	const char *argv3[] = {prgname, "-c", "1", "-m",
+	const char *argv3[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and default mem mode */
-	const char *argv4[] = {prgname, "-c", "1", "-m",
+	const char *argv4[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
 
 	/* check if files for current prefix are present */
@@ -1159,39 +1164,39 @@ enum hugepage_action {
 #endif
 
 	/* valid -m flag and mp flag */
-	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
+	const char *argv0[] = {prgname, prefix, mp_flag,
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
-	const char *argv1[] = {prgname, "-c", "10",
+	const char *argv1[] = {prgname,
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* valid (zero) --socket-mem flag */
-	const char *argv2[] = {prgname, "-c", "10",
+	const char *argv2[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
 
 	/* invalid (incomplete) --socket-mem flag */
-	const char *argv3[] = {prgname, "-c", "10",
+	const char *argv3[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,"};
 
 	/* invalid (mixed with invalid data) --socket-mem flag */
-	const char *argv4[] = {prgname, "-c", "10",
+	const char *argv4[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred"};
 
 	/* invalid (with numeric value as last character) --socket-mem flag */
-	const char *argv5[] = {prgname, "-c", "10",
+	const char *argv5[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred0"};
 
 	/* invalid (with empty socket) --socket-mem flag */
-	const char *argv6[] = {prgname, "-c", "10",
+	const char *argv6[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,,2"};
 
 	/* invalid (null) --socket-mem flag */
-	const char *argv7[] = {prgname, "-c", "10",
+	const char *argv7[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem="};
 
 	/* valid --socket-mem specified together with -m flag */
-	const char *argv8[] = {prgname, "-c", "10",
+	const char *argv8[] = {prgname,
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
@@ -1243,11 +1248,11 @@ enum hugepage_action {
 	}
 
 	/* invalid --socket-mem flag (with extra socket) */
-	const char *argv9[] = {prgname, "-c", "10",
+	const char *argv9[] = {prgname,
 			"--file-prefix=" memtest, invalid_socket_mem};
 
 	/* valid --socket-mem flag */
-	const char *argv10[] = {prgname, "-c", "10",
+	const char *argv10[] = {prgname,
 			"--file-prefix=" memtest, valid_socket_mem};
 
 	if (launch_proc(argv0) != 0) {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 10/14] test/eal: set core mask/list config only in dedicated test

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Setting a coremask was mandatory a long time ago but has been optional
> for a while.
> The checks on PCI whitelist/blacklist, vdev, memory rank, memory channel,
> HPET, memory size and other miscs options have no requirement wrt cores.
>
> Let's remove those coremasks so that we only care about it in the
> dedicated checks.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

Re: [dpdk-dev] [PATCH 10/14] test/eal: set core mask/list config only in dedicated test

[Burakov, Anatoly]

On 04-Jun-19 9:59 AM, David Marchand wrote:
> Setting a coremask was mandatory a long time ago but has been optional
> for a while.
> The checks on PCI whitelist/blacklist, vdev, memory rank, memory channel,
> HPET, memory size and other miscs options have no requirement wrt cores.
> 
> Let's remove those coremasks so that we only care about it in the
> dedicated checks.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

[dpdk-dev] [PATCH 11/14] test/eal: check number of cores before running subtests

[David Marchand]

From: Michael Santana <msantana@redhat.com>

The eal flags unit test assumes that a certain number of cores are
available (4 and 8 cores), however this may not always be the case.
Individual developers may run the unit test on their local desktop
which typically have 2 to 4 cores, in said case the test is bound
to fail for lacking 4 or 8 cores.

Additionally, as we push forward introducing CI into DPDK we are limited
to the hardware specification of CI services (e.g. Travis CI) that only
have 2 cores on their servers, in which case the test would fail.

To fix this we check available cores before running a subtest. This
applies to subtests that are dedicated to test that the -l and --lcore
flags work correctly. If not enough cores are available the subtest is
simply skipped, otherwise the subtest is run.

Signed-off-by: Michael Santana <msantana@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_eal_flags.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 5e11b9f..cfa8a61 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -19,7 +19,7 @@
 #include <limits.h>
 #include <fcntl.h>
 
-#include <rte_per_lcore.h>
+#include <rte_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -560,7 +560,9 @@ enum hugepage_action {
 		       "process ran without error with invalid -l flag\n");
 		return -1;
 	}
-	if (launch_proc(argv15) != 0) {
+	if (rte_lcore_is_enabled(0) && rte_lcore_is_enabled(1) &&
+	    rte_lcore_is_enabled(2) && rte_lcore_is_enabled(3) &&
+	    launch_proc(argv15) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
@@ -579,7 +581,11 @@ enum hugepage_action {
 		return -1;
 	}
 
-	if (launch_proc(argv29) != 0) {
+	if (rte_lcore_is_enabled(0) && rte_lcore_is_enabled(1) &&
+	    rte_lcore_is_enabled(2) && rte_lcore_is_enabled(3) &&
+	    rte_lcore_is_enabled(3) && rte_lcore_is_enabled(5) &&
+	    rte_lcore_is_enabled(4) && rte_lcore_is_enabled(7) &&
+	    launch_proc(argv29) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
@@ -606,6 +612,9 @@ enum hugepage_action {
 	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
 #endif
 
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1))
+		return TEST_SKIPPED;
+
 	/* --master-lcore flag but no value */
 	const char *argv1[] = { prgname, prefix, mp_flag,
 				"-c", "3", "--master-lcore"};
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 11/14] test/eal: check number of cores before running subtests

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> From: Michael Santana <msantana@redhat.com>
>
> The eal flags unit test assumes that a certain number of cores are
> available (4 and 8 cores), however this may not always be the case.
> Individual developers may run the unit test on their local desktop
> which typically have 2 to 4 cores, in said case the test is bound
> to fail for lacking 4 or 8 cores.
>
> Additionally, as we push forward introducing CI into DPDK we are limited
> to the hardware specification of CI services (e.g. Travis CI) that only
> have 2 cores on their servers, in which case the test would fail.
>
> To fix this we check available cores before running a subtest. This
> applies to subtests that are dedicated to test that the -l and --lcore
> flags work correctly. If not enough cores are available the subtest is
> simply skipped, otherwise the subtest is run.
>
> Signed-off-by: Michael Santana <msantana@redhat.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

Re: [dpdk-dev] [PATCH 11/14] test/eal: check number of cores before running subtests

[Burakov, Anatoly]

On 04-Jun-19 9:59 AM, David Marchand wrote:
> From: Michael Santana <msantana@redhat.com>
> 
> The eal flags unit test assumes that a certain number of cores are
> available (4 and 8 cores), however this may not always be the case.
> Individual developers may run the unit test on their local desktop
> which typically have 2 to 4 cores, in said case the test is bound
> to fail for lacking 4 or 8 cores.
> 
> Additionally, as we push forward introducing CI into DPDK we are limited
> to the hardware specification of CI services (e.g. Travis CI) that only
> have 2 cores on their servers, in which case the test would fail.
> 
> To fix this we check available cores before running a subtest. This
> applies to subtests that are dedicated to test that the -l and --lcore
> flags work correctly. If not enough cores are available the subtest is
> simply skipped, otherwise the subtest is run.
> 
> Signed-off-by: Michael Santana <msantana@redhat.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

While i think it's OK to skip tests we *can't* run without having many 
cores, maybe we should also look into reducing core consumption in our 
tests - i'm pretty sure we'll find cases where we don't really need 
those 8+ cores running our tests.

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

[dpdk-dev] [PATCH 12/14] test/eal: make the test pass again

[David Marchand]

From: Michael Santana <msantana@redhat.com>

The eal_flags_autotest test currently fails due to a memory leak in the
timer library[1][2]. This failure occurs when the test calls one of its
subtests test_file_prefix().

Fixing the memory leak is not trivial, so this patch is a workaround that
makes the eal_flags_autotest test pass. This is accomplished by moving the
test_file_prefix test to its own test unit. This is a temporary measure
until the leak is fixed.

[1] http://patchwork.dpdk.org/patch/53268/
[2] http://patchwork.dpdk.org/patch/53334/

Signed-off-by: Michael Santana <msantana@redhat.com>
---
 app/test/autotest_data.py | 6 ++++++
 app/test/meson.build      | 1 +
 app/test/test_eal_flags.c | 7 +------
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 6cf7eca..15e672f 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -93,6 +93,12 @@
         "Report":  None,
     },
     {
+        "Name":    "EAL flags file prefix autotest",
+        "Command": "eal_flags_prefix_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
         "Name":    "Hash autotest",
         "Command": "hash_autotest",
         "Func":    default_autotest,
diff --git a/app/test/meson.build b/app/test/meson.build
index 7ad3684..212cd1b 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -156,6 +156,7 @@ fast_parallel_test_names = [
         'cycles_autotest',
         'debug_autotest',
         'eal_flags_autotest',
+        'eal_flags_prefix_autotest',
         'eal_fs_autotest',
         'errno_autotest',
         'event_ring_autotest',
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index cfa8a61..1e227aa 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1397,12 +1397,6 @@ enum hugepage_action {
 		return ret;
 	}
 
-	ret = test_file_prefix();
-	if (ret < 0) {
-		printf("Error in test_file_prefix()\n");
-		return ret;
-	}
-
 	ret = test_misc_flags();
 	if (ret < 0) {
 		printf("Error in test_misc_flags()");
@@ -1413,3 +1407,4 @@ enum hugepage_action {
 }
 
 REGISTER_TEST_COMMAND(eal_flags_autotest, test_eal_flags);
+REGISTER_TEST_COMMAND(eal_flags_prefix_autotest, test_file_prefix);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 12/14] test/eal: make the test pass again

[Aaron Conole]

Hi David and Michael,

David Marchand <david.marchand@redhat.com> writes:

> From: Michael Santana <msantana@redhat.com>
>
> The eal_flags_autotest test currently fails due to a memory leak in the
> timer library[1][2]. This failure occurs when the test calls one of its
> subtests test_file_prefix().
>
> Fixing the memory leak is not trivial, so this patch is a workaround that
> makes the eal_flags_autotest test pass. This is accomplished by moving the
> test_file_prefix test to its own test unit. This is a temporary measure
> until the leak is fixed.
>
> [1] http://patchwork.dpdk.org/patch/53268/
> [2] http://patchwork.dpdk.org/patch/53334/
>
> Signed-off-by: Michael Santana <msantana@redhat.com>
> ---

I'm wondering if it's better to just fix the leak outright.

Then again, it might be useful to have the file-prefix test as an
enumerable test anyway.

CC'd Anatoly.

>  app/test/autotest_data.py | 6 ++++++
>  app/test/meson.build      | 1 +
>  app/test/test_eal_flags.c | 7 +------
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
> index 6cf7eca..15e672f 100644
> --- a/app/test/autotest_data.py
> +++ b/app/test/autotest_data.py
> @@ -93,6 +93,12 @@
>          "Report":  None,
>      },
>      {
> +        "Name":    "EAL flags file prefix autotest",
> +        "Command": "eal_flags_prefix_autotest",
> +        "Func":    default_autotest,
> +        "Report":  None,
> +    },
> +    {
>          "Name":    "Hash autotest",
>          "Command": "hash_autotest",
>          "Func":    default_autotest,
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 7ad3684..212cd1b 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -156,6 +156,7 @@ fast_parallel_test_names = [
>          'cycles_autotest',
>          'debug_autotest',
>          'eal_flags_autotest',
> +        'eal_flags_prefix_autotest',
>          'eal_fs_autotest',
>          'errno_autotest',
>          'event_ring_autotest',
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index cfa8a61..1e227aa 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -1397,12 +1397,6 @@ enum hugepage_action {
>  		return ret;
>  	}
>  
> -	ret = test_file_prefix();
> -	if (ret < 0) {
> -		printf("Error in test_file_prefix()\n");
> -		return ret;
> -	}
> -
>  	ret = test_misc_flags();
>  	if (ret < 0) {
>  		printf("Error in test_misc_flags()");
> @@ -1413,3 +1407,4 @@ enum hugepage_action {
>  }
>  
>  REGISTER_TEST_COMMAND(eal_flags_autotest, test_eal_flags);
> +REGISTER_TEST_COMMAND(eal_flags_prefix_autotest, test_file_prefix);

Re: [dpdk-dev] [PATCH 12/14] test/eal: make the test pass again

[David Marchand]

On Tue, Jun 4, 2019 at 3:29 PM Aaron Conole <aconole@redhat.com> wrote:

> Hi David and Michael,
>
> David Marchand <david.marchand@redhat.com> writes:
>
> > From: Michael Santana <msantana@redhat.com>
> >
> > The eal_flags_autotest test currently fails due to a memory leak in the
> > timer library[1][2]. This failure occurs when the test calls one of its
> > subtests test_file_prefix().
> >
> > Fixing the memory leak is not trivial, so this patch is a workaround that
> > makes the eal_flags_autotest test pass. This is accomplished by moving
> the
> > test_file_prefix test to its own test unit. This is a temporary measure
> > until the leak is fixed.
> >
> > [1] http://patchwork.dpdk.org/patch/53268/
> > [2] http://patchwork.dpdk.org/patch/53334/
> >
> > Signed-off-by: Michael Santana <msantana@redhat.com>
> > ---
>
> I'm wondering if it's better to just fix the leak outright.
>
> Then again, it might be useful to have the file-prefix test as an
> enumerable test anyway.
>

We have a lot of tests that embed subtests that can be entirely skipped at
the first subtest failure.
Their execution time can also be quite long making it harder to adjust the
timeout.
So splitting the existing tests into their subtests is something I have
been considering.


-- 
David Marchand

Re: [dpdk-dev] [PATCH 12/14] test/eal: make the test pass again

[Burakov, Anatoly]

On 04-Jun-19 9:59 AM, David Marchand wrote:
> From: Michael Santana <msantana@redhat.com>
> 
> The eal_flags_autotest test currently fails due to a memory leak in the
> timer library[1][2]. This failure occurs when the test calls one of its
> subtests test_file_prefix().
> 
> Fixing the memory leak is not trivial, so this patch is a workaround that
> makes the eal_flags_autotest test pass. This is accomplished by moving the
> test_file_prefix test to its own test unit. This is a temporary measure
> until the leak is fixed.
> 
> [1] http://patchwork.dpdk.org/patch/53268/
> [2] http://patchwork.dpdk.org/patch/53334/
> 
> Signed-off-by: Michael Santana <msantana@redhat.com>
> ---

There is now a patchset that fixes the leak [1], so this may not be 
necessary.

[1] https://mails.dpdk.org/archives/dev/2019-June/135774.html

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 12/14] test/eal: make the test pass again

[David Marchand]

On Wed, Jun 26, 2019 at 11:49 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 04-Jun-19 9:59 AM, David Marchand wrote:
> > From: Michael Santana <msantana@redhat.com>
> >
> > The eal_flags_autotest test currently fails due to a memory leak in the
> > timer library[1][2]. This failure occurs when the test calls one of its
> > subtests test_file_prefix().
> >
> > Fixing the memory leak is not trivial, so this patch is a workaround that
> > makes the eal_flags_autotest test pass. This is accomplished by moving
> the
> > test_file_prefix test to its own test unit. This is a temporary measure
> > until the leak is fixed.
> >
> > [1] http://patchwork.dpdk.org/patch/53268/
> > [2] http://patchwork.dpdk.org/patch/53334/
> >
> > Signed-off-by: Michael Santana <msantana@redhat.com>
> > ---
>
> There is now a patchset that fixes the leak [1], so this may not be
> necessary.
>
> [1] https://mails.dpdk.org/archives/dev/2019-June/135774.html


As discussed on irc, there is a v2 for this series and this patch has been
reworked to be more generic and also fix the CI timeout issues.

I will propagate those previous acks you gave on the v3 if I ever send one.

Thanks.


-- 
David Marchand

[dpdk-dev] [PATCH 13/14] test: do not start tests in parallel

[David Marchand]

Running the tests in parallel has two drawbacks:
- the tests are racing on the hugepages allocations,
- the tests are sharing the cores to run their checks which results in
  undeterministic execution time,

This results in random failures.
For better reproducibility in CI, start them all in a serialised way.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 212cd1b..f0797c9 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -144,8 +144,7 @@ test_deps = ['acl',
 	'timer'
 ]
 
-# All test cases in fast_parallel_test_names list are parallel
-fast_parallel_test_names = [
+fast_test_names = [
         'acl_autotest',
         'alarm_autotest',
         'atomic_autotest',
@@ -192,10 +191,6 @@ fast_parallel_test_names = [
         'timer_autotest',
         'user_delay_us',
         'version_autotest',
-]
-
-# All test cases in fast_non_parallel_test_names list are non-parallel
-fast_non_parallel_test_names = [
         'bitratestats_autotest',
         'crc_autotest',
         'delay_us_sleep_autotest',
@@ -219,7 +214,6 @@ fast_non_parallel_test_names = [
         'thash_autotest',
 ]
 
-# All test cases in perf_test_names list are non-parallel
 perf_test_names = [
         'ring_perf_autotest',
         'mempool_perf_autotest',
@@ -247,7 +241,6 @@ perf_test_names = [
         'stack_lf_perf_autotest',
 ]
 
-# All test cases in driver_test_names list are non-parallel
 driver_test_names = [
         'cryptodev_aesni_mb_autotest',
         'cryptodev_aesni_gcm_autotest',
@@ -269,7 +262,6 @@ driver_test_names = [
         'link_bonding_rssconf_autotest',
 ]
 
-# All test cases in dump_test_names list are non-parallel
 dump_test_names = [
         'dump_struct_sizes',
         'dump_mempool',
@@ -318,7 +310,7 @@ if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
 		test_dep_objs += compress_test_dep
 		test_sources += 'test_compressdev.c'
 		test_deps += 'compressdev'
-		fast_non_parallel_test_names += 'compressdev_autotest'
+		fast_test_names += 'compressdev_autotest'
 	endif
 endif
 
@@ -367,30 +359,23 @@ if get_option('tests')
 	num_cores_arg = '-l ' + num_cores
 
 	test_args = [num_cores_arg]
-	foreach arg : fast_parallel_test_names
+	foreach arg : fast_test_names
 		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,
+			is_parallel : false,
 			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')
+		endif
 	endforeach
 
 	foreach arg : perf_test_names
-- 
1.8.3.1

[dpdk-dev] [PATCH 14/14] test: skip tests when missing requirements

[David Marchand]

Let's mark as skipped the tests when they are missing some requirements
like a number of used cores or specific hardware availability, like
compress, crypto or eventdev devices.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test.c                     | 24 ++++++++++++++++--------
 app/test/test_compressdev.c         |  4 ++--
 app/test/test_cryptodev.c           |  4 ++--
 app/test/test_distributor.c         |  4 ++--
 app/test/test_distributor_perf.c    |  4 ++--
 app/test/test_event_timer_adapter.c |  5 +++--
 app/test/test_eventdev.c            |  2 ++
 app/test/test_func_reentrancy.c     |  6 +++---
 app/test/test_hash_multiwriter.c    |  7 +++----
 app/test/test_hash_readwrite.c      |  7 +++----
 app/test/test_hash_readwrite_lf.c   |  8 ++++----
 app/test/test_ipsec.c               |  4 ++--
 app/test/test_mbuf.c                | 13 ++++++-------
 app/test/test_rcu_qsbr.c            | 10 +++++-----
 app/test/test_rcu_qsbr_perf.c       |  9 +++++----
 app/test/test_service_cores.c       | 14 ++++++++++++++
 app/test/test_stack.c               |  8 +++++---
 app/test/test_timer.c               | 10 +++++-----
 app/test/test_timer_secondary.c     | 10 ++++++----
 19 files changed, 90 insertions(+), 63 deletions(-)

diff --git a/app/test/test.c b/app/test/test.c
index ea1e98f..194a92a 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -208,14 +208,16 @@
 		printf(" + Test Suite : %s\n", suite->suite_name);
 	}
 
-	if (suite->setup)
-		if (suite->setup() != 0) {
+	if (suite->setup) {
+		test_success = suite->setup();
+		if (test_success != 0) {
 			/*
-			 * setup failed, so count all enabled tests and mark
-			 * them as failed
+			 * setup did not pass, so count all enabled tests and
+			 * mark them as failed/skipped
 			 */
 			while (suite->unit_test_cases[total].testcase) {
-				if (!suite->unit_test_cases[total].enabled)
+				if (!suite->unit_test_cases[total].enabled ||
+				    test_success == TEST_SKIPPED)
 					skipped++;
 				else
 					failed++;
@@ -223,6 +225,7 @@
 			}
 			goto suite_summary;
 		}
+	}
 
 	printf(" + ------------------------------------------------------- +\n");
 
@@ -246,6 +249,8 @@
 			test_success = suite->unit_test_cases[total].testcase();
 			if (test_success == TEST_SUCCESS)
 				succeeded++;
+			else if (test_success == TEST_SKIPPED)
+				skipped++;
 			else if (test_success == -ENOTSUP)
 				unsupported++;
 			else
@@ -262,6 +267,8 @@
 
 		if (test_success == TEST_SUCCESS)
 			status = "succeeded";
+		else if (test_success == TEST_SKIPPED)
+			status = "skipped";
 		else if (test_success == -ENOTSUP)
 			status = "unsupported";
 		else
@@ -293,7 +300,8 @@
 	last_test_result = failed;
 
 	if (failed)
-		return -1;
-
-	return 0;
+		return TEST_FAILED;
+	if (total == skipped)
+		return TEST_SKIPPED;
+	return TEST_SUCCESS;
 }
diff --git a/app/test/test_compressdev.c b/app/test/test_compressdev.c
index 1b1983e..cf78775 100644
--- a/app/test/test_compressdev.c
+++ b/app/test/test_compressdev.c
@@ -134,8 +134,8 @@ struct test_data_params {
 	unsigned int i;
 
 	if (rte_compressdev_count() == 0) {
-		RTE_LOG(ERR, USER1, "Need at least one compress device\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "Need at least one compress device\n");
+		return TEST_SKIPPED;
 	}
 
 	RTE_LOG(NOTICE, USER1, "Running tests on device %s\n",
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index eca6d3d..0509af7 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -408,8 +408,8 @@ struct crypto_unittest_params {
 
 	nb_devs = rte_cryptodev_count();
 	if (nb_devs < 1) {
-		RTE_LOG(ERR, USER1, "No crypto devices found?\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "No crypto devices found?\n");
+		return TEST_SKIPPED;
 	}
 
 	/* Create list of valid crypto devs */
diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index da3348f..8084c07 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -594,8 +594,8 @@ int test_error_distributor_create_numworkers(void)
 	int i;
 
 	if (rte_lcore_count() < 2) {
-		printf("ERROR: not enough cores to test distributor\n");
-		return -1;
+		printf("Not enough cores for distributor_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	if (db == NULL) {
diff --git a/app/test/test_distributor_perf.c b/app/test/test_distributor_perf.c
index edf1998..f6f1139 100644
--- a/app/test/test_distributor_perf.c
+++ b/app/test/test_distributor_perf.c
@@ -208,8 +208,8 @@ struct worker_stats {
 	static struct rte_mempool *p;
 
 	if (rte_lcore_count() < 2) {
-		printf("ERROR: not enough cores to test distributor\n");
-		return -1;
+		printf("Not enough cores for distributor_perf_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	/* first time how long it takes to round-trip a cache line */
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index 742ca81..ad3f4dc 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -158,8 +158,9 @@
 	}
 
 	if (rte_lcore_count() < required_lcore_count) {
-		printf("%d lcores needed to run tests", required_lcore_count);
-		return TEST_FAILED;
+		printf("Not enough cores for event_timer_adapter_test, expecting at least %u\n",
+		       required_lcore_count);
+		return TEST_SKIPPED;
 	}
 
 	/* Assign lcores for various tasks */
diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
index 00d7327..c745e99 100644
--- a/app/test/test_eventdev.c
+++ b/app/test/test_eventdev.c
@@ -997,6 +997,8 @@
 test_eventdev_selftest_impl(const char *pmd, const char *opts)
 {
 	rte_vdev_init(pmd, opts);
+	if (rte_event_dev_get_dev_id(pmd) == -ENODEV)
+		return TEST_SKIPPED;
 	return rte_event_dev_selftest(rte_event_dev_get_dev_id(pmd));
 }
 
diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index e27d1e0..99ad902 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -473,9 +473,9 @@ struct test_case test_cases[] = {
 	uint32_t case_id;
 	struct test_case *pt_case = NULL;
 
-	if (rte_lcore_count() <= 1) {
-		printf("Not enough lcore for testing\n");
-		return -1;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for func_reentrancy_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 	else if (rte_lcore_count() > MAX_LCORES)
 		printf("Too many lcores, some cores will be disabled\n");
diff --git a/app/test/test_hash_multiwriter.c b/app/test/test_hash_multiwriter.c
index 50018db..46ab45f 100644
--- a/app/test/test_hash_multiwriter.c
+++ b/app/test/test_hash_multiwriter.c
@@ -260,12 +260,11 @@ struct {
 static int
 test_hash_multiwriter_main(void)
 {
-	if (rte_lcore_count() == 1) {
-		printf("More than one lcore is required to do multiwriter test\n");
-		return 0;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for distributor_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
-
 	setlocale(LC_NUMERIC, "");
 
 
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 480ae97..4376b09 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -618,10 +618,9 @@ struct {
 	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
-	if (rte_lcore_count() <= 2) {
-		printf("More than two lcores are required "
-			"to do read write test\n");
-		return -1;
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
 	}
 
 	RTE_LCORE_FOREACH_SLAVE(core_id) {
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 5644361..2664f51 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1254,10 +1254,10 @@ struct {
 	int htm;
 	int use_jhash = 0;
 	int ext_bkt = 0;
-	if (rte_lcore_count() == 1) {
-		printf("More than one lcore is required "
-			"to do read write lock-free concurrency test\n");
-		return -1;
+
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for hash_readwrite_lf_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	setlocale(LC_NUMERIC, "");
diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
index 3993ff4..83f7895 100644
--- a/app/test/test_ipsec.c
+++ b/app/test/test_ipsec.c
@@ -296,8 +296,8 @@ struct supported_auth_algo {
 
 	nb_devs = rte_cryptodev_count();
 	if (nb_devs < 1) {
-		RTE_LOG(ERR, USER1, "No crypto devices found?\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "No crypto devices found?\n");
+		return TEST_SKIPPED;
 	}
 
 	/* Find first valid crypto device */
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 030385e..2a97afe 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -753,18 +753,17 @@
 test_refcnt_mbuf(void)
 {
 #ifdef RTE_MBUF_REFCNT_ATOMIC
-	unsigned lnum, master, slave, tref;
+	unsigned int master, slave, tref;
 	int ret = -1;
 	struct rte_mempool *refcnt_pool = NULL;
 	struct rte_ring *refcnt_mbuf_ring = NULL;
 
-	if ((lnum = rte_lcore_count()) == 1) {
-		printf("skipping %s, number of lcores: %u is not enough\n",
-		    __func__, lnum);
-		return 0;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for test_refcnt_mbuf, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
-	printf("starting %s, at %u lcores\n", __func__, lnum);
+	printf("starting %s, at %u lcores\n", __func__, rte_lcore_count());
 
 	/* create refcnt pool & ring if they don't exist */
 
@@ -1206,7 +1205,7 @@
 		goto err;
 	}
 
-	if (test_refcnt_mbuf()<0){
+	if (test_refcnt_mbuf() < 0) {
 		printf("test_refcnt_mbuf() failed \n");
 		goto err;
 	}
diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c
index 92ab0c2..725d27d 100644
--- a/app/test/test_rcu_qsbr.c
+++ b/app/test/test_rcu_qsbr.c
@@ -949,14 +949,14 @@
 static int
 test_rcu_qsbr_main(void)
 {
+	if (rte_lcore_count() < 5) {
+		printf("Not enough cores for rcu_qsbr_autotest, expecting at least 5\n");
+		return TEST_SKIPPED;
+	}
+
 	if (get_enabled_cores_mask() != 0)
 		return -1;
 
-	if (num_cores < 4) {
-		printf("Test failed! Need 4 or more cores\n");
-		goto test_fail;
-	}
-
 	/* Error-checking test cases */
 	if (test_rcu_qsbr_get_memsize() < 0)
 		goto test_fail;
diff --git a/app/test/test_rcu_qsbr_perf.c b/app/test/test_rcu_qsbr_perf.c
index 6b1912c..dcdd9da 100644
--- a/app/test/test_rcu_qsbr_perf.c
+++ b/app/test/test_rcu_qsbr_perf.c
@@ -623,6 +623,11 @@
 static int
 test_rcu_qsbr_main(void)
 {
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for rcu_qsbr_perf_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
 	rte_atomic64_init(&updates);
 	rte_atomic64_init(&update_cycles);
 	rte_atomic64_init(&checks);
@@ -632,10 +637,6 @@
 		return -1;
 
 	printf("Number of cores provided = %d\n", num_cores);
-	if (num_cores < 2) {
-		printf("Test failed! Need 2 or more cores\n");
-		goto test_fail;
-	}
 	if (num_cores > TEST_RCU_MAX_LCORE) {
 		printf("Test failed! %d cores supported\n", TEST_RCU_MAX_LCORE);
 		goto test_fail;
diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index 82bb2ce..9fe38f5 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -502,6 +502,10 @@ static int32_t dummy_mt_safe_cb(void *args)
 static int
 service_lcore_add_del(void)
 {
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2) || !rte_lcore_is_enabled(3))
+		return TEST_SKIPPED;
+
 	/* check initial count */
 	TEST_ASSERT_EQUAL(0, rte_service_lcore_count(),
 			"Service lcore count has value before adding a lcore");
@@ -669,6 +673,11 @@ static int32_t dummy_mt_safe_cb(void *args)
 service_mt_safe_poll(void)
 {
 	int mt_safe = 1;
+
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2))
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_EQUAL(1, service_threaded_test(mt_safe),
 			"Error: MT Safe service not run by two cores concurrently");
 	return TEST_SUCCESS;
@@ -681,6 +690,11 @@ static int32_t dummy_mt_safe_cb(void *args)
 service_mt_unsafe_poll(void)
 {
 	int mt_safe = 0;
+
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2))
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_EQUAL(1, service_threaded_test(mt_safe),
 			"Error: NON MT Safe service run by two cores concurrently");
 	return TEST_SUCCESS;
diff --git a/app/test/test_stack.c b/app/test/test_stack.c
index e972a61..c8dac1f 100644
--- a/app/test/test_stack.c
+++ b/app/test/test_stack.c
@@ -336,12 +336,14 @@ struct test_args {
 	struct rte_stack *s;
 	rte_atomic64_t size;
 
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for test_stack_multithreaded, expecting at least 2\n");
+		return TEST_SKIPPED;
+	}
+
 	printf("[%s():%u] Running with %u lcores\n",
 	       __func__, __LINE__, rte_lcore_count());
 
-	if (rte_lcore_count() < 2)
-		return 0;
-
 	args = rte_malloc(NULL, sizeof(struct test_args) * RTE_MAX_LCORE, 0);
 	if (args == NULL) {
 		printf("[%s():%u] failed to malloc %zu bytes\n",
diff --git a/app/test/test_timer.c b/app/test/test_timer.c
index e2aab53..8e0a589 100644
--- a/app/test/test_timer.c
+++ b/app/test/test_timer.c
@@ -538,17 +538,17 @@ struct mytimerinfo {
 	uint64_t cur_time;
 	uint64_t hz;
 
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for timer_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
+	}
+
 	/* sanity check our timer sources and timer config values */
 	if (timer_sanity_check() < 0) {
 		printf("Timer sanity checks failed\n");
 		return TEST_FAILED;
 	}
 
-	if (rte_lcore_count() < 2) {
-		printf("not enough lcores for this test\n");
-		return TEST_FAILED;
-	}
-
 	/* init timer */
 	for (i=0; i<NB_TIMER; i++) {
 		memset(&mytiminfo[i], 0, sizeof(struct mytimerinfo));
diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c
index 31e4a7f..790f180 100644
--- a/app/test/test_timer_secondary.c
+++ b/app/test/test_timer_secondary.c
@@ -118,16 +118,18 @@ struct test_info {
 	int ret;
 
 	if (proc_type == RTE_PROC_PRIMARY) {
+		if (rte_lcore_count() < NUM_LCORES_NEEDED) {
+			printf("Not enough cores for test_timer_secondary, expecting at least %u\n",
+			       NUM_LCORES_NEEDED);
+			return TEST_SKIPPED;
+		}
+
 		mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info),
 					 SOCKET_ID_ANY, 0);
 		test_info = mz->addr;
 		TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for "
 				     "test data");
 
-		TEST_ASSERT(rte_lcore_count() >= NUM_LCORES_NEEDED,
-			    "at least %d lcores needed to run tests",
-			    NUM_LCORES_NEEDED);
-
 		test_info->tim_mempool = rte_mempool_create("test_timer_mp",
 				NUM_TIMERS, sizeof(struct rte_timer), 0, 0,
 				NULL, NULL, NULL, NULL, rte_socket_id(), 0);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 14/14] test: skip tests when missing requirements

[Honnappa Nagarahalli]

> 
> Let's mark as skipped the tests when they are missing some requirements like a
> number of used cores or specific hardware availability, like compress, crypto or
> eventdev devices.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  app/test/test.c                     | 24 ++++++++++++++++--------
>  app/test/test_compressdev.c         |  4 ++--
>  app/test/test_cryptodev.c           |  4 ++--
>  app/test/test_distributor.c         |  4 ++--
>  app/test/test_distributor_perf.c    |  4 ++--
>  app/test/test_event_timer_adapter.c |  5 +++--
>  app/test/test_eventdev.c            |  2 ++
>  app/test/test_func_reentrancy.c     |  6 +++---
>  app/test/test_hash_multiwriter.c    |  7 +++----
>  app/test/test_hash_readwrite.c      |  7 +++----
>  app/test/test_hash_readwrite_lf.c   |  8 ++++----
>  app/test/test_ipsec.c               |  4 ++--
>  app/test/test_mbuf.c                | 13 ++++++-------
>  app/test/test_rcu_qsbr.c            | 10 +++++-----
>  app/test/test_rcu_qsbr_perf.c       |  9 +++++----
>  app/test/test_service_cores.c       | 14 ++++++++++++++
>  app/test/test_stack.c               |  8 +++++---
>  app/test/test_timer.c               | 10 +++++-----
>  app/test/test_timer_secondary.c     | 10 ++++++----
>  19 files changed, 90 insertions(+), 63 deletions(-)
> 

<snip>

> 
>  	RTE_LCORE_FOREACH_SLAVE(core_id) {
> diff --git a/app/test/test_hash_readwrite_lf.c
> b/app/test/test_hash_readwrite_lf.c
> index 5644361..2664f51 100644
> --- a/app/test/test_hash_readwrite_lf.c
> +++ b/app/test/test_hash_readwrite_lf.c
> @@ -1254,10 +1254,10 @@ struct {
>  	int htm;
>  	int use_jhash = 0;
>  	int ext_bkt = 0;
> -	if (rte_lcore_count() == 1) {
> -		printf("More than one lcore is required "
> -			"to do read write lock-free concurrency test\n");
> -		return -1;
> +
> +	if (rte_lcore_count() < 2) {
> +		printf("Not enough cores for hash_readwrite_lf_autotest,
> expecting at least 2\n");
> +		return TEST_SKIPPED;
>  	}
Looks good

> diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c index
> 92ab0c2..725d27d 100644
> --- a/app/test/test_rcu_qsbr.c
> +++ b/app/test/test_rcu_qsbr.c
> @@ -949,14 +949,14 @@
>  static int
>  test_rcu_qsbr_main(void)
>  {
> +	if (rte_lcore_count() < 5) {
Should be '4'. 4 cores are enough for the test.

> +		printf("Not enough cores for rcu_qsbr_autotest, expecting at
> least 5\n");
> +		return TEST_SKIPPED;
> +	}
> +
>  	if (get_enabled_cores_mask() != 0)
>  		return -1;
> 
> -	if (num_cores < 4) {
> -		printf("Test failed! Need 4 or more cores\n");
> -		goto test_fail;
> -	}
There is another check in 'get_enabled_cores_mask' function. We should convert that as well. Suggest pulling the check in 'get_enabled_cores_mask' to 'test_rcu_qsbr_main'

> -
>  	/* Error-checking test cases */
>  	if (test_rcu_qsbr_get_memsize() < 0)
>  		goto test_fail;
> diff --git a/app/test/test_rcu_qsbr_perf.c b/app/test/test_rcu_qsbr_perf.c
> index 6b1912c..dcdd9da 100644
> --- a/app/test/test_rcu_qsbr_perf.c
> +++ b/app/test/test_rcu_qsbr_perf.c
> @@ -623,6 +623,11 @@
>  static int
>  test_rcu_qsbr_main(void)
>  {
> +	if (rte_lcore_count() < 3) {
Should be 2. Minimum 2 cores are required.

> +		printf("Not enough cores for rcu_qsbr_perf_autotest,
> expecting at least 3\n");
> +		return TEST_SKIPPED;
> +	}
> +
>  	rte_atomic64_init(&updates);
>  	rte_atomic64_init(&update_cycles);
>  	rte_atomic64_init(&checks);
> @@ -632,10 +637,6 @@
>  		return -1;
> 
>  	printf("Number of cores provided = %d\n", num_cores);
> -	if (num_cores < 2) {
> -		printf("Test failed! Need 2 or more cores\n");
> -		goto test_fail;
> -	}
>  	if (num_cores > TEST_RCU_MAX_LCORE) {
Should convert this check as well to return TEST_SKIPPED.

>  		printf("Test failed! %d cores supported\n",
> TEST_RCU_MAX_LCORE);
>  		goto test_fail;

<snip>

Re: [dpdk-dev] [PATCH 14/14] test: skip tests when missing requirements

[David Marchand]

(pruning a little bit of the CC: list...)

On Fri, Jun 7, 2019 at 10:55 PM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:

> >
> > Let's mark as skipped the tests when they are missing some requirements
> like a
> > number of used cores or specific hardware availability, like compress,
> crypto or
> > eventdev devices.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  app/test/test.c                     | 24 ++++++++++++++++--------
> >  app/test/test_compressdev.c         |  4 ++--
> >  app/test/test_cryptodev.c           |  4 ++--
> >  app/test/test_distributor.c         |  4 ++--
> >  app/test/test_distributor_perf.c    |  4 ++--
> >  app/test/test_event_timer_adapter.c |  5 +++--
> >  app/test/test_eventdev.c            |  2 ++
> >  app/test/test_func_reentrancy.c     |  6 +++---
> >  app/test/test_hash_multiwriter.c    |  7 +++----
> >  app/test/test_hash_readwrite.c      |  7 +++----
> >  app/test/test_hash_readwrite_lf.c   |  8 ++++----
> >  app/test/test_ipsec.c               |  4 ++--
> >  app/test/test_mbuf.c                | 13 ++++++-------
> >  app/test/test_rcu_qsbr.c            | 10 +++++-----
> >  app/test/test_rcu_qsbr_perf.c       |  9 +++++----
> >  app/test/test_service_cores.c       | 14 ++++++++++++++
> >  app/test/test_stack.c               |  8 +++++---
> >  app/test/test_timer.c               | 10 +++++-----
> >  app/test/test_timer_secondary.c     | 10 ++++++----
> >  19 files changed, 90 insertions(+), 63 deletions(-)
> >
>
> <snip>
>
> >
> >       RTE_LCORE_FOREACH_SLAVE(core_id) {
> > diff --git a/app/test/test_hash_readwrite_lf.c
> > b/app/test/test_hash_readwrite_lf.c
> > index 5644361..2664f51 100644
> > --- a/app/test/test_hash_readwrite_lf.c
> > +++ b/app/test/test_hash_readwrite_lf.c
> > @@ -1254,10 +1254,10 @@ struct {
> >       int htm;
> >       int use_jhash = 0;
> >       int ext_bkt = 0;
> > -     if (rte_lcore_count() == 1) {
> > -             printf("More than one lcore is required "
> > -                     "to do read write lock-free concurrency test\n");
> > -             return -1;
> > +
> > +     if (rte_lcore_count() < 2) {
> > +             printf("Not enough cores for hash_readwrite_lf_autotest,
> > expecting at least 2\n");
> > +             return TEST_SKIPPED;
> >       }
> Looks good
>
> > diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c index
> > 92ab0c2..725d27d 100644
> > --- a/app/test/test_rcu_qsbr.c
> > +++ b/app/test/test_rcu_qsbr.c
> > @@ -949,14 +949,14 @@
> >  static int
> >  test_rcu_qsbr_main(void)
> >  {
> > +     if (rte_lcore_count() < 5) {
> Should be '4'. 4 cores are enough for the test.
>

Well, if we make it 4, then there was an issue before.
num_cores < 4 means 'at least 4 slave cores', so with the master core, we
are at 5.

See:
static inline int
get_enabled_cores_mask(void)
{
        uint16_t core_id;
        uint32_t max_cores = rte_lcore_count();

        if (max_cores > TEST_RCU_MAX_LCORE) {
                printf("Number of cores exceed %d\n", TEST_RCU_MAX_LCORE);
                return -1;
        }

        core_id = 0;
        num_cores = 0;
        RTE_LCORE_FOREACH_SLAVE(core_id) {
                enabled_core_ids[num_cores] = core_id;
                num_cores++;
        }

        return 0;
}



> > +             printf("Not enough cores for rcu_qsbr_autotest, expecting
> at
> > least 5\n");
> > +             return TEST_SKIPPED;
> > +     }
> > +
> >       if (get_enabled_cores_mask() != 0)
> >               return -1;
> >
> > -     if (num_cores < 4) {
> > -             printf("Test failed! Need 4 or more cores\n");
> > -             goto test_fail;
> > -     }
> There is another check in 'get_enabled_cores_mask' function. We should
> convert that as well. Suggest pulling the check in 'get_enabled_cores_mask'
> to 'test_rcu_qsbr_main'
>

Already said it before, can't we just shoot this enabled_core_ids[] array?
Is there a real need to enumerate per core rank?



> > -
> >       /* Error-checking test cases */
> >       if (test_rcu_qsbr_get_memsize() < 0)
> >               goto test_fail;
> > diff --git a/app/test/test_rcu_qsbr_perf.c
> b/app/test/test_rcu_qsbr_perf.c
> > index 6b1912c..dcdd9da 100644
> > --- a/app/test/test_rcu_qsbr_perf.c
> > +++ b/app/test/test_rcu_qsbr_perf.c
> > @@ -623,6 +623,11 @@
> >  static int
> >  test_rcu_qsbr_main(void)
> >  {
> > +     if (rte_lcore_count() < 3) {
> Should be 2. Minimum 2 cores are required.
>

Idem num_cores < 2.
Was the check incorrect before?



> > +             printf("Not enough cores for rcu_qsbr_perf_autotest,
> > expecting at least 3\n");
> > +             return TEST_SKIPPED;
> > +     }
> > +
> >       rte_atomic64_init(&updates);
> >       rte_atomic64_init(&update_cycles);
> >       rte_atomic64_init(&checks);
> > @@ -632,10 +637,6 @@
> >               return -1;
> >
> >       printf("Number of cores provided = %d\n", num_cores);
> > -     if (num_cores < 2) {
> > -             printf("Test failed! Need 2 or more cores\n");
> > -             goto test_fail;
> > -     }
> >       if (num_cores > TEST_RCU_MAX_LCORE) {
> Should convert this check as well to return TEST_SKIPPED.
>

Hum, skipped if there is a real issue at running the test with more than
128 cores (I'd like to hear how this value was chosen).
Or, we size this array RTE_MAX_LCORES and there is no check at all.
Or, we shoot enabled_core_ids[] array :-)


-- 
David Marchand

Re: [dpdk-dev] [PATCH 14/14] test: skip tests when missing requirements

[Honnappa Nagarahalli]

(pruning a little bit of the CC: list...)

On Fri, Jun 7, 2019 at 10:55 PM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com<mailto:Honnappa.Nagarahalli@arm.com>> wrote:
>
> Let's mark as skipped the tests when they are missing some requirements like a
> number of used cores or specific hardware availability, like compress, crypto or
> eventdev devices.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com<mailto:david.marchand@redhat.com>>
> ---
>  app/test/test.c                     | 24 ++++++++++++++++--------
>  app/test/test_compressdev.c         |  4 ++--
>  app/test/test_cryptodev.c           |  4 ++--
>  app/test/test_distributor.c         |  4 ++--
>  app/test/test_distributor_perf.c    |  4 ++--
>  app/test/test_event_timer_adapter.c |  5 +++--
>  app/test/test_eventdev.c            |  2 ++
>  app/test/test_func_reentrancy.c     |  6 +++---
>  app/test/test_hash_multiwriter.c    |  7 +++----
>  app/test/test_hash_readwrite.c      |  7 +++----
>  app/test/test_hash_readwrite_lf.c   |  8 ++++----
>  app/test/test_ipsec.c               |  4 ++--
>  app/test/test_mbuf.c                | 13 ++++++-------
>  app/test/test_rcu_qsbr.c            | 10 +++++-----
>  app/test/test_rcu_qsbr_perf.c       |  9 +++++----
>  app/test/test_service_cores.c       | 14 ++++++++++++++
>  app/test/test_stack.c               |  8 +++++---
>  app/test/test_timer.c               | 10 +++++-----
>  app/test/test_timer_secondary.c     | 10 ++++++----
>  19 files changed, 90 insertions(+), 63 deletions(-)
>

<snip>

>
>       RTE_LCORE_FOREACH_SLAVE(core_id) {
> diff --git a/app/test/test_hash_readwrite_lf.c
> b/app/test/test_hash_readwrite_lf.c
> index 5644361..2664f51 100644
> --- a/app/test/test_hash_readwrite_lf.c
> +++ b/app/test/test_hash_readwrite_lf.c
> @@ -1254,10 +1254,10 @@ struct {
>       int htm;
>       int use_jhash = 0;
>       int ext_bkt = 0;
> -     if (rte_lcore_count() == 1) {
> -             printf("More than one lcore is required "
> -                     "to do read write lock-free concurrency test\n");
> -             return -1;
> +
> +     if (rte_lcore_count() < 2) {
> +             printf("Not enough cores for hash_readwrite_lf_autotest,
> expecting at least 2\n");
> +             return TEST_SKIPPED;
>       }
Looks good

> diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c index
> 92ab0c2..725d27d 100644
> --- a/app/test/test_rcu_qsbr.c
> +++ b/app/test/test_rcu_qsbr.c
> @@ -949,14 +949,14 @@
>  static int
>  test_rcu_qsbr_main(void)
>  {
> +     if (rte_lcore_count() < 5) {
Should be '4'. 4 cores are enough for the test.

Well, if we make it 4, then there was an issue before.
num_cores < 4 means 'at least 4 slave cores', so with the master core, we are at 5.
[Honnappa] Ok

See:
static inline int
get_enabled_cores_mask(void)
{
        uint16_t core_id;
        uint32_t max_cores = rte_lcore_count();

        if (max_cores > TEST_RCU_MAX_LCORE) {
                printf("Number of cores exceed %d\n", TEST_RCU_MAX_LCORE);
                return -1;
        }

        core_id = 0;
        num_cores = 0;
        RTE_LCORE_FOREACH_SLAVE(core_id) {
                enabled_core_ids[num_cores] = core_id;
                num_cores++;
        }

        return 0;
}



> +             printf("Not enough cores for rcu_qsbr_autotest, expecting at
> least 5\n");
> +             return TEST_SKIPPED;
> +     }
> +
>       if (get_enabled_cores_mask() != 0)
>               return -1;
>
> -     if (num_cores < 4) {
> -             printf("Test failed! Need 4 or more cores\n");
> -             goto test_fail;
> -     }
There is another check in 'get_enabled_cores_mask' function. We should convert that as well. Suggest pulling the check in 'get_enabled_cores_mask' to 'test_rcu_qsbr_main'

Already said it before, can't we just shoot this enabled_core_ids[] array?
Is there a real need to enumerate per core rank?
[Honnappa] It is being used in the case where non-contiguous cores are enabled



> -
>       /* Error-checking test cases */
>       if (test_rcu_qsbr_get_memsize() < 0)
>               goto test_fail;
> diff --git a/app/test/test_rcu_qsbr_perf.c b/app/test/test_rcu_qsbr_perf.c
> index 6b1912c..dcdd9da 100644
> --- a/app/test/test_rcu_qsbr_perf.c
> +++ b/app/test/test_rcu_qsbr_perf.c
> @@ -623,6 +623,11 @@
>  static int
>  test_rcu_qsbr_main(void)
>  {
> +     if (rte_lcore_count() < 3) {
Should be 2. Minimum 2 cores are required.

Idem num_cores < 2.
Was the check incorrect before?
[Honnappa] It is fine



> +             printf("Not enough cores for rcu_qsbr_perf_autotest,
> expecting at least 3\n");
> +             return TEST_SKIPPED;
> +     }
> +
>       rte_atomic64_init(&updates);
>       rte_atomic64_init(&update_cycles);
>       rte_atomic64_init(&checks);
> @@ -632,10 +637,6 @@
>               return -1;
>
>       printf("Number of cores provided = %d\n", num_cores);
> -     if (num_cores < 2) {
> -             printf("Test failed! Need 2 or more cores\n");
> -             goto test_fail;
> -     }
>       if (num_cores > TEST_RCU_MAX_LCORE) {
Should convert this check as well to return TEST_SKIPPED.

Hum, skipped if there is a real issue at running the test with more than 128 cores (I'd like to hear how this value was chosen).
Or, we size this array RTE_MAX_LCORES and there is no check at all.
[Honnappa] We can size it to RTE_MAX_LCORES.
Or, we shoot enabled_core_ids[] array :-)


--
David Marchand

Re: [dpdk-dev] [PATCH 00/14] Unit tests fixes for CI

[Michael Santana Francisco]

On 6/4/19 4:59 AM, David Marchand wrote:
> This is a joint effort to make the unit tests ready for CI.
> The first 8 patches are fixes that I had accumulated.
> Then the second part of the series focuses on skipping tests when some
> requirements are not fulfilled so that we can start them in a restrained
> environment like Travis virtual machines that gives us two cores and does
> not have specific hw devices.
>
> We are still not ready for enabling those tests in Travis.
> At least, the following issues remain:
> - some fixes on librte_acl have not been merged yet [1],
> - the tests on --file-prefix are still ko, and have been isolated in a
>    test that we could disable while waiting for the fixes,
> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>    than 10s,
It would be a good idea to increase timeout. These are probably not the 
only tests that would pass with just a little bit more time
> - librte_table unit test crashes on ipv6 [2],
> - the "perf" tests are taking way too long for my taste,

We should still fix them. However I don't know if we should be running 
the perf test for every job and every patch on travis. It takes too 
long. The travis queue will be delayed too far behind for it to be of 
any use.

OTOH we could have one job as part of the travis build dedicated to 
running tests (or just perf test). It's still time consuming but better 
than running the test on every travis job. For this to work we would 
need to decreased the timeout for the perf tests as the timeout for it 
and the travis are both 10 minutes

> - the shared build unit tests all fail when depending on mempool since
>    the mempool drivers are not loaded,
>
> 1: http://patchwork.dpdk.org/project/dpdk/list/?series=4242
> 2: https://bugs.dpdk.org/show_bug.cgi?id=285
>
> Comments/reviews welcome!
>

Re: [dpdk-dev] [PATCH 00/14] Unit tests fixes for CI

[Thomas Monjalon]

04/06/2019 17:49, Michael Santana Francisco:
> On 6/4/19 4:59 AM, David Marchand wrote:
> > - the "perf" tests are taking way too long for my taste,
> 
> We should still fix them. However I don't know if we should be running 
> the perf test for every job and every patch on travis. It takes too 
> long. The travis queue will be delayed too far behind for it to be of 
> any use.
> 
> OTOH we could have one job as part of the travis build dedicated to 
> running tests (or just perf test). It's still time consuming but better 
> than running the test on every travis job. For this to work we would 
> need to decreased the timeout for the perf tests as the timeout for it 
> and the travis are both 10 minutes

+Cc ci@dpdk.org

I don't think we should run the perf tests in basic CI like Travis.
We can run perf tests if the purpose is to compare the performance
with previous releases, as some other tests in the community lab.

Re: [dpdk-dev] [PATCH 00/14] Unit tests fixes for CI

[Aaron Conole]

Thomas Monjalon <thomas@monjalon.net> writes:

> 04/06/2019 17:49, Michael Santana Francisco:
>> On 6/4/19 4:59 AM, David Marchand wrote:
>> > - the "perf" tests are taking way too long for my taste,

+1 here.

>> 
>> We should still fix them. However I don't know if we should be running 
>> the perf test for every job and every patch on travis. It takes too 
>> long. The travis queue will be delayed too far behind for it to be of 
>> any use.
>> 
>> OTOH we could have one job as part of the travis build dedicated to 
>> running tests (or just perf test). It's still time consuming but better 
>> than running the test on every travis job. For this to work we would 
>> need to decreased the timeout for the perf tests as the timeout for it 
>> and the travis are both 10 minutes
>
> +Cc ci@dpdk.org
>
> I don't think we should run the perf tests in basic CI like Travis.
> We can run perf tests if the purpose is to compare the performance
> with previous releases, as some other tests in the community lab.

+1 - some of the perf tests aren't going to complete in any sort of
reasonable time.  While we could claim it's a separate problem, we
should also not enable something that will make the travis runs so much
longer.

I do like the idea of running tests in the travis build, and I think it
would make sense to have just a single job for it (or maybe one for
clang and one for gcc?  maybe even that is overkill).

I would rather not do performance tests during the travis run, though.
It doesn't really make sense.  Travis isn't any kind of an 'optimized'
environment, so I don't know what 'performance' should mean.

[dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

This is a joint effort to make the unit tests ready for CI.
The first patches are fixes that I had accumulated.
Then the second part of the series focuses on skipping tests when some
requirements are not fulfilled so that we can start them in a restrained
environment like Travis virtual machines that gives us two cores and does
not have specific hw devices.

We are still not ready for enabling those tests in Travis.
At least, the following issues remain:
- some fixes on librte_acl have not been merged yet [1],
- the tests on --file-prefix are still ko, and have been isolated in a
  test that we could disable while waiting for the fixes,
- rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
  than 10s,
- librte_table unit test crashes on ipv6 [2],
- the "perf" tests are taking way too long for my taste,
- the shared build unit tests all fail when depending on mempool since
  the mempool drivers are not loaded,

1: http://patchwork.dpdk.org/project/dpdk/list/?series=4242
2: https://bugs.dpdk.org/show_bug.cgi?id=285

Changelog since v1:
- removed limit on 128 cores in rcu tests,
- reworked Michael patch on eal test and started splitting big tests into
  subtests: when a subtest fails, it does not impact the other subtests;
  plus, subtests are shorter to run, so easier to make them fit in 10s,

Comments/reviews welcome!

-- 
David Marchand

David Marchand (13):
  test/bonding: add missing sources for link bonding RSS
  test/crypto: move tests to the driver specific list
  test/eventdev: move tests to the driver specific list
  test/hash: fix off-by-one check on core count
  test/hash: clean remaining trace of scaling autotest
  test/latencystats: fix stack smashing
  test/rcu: remove arbitrary limit on max core count
  test/stack: fix lock-free test name
  test/eal: set memory channel config only in dedicated test
  test/eal: set core mask/list config only in dedicated test
  test: split into shorter subtests for CI
  test: do not start tests in parallel
  test: skip tests when missing requirements

Dharmik Thakkar (1):
  test/hash: rectify slaveid to point to valid cores

Michael Santana (1):
  test/eal: check number of cores before running subtests

 app/test/autotest.py                |   2 +-
 app/test/autotest_data.py           |   4 +-
 app/test/meson.build                |  84 ++++++------
 app/test/test.c                     |  24 ++--
 app/test/test_compressdev.c         |   4 +-
 app/test/test_cryptodev.c           |   4 +-
 app/test/test_distributor.c         |   4 +-
 app/test/test_distributor_perf.c    |   4 +-
 app/test/test_eal_flags.c           | 265 ++++++++++++++++++++----------------
 app/test/test_event_timer_adapter.c |   5 +-
 app/test/test_eventdev.c            |   2 +
 app/test/test_func_reentrancy.c     |   6 +-
 app/test/test_hash_multiwriter.c    |   7 +-
 app/test/test_hash_readwrite.c      |   7 +-
 app/test/test_hash_readwrite_lf.c   |  34 +++--
 app/test/test_ipsec.c               |   4 +-
 app/test/test_latencystats.c        |  18 ---
 app/test/test_mbuf.c                |  13 +-
 app/test/test_rcu_qsbr.c            | 139 +++++++++----------
 app/test/test_rcu_qsbr_perf.c       |  73 ++++------
 app/test/test_rwlock.c              |   6 +
 app/test/test_service_cores.c       |  14 ++
 app/test/test_stack.c               |   8 +-
 app/test/test_timer.c               |  10 +-
 app/test/test_timer_secondary.c     |  10 +-
 25 files changed, 381 insertions(+), 370 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 01/15] test/bonding: add missing sources for link bonding RSS

[David Marchand]

Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test/meson.build b/app/test/meson.build
index 4de856f..a51b50a 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -62,6 +62,7 @@ test_sources = files('commands.c',
 	'test_latencystats.c',
 	'test_link_bonding.c',
 	'test_link_bonding_mode4.c',
+	'test_link_bonding_rssconf.c',
 	'test_logs.c',
 	'test_lpm.c',
 	'test_lpm6.c',
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 02/15] test/crypto: move tests to the driver specific list

[David Marchand]

For consistency, put all specific crypto driver tests in the dedicated
list (in alphabetic order).

While at it:
- remove dead reference to cryptodev_sw_mrvl_autotest (renamed as
  cryptodev_sw_mvsam_autotest),
- call the crypto scheduler test only when built,

Fixes: 9eabcb682493 ("test: update autotest list")
Fixes: 3d20ffe6ddb1 ("test: reorder test cases in meson")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/autotest_data.py |  4 ++--
 app/test/meson.build      | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 0f2c9a7..6cf7eca 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -393,8 +393,8 @@
         "Report":  None,
     },
     {
-        "Name":    "Cryptodev sw mrvl autotest",
-        "Command": "cryptodev_sw_mrvl_autotest",
+        "Name":    "Cryptodev sw mvsam autotest",
+        "Command": "cryptodev_sw_mvsam_autotest",
         "Func":    default_autotest,
         "Report":  None,
     },
diff --git a/app/test/meson.build b/app/test/meson.build
index a51b50a..cb3de71 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -200,10 +200,7 @@ fast_parallel_test_names = [
 # All test cases in fast_non_parallel_test_names list are non-parallel
 fast_non_parallel_test_names = [
         'bitratestats_autotest',
-        'cryptodev_sw_armv8_autotest',
         'crc_autotest',
-        'cryptodev_openssl_asym_autotest',
-        'cryptodev_sw_mvsam_autotest',
         'delay_us_sleep_autotest',
         'distributor_autotest',
         'eventdev_common_autotest',
@@ -259,21 +256,22 @@ perf_test_names = [
 
 # All test cases in driver_test_names list are non-parallel
 driver_test_names = [
-        'link_bonding_autotest',
-        'link_bonding_mode4_autotest',
-        'link_bonding_rssconf_autotest',
-        'cryptodev_sw_mrvl_autotest',
-        'cryptodev_dpaa2_sec_autotest',
-        'cryptodev_dpaa_sec_autotest',
-        'cryptodev_qat_autotest',
         'cryptodev_aesni_mb_autotest',
-        'cryptodev_openssl_autotest',
-        'cryptodev_scheduler_autotest',
         'cryptodev_aesni_gcm_autotest',
+        'cryptodev_dpaa_sec_autotest',
+        'cryptodev_dpaa2_sec_autotest',
         'cryptodev_null_autotest',
-        'cryptodev_sw_snow3g_autotest',
+        'cryptodev_openssl_autotest',
+        'cryptodev_openssl_asym_autotest',
+        'cryptodev_qat_autotest',
+        'cryptodev_sw_armv8_autotest',
         'cryptodev_sw_kasumi_autotest',
+        'cryptodev_sw_mvsam_autotest',
+        'cryptodev_sw_snow3g_autotest',
         'cryptodev_sw_zuc_autotest',
+        'link_bonding_autotest',
+        'link_bonding_mode4_autotest',
+        'link_bonding_rssconf_autotest',
 ]
 
 # All test cases in dump_test_names list are non-parallel
@@ -329,6 +327,10 @@ if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
 	endif
 endif
 
+if dpdk_conf.has('RTE_LIBRTE_PMD_CRYPTO_SCHEDULER')
+	driver_test_names += 'cryptodev_scheduler_autotest'
+endif
+
 foreach d:test_deps
 	def_lib = get_option('default_library')
 	test_dep_objs += get_variable(def_lib + '_rte_' + d)
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 03/15] test/eventdev: move tests to the driver specific list

[David Marchand]

Same treatment than crypto tests, move the eventdev drivers tests in the
driver list.

While at it:
- eventdev_octeontx_autotest has been renamed as
  eventdev_selftest_octeontx,
- eventdev_sw_autotest has been renamed as
  eventdev_selftest_sw,

Fixes: 50fb749a3972 ("event/octeontx: move test to driver")
Fixes: 85fb515b7318 ("event/sw: move test to driver")
Fixes: 123d67c73b06 ("test/event: register selftests")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/meson.build | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index cb3de71..ad46515 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -204,8 +204,6 @@ fast_non_parallel_test_names = [
         'delay_us_sleep_autotest',
         'distributor_autotest',
         'eventdev_common_autotest',
-        'eventdev_octeontx_autotest',
-        'eventdev_sw_autotest',
         'fbarray_autotest',
         'hash_readwrite_autotest',
         'hash_readwrite_lf_autotest',
@@ -241,7 +239,6 @@ perf_test_names = [
         'timer_racecond_autotest',
         'efd_autotest',
         'hash_functions_autotest',
-        'eventdev_selftest_sw',
         'member_perf_autotest',
         'efd_perf_autotest',
         'lpm6_perf_autotest',
@@ -269,6 +266,8 @@ driver_test_names = [
         'cryptodev_sw_mvsam_autotest',
         'cryptodev_sw_snow3g_autotest',
         'cryptodev_sw_zuc_autotest',
+        'eventdev_selftest_octeontx',
+        'eventdev_selftest_sw',
         'link_bonding_autotest',
         'link_bonding_mode4_autotest',
         'link_bonding_rssconf_autotest',
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 04/15] test/hash: fix off-by-one check on core count

[David Marchand]

This subtest wants to start rwc_core_cnt[n] reader threads, while the
master core is waiting for them to report.

Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 app/test/test_hash_readwrite_lf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 343a338..f9f233a 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -939,7 +939,7 @@ struct {
 		}
 		for (n = 0; n < NUM_TEST; n++) {
 			unsigned int tot_lcore = rte_lcore_count();
-			if (tot_lcore < rwc_core_cnt[n])
+			if (tot_lcore < rwc_core_cnt[n] + 1)
 				goto finish;
 
 			printf("\nNumber of readers: %u\n", rwc_core_cnt[n]);
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 05/15] test/hash: rectify slaveid to point to valid cores

[David Marchand]

From: Dharmik Thakkar <dharmik.thakkar@arm.com>

This patch rectifies slave_id to point to valid core indexes rather than
core ranks in read-write lock-free concurrency test.

It also replaces a 'for' loop with RTE_LCORE_FOREACH API.

Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
Cc: stable@dpdk.org

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 app/test/test_hash_readwrite_lf.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index f9f233a..5644361 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -126,11 +126,9 @@ struct {
 	uint32_t i = 0;
 	uint16_t core_id;
 	uint32_t max_cores = rte_lcore_count();
-	for (core_id = 0; core_id < RTE_MAX_LCORE && i < max_cores; core_id++) {
-		if (rte_lcore_is_enabled(core_id)) {
-			enabled_core_ids[i] = core_id;
-			i++;
-		}
+	RTE_LCORE_FOREACH(core_id) {
+		enabled_core_ids[i] = core_id;
+		i++;
 	}
 
 	if (i != max_cores) {
@@ -738,7 +736,7 @@ struct {
 							enabled_core_ids[i]);
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -810,7 +808,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -886,7 +884,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -962,7 +960,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1037,7 +1035,7 @@ struct {
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1132,12 +1130,12 @@ struct {
 				for (i = rwc_core_cnt[n] + 1;
 				     i <= rwc_core_cnt[m] + rwc_core_cnt[n];
 				     i++)
-					rte_eal_wait_lcore(i);
+					rte_eal_wait_lcore(enabled_core_ids[i]);
 
 				writer_done = 1;
 
 				for (i = 1; i <= rwc_core_cnt[n]; i++)
-					if (rte_eal_wait_lcore(i) < 0)
+					if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 						goto err;
 
 				unsigned long long cycles_per_lookup =
@@ -1221,7 +1219,7 @@ struct {
 			writer_done = 1;
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (rte_eal_wait_lcore(i) < 0)
+				if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 06/15] test/hash: clean remaining trace of scaling autotest

[David Marchand]

Fixes: 3c518ca41ffa ("test/hash: remove hash scaling unit test")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index ad46515..44cf561 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -207,7 +207,6 @@ fast_non_parallel_test_names = [
         'fbarray_autotest',
         'hash_readwrite_autotest',
         'hash_readwrite_lf_autotest',
-        'hash_scaling_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 07/15] test/latencystats: fix stack smashing

[David Marchand]

Caught in one Travis run:
 + ------------------------------------------------------- +
 + Test Suite : Latency Stats Unit Test Suite
 + ------------------------------------------------------- +
 + TestCase [ 0] : test_latency_init succeeded
 + TestCase [ 1] : test_latency_update succeeded
[snip]
 + TestCase [1601724781] : test_latencystats_get_names succeeded
[snip]
 + Tests Failed :      1601790830

htonl(1601724781) -> "m", "a", "x", "_"
htonl(1601790830) -> "n", "c", "y", "_"

Looks like someone went too far.

The test passes a bigger size than the array it passes along.

Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/test_latencystats.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 039c508..8dd794b 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -69,13 +69,10 @@ static int test_latencystats_get_names(void)
 	int ret = 0, i = 0;
 	int size = 0;
 	struct rte_metric_name names[NUM_STATS];
-	struct rte_metric_name wrongnames[NUM_STATS - 2];
 
 	size_t m_size = sizeof(struct rte_metric_name);
 	for (i = 0; i < NUM_STATS; i++)
 		memset(&names[i], 0, m_size);
-	for (i = 0; i < NUM_STATS - 2; i++)
-		memset(&wrongnames[i], 0, m_size);
 
 	/* Success Test: Valid names and size */
 	size = NUM_STATS;
@@ -99,10 +96,6 @@ static int test_latencystats_get_names(void)
 	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get the metrics count,"
 		    "Actual: %d Expected: %d", ret, NUM_STATS);
 
-	/* Failure Test: Invalid names (array size lesser than size) */
-	size = NUM_STATS + 1;
-	ret = rte_latencystats_get_names(wrongnames, size);
-	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get metrics names");
 	return TEST_SUCCESS;
 }
 
@@ -112,13 +105,10 @@ static int test_latencystats_get(void)
 	int ret = 0, i = 0;
 	int size = 0;
 	struct rte_metric_value values[NUM_STATS];
-	struct rte_metric_value wrongvalues[NUM_STATS - 2];
 
 	size_t v_size = sizeof(struct rte_metric_value);
 	for (i = 0; i < NUM_STATS; i++)
 		memset(&values[i], 0, v_size);
-	for (i = 0; i < NUM_STATS - 2; i++)
-		memset(&wrongvalues[i], 0, v_size);
 
 	/* Success Test: Valid values and valid size */
 	size = NUM_STATS;
@@ -137,14 +127,6 @@ static int test_latencystats_get(void)
 	TEST_ASSERT((ret == NUM_STATS), "Test Failed to get the stats count,"
 		    "Actual: %d Expected: %d", ret, NUM_STATS);
 
-	/* Failure Test: Invalid values(array size lesser than size)
-	 * and invalid size
-	 */
-	size = NUM_STATS + 2;
-	ret = rte_latencystats_get(wrongvalues, size);
-	TEST_ASSERT(ret == NUM_STATS, "Test Failed to get latency metrics"
-			" values");
-
 	return TEST_SUCCESS;
 }
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 08/15] test/rcu: remove arbitrary limit on max core count

[David Marchand]

We can have up to RTE_MAX_LCORE in a dpdk application.
Remove the limit on 128 cores and tests that are now always false.

Fixes: b87089b0bb19 ("test/rcu: add API and functional tests")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- new patch added to remove the local TEST_RCU_MAX_LCORE limit
- changed some integer types to accomodate with the change

---
 app/test/test_rcu_qsbr.c      | 133 ++++++++++++++++++------------------------
 app/test/test_rcu_qsbr_perf.c |  63 +++++++-------------
 2 files changed, 79 insertions(+), 117 deletions(-)

diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c
index 5f0b138..97af087 100644
--- a/app/test/test_rcu_qsbr.c
+++ b/app/test/test_rcu_qsbr.c
@@ -26,19 +26,18 @@
 /* Make sure that this has the same value as __RTE_QSBR_CNT_INIT */
 #define TEST_RCU_QSBR_CNT_INIT 1
 
-#define TEST_RCU_MAX_LCORE 128
-uint16_t enabled_core_ids[TEST_RCU_MAX_LCORE];
+uint16_t enabled_core_ids[RTE_MAX_LCORE];
 uint8_t num_cores;
 
 static uint32_t *keys;
 #define TOTAL_ENTRY (1024 * 8)
 #define COUNTER_VALUE 4096
-static uint32_t *hash_data[TEST_RCU_MAX_LCORE][TOTAL_ENTRY];
+static uint32_t *hash_data[RTE_MAX_LCORE][TOTAL_ENTRY];
 static uint8_t writer_done;
 
-static struct rte_rcu_qsbr *t[TEST_RCU_MAX_LCORE];
-struct rte_hash *h[TEST_RCU_MAX_LCORE];
-char hash_name[TEST_RCU_MAX_LCORE][8];
+static struct rte_rcu_qsbr *t[RTE_MAX_LCORE];
+struct rte_hash *h[RTE_MAX_LCORE];
+char hash_name[RTE_MAX_LCORE][8];
 
 struct test_rcu_thread_info {
 	/* Index in RCU array */
@@ -48,28 +47,7 @@ struct test_rcu_thread_info {
 	/* lcore IDs registered on the RCU variable */
 	uint16_t r_core_ids[2];
 };
-struct test_rcu_thread_info thread_info[TEST_RCU_MAX_LCORE/4];
-
-static inline int
-get_enabled_cores_mask(void)
-{
-	uint16_t core_id;
-	uint32_t max_cores = rte_lcore_count();
-
-	if (max_cores > TEST_RCU_MAX_LCORE) {
-		printf("Number of cores exceed %d\n", TEST_RCU_MAX_LCORE);
-		return -1;
-	}
-
-	core_id = 0;
-	num_cores = 0;
-	RTE_LCORE_FOREACH_SLAVE(core_id) {
-		enabled_core_ids[num_cores] = core_id;
-		num_cores++;
-	}
-
-	return 0;
-}
+struct test_rcu_thread_info thread_info[RTE_MAX_LCORE/4];
 
 static int
 alloc_rcu(void)
@@ -77,9 +55,9 @@ struct test_rcu_thread_info {
 	int i;
 	uint32_t sz;
 
-	sz = rte_rcu_qsbr_get_memsize(TEST_RCU_MAX_LCORE);
+	sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
 
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		t[i] = (struct rte_rcu_qsbr *)rte_zmalloc(NULL, sz,
 						RTE_CACHE_LINE_SIZE);
 
@@ -91,7 +69,7 @@ struct test_rcu_thread_info {
 {
 	int i;
 
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_free(t[i]);
 
 	return 0;
@@ -111,7 +89,7 @@ struct test_rcu_thread_info {
 	sz = rte_rcu_qsbr_get_memsize(0);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 1), "Get Memsize for 0 threads");
 
-	sz = rte_rcu_qsbr_get_memsize(TEST_RCU_MAX_LCORE);
+	sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
 	/* For 128 threads,
 	 * for machines with cache line size of 64B - 8384
 	 * for machines with cache line size of 128 - 16768
@@ -132,7 +110,7 @@ struct test_rcu_thread_info {
 
 	printf("\nTest rte_rcu_qsbr_init()\n");
 
-	r = rte_rcu_qsbr_init(NULL, TEST_RCU_MAX_LCORE);
+	r = rte_rcu_qsbr_init(NULL, RTE_MAX_LCORE);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((r != 1), "NULL variable");
 
 	return 0;
@@ -156,7 +134,7 @@ struct test_rcu_thread_info {
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0),
 		"NULL variable, invalid thread id");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	/* Register valid thread id */
 	ret = rte_rcu_qsbr_thread_register(t[0], enabled_core_ids[0]);
@@ -168,7 +146,7 @@ struct test_rcu_thread_info {
 		"Already registered thread id");
 
 	/* Register valid thread id - max allowed thread id */
-	ret = rte_rcu_qsbr_thread_register(t[0], TEST_RCU_MAX_LCORE - 1);
+	ret = rte_rcu_qsbr_thread_register(t[0], RTE_MAX_LCORE - 1);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 1), "Max thread id");
 
 	ret = rte_rcu_qsbr_thread_register(t[0], 100000);
@@ -185,9 +163,10 @@ struct test_rcu_thread_info {
 static int
 test_rcu_qsbr_thread_unregister(void)
 {
-	int i, j, ret;
+	unsigned int num_threads[3] = {1, RTE_MAX_LCORE, 1};
+	unsigned int i, j;
 	uint64_t token;
-	uint8_t num_threads[3] = {1, TEST_RCU_MAX_LCORE, 1};
+	int ret;
 
 	printf("\nTest rte_rcu_qsbr_thread_unregister()\n");
 
@@ -198,7 +177,7 @@ struct test_rcu_thread_info {
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0),
 		"NULL variable, invalid thread id");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	rte_rcu_qsbr_thread_register(t[0], enabled_core_ids[0]);
 
@@ -207,7 +186,7 @@ struct test_rcu_thread_info {
 		"NULL variable, invalid thread id");
 
 	/* Find first disabled core */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++) {
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
 		if (enabled_core_ids[i] == 0)
 			break;
 	}
@@ -232,15 +211,15 @@ struct test_rcu_thread_info {
 	/*
 	 * Test with different thread_ids:
 	 * 1 - thread_id = 0
-	 * 2 - All possible thread_ids, from 0 to TEST_RCU_MAX_LCORE
-	 * 3 - thread_id = TEST_RCU_MAX_LCORE - 1
+	 * 2 - All possible thread_ids, from 0 to RTE_MAX_LCORE
+	 * 3 - thread_id = RTE_MAX_LCORE - 1
 	 */
 	for (j = 0; j < 3; j++) {
-		rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+		rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 		for (i = 0; i < num_threads[j]; i++)
 			rte_rcu_qsbr_thread_register(t[0],
-				(j == 2) ? (TEST_RCU_MAX_LCORE - 1) : i);
+				(j == 2) ? (RTE_MAX_LCORE - 1) : i);
 
 		token = rte_rcu_qsbr_start(t[0]);
 		TEST_RCU_QSBR_RETURN_IF_ERROR(
@@ -248,10 +227,10 @@ struct test_rcu_thread_info {
 		/* Update quiescent state counter */
 		for (i = 0; i < num_threads[j]; i++) {
 			/* Skip one update */
-			if (i == (TEST_RCU_MAX_LCORE - 10))
+			if (i == (RTE_MAX_LCORE - 10))
 				continue;
 			rte_rcu_qsbr_quiescent(t[0],
-				(j == 2) ? (TEST_RCU_MAX_LCORE - 1) : i);
+				(j == 2) ? (RTE_MAX_LCORE - 1) : i);
 		}
 
 		if (j == 1) {
@@ -260,7 +239,7 @@ struct test_rcu_thread_info {
 			TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0),
 						"Non-blocking QSBR check");
 			/* Update the previously skipped thread */
-			rte_rcu_qsbr_quiescent(t[0], TEST_RCU_MAX_LCORE - 10);
+			rte_rcu_qsbr_quiescent(t[0], RTE_MAX_LCORE - 10);
 		}
 
 		/* Validate the updates */
@@ -270,7 +249,7 @@ struct test_rcu_thread_info {
 
 		for (i = 0; i < num_threads[j]; i++)
 			rte_rcu_qsbr_thread_unregister(t[0],
-				(j == 2) ? (TEST_RCU_MAX_LCORE - 1) : i);
+				(j == 2) ? (RTE_MAX_LCORE - 1) : i);
 
 		/* Check with no thread registered */
 		ret = rte_rcu_qsbr_check(t[0], token, true);
@@ -292,7 +271,7 @@ struct test_rcu_thread_info {
 
 	printf("\nTest rte_rcu_qsbr_start()\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	for (i = 0; i < 3; i++)
 		rte_rcu_qsbr_thread_register(t[0], enabled_core_ids[i]);
@@ -331,7 +310,7 @@ struct test_rcu_thread_info {
 
 	printf("\nTest rte_rcu_qsbr_check()\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	token = rte_rcu_qsbr_start(t[0]);
 	TEST_RCU_QSBR_RETURN_IF_ERROR(
@@ -365,7 +344,7 @@ struct test_rcu_thread_info {
 	ret = rte_rcu_qsbr_check(t[0], token, true);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "Blocking QSBR check");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	for (i = 0; i < 4; i++)
 		rte_rcu_qsbr_thread_register(t[0], enabled_core_ids[i]);
@@ -414,7 +393,7 @@ struct test_rcu_thread_info {
 
 	printf("\nTest rte_rcu_qsbr_synchronize()\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	/* Test if the API returns when there are no threads reporting
 	 * QS on the variable.
@@ -424,7 +403,7 @@ struct test_rcu_thread_info {
 	/* Test if the API returns when there are threads registered
 	 * but not online.
 	 */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_thread_register(t[0], i);
 	rte_rcu_qsbr_synchronize(t[0], RTE_QSBR_THRID_INVALID);
 
@@ -436,12 +415,12 @@ struct test_rcu_thread_info {
 	rte_rcu_qsbr_thread_offline(t[0], 0);
 
 	/* Check the other boundary */
-	rte_rcu_qsbr_thread_online(t[0], TEST_RCU_MAX_LCORE - 1);
-	rte_rcu_qsbr_synchronize(t[0], TEST_RCU_MAX_LCORE - 1);
-	rte_rcu_qsbr_thread_offline(t[0], TEST_RCU_MAX_LCORE - 1);
+	rte_rcu_qsbr_thread_online(t[0], RTE_MAX_LCORE - 1);
+	rte_rcu_qsbr_synchronize(t[0], RTE_MAX_LCORE - 1);
+	rte_rcu_qsbr_thread_offline(t[0], RTE_MAX_LCORE - 1);
 
 	/* Test if the API returns after unregisterng all the threads */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_thread_unregister(t[0], i);
 	rte_rcu_qsbr_synchronize(t[0], RTE_QSBR_THRID_INVALID);
 
@@ -474,7 +453,7 @@ struct test_rcu_thread_info {
 
 	printf("Test rte_rcu_qsbr_thread_online()\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	/* Register 2 threads to validate that only the
 	 * online thread is waited upon.
@@ -501,9 +480,9 @@ struct test_rcu_thread_info {
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "thread update");
 
 	/* Make all the threads online */
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 	token = rte_rcu_qsbr_start(t[0]);
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++) {
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
 		rte_rcu_qsbr_thread_register(t[0], i);
 		rte_rcu_qsbr_thread_online(t[0], i);
 	}
@@ -512,7 +491,7 @@ struct test_rcu_thread_info {
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "thread online");
 	/* Check if all the online threads can report QS */
 	token = rte_rcu_qsbr_start(t[0]);
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_quiescent(t[0], i);
 	ret = rte_rcu_qsbr_check(t[0], token, true);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "thread update");
@@ -532,7 +511,7 @@ struct test_rcu_thread_info {
 
 	printf("\nTest rte_rcu_qsbr_thread_offline()\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	rte_rcu_qsbr_thread_register(t[0], enabled_core_ids[0]);
 
@@ -560,10 +539,10 @@ struct test_rcu_thread_info {
 	/*
 	 * Check a sequence of online/status/offline/status/online/status
 	 */
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 	token = rte_rcu_qsbr_start(t[0]);
 	/* Make the threads online */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++) {
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
 		rte_rcu_qsbr_thread_register(t[0], i);
 		rte_rcu_qsbr_thread_online(t[0], i);
 	}
@@ -574,13 +553,13 @@ struct test_rcu_thread_info {
 
 	/* Check if all the online threads can report QS */
 	token = rte_rcu_qsbr_start(t[0]);
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_quiescent(t[0], i);
 	ret = rte_rcu_qsbr_check(t[0], token, true);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "report QS");
 
 	/* Make all the threads offline */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_thread_offline(t[0], i);
 	/* Make sure these threads are not being waited on */
 	token = rte_rcu_qsbr_start(t[0]);
@@ -588,11 +567,11 @@ struct test_rcu_thread_info {
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "offline QS");
 
 	/* Make the threads online */
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_thread_online(t[0], i);
 	/* Check if all the online threads can report QS */
 	token = rte_rcu_qsbr_start(t[0]);
-	for (i = 0; i < TEST_RCU_MAX_LCORE; i++)
+	for (i = 0; i < RTE_MAX_LCORE; i++)
 		rte_rcu_qsbr_quiescent(t[0], i);
 	ret = rte_rcu_qsbr_check(t[0], token, true);
 	TEST_RCU_QSBR_RETURN_IF_ERROR((ret == 0), "online again");
@@ -615,8 +594,8 @@ struct test_rcu_thread_info {
 	rte_rcu_qsbr_dump(stdout, NULL);
 	rte_rcu_qsbr_dump(NULL, NULL);
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
-	rte_rcu_qsbr_init(t[1], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
+	rte_rcu_qsbr_init(t[1], RTE_MAX_LCORE);
 
 	/* QS variable with 0 core mask */
 	rte_rcu_qsbr_dump(stdout, t[0]);
@@ -738,8 +717,7 @@ struct test_rcu_thread_info {
 
 	for (i = 0; i < TOTAL_ENTRY; i++) {
 		hash_data[hash_id][i] =
-			rte_zmalloc(NULL,
-				sizeof(uint32_t) * TEST_RCU_MAX_LCORE, 0);
+			rte_zmalloc(NULL, sizeof(uint32_t) * RTE_MAX_LCORE, 0);
 		if (hash_data[hash_id][i] == NULL) {
 			printf("No memory\n");
 			return NULL;
@@ -781,7 +759,7 @@ struct test_rcu_thread_info {
 
 	printf("Test: 1 writer, 1 QSBR variable, simultaneous QSBR queries\n");
 
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	/* Shared data structure created */
 	h[0] = init_hash(0);
@@ -924,7 +902,7 @@ struct test_rcu_thread_info {
 
 	for (i = 0; i < test_cores / 4; i++) {
 		j = i * 4;
-		rte_rcu_qsbr_init(t[i], TEST_RCU_MAX_LCORE);
+		rte_rcu_qsbr_init(t[i], RTE_MAX_LCORE);
 		h[i] = init_hash(i);
 		if (h[i] == NULL) {
 			printf("Hash init failed\n");
@@ -998,8 +976,13 @@ struct test_rcu_thread_info {
 static int
 test_rcu_qsbr_main(void)
 {
-	if (get_enabled_cores_mask() != 0)
-		return -1;
+	uint16_t core_id;
+
+	num_cores = 0;
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		enabled_core_ids[num_cores] = core_id;
+		num_cores++;
+	}
 
 	if (num_cores < 4) {
 		printf("Test failed! Need 4 or more cores\n");
diff --git a/app/test/test_rcu_qsbr_perf.c b/app/test/test_rcu_qsbr_perf.c
index 33ca36c..c918a94 100644
--- a/app/test/test_rcu_qsbr_perf.c
+++ b/app/test/test_rcu_qsbr_perf.c
@@ -16,8 +16,7 @@
 #include "test.h"
 
 /* Check condition and return an error if true. */
-#define TEST_RCU_MAX_LCORE 128
-static uint16_t enabled_core_ids[TEST_RCU_MAX_LCORE];
+static uint16_t enabled_core_ids[RTE_MAX_LCORE];
 static uint8_t num_cores;
 
 static uint32_t *keys;
@@ -28,7 +27,7 @@
 static volatile uint8_t all_registered;
 static volatile uint32_t thr_id;
 
-static struct rte_rcu_qsbr *t[TEST_RCU_MAX_LCORE];
+static struct rte_rcu_qsbr *t[RTE_MAX_LCORE];
 static struct rte_hash *h;
 static char hash_name[8];
 static rte_atomic64_t updates, checks;
@@ -39,40 +38,19 @@
  */
 #define RCU_SCALE_DOWN 1000
 
-/* Simple way to allocate thread ids in 0 to TEST_RCU_MAX_LCORE space */
+/* Simple way to allocate thread ids in 0 to RTE_MAX_LCORE space */
 static inline uint32_t
 alloc_thread_id(void)
 {
 	uint32_t tmp_thr_id;
 
 	tmp_thr_id = __atomic_fetch_add(&thr_id, 1, __ATOMIC_RELAXED);
-	if (tmp_thr_id >= TEST_RCU_MAX_LCORE)
+	if (tmp_thr_id >= RTE_MAX_LCORE)
 		printf("Invalid thread id %u\n", tmp_thr_id);
 
 	return tmp_thr_id;
 }
 
-static inline int
-get_enabled_cores_mask(void)
-{
-	uint16_t core_id;
-	uint32_t max_cores = rte_lcore_count();
-
-	if (max_cores > TEST_RCU_MAX_LCORE) {
-		printf("Number of cores exceed %d\n", TEST_RCU_MAX_LCORE);
-		return -1;
-	}
-
-	core_id = 0;
-	num_cores = 0;
-	RTE_LCORE_FOREACH_SLAVE(core_id) {
-		enabled_core_ids[num_cores] = core_id;
-		num_cores++;
-	}
-
-	return 0;
-}
-
 static int
 test_rcu_qsbr_reader_perf(void *arg)
 {
@@ -165,7 +143,7 @@
 	if (all_registered == 1)
 		tmp_num_cores = num_cores - 1;
 	else
-		tmp_num_cores = TEST_RCU_MAX_LCORE;
+		tmp_num_cores = RTE_MAX_LCORE;
 
 	sz = rte_rcu_qsbr_get_memsize(tmp_num_cores);
 	t[0] = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz,
@@ -223,7 +201,7 @@
 	if (all_registered == 1)
 		tmp_num_cores = num_cores;
 	else
-		tmp_num_cores = TEST_RCU_MAX_LCORE;
+		tmp_num_cores = RTE_MAX_LCORE;
 
 	sz = rte_rcu_qsbr_get_memsize(tmp_num_cores);
 	t[0] = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz,
@@ -269,11 +247,11 @@
 	/* Number of readers does not matter for QS variable in this test
 	 * case as no reader will be registered.
 	 */
-	sz = rte_rcu_qsbr_get_memsize(TEST_RCU_MAX_LCORE);
+	sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
 	t[0] = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz,
 						RTE_CACHE_LINE_SIZE);
 	/* QS variable is initialized */
-	rte_rcu_qsbr_init(t[0], TEST_RCU_MAX_LCORE);
+	rte_rcu_qsbr_init(t[0], RTE_MAX_LCORE);
 
 	/* Writer threads are launched */
 	for (i = 0; i < num_cores; i++)
@@ -367,7 +345,7 @@ static struct rte_hash *init_hash(void)
 
 	for (i = 0; i < TOTAL_ENTRY; i++) {
 		hash_data[i] = rte_zmalloc(NULL,
-				sizeof(uint32_t) * TEST_RCU_MAX_LCORE, 0);
+				sizeof(uint32_t) * RTE_MAX_LCORE, 0);
 		if (hash_data[i] == NULL) {
 			printf("No memory\n");
 			return NULL;
@@ -417,7 +395,7 @@ static struct rte_hash *init_hash(void)
 	if (all_registered == 1)
 		tmp_num_cores = num_cores;
 	else
-		tmp_num_cores = TEST_RCU_MAX_LCORE;
+		tmp_num_cores = RTE_MAX_LCORE;
 
 	sz = rte_rcu_qsbr_get_memsize(tmp_num_cores);
 	t[0] = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz,
@@ -530,7 +508,7 @@ static struct rte_hash *init_hash(void)
 	if (all_registered == 1)
 		tmp_num_cores = num_cores;
 	else
-		tmp_num_cores = TEST_RCU_MAX_LCORE;
+		tmp_num_cores = RTE_MAX_LCORE;
 
 	sz = rte_rcu_qsbr_get_memsize(tmp_num_cores);
 	t[0] = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz,
@@ -626,23 +604,24 @@ static struct rte_hash *init_hash(void)
 static int
 test_rcu_qsbr_main(void)
 {
+	uint16_t core_id;
+
 	rte_atomic64_init(&updates);
 	rte_atomic64_init(&update_cycles);
 	rte_atomic64_init(&checks);
 	rte_atomic64_init(&check_cycles);
 
-	if (get_enabled_cores_mask() != 0)
-		return -1;
+	num_cores = 0;
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		enabled_core_ids[num_cores] = core_id;
+		num_cores++;
+	}
 
 	printf("Number of cores provided = %d\n", num_cores);
 	if (num_cores < 2) {
 		printf("Test failed! Need 2 or more cores\n");
 		goto test_fail;
 	}
-	if (num_cores > TEST_RCU_MAX_LCORE) {
-		printf("Test failed! %d cores supported\n", TEST_RCU_MAX_LCORE);
-		goto test_fail;
-	}
 
 	printf("Perf test with all reader threads registered\n");
 	printf("--------------------------------------------\n");
@@ -664,12 +643,12 @@ static struct rte_hash *init_hash(void)
 		goto test_fail;
 
 	/* Make sure the actual number of cores provided is less than
-	 * TEST_RCU_MAX_LCORE. This will allow for some threads not
+	 * RTE_MAX_LCORE. This will allow for some threads not
 	 * to be registered on the QS variable.
 	 */
-	if (num_cores >= TEST_RCU_MAX_LCORE) {
+	if (num_cores >= RTE_MAX_LCORE) {
 		printf("Test failed! number of cores provided should be less than %d\n",
-			TEST_RCU_MAX_LCORE);
+			RTE_MAX_LCORE);
 		goto test_fail;
 	}
 
-- 
1.8.3.1

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 08/15] test/rcu: remove arbitrary limit on max core count

[Ferruh Yigit]

On 6/15/2019 7:42 AM, David Marchand wrote:
> We can have up to RTE_MAX_LCORE in a dpdk application.
> Remove the limit on 128 cores and tests that are now always false.
> 
> Fixes: b87089b0bb19 ("test/rcu: add API and functional tests")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

<...>

> @@ -664,12 +643,12 @@ static struct rte_hash *init_hash(void)
>  		goto test_fail;
>  
>  	/* Make sure the actual number of cores provided is less than
> -	 * TEST_RCU_MAX_LCORE. This will allow for some threads not
> +	 * RTE_MAX_LCORE. This will allow for some threads not
>  	 * to be registered on the QS variable.
>  	 */
> -	if (num_cores >= TEST_RCU_MAX_LCORE) {
> +	if (num_cores >= RTE_MAX_LCORE) {
>  		printf("Test failed! number of cores provided should be less than %d\n",
> -			TEST_RCU_MAX_LCORE);
> +			RTE_MAX_LCORE);
>  		goto test_fail;
>  	}
>  
> 

Hi David,

This is causing build error for arm [1] since in config they have
"CONFIG_RTE_MAX_LCORE=256"
Can we make 'num_cores' 'uint16_t'?

[1]
.../dpdk/app/test/test_rcu_qsbr_perf.c:649:16: error: comparison is always false
due to limited range of data type [-Werror=type-limits]
  if (num_cores >= RTE_MAX_LCORE) {
                ^~

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 08/15] test/rcu: remove arbitrary limit on max core count

[David Marchand]

On Fri, Jun 28, 2019 at 2:57 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 6/15/2019 7:42 AM, David Marchand wrote:
> > We can have up to RTE_MAX_LCORE in a dpdk application.
> > Remove the limit on 128 cores and tests that are now always false.
> >
> > Fixes: b87089b0bb19 ("test/rcu: add API and functional tests")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> <...>
>
> > @@ -664,12 +643,12 @@ static struct rte_hash *init_hash(void)
> >               goto test_fail;
> >
> >       /* Make sure the actual number of cores provided is less than
> > -      * TEST_RCU_MAX_LCORE. This will allow for some threads not
> > +      * RTE_MAX_LCORE. This will allow for some threads not
> >        * to be registered on the QS variable.
> >        */
> > -     if (num_cores >= TEST_RCU_MAX_LCORE) {
> > +     if (num_cores >= RTE_MAX_LCORE) {
> >               printf("Test failed! number of cores provided should be
> less than %d\n",
> > -                     TEST_RCU_MAX_LCORE);
> > +                     RTE_MAX_LCORE);
> >               goto test_fail;
> >       }
> >
> >
>
> Hi David,
>
> This is causing build error for arm [1] since in config they have
> "CONFIG_RTE_MAX_LCORE=256"
> Can we make 'num_cores' 'uint16_t'?
>
> [1]
> .../dpdk/app/test/test_rcu_qsbr_perf.c:649:16: error: comparison is always
> false
> due to limited range of data type [-Werror=type-limits]
>   if (num_cores >= RTE_MAX_LCORE) {
>                 ^~
>

I checked and I am not reproducing it in travis arm64 compilation, but did
not investigate why.
Indeed there is an issue, I can send a patch to fix, but Honnappa also
reported it and sent a patch [1].


1: http://patchwork.dpdk.org/patch/55530/

-- 
David Marchand

[dpdk-dev] [PATCH v2 09/15] test/stack: fix lock-free test name

[David Marchand]

Fixes: 0420378bbfc4 ("test/stack: check lock-free implementation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 44cf561..7628ed9 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -188,7 +188,7 @@ fast_parallel_test_names = [
         'sched_autotest',
         'spinlock_autotest',
         'stack_autotest',
-        'stack_nb_autotest',
+        'stack_lf_autotest',
         'string_autotest',
         'table_autotest',
         'tailq_autotest',
@@ -247,7 +247,7 @@ perf_test_names = [
         'ring_pmd_perf_autotest',
         'pmd_perf_autotest',
         'stack_perf_autotest',
-        'stack_nb_perf_autotest',
+        'stack_lf_perf_autotest',
 ]
 
 # All test cases in driver_test_names list are non-parallel
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 10/15] test/eal: set memory channel config only in dedicated test

[David Marchand]

The -n option is an optimisation configuration option that defaults to 0.
Such a default value makes the mempool library distributes objects as if
there was 4 memory channels, so -n 4 is the same as the default behavior.

This parameter was mandatory a long time ago, but has been optional for
a while. We check that setting this value works fine in its own test.
Remove it everywhere else.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
Changelog since v1:
- rebased on master

---
 app/test/autotest.py      |   2 +-
 app/test/meson.build      |   2 +-
 app/test/test_eal_flags.c | 191 ++++++++++++++++++++++++----------------------
 3 files changed, 101 insertions(+), 94 deletions(-)

diff --git a/app/test/autotest.py b/app/test/autotest.py
index 46c469e..b42f488 100644
--- a/app/test/autotest.py
+++ b/app/test/autotest.py
@@ -32,7 +32,7 @@ def usage():
     else:
         test_whitelist = testlist
 
-cmdline = "%s -c f -n 4" % (sys.argv[1])
+cmdline = "%s -c f" % (sys.argv[1])
 
 print(cmdline)
 
diff --git a/app/test/meson.build b/app/test/meson.build
index 7628ed9..a9dddf8 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -368,7 +368,7 @@ endif
 
 num_cores_arg = '-l ' + num_cores
 
-test_args = [num_cores_arg, '-n 4']
+test_args = [num_cores_arg]
 foreach arg : fast_parallel_test_names
 	if host_machine.system() == 'linux'
 		test(arg, dpdk_test,
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 9112c96..e82e56a 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -245,25 +245,25 @@ enum hugepage_action {
 #endif
 
 	const char *wlinval[][11] = {
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "error", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:error:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0.1error", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "error0:0:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+		{prgname, prefix, mp_flag, "-c", "1",
 				pci_whitelist, "0:0:0.1.2", "", ""},
 	};
 	/* Test with valid whitelist option */
-	const char *wlval1[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval1[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "00FF:09:0B.3"};
-	const char *wlval2[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval2[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "09:0B.3", pci_whitelist, "0a:0b.1"};
-	const char *wlval3[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1",
+	const char *wlval3[] = {prgname, prefix, mp_flag, "-c", "1",
 			pci_whitelist, "09:0B.3,type=test",
 			pci_whitelist, "08:00.1,type=normal",
 	};
@@ -311,15 +311,16 @@ enum hugepage_action {
 #endif
 
 	const char *blinval[][9] = {
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "error"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:error:0.1"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0.1error"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "error0:0:0.1"},
-		{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "0:0:0.1.2"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "error"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:error:0.1"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1error"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "error0:0:0.1"},
+		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1.2"},
 	};
 	/* Test with valid blacklist option */
-	const char *blval[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-b", "FF:09:0B.3"};
+	const char *blval[] = {prgname, prefix, mp_flag, "-c", "1",
+			       "-b", "FF:09:0B.3"};
 
 	int i;
 
@@ -354,17 +355,17 @@ enum hugepage_action {
 #endif
 
 	/* Test with invalid vdev option */
-	const char *vdevinval[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevinval[] = {prgname, prefix, no_huge,
 				"-c", "1", vdev, "eth_dummy"};
 
 	/* Test with valid vdev option */
-	const char *vdevval1[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval1[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0"};
 
-	const char *vdevval2[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval2[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0,args=test"};
 
-	const char *vdevval3[] = {prgname, prefix, no_huge, "-n", "1",
+	const char *vdevval3[] = {prgname, prefix, no_huge,
 	"-c", "1", vdev, "net_ring0,nodeaction=r1:0:CREATE"};
 
 	if (launch_proc(vdevinval) == 0) {
@@ -412,13 +413,13 @@ enum hugepage_action {
 #endif
 
 	const char *rinval[][9] = {
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "error"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "0"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "-1"},
-			{prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "17"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "error"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "0"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "-1"},
+			{prgname, prefix, mp_flag, "-c", "1", "-r", "17"},
 	};
 	/* Test with valid blacklist option */
-	const char *rval[] = {prgname, prefix, mp_flag, "-n", "1", "-c", "1", "-r", "16"};
+	const char *rval[] = {prgname, prefix, mp_flag, "-c", "1", "-r", "16"};
 
 	int i;
 
@@ -456,74 +457,74 @@ enum hugepage_action {
 #endif
 
 	/* -c flag but no coremask value */
-	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
+	const char *argv1[] = { prgname, prefix, mp_flag, "-c"};
 	/* No -c, -l or --lcores flag at all */
-	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
+	const char *argv2[] = { prgname, prefix, mp_flag};
 	/* bad coremask value */
 	const char *argv3[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-c", "error" };
+				"-c", "error" };
 	/* sanity check of tests - valid coremask value */
 	const char *argv4[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-c", "1" };
+				"-c", "1" };
 	/* -l flag but no corelist value */
 	const char *argv5[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l"};
+				"-l"};
 	const char *argv6[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", " " };
+				"-l", " " };
 	/* bad corelist values */
 	const char *argv7[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "error" };
+				"-l", "error" };
 	const char *argv8[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1-" };
+				"-l", "1-" };
 	const char *argv9[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1," };
+				"-l", "1," };
 	const char *argv10[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "-l", "1#2" };
+				 "-l", "1#2" };
 	/* core number is negative value */
 	const char * const argv11[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "-5" };
+				"-l", "-5" };
 	const char * const argv12[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "-5-7" };
+				"-l", "-5-7" };
 	/* core number is maximum value */
 	const char * const argv13[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+				"-l", RTE_STR(RTE_MAX_LCORE) };
 	const char * const argv14[] = { prgname, prefix, mp_flag,
-				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
+				"-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 	/* sanity check test - valid corelist value */
 	const char * const argv15[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "-l", "1-2,3" };
+				 "-l", "1-2,3" };
 
 	/* --lcores flag but no lcores value */
 	const char * const argv16[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores" };
+				 "--lcores" };
 	const char * const argv17[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", " " };
+				 "--lcores", " " };
 	/* bad lcores value */
 	const char * const argv18[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "1-3-5" };
+				 "--lcores", "1-3-5" };
 	const char * const argv19[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "0-1,,2" };
+				 "--lcores", "0-1,,2" };
 	const char * const argv20[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "0-,1" };
+				 "--lcores", "0-,1" };
 	const char * const argv21[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(0-,2-4)" };
+				 "--lcores", "(0-,2-4)" };
 	const char * const argv22[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(-1,2)" };
+				 "--lcores", "(-1,2)" };
 	const char * const argv23[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(2-4)@(2-4-6)" };
+				 "--lcores", "(2-4)@(2-4-6)" };
 	const char * const argv24[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(a,2)" };
+				 "--lcores", "(a,2)" };
 	const char * const argv25[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "1-3@(1,3)" };
+				 "--lcores", "1-3@(1,3)" };
 	const char * const argv26[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "3@((1,3)" };
+				 "--lcores", "3@((1,3)" };
 	const char * const argv27[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "(4-7)=(1,3)" };
+				 "--lcores", "(4-7)=(1,3)" };
 	const char * const argv28[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores", "[4-7]@(1,3)" };
+				 "--lcores", "[4-7]@(1,3)" };
 	/* sanity check of tests - valid lcores value */
 	const char * const argv29[] = { prgname, prefix, mp_flag,
-				 "-n", "3", "--lcores",
+				 "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
 
 	if (launch_proc(argv2) != 0) {
@@ -606,16 +607,22 @@ enum hugepage_action {
 #endif
 
 	/* --master-lcore flag but no value */
-	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore"};
+	const char *argv1[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore"};
 	/* --master-lcore flag with invalid value */
-	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "-1"};
-	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "X"};
+	const char *argv2[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "-1"};
+	const char *argv3[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "X"};
 	/* master lcore not in coremask */
-	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "2"};
+	const char *argv4[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "2"};
 	/* valid value */
-	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "1"};
+	const char *argv5[] = { prgname, prefix, mp_flag,
+				"-c", "3", "--master-lcore", "1"};
 	/* valid value set before coremask */
-	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "1", "--master-lcore", "1", "-c", "3"};
+	const char *argv6[] = { prgname, prefix, mp_flag,
+				"--master-lcore", "1", "-c", "3"};
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -703,9 +710,9 @@ enum hugepage_action {
 #endif
 
 	/* With --no-hpet */
-	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1", "-n", "2"};
+	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1"};
 	/* Without --no-hpet */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1", "-n", "2"};
+	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1"};
 
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-hpet flag\n");
@@ -734,16 +741,16 @@ enum hugepage_action {
 #endif
 
 	/* With --no-huge */
-	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2"};
+	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1"};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1",
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
-	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1",
 			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
-	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1",
 			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
@@ -844,17 +851,17 @@ enum hugepage_action {
 	/* With invalid --syslog */
 	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
-	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv6[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix, no_huge};
 
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv7[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv8[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
+	const char *argv9[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
@@ -862,23 +869,23 @@ enum hugepage_action {
 
 	/* try running with base-virtaddr param */
 	const char *argv11[] = {prgname, "--file-prefix=virtaddr",
-			"-c", "1", "-n", "2", "--base-virtaddr=0x12345678"};
+			"-c", "1", "--base-virtaddr=0x12345678"};
 
 	/* try running with --vfio-intr INTx flag */
 	const char *argv12[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=legacy"};
+			"-c", "1", "--vfio-intr=legacy"};
 
 	/* try running with --vfio-intr MSI flag */
 	const char *argv13[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=msi"};
+			"-c", "1", "--vfio-intr=msi"};
 
 	/* try running with --vfio-intr MSI-X flag */
 	const char *argv14[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=msix"};
+			"-c", "1", "--vfio-intr=msix"};
 
 	/* try running with --vfio-intr invalid flag */
 	const char *argv15[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "-n", "2", "--vfio-intr=invalid"};
+			"-c", "1", "--vfio-intr=invalid"};
 
 	/* run all tests also applicable to FreeBSD first */
 
@@ -989,25 +996,25 @@ enum hugepage_action {
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
-			"--file-prefix=" memtest };
+	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest };
 
 	/* primary process with memtest1 and default mem mode */
-	const char *argv1[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv1[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
 
 	/* primary process with memtest1 and legacy mem mode */
-	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv2[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and legacy mem mode */
-	const char *argv3[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv3[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and default mem mode */
-	const char *argv4[] = {prgname, "-c", "1", "-n", "2", "-m",
+	const char *argv4[] = {prgname, "-c", "1", "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
 
 	/* check if files for current prefix are present */
@@ -1153,38 +1160,38 @@ enum hugepage_action {
 
 	/* valid -m flag and mp flag */
 	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
-			"-n", "2", "-m", DEFAULT_MEM_SIZE};
+			"-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
-	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv1[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* valid (zero) --socket-mem flag */
-	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv2[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
 
 	/* invalid (incomplete) --socket-mem flag */
-	const char *argv3[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv3[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,"};
 
 	/* invalid (mixed with invalid data) --socket-mem flag */
-	const char *argv4[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv4[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred"};
 
 	/* invalid (with numeric value as last character) --socket-mem flag */
-	const char *argv5[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv5[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred0"};
 
 	/* invalid (with empty socket) --socket-mem flag */
-	const char *argv6[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv6[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem=2,,2"};
 
 	/* invalid (null) --socket-mem flag */
-	const char *argv7[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv7[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "--socket-mem="};
 
 	/* valid --socket-mem specified together with -m flag */
-	const char *argv8[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv8[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
@@ -1236,11 +1243,11 @@ enum hugepage_action {
 	}
 
 	/* invalid --socket-mem flag (with extra socket) */
-	const char *argv9[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv9[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, invalid_socket_mem};
 
 	/* valid --socket-mem flag */
-	const char *argv10[] = {prgname, "-c", "10", "-n", "2",
+	const char *argv10[] = {prgname, "-c", "10",
 			"--file-prefix=" memtest, valid_socket_mem};
 
 	if (launch_proc(argv0) != 0) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 11/15] test/eal: set core mask/list config only in dedicated test

[David Marchand]

Setting a coremask was mandatory a long time ago but has been optional
for a while.
The checks on PCI whitelist/blacklist, vdev, memory rank, memory channel,
HPET, memory size and other miscs options have no requirement wrt cores.

Let's remove those coremasks so that we only care about it in the
dedicated checks.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/test_eal_flags.c | 141 ++++++++++++++++++++++++----------------------
 1 file changed, 73 insertions(+), 68 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index e82e56a..5e11b9f 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -245,25 +245,25 @@ enum hugepage_action {
 #endif
 
 	const char *wlinval[][11] = {
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "error", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:error:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0.1error", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "error0:0:0.1", "", ""},
-		{prgname, prefix, mp_flag, "-c", "1",
+		{prgname, prefix, mp_flag,
 				pci_whitelist, "0:0:0.1.2", "", ""},
 	};
 	/* Test with valid whitelist option */
-	const char *wlval1[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval1[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "00FF:09:0B.3"};
-	const char *wlval2[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval2[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "09:0B.3", pci_whitelist, "0a:0b.1"};
-	const char *wlval3[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *wlval3[] = {prgname, prefix, mp_flag,
 			pci_whitelist, "09:0B.3,type=test",
 			pci_whitelist, "08:00.1,type=normal",
 	};
@@ -311,15 +311,15 @@ enum hugepage_action {
 #endif
 
 	const char *blinval[][9] = {
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "error"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:error:0.1"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1error"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "error0:0:0.1"},
-		{prgname, prefix, mp_flag, "-c", "1", "-b", "0:0:0.1.2"},
+		{prgname, prefix, mp_flag, "-b", "error"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0"},
+		{prgname, prefix, mp_flag, "-b", "0:error:0.1"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0.1error"},
+		{prgname, prefix, mp_flag, "-b", "error0:0:0.1"},
+		{prgname, prefix, mp_flag, "-b", "0:0:0.1.2"},
 	};
 	/* Test with valid blacklist option */
-	const char *blval[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *blval[] = {prgname, prefix, mp_flag,
 			       "-b", "FF:09:0B.3"};
 
 	int i;
@@ -356,17 +356,17 @@ enum hugepage_action {
 
 	/* Test with invalid vdev option */
 	const char *vdevinval[] = {prgname, prefix, no_huge,
-				"-c", "1", vdev, "eth_dummy"};
+				vdev, "eth_dummy"};
 
 	/* Test with valid vdev option */
 	const char *vdevval1[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0"};
+	vdev, "net_ring0"};
 
 	const char *vdevval2[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0,args=test"};
+	vdev, "net_ring0,args=test"};
 
 	const char *vdevval3[] = {prgname, prefix, no_huge,
-	"-c", "1", vdev, "net_ring0,nodeaction=r1:0:CREATE"};
+	vdev, "net_ring0,nodeaction=r1:0:CREATE"};
 
 	if (launch_proc(vdevinval) == 0) {
 		printf("Error - process did run ok with invalid "
@@ -413,13 +413,13 @@ enum hugepage_action {
 #endif
 
 	const char *rinval[][9] = {
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "error"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "0"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "-1"},
-			{prgname, prefix, mp_flag, "-c", "1", "-r", "17"},
+			{prgname, prefix, mp_flag, "-r", "error"},
+			{prgname, prefix, mp_flag, "-r", "0"},
+			{prgname, prefix, mp_flag, "-r", "-1"},
+			{prgname, prefix, mp_flag, "-r", "17"},
 	};
 	/* Test with valid blacklist option */
-	const char *rval[] = {prgname, prefix, mp_flag, "-c", "1", "-r", "16"};
+	const char *rval[] = {prgname, prefix, mp_flag, "-r", "16"};
 
 	int i;
 
@@ -661,15 +661,19 @@ enum hugepage_action {
 #endif
 
 	/* -n flag but no value */
-	const char *argv1[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n"};
+	const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
+				"-n"};
 	/* bad numeric value */
-	const char *argv2[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "e" };
+	const char *argv2[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "e" };
 	/* zero is invalid */
-	const char *argv3[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "0" };
+	const char *argv3[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "0" };
 	/* sanity test - check with good value */
-	const char *argv4[] = { prgname, prefix, no_huge, no_shconf, "-c", "1", "-n", "2" };
+	const char *argv4[] = { prgname, prefix, no_huge, no_shconf,
+				"-n", "2" };
 	/* sanity test - check with no -n flag */
-	const char *argv5[] = { prgname, prefix, no_huge, no_shconf, "-c", "1"};
+	const char *argv5[] = { prgname, prefix, no_huge, no_shconf};
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -710,9 +714,9 @@ enum hugepage_action {
 #endif
 
 	/* With --no-hpet */
-	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet, "-c", "1"};
+	const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
 	/* Without --no-hpet */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1"};
+	const char *argv2[] = {prgname, prefix, mp_flag};
 
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-hpet flag\n");
@@ -741,16 +745,16 @@ enum hugepage_action {
 #endif
 
 	/* With --no-huge */
-	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1"};
+	const char *argv1[] = {prgname, prefix, no_huge};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv2[] = {prgname, prefix, no_huge,
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
-	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv3[] = {prgname, prefix, no_huge,
 			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
-	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1",
+	const char *argv4[] = {prgname, prefix, no_huge,
 			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
@@ -837,55 +841,56 @@ enum hugepage_action {
 	 * No further testing of output done.
 	 */
 	/* sanity check - failure with invalid option */
-	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "1", "--invalid-opt"};
+	const char *argv0[] = {prgname, prefix, mp_flag, "--invalid-opt"};
 
 	/* With --no-pci */
-	const char *argv1[] = {prgname, prefix, mp_flag, "-c", "1", "--no-pci"};
+	const char *argv1[] = {prgname, prefix, mp_flag, "--no-pci"};
 	/* With -v */
-	const char *argv2[] = {prgname, prefix, mp_flag, "-c", "1", "-v"};
+	const char *argv2[] = {prgname, prefix, mp_flag, "-v"};
 	/* With valid --syslog */
-	const char *argv3[] = {prgname, prefix, mp_flag, "-c", "1",
+	const char *argv3[] = {prgname, prefix, mp_flag,
 			"--syslog", "syslog"};
 	/* With empty --syslog (should fail) */
-	const char *argv4[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog"};
+	const char *argv4[] = {prgname, prefix, mp_flag, "--syslog"};
 	/* With invalid --syslog */
-	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
+	const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
-	const char *argv6[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv6[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix, no_huge};
 
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv7[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv8[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-m", DEFAULT_MEM_SIZE,
+	const char *argv9[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
-	const char *argv10[] = {prgname, prefix, mp_flag, "-c", "1", "--huge-dir", "invalid"};
+	const char *argv10[] = {prgname, prefix, mp_flag,
+			"--huge-dir", "invalid"};
 
 	/* try running with base-virtaddr param */
 	const char *argv11[] = {prgname, "--file-prefix=virtaddr",
-			"-c", "1", "--base-virtaddr=0x12345678"};
+			"--base-virtaddr=0x12345678"};
 
 	/* try running with --vfio-intr INTx flag */
 	const char *argv12[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=legacy"};
+			"--vfio-intr=legacy"};
 
 	/* try running with --vfio-intr MSI flag */
 	const char *argv13[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=msi"};
+			"--vfio-intr=msi"};
 
 	/* try running with --vfio-intr MSI-X flag */
 	const char *argv14[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=msix"};
+			"--vfio-intr=msix"};
 
 	/* try running with --vfio-intr invalid flag */
 	const char *argv15[] = {prgname, "--file-prefix=intr",
-			"-c", "1", "--vfio-intr=invalid"};
+			"--vfio-intr=invalid"};
 
 	/* run all tests also applicable to FreeBSD first */
 
@@ -996,25 +1001,25 @@ enum hugepage_action {
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-m",
+	const char *argv0[] = {prgname, mp_flag, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest };
 
 	/* primary process with memtest1 and default mem mode */
-	const char *argv1[] = {prgname, "-c", "1", "-m",
+	const char *argv1[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
 
 	/* primary process with memtest1 and legacy mem mode */
-	const char *argv2[] = {prgname, "-c", "1", "-m",
+	const char *argv2[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and legacy mem mode */
-	const char *argv3[] = {prgname, "-c", "1", "-m",
+	const char *argv3[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2,
 			"--legacy-mem" };
 
 	/* primary process with memtest2 and default mem mode */
-	const char *argv4[] = {prgname, "-c", "1", "-m",
+	const char *argv4[] = {prgname, "-m",
 			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
 
 	/* check if files for current prefix are present */
@@ -1159,39 +1164,39 @@ enum hugepage_action {
 #endif
 
 	/* valid -m flag and mp flag */
-	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
+	const char *argv0[] = {prgname, prefix, mp_flag,
 			"-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
-	const char *argv1[] = {prgname, "-c", "10",
+	const char *argv1[] = {prgname,
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* valid (zero) --socket-mem flag */
-	const char *argv2[] = {prgname, "-c", "10",
+	const char *argv2[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
 
 	/* invalid (incomplete) --socket-mem flag */
-	const char *argv3[] = {prgname, "-c", "10",
+	const char *argv3[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,"};
 
 	/* invalid (mixed with invalid data) --socket-mem flag */
-	const char *argv4[] = {prgname, "-c", "10",
+	const char *argv4[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred"};
 
 	/* invalid (with numeric value as last character) --socket-mem flag */
-	const char *argv5[] = {prgname, "-c", "10",
+	const char *argv5[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,2,Fred0"};
 
 	/* invalid (with empty socket) --socket-mem flag */
-	const char *argv6[] = {prgname, "-c", "10",
+	const char *argv6[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem=2,,2"};
 
 	/* invalid (null) --socket-mem flag */
-	const char *argv7[] = {prgname, "-c", "10",
+	const char *argv7[] = {prgname,
 			"--file-prefix=" memtest, "--socket-mem="};
 
 	/* valid --socket-mem specified together with -m flag */
-	const char *argv8[] = {prgname, "-c", "10",
+	const char *argv8[] = {prgname,
 			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
@@ -1243,11 +1248,11 @@ enum hugepage_action {
 	}
 
 	/* invalid --socket-mem flag (with extra socket) */
-	const char *argv9[] = {prgname, "-c", "10",
+	const char *argv9[] = {prgname,
 			"--file-prefix=" memtest, invalid_socket_mem};
 
 	/* valid --socket-mem flag */
-	const char *argv10[] = {prgname, "-c", "10",
+	const char *argv10[] = {prgname,
 			"--file-prefix=" memtest, valid_socket_mem};
 
 	if (launch_proc(argv0) != 0) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 12/15] test/eal: check number of cores before running subtests

[David Marchand]

From: Michael Santana <msantana@redhat.com>

The eal flags unit test assumes that a certain number of cores are
available (4 and 8 cores), however this may not always be the case.
Individual developers may run the unit test on their local desktop
which typically have 2 to 4 cores, in said case the test is bound
to fail for lacking 4 or 8 cores.

Additionally, as we push forward introducing CI into DPDK we are limited
to the hardware specification of CI services (e.g. Travis CI) that only
have 2 cores on their servers, in which case the test would fail.

To fix this we check available cores before running a subtest. This
applies to subtests that are dedicated to test that the -l and --lcore
flags work correctly. If not enough cores are available the subtest is
simply skipped, otherwise the subtest is run.

Signed-off-by: Michael Santana <msantana@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 app/test/test_eal_flags.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 5e11b9f..cfa8a61 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -19,7 +19,7 @@
 #include <limits.h>
 #include <fcntl.h>
 
-#include <rte_per_lcore.h>
+#include <rte_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -560,7 +560,9 @@ enum hugepage_action {
 		       "process ran without error with invalid -l flag\n");
 		return -1;
 	}
-	if (launch_proc(argv15) != 0) {
+	if (rte_lcore_is_enabled(0) && rte_lcore_is_enabled(1) &&
+	    rte_lcore_is_enabled(2) && rte_lcore_is_enabled(3) &&
+	    launch_proc(argv15) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
@@ -579,7 +581,11 @@ enum hugepage_action {
 		return -1;
 	}
 
-	if (launch_proc(argv29) != 0) {
+	if (rte_lcore_is_enabled(0) && rte_lcore_is_enabled(1) &&
+	    rte_lcore_is_enabled(2) && rte_lcore_is_enabled(3) &&
+	    rte_lcore_is_enabled(3) && rte_lcore_is_enabled(5) &&
+	    rte_lcore_is_enabled(4) && rte_lcore_is_enabled(7) &&
+	    launch_proc(argv29) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
@@ -606,6 +612,9 @@ enum hugepage_action {
 	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
 #endif
 
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1))
+		return TEST_SKIPPED;
+
 	/* --master-lcore flag but no value */
 	const char *argv1[] = { prgname, prefix, mp_flag,
 				"-c", "3", "--master-lcore"};
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 13/15] test: split into shorter subtests for CI

[David Marchand]

Based on Michael initial idea of separating the file-prefix subtest in
the eal flags test.

Let's split the biggest tests into their subparts.
It is then easier to have them fit in the 10s timeout we have configured
in Travis.
We also get a better idea of which part fails in the previously big tests
we had.

Those new subtests are called from the meson testsuite.
The autotest tool is left untouched.

Note: we still have an issue with test_hash_readwrite_lf.c, any help from
the original authors would be appreciated.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- new patch added,

---
 app/test/meson.build      | 18 ++++++++++++++++--
 app/test/test_eal_flags.c | 20 ++++++++++++++++++--
 app/test/test_rwlock.c    |  6 ++++++
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index a9dddf8..f799c8e 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -159,7 +159,18 @@ fast_parallel_test_names = [
         'cpuflags_autotest',
         'cycles_autotest',
         'debug_autotest',
-        'eal_flags_autotest',
+        'eal_flags_c_opt_autotest',
+        'eal_flags_master_opt_autotest',
+        'eal_flags_n_opt_autotest',
+        'eal_flags_hpet_autotest',
+        'eal_flags_no_huge_autotest',
+        'eal_flags_w_opt_autotest',
+        'eal_flags_b_opt_autotest',
+        'eal_flags_vdev_opt_autotest',
+        'eal_flags_r_opt_autotest',
+        'eal_flags_mem_autotest',
+        'eal_flags_file_prefix_autotest',
+        'eal_flags_misc_autotest',
         'eal_fs_autotest',
         'errno_autotest',
         'event_ring_autotest',
@@ -184,7 +195,10 @@ fast_parallel_test_names = [
         'red_autotest',
         'ring_autotest',
         'ring_pmd_autotest',
-        'rwlock_autotest',
+        'rwlock_test1_autotest',
+        'rwlock_rda_autotest',
+        'rwlock_rds_wrm_autotest',
+        'rwlock_rde_wro_autotest',
         'sched_autotest',
         'spinlock_autotest',
         'stack_autotest',
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index cfa8a61..9985ee9 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -342,10 +342,10 @@ enum hugepage_action {
  *  Test that the app doesn't run with invalid vdev option.
  *  Final test ensures it does run with valid options as sanity check
  */
-#ifdef RTE_LIBRTE_PMD_RING
 static int
 test_invalid_vdev_flag(void)
 {
+#ifdef RTE_LIBRTE_PMD_RING
 #ifdef RTE_EXEC_ENV_FREEBSD
 	/* BSD target doesn't support prefixes at this point, and we also need to
 	 * run another primary process here */
@@ -391,8 +391,10 @@ enum hugepage_action {
 		return -1;
 	}
 	return 0;
-}
+#else
+	return TEST_SKIPPED;
 #endif
+}
 
 /*
  * Test that the app doesn't run with invalid -r option.
@@ -1413,3 +1415,17 @@ enum hugepage_action {
 }
 
 REGISTER_TEST_COMMAND(eal_flags_autotest, test_eal_flags);
+
+/* subtests used in meson for CI */
+REGISTER_TEST_COMMAND(eal_flags_c_opt_autotest, test_missing_c_flag);
+REGISTER_TEST_COMMAND(eal_flags_master_opt_autotest, test_master_lcore_flag);
+REGISTER_TEST_COMMAND(eal_flags_n_opt_autotest, test_invalid_n_flag);
+REGISTER_TEST_COMMAND(eal_flags_hpet_autotest, test_no_hpet_flag);
+REGISTER_TEST_COMMAND(eal_flags_no_huge_autotest, test_no_huge_flag);
+REGISTER_TEST_COMMAND(eal_flags_w_opt_autotest, test_whitelist_flag);
+REGISTER_TEST_COMMAND(eal_flags_b_opt_autotest, test_invalid_b_flag);
+REGISTER_TEST_COMMAND(eal_flags_vdev_opt_autotest, test_invalid_vdev_flag);
+REGISTER_TEST_COMMAND(eal_flags_r_opt_autotest, test_invalid_r_flag);
+REGISTER_TEST_COMMAND(eal_flags_mem_autotest, test_memory_flags);
+REGISTER_TEST_COMMAND(eal_flags_file_prefix_autotest, test_file_prefix);
+REGISTER_TEST_COMMAND(eal_flags_misc_autotest, test_misc_flags);
diff --git a/app/test/test_rwlock.c b/app/test/test_rwlock.c
index c3d656a..40f9175 100644
--- a/app/test/test_rwlock.c
+++ b/app/test/test_rwlock.c
@@ -547,3 +547,9 @@ struct try_rwlock_lcore {
 }
 
 REGISTER_TEST_COMMAND(rwlock_autotest, test_rwlock);
+
+/* subtests used in meson for CI */
+REGISTER_TEST_COMMAND(rwlock_test1_autotest, rwlock_test1);
+REGISTER_TEST_COMMAND(rwlock_rda_autotest, try_rwlock_test_rda);
+REGISTER_TEST_COMMAND(rwlock_rds_wrm_autotest, try_rwlock_test_rds_wrm);
+REGISTER_TEST_COMMAND(rwlock_rde_wro_autotest, try_rwlock_test_rde_wro);
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 14/15] test: do not start tests in parallel

[David Marchand]

Running the tests in parallel has two drawbacks:
- the tests are racing on the hugepages allocations,
- the tests are sharing the cores to run their checks which results in
  undeterministic execution time,

This results in random failures.
For better reproducibility in CI, start them all in a serialised way.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- rebased on master

---
 app/test/meson.build | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index f799c8e..f1db02f 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -148,8 +148,7 @@ test_deps = ['acl',
 	'timer'
 ]
 
-# All test cases in fast_parallel_test_names list are parallel
-fast_parallel_test_names = [
+fast_test_names = [
         'acl_autotest',
         'alarm_autotest',
         'atomic_autotest',
@@ -209,10 +208,6 @@ fast_parallel_test_names = [
         'timer_autotest',
         'user_delay_us',
         'version_autotest',
-]
-
-# All test cases in fast_non_parallel_test_names list are non-parallel
-fast_non_parallel_test_names = [
         'bitratestats_autotest',
         'crc_autotest',
         'delay_us_sleep_autotest',
@@ -236,7 +231,6 @@ fast_non_parallel_test_names = [
         'thash_autotest',
 ]
 
-# All test cases in perf_test_names list are non-parallel
 perf_test_names = [
         'ring_perf_autotest',
         'mempool_perf_autotest',
@@ -264,7 +258,6 @@ perf_test_names = [
         'stack_lf_perf_autotest',
 ]
 
-# All test cases in driver_test_names list are non-parallel
 driver_test_names = [
         'cryptodev_aesni_mb_autotest',
         'cryptodev_aesni_gcm_autotest',
@@ -286,7 +279,6 @@ driver_test_names = [
         'link_bonding_rssconf_autotest',
 ]
 
-# All test cases in dump_test_names list are non-parallel
 dump_test_names = [
         'dump_struct_sizes',
         'dump_mempool',
@@ -335,7 +327,7 @@ if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
 		test_dep_objs += compress_test_dep
 		test_sources += 'test_compressdev.c'
 		test_deps += 'compressdev'
-		fast_non_parallel_test_names += 'compressdev_autotest'
+		fast_test_names += 'compressdev_autotest'
 	endif
 endif
 
@@ -383,30 +375,23 @@ endif
 num_cores_arg = '-l ' + num_cores
 
 test_args = [num_cores_arg]
-foreach arg : fast_parallel_test_names
+foreach arg : fast_test_names
 	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,
+		is_parallel : false,
 		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')
+	endif
 endforeach
 
 foreach arg : perf_test_names
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 15/15] test: skip tests when missing requirements

[David Marchand]

Let's mark as skipped the tests when they are missing some requirements
like a number of used cores or specific hardware availability, like
compress, crypto or eventdev devices.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- adapted rcu parts with changes from newly added patch 8

---
 app/test/test.c                     | 24 ++++++++++++++++--------
 app/test/test_compressdev.c         |  4 ++--
 app/test/test_cryptodev.c           |  4 ++--
 app/test/test_distributor.c         |  4 ++--
 app/test/test_distributor_perf.c    |  4 ++--
 app/test/test_event_timer_adapter.c |  5 +++--
 app/test/test_eventdev.c            |  2 ++
 app/test/test_func_reentrancy.c     |  6 +++---
 app/test/test_hash_multiwriter.c    |  7 +++----
 app/test/test_hash_readwrite.c      |  7 +++----
 app/test/test_hash_readwrite_lf.c   |  8 ++++----
 app/test/test_ipsec.c               |  4 ++--
 app/test/test_mbuf.c                | 13 ++++++-------
 app/test/test_rcu_qsbr.c            | 10 +++++-----
 app/test/test_rcu_qsbr_perf.c       | 10 +++++-----
 app/test/test_service_cores.c       | 14 ++++++++++++++
 app/test/test_stack.c               |  8 +++++---
 app/test/test_timer.c               | 10 +++++-----
 app/test/test_timer_secondary.c     | 10 ++++++----
 19 files changed, 90 insertions(+), 64 deletions(-)

diff --git a/app/test/test.c b/app/test/test.c
index ea1e98f..194a92a 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -208,14 +208,16 @@
 		printf(" + Test Suite : %s\n", suite->suite_name);
 	}
 
-	if (suite->setup)
-		if (suite->setup() != 0) {
+	if (suite->setup) {
+		test_success = suite->setup();
+		if (test_success != 0) {
 			/*
-			 * setup failed, so count all enabled tests and mark
-			 * them as failed
+			 * setup did not pass, so count all enabled tests and
+			 * mark them as failed/skipped
 			 */
 			while (suite->unit_test_cases[total].testcase) {
-				if (!suite->unit_test_cases[total].enabled)
+				if (!suite->unit_test_cases[total].enabled ||
+				    test_success == TEST_SKIPPED)
 					skipped++;
 				else
 					failed++;
@@ -223,6 +225,7 @@
 			}
 			goto suite_summary;
 		}
+	}
 
 	printf(" + ------------------------------------------------------- +\n");
 
@@ -246,6 +249,8 @@
 			test_success = suite->unit_test_cases[total].testcase();
 			if (test_success == TEST_SUCCESS)
 				succeeded++;
+			else if (test_success == TEST_SKIPPED)
+				skipped++;
 			else if (test_success == -ENOTSUP)
 				unsupported++;
 			else
@@ -262,6 +267,8 @@
 
 		if (test_success == TEST_SUCCESS)
 			status = "succeeded";
+		else if (test_success == TEST_SKIPPED)
+			status = "skipped";
 		else if (test_success == -ENOTSUP)
 			status = "unsupported";
 		else
@@ -293,7 +300,8 @@
 	last_test_result = failed;
 
 	if (failed)
-		return -1;
-
-	return 0;
+		return TEST_FAILED;
+	if (total == skipped)
+		return TEST_SKIPPED;
+	return TEST_SUCCESS;
 }
diff --git a/app/test/test_compressdev.c b/app/test/test_compressdev.c
index 1b1983e..cf78775 100644
--- a/app/test/test_compressdev.c
+++ b/app/test/test_compressdev.c
@@ -134,8 +134,8 @@ struct test_data_params {
 	unsigned int i;
 
 	if (rte_compressdev_count() == 0) {
-		RTE_LOG(ERR, USER1, "Need at least one compress device\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "Need at least one compress device\n");
+		return TEST_SKIPPED;
 	}
 
 	RTE_LOG(NOTICE, USER1, "Running tests on device %s\n",
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index eca6d3d..0509af7 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -408,8 +408,8 @@ struct crypto_unittest_params {
 
 	nb_devs = rte_cryptodev_count();
 	if (nb_devs < 1) {
-		RTE_LOG(ERR, USER1, "No crypto devices found?\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "No crypto devices found?\n");
+		return TEST_SKIPPED;
 	}
 
 	/* Create list of valid crypto devs */
diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index da3348f..8084c07 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -594,8 +594,8 @@ int test_error_distributor_create_numworkers(void)
 	int i;
 
 	if (rte_lcore_count() < 2) {
-		printf("ERROR: not enough cores to test distributor\n");
-		return -1;
+		printf("Not enough cores for distributor_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	if (db == NULL) {
diff --git a/app/test/test_distributor_perf.c b/app/test/test_distributor_perf.c
index edf1998..f6f1139 100644
--- a/app/test/test_distributor_perf.c
+++ b/app/test/test_distributor_perf.c
@@ -208,8 +208,8 @@ struct worker_stats {
 	static struct rte_mempool *p;
 
 	if (rte_lcore_count() < 2) {
-		printf("ERROR: not enough cores to test distributor\n");
-		return -1;
+		printf("Not enough cores for distributor_perf_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	/* first time how long it takes to round-trip a cache line */
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index 742ca81..ad3f4dc 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -158,8 +158,9 @@
 	}
 
 	if (rte_lcore_count() < required_lcore_count) {
-		printf("%d lcores needed to run tests", required_lcore_count);
-		return TEST_FAILED;
+		printf("Not enough cores for event_timer_adapter_test, expecting at least %u\n",
+		       required_lcore_count);
+		return TEST_SKIPPED;
 	}
 
 	/* Assign lcores for various tasks */
diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
index 00d7327..c745e99 100644
--- a/app/test/test_eventdev.c
+++ b/app/test/test_eventdev.c
@@ -997,6 +997,8 @@
 test_eventdev_selftest_impl(const char *pmd, const char *opts)
 {
 	rte_vdev_init(pmd, opts);
+	if (rte_event_dev_get_dev_id(pmd) == -ENODEV)
+		return TEST_SKIPPED;
 	return rte_event_dev_selftest(rte_event_dev_get_dev_id(pmd));
 }
 
diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index e27d1e0..99ad902 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -473,9 +473,9 @@ struct test_case test_cases[] = {
 	uint32_t case_id;
 	struct test_case *pt_case = NULL;
 
-	if (rte_lcore_count() <= 1) {
-		printf("Not enough lcore for testing\n");
-		return -1;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for func_reentrancy_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 	else if (rte_lcore_count() > MAX_LCORES)
 		printf("Too many lcores, some cores will be disabled\n");
diff --git a/app/test/test_hash_multiwriter.c b/app/test/test_hash_multiwriter.c
index 50018db..46ab45f 100644
--- a/app/test/test_hash_multiwriter.c
+++ b/app/test/test_hash_multiwriter.c
@@ -260,12 +260,11 @@ struct {
 static int
 test_hash_multiwriter_main(void)
 {
-	if (rte_lcore_count() == 1) {
-		printf("More than one lcore is required to do multiwriter test\n");
-		return 0;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for distributor_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
-
 	setlocale(LC_NUMERIC, "");
 
 
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 480ae97..4376b09 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -618,10 +618,9 @@ struct {
 	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
-	if (rte_lcore_count() <= 2) {
-		printf("More than two lcores are required "
-			"to do read write test\n");
-		return -1;
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
 	}
 
 	RTE_LCORE_FOREACH_SLAVE(core_id) {
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 5644361..2664f51 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1254,10 +1254,10 @@ struct {
 	int htm;
 	int use_jhash = 0;
 	int ext_bkt = 0;
-	if (rte_lcore_count() == 1) {
-		printf("More than one lcore is required "
-			"to do read write lock-free concurrency test\n");
-		return -1;
+
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for hash_readwrite_lf_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
 	setlocale(LC_NUMERIC, "");
diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
index 3993ff4..83f7895 100644
--- a/app/test/test_ipsec.c
+++ b/app/test/test_ipsec.c
@@ -296,8 +296,8 @@ struct supported_auth_algo {
 
 	nb_devs = rte_cryptodev_count();
 	if (nb_devs < 1) {
-		RTE_LOG(ERR, USER1, "No crypto devices found?\n");
-		return TEST_FAILED;
+		RTE_LOG(WARNING, USER1, "No crypto devices found?\n");
+		return TEST_SKIPPED;
 	}
 
 	/* Find first valid crypto device */
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 030385e..2a97afe 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -753,18 +753,17 @@
 test_refcnt_mbuf(void)
 {
 #ifdef RTE_MBUF_REFCNT_ATOMIC
-	unsigned lnum, master, slave, tref;
+	unsigned int master, slave, tref;
 	int ret = -1;
 	struct rte_mempool *refcnt_pool = NULL;
 	struct rte_ring *refcnt_mbuf_ring = NULL;
 
-	if ((lnum = rte_lcore_count()) == 1) {
-		printf("skipping %s, number of lcores: %u is not enough\n",
-		    __func__, lnum);
-		return 0;
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for test_refcnt_mbuf, expecting at least 2\n");
+		return TEST_SKIPPED;
 	}
 
-	printf("starting %s, at %u lcores\n", __func__, lnum);
+	printf("starting %s, at %u lcores\n", __func__, rte_lcore_count());
 
 	/* create refcnt pool & ring if they don't exist */
 
@@ -1206,7 +1205,7 @@
 		goto err;
 	}
 
-	if (test_refcnt_mbuf()<0){
+	if (test_refcnt_mbuf() < 0) {
 		printf("test_refcnt_mbuf() failed \n");
 		goto err;
 	}
diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c
index 97af087..943a1e3 100644
--- a/app/test/test_rcu_qsbr.c
+++ b/app/test/test_rcu_qsbr.c
@@ -978,17 +978,17 @@ struct test_rcu_thread_info {
 {
 	uint16_t core_id;
 
+	if (rte_lcore_count() < 5) {
+		printf("Not enough cores for rcu_qsbr_autotest, expecting at least 5\n");
+		return TEST_SKIPPED;
+	}
+
 	num_cores = 0;
 	RTE_LCORE_FOREACH_SLAVE(core_id) {
 		enabled_core_ids[num_cores] = core_id;
 		num_cores++;
 	}
 
-	if (num_cores < 4) {
-		printf("Test failed! Need 4 or more cores\n");
-		goto test_fail;
-	}
-
 	/* Error-checking test cases */
 	if (test_rcu_qsbr_get_memsize() < 0)
 		goto test_fail;
diff --git a/app/test/test_rcu_qsbr_perf.c b/app/test/test_rcu_qsbr_perf.c
index c918a94..363365f 100644
--- a/app/test/test_rcu_qsbr_perf.c
+++ b/app/test/test_rcu_qsbr_perf.c
@@ -606,6 +606,11 @@ static struct rte_hash *init_hash(void)
 {
 	uint16_t core_id;
 
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for rcu_qsbr_perf_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
 	rte_atomic64_init(&updates);
 	rte_atomic64_init(&update_cycles);
 	rte_atomic64_init(&checks);
@@ -618,11 +623,6 @@ static struct rte_hash *init_hash(void)
 	}
 
 	printf("Number of cores provided = %d\n", num_cores);
-	if (num_cores < 2) {
-		printf("Test failed! Need 2 or more cores\n");
-		goto test_fail;
-	}
-
 	printf("Perf test with all reader threads registered\n");
 	printf("--------------------------------------------\n");
 	all_registered = 1;
diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index 82bb2ce..9fe38f5 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -502,6 +502,10 @@ static int32_t dummy_mt_safe_cb(void *args)
 static int
 service_lcore_add_del(void)
 {
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2) || !rte_lcore_is_enabled(3))
+		return TEST_SKIPPED;
+
 	/* check initial count */
 	TEST_ASSERT_EQUAL(0, rte_service_lcore_count(),
 			"Service lcore count has value before adding a lcore");
@@ -669,6 +673,11 @@ static int32_t dummy_mt_safe_cb(void *args)
 service_mt_safe_poll(void)
 {
 	int mt_safe = 1;
+
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2))
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_EQUAL(1, service_threaded_test(mt_safe),
 			"Error: MT Safe service not run by two cores concurrently");
 	return TEST_SUCCESS;
@@ -681,6 +690,11 @@ static int32_t dummy_mt_safe_cb(void *args)
 service_mt_unsafe_poll(void)
 {
 	int mt_safe = 0;
+
+	if (!rte_lcore_is_enabled(0) || !rte_lcore_is_enabled(1) ||
+	    !rte_lcore_is_enabled(2))
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_EQUAL(1, service_threaded_test(mt_safe),
 			"Error: NON MT Safe service run by two cores concurrently");
 	return TEST_SUCCESS;
diff --git a/app/test/test_stack.c b/app/test/test_stack.c
index e972a61..c8dac1f 100644
--- a/app/test/test_stack.c
+++ b/app/test/test_stack.c
@@ -336,12 +336,14 @@ struct test_args {
 	struct rte_stack *s;
 	rte_atomic64_t size;
 
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for test_stack_multithreaded, expecting at least 2\n");
+		return TEST_SKIPPED;
+	}
+
 	printf("[%s():%u] Running with %u lcores\n",
 	       __func__, __LINE__, rte_lcore_count());
 
-	if (rte_lcore_count() < 2)
-		return 0;
-
 	args = rte_malloc(NULL, sizeof(struct test_args) * RTE_MAX_LCORE, 0);
 	if (args == NULL) {
 		printf("[%s():%u] failed to malloc %zu bytes\n",
diff --git a/app/test/test_timer.c b/app/test/test_timer.c
index e2aab53..8e0a589 100644
--- a/app/test/test_timer.c
+++ b/app/test/test_timer.c
@@ -538,17 +538,17 @@ struct mytimerinfo {
 	uint64_t cur_time;
 	uint64_t hz;
 
+	if (rte_lcore_count() < 2) {
+		printf("Not enough cores for timer_autotest, expecting at least 2\n");
+		return TEST_SKIPPED;
+	}
+
 	/* sanity check our timer sources and timer config values */
 	if (timer_sanity_check() < 0) {
 		printf("Timer sanity checks failed\n");
 		return TEST_FAILED;
 	}
 
-	if (rte_lcore_count() < 2) {
-		printf("not enough lcores for this test\n");
-		return TEST_FAILED;
-	}
-
 	/* init timer */
 	for (i=0; i<NB_TIMER; i++) {
 		memset(&mytiminfo[i], 0, sizeof(struct mytimerinfo));
diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c
index 31e4a7f..790f180 100644
--- a/app/test/test_timer_secondary.c
+++ b/app/test/test_timer_secondary.c
@@ -118,16 +118,18 @@ struct test_info {
 	int ret;
 
 	if (proc_type == RTE_PROC_PRIMARY) {
+		if (rte_lcore_count() < NUM_LCORES_NEEDED) {
+			printf("Not enough cores for test_timer_secondary, expecting at least %u\n",
+			       NUM_LCORES_NEEDED);
+			return TEST_SKIPPED;
+		}
+
 		mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info),
 					 SOCKET_ID_ANY, 0);
 		test_info = mz->addr;
 		TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for "
 				     "test data");
 
-		TEST_ASSERT(rte_lcore_count() >= NUM_LCORES_NEEDED,
-			    "at least %d lcores needed to run tests",
-			    NUM_LCORES_NEEDED);
-
 		test_info->tim_mempool = rte_mempool_create("test_timer_mp",
 				NUM_TIMERS, sizeof(struct rte_timer), 0, 0,
 				NULL, NULL, NULL, NULL, rte_socket_id(), 0);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Bruce Richardson]

On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand wrote:
> This is a joint effort to make the unit tests ready for CI.
> The first patches are fixes that I had accumulated.
> Then the second part of the series focuses on skipping tests when some
> requirements are not fulfilled so that we can start them in a restrained
> environment like Travis virtual machines that gives us two cores and does
> not have specific hw devices.
> 
> We are still not ready for enabling those tests in Travis.
> At least, the following issues remain:
> - some fixes on librte_acl have not been merged yet [1],
> - the tests on --file-prefix are still ko, and have been isolated in a
>   test that we could disable while waiting for the fixes,
> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>   than 10s,
> - librte_table unit test crashes on ipv6 [2],
> - the "perf" tests are taking way too long for my taste,
> - the shared build unit tests all fail when depending on mempool since
>   the mempool drivers are not loaded,
> 

For the autotest app shared builds, it is probably worthwhile linking in
all drivers explicitly to avoid issues like this.

/Bruce

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

On Mon, Jun 17, 2019 at 12:02 PM Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand wrote:
> > This is a joint effort to make the unit tests ready for CI.
> > The first patches are fixes that I had accumulated.
> > Then the second part of the series focuses on skipping tests when some
> > requirements are not fulfilled so that we can start them in a restrained
> > environment like Travis virtual machines that gives us two cores and does
> > not have specific hw devices.
> >
> > We are still not ready for enabling those tests in Travis.
> > At least, the following issues remain:
> > - some fixes on librte_acl have not been merged yet [1],
> > - the tests on --file-prefix are still ko, and have been isolated in a
> >   test that we could disable while waiting for the fixes,
> > - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
> >   than 10s,
> > - librte_table unit test crashes on ipv6 [2],
> > - the "perf" tests are taking way too long for my taste,
> > - the shared build unit tests all fail when depending on mempool since
> >   the mempool drivers are not loaded,
> >
>
> For the autotest app shared builds, it is probably worthwhile linking in
> all drivers explicitly to avoid issues like this.
>

Yes, I'll look into this.

While at it, do you know why the i40e and ixgbe drivers are linked to
app/test in meson?


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Bruce Richardson]

On Mon, Jun 17, 2019 at 12:46:03PM +0200, David Marchand wrote:
>    On Mon, Jun 17, 2019 at 12:02 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand wrote:
>      > This is a joint effort to make the unit tests ready for CI.
>      > The first patches are fixes that I had accumulated.
>      > Then the second part of the series focuses on skipping tests when
>      some
>      > requirements are not fulfilled so that we can start them in a
>      restrained
>      > environment like Travis virtual machines that gives us two cores
>      and does
>      > not have specific hw devices.
>      >
>      > We are still not ready for enabling those tests in Travis.
>      > At least, the following issues remain:
>      > - some fixes on librte_acl have not been merged yet [1],
>      > - the tests on --file-prefix are still ko, and have been isolated
>      in a
>      >   test that we could disable while waiting for the fixes,
>      > - rwlock_autotest and hash_readwrite_lf_autotest are taking a
>      little more
>      >   than 10s,
>      > - librte_table unit test crashes on ipv6 [2],
>      > - the "perf" tests are taking way too long for my taste,
>      > - the shared build unit tests all fail when depending on mempool
>      since
>      >   the mempool drivers are not loaded,
>      >
>      For the autotest app shared builds, it is probably worthwhile
>      linking in
>      all drivers explicitly to avoid issues like this.
> 
>    Yes, I'll look into this.
>    While at it, do you know why the i40e and ixgbe drivers are linked to
>    app/test in meson?
>    --

There are unit tests for the device-specific functions in those drivers, so
they need to be given at link time.

/Bruce

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

On Mon, Jun 17, 2019 at 1:18 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Mon, Jun 17, 2019 at 12:46:03PM +0200, David Marchand wrote:
> >    On Mon, Jun 17, 2019 at 12:02 PM Bruce Richardson
> >    <[1]bruce.richardson@intel.com> wrote:
> >
> >      On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand wrote:
> >      > This is a joint effort to make the unit tests ready for CI.
> >      > The first patches are fixes that I had accumulated.
> >      > Then the second part of the series focuses on skipping tests when
> >      some
> >      > requirements are not fulfilled so that we can start them in a
> >      restrained
> >      > environment like Travis virtual machines that gives us two cores
> >      and does
> >      > not have specific hw devices.
> >      >
> >      > We are still not ready for enabling those tests in Travis.
> >      > At least, the following issues remain:
> >      > - some fixes on librte_acl have not been merged yet [1],
> >      > - the tests on --file-prefix are still ko, and have been isolated
> >      in a
> >      >   test that we could disable while waiting for the fixes,
> >      > - rwlock_autotest and hash_readwrite_lf_autotest are taking a
> >      little more
> >      >   than 10s,
> >      > - librte_table unit test crashes on ipv6 [2],
> >      > - the "perf" tests are taking way too long for my taste,
> >      > - the shared build unit tests all fail when depending on mempool
> >      since
> >      >   the mempool drivers are not loaded,
> >      >
> >      For the autotest app shared builds, it is probably worthwhile
> >      linking in
> >      all drivers explicitly to avoid issues like this.
> >
> >    Yes, I'll look into this.
> >    While at it, do you know why the i40e and ixgbe drivers are linked to
> >    app/test in meson?
> >    --
>
> There are unit tests for the device-specific functions in those drivers, so
> they need to be given at link time.
>

For testpmd, I can understand.

But I can't see code for driver specific apis in app/test.
It looks like a copy/paste error when adding meson support.



-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Bruce Richardson]

On Mon, Jun 17, 2019 at 01:41:21PM +0200, David Marchand wrote:
>    On Mon, Jun 17, 2019 at 1:18 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Mon, Jun 17, 2019 at 12:46:03PM +0200, David Marchand wrote:
>      >    On Mon, Jun 17, 2019 at 12:02 PM Bruce Richardson
>      >    <[1][2]bruce.richardson@intel.com> wrote:
>      >
>      >      On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand
>      wrote:
>      >      > This is a joint effort to make the unit tests ready for CI.
>      >      > The first patches are fixes that I had accumulated.
>      >      > Then the second part of the series focuses on skipping
>      tests when
>      >      some
>      >      > requirements are not fulfilled so that we can start them in
>      a
>      >      restrained
>      >      > environment like Travis virtual machines that gives us two
>      cores
>      >      and does
>      >      > not have specific hw devices.
>      >      >
>      >      > We are still not ready for enabling those tests in Travis.
>      >      > At least, the following issues remain:
>      >      > - some fixes on librte_acl have not been merged yet [1],
>      >      > - the tests on --file-prefix are still ko, and have been
>      isolated
>      >      in a
>      >      >   test that we could disable while waiting for the fixes,
>      >      > - rwlock_autotest and hash_readwrite_lf_autotest are taking
>      a
>      >      little more
>      >      >   than 10s,
>      >      > - librte_table unit test crashes on ipv6 [2],
>      >      > - the "perf" tests are taking way too long for my taste,
>      >      > - the shared build unit tests all fail when depending on
>      mempool
>      >      since
>      >      >   the mempool drivers are not loaded,
>      >      >
>      >      For the autotest app shared builds, it is probably worthwhile
>      >      linking in
>      >      all drivers explicitly to avoid issues like this.
>      >
>      >    Yes, I'll look into this.
>      >    While at it, do you know why the i40e and ixgbe drivers are
>      linked to
>      >    app/test in meson?
>      >    --
>      There are unit tests for the device-specific functions in those
>      drivers, so
>      they need to be given at link time.
> 
>    For testpmd, I can understand.
>    But I can't see code for driver specific apis in app/test.
>    It looks like a copy/paste error when adding meson support.
>    --
Ok, could be. Simple question is does it still build ok if you remove them?

/Bruce

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

On Mon, Jun 17, 2019 at 1:57 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Mon, Jun 17, 2019 at 01:41:21PM +0200, David Marchand wrote:
> >    On Mon, Jun 17, 2019 at 1:18 PM Bruce Richardson
> >    <[1]bruce.richardson@intel.com> wrote:
> >
> >      On Mon, Jun 17, 2019 at 12:46:03PM +0200, David Marchand wrote:
> >      >    On Mon, Jun 17, 2019 at 12:02 PM Bruce Richardson
> >      >    <[1][2]bruce.richardson@intel.com> wrote:
> >      >
> >      >      On Sat, Jun 15, 2019 at 08:42:15AM +0200, David Marchand
> >      wrote:
> >      >      > This is a joint effort to make the unit tests ready for CI.
> >      >      > The first patches are fixes that I had accumulated.
> >      >      > Then the second part of the series focuses on skipping
> >      tests when
> >      >      some
> >      >      > requirements are not fulfilled so that we can start them in
> >      a
> >      >      restrained
> >      >      > environment like Travis virtual machines that gives us two
> >      cores
> >      >      and does
> >      >      > not have specific hw devices.
> >      >      >
> >      >      > We are still not ready for enabling those tests in Travis.
> >      >      > At least, the following issues remain:
> >      >      > - some fixes on librte_acl have not been merged yet [1],
> >      >      > - the tests on --file-prefix are still ko, and have been
> >      isolated
> >      >      in a
> >      >      >   test that we could disable while waiting for the fixes,
> >      >      > - rwlock_autotest and hash_readwrite_lf_autotest are taking
> >      a
> >      >      little more
> >      >      >   than 10s,
> >      >      > - librte_table unit test crashes on ipv6 [2],
> >      >      > - the "perf" tests are taking way too long for my taste,
> >      >      > - the shared build unit tests all fail when depending on
> >      mempool
> >      >      since
> >      >      >   the mempool drivers are not loaded,
> >      >      >
> >      >      For the autotest app shared builds, it is probably worthwhile
> >      >      linking in
> >      >      all drivers explicitly to avoid issues like this.
> >      >
> >      >    Yes, I'll look into this.
> >      >    While at it, do you know why the i40e and ixgbe drivers are
> >      linked to
> >      >    app/test in meson?
> >      >    --
> >      There are unit tests for the device-specific functions in those
> >      drivers, so
> >      they need to be given at link time.
> >
> >    For testpmd, I can understand.
> >    But I can't see code for driver specific apis in app/test.
> >    It looks like a copy/paste error when adding meson support.
> >    --
> Ok, could be. Simple question is does it still build ok if you remove them?
>

It would have been strange if it did not build, since on Makefile side we
do nothing.
Yes, it builds fine with meson without this.

I managed to get the same test results than with static builds by linking
the skeleton eventdev driver and the mempool sw drivers.
Should be enough.



-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Thomas Monjalon]

15/06/2019 08:42, David Marchand:
> This is a joint effort to make the unit tests ready for CI.

Applied, thanks

Remaining work below from your list:
[...]
> - the tests on --file-prefix are still ko, and have been isolated in a
>   test that we could disable while waiting for the fixes,
> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>   than 10s,
> - librte_table unit test crashes on ipv6 [2],
> - the "perf" tests are taking way too long for my taste,
> - the shared build unit tests all fail when depending on mempool since
>   the mempool drivers are not loaded,
> 
> 2: https://bugs.dpdk.org/show_bug.cgi?id=285

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Aaron Conole]

Thomas Monjalon <thomas@monjalon.net> writes:

> 15/06/2019 08:42, David Marchand:
>> This is a joint effort to make the unit tests ready for CI.
>
> Applied, thanks
>
> Remaining work below from your list:
> [...]
>> - the tests on --file-prefix are still ko, and have been isolated in a
>>   test that we could disable while waiting for the fixes,

Yes, I think it's good to do that for now.

>> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>>   than 10s,

Occasionally the distributor test times out as well.  I've moved them as
part of a separate patch, that I'll post along with a bigger series to
enable the unit tests under travis.  Michael and I are leaning toward
introducing a new variable called RUN_TESTS which will do the docs and
unit testing since those combined would add quite a bit to the execution
time of each job (and feel free to bike shed the name, since the patches
aren't final).

>> - librte_table unit test crashes on ipv6 [2],

I guess we're waiting on a patch from Jananee (CC'd)?

>> - the "perf" tests are taking way too long for my taste,

Agreed, so I think we'll disable the perf tests in the CI environment
anyway.

>> - the shared build unit tests all fail when depending on mempool since
>>   the mempool drivers are not loaded,

I think Michael is working on a fix for this right now.

>> 2: https://bugs.dpdk.org/show_bug.cgi?id=285

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Thomas Monjalon]

01/07/2019 18:04, Aaron Conole:
> Michael and I are leaning toward
> introducing a new variable called RUN_TESTS which will do the docs and
> unit testing since those combined would add quite a bit to the execution
> time of each job (and feel free to bike shed the name, since the patches
> aren't final).

Please would you like to send a RFC so we can discuss it before
you do the final patches?

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

On Mon, Jul 1, 2019 at 6:04 PM Aaron Conole <aconole@redhat.com> wrote:

> >> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little
> more
> >>   than 10s,
>
> Occasionally the distributor test times out as well.  I've moved them as
> part of a separate patch, that I'll post along with a bigger series to
> enable the unit tests under travis.  Michael and I are leaning toward
> introducing a new variable called RUN_TESTS which will do the docs and
> unit testing since those combined would add quite a bit to the execution
> time of each job (and feel free to bike shed the name, since the patches
> aren't final).
>

Seeing how the distributor autotest usually takes less than a second to
complete, this sounds like a bug.
I don't think I caught this so far.


Yes, we need a variable to control this and select the targets that will do
the tests and/or build the doc.
About the name, RUN_TESTS is ok for me.

What do you want to make of this variable?
Have it as a simple boolean that enables everything? Or a selector with
strings like unit-tests+doc+perf-tests?



> >> - librte_table unit test crashes on ipv6 [2],
>
> I guess we're waiting on a patch from Jananee (CC'd)?
>

Yep.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Michael Santana Francisco]

>
>
>
> On Mon, Jul 1, 2019 at 6:04 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>> >> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>> >>   than 10s,
>>
>> Occasionally the distributor test times out as well.  I've moved them as
>> part of a separate patch, that I'll post along with a bigger series to
>> enable the unit tests under travis.  Michael and I are leaning toward
>> introducing a new variable called RUN_TESTS which will do the docs and
>> unit testing since those combined would add quite a bit to the execution
>> time of each job (and feel free to bike shed the name, since the patches
>> aren't final).
>
>
> Seeing how the distributor autotest usually takes less than a second to complete, this sounds like a bug.
> I don't think I caught this so far.
So I actually ran into the distributor test timing out. I agree with
David in that it is a bug with the test. Looking at the logs that test
normally finishes in less than 1/2 a second, so running to 10 seconds
and timing out is a big jump in run time. I ran into the issue where
it timedout, so I restarted the job and it finished no problem.
The test fails every so often for no good reason and the logs[1] dont
really say much. I speculate that it is waiting for a resource to
become available or in the worse case a deadlock. Seeing that it only
fails every so often and it passes when restarted I don't think it's a
big deal, nevertheless it's worth investing time figuring out what's
wrong

[1] https://api.travis-ci.com/v3/job/212335916/log.txt
>
>
> Yes, we need a variable to control this and select the targets that will do the tests and/or build the doc.
> About the name, RUN_TESTS is ok for me.
>
> What do you want to make of this variable?
> Have it as a simple boolean that enables everything? Or a selector with strings like unit-tests+doc+perf-tests?
>
>
>>
>> >> - librte_table unit test crashes on ipv6 [2],
>>
>> I guess we're waiting on a patch from Jananee (CC'd)?
>
>
> Yep.
>
>
> --
> David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[Michael Santana Francisco]

On 7/1/19 2:07 PM, Michael Santana Francisco wrote:
>>
>>
>> On Mon, Jul 1, 2019 at 6:04 PM Aaron Conole <aconole@redhat.com> wrote:
>>>>> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little more
>>>>>    than 10s,
>>> Occasionally the distributor test times out as well.  I've moved them as
>>> part of a separate patch, that I'll post along with a bigger series to
>>> enable the unit tests under travis.  Michael and I are leaning toward
>>> introducing a new variable called RUN_TESTS which will do the docs and
>>> unit testing since those combined would add quite a bit to the execution
>>> time of each job (and feel free to bike shed the name, since the patches
>>> aren't final).
>>
>> Seeing how the distributor autotest usually takes less than a second to complete, this sounds like a bug.
>> I don't think I caught this so far.
> So I actually ran into the distributor test timing out. I agree with
> David in that it is a bug with the test. Looking at the logs that test
> normally finishes in less than 1/2 a second, so running to 10 seconds
> and timing out is a big jump in run time. I ran into the issue where
> it timedout, so I restarted the job and it finished no problem.
> The test fails every so often for no good reason and the logs[1] dont
> really say much. I speculate that it is waiting for a resource to
> become available or in the worse case a deadlock. Seeing that it only
> fails every so often and it passes when restarted I don't think it's a
> big deal, nevertheless it's worth investing time figuring out what's
> wrong
>
> [1] https://api.travis-ci.com/v3/job/212335916/log.txt

I investigated a little bit on this this test. CC'd David Hunt,

I was able to reproduce the problem on v19.08-rc1 with:

`while sudo sh -c "echo 'distributor_autotest' | 
./build/app/test/dpdk-test"; do :; done`

It runs a couple of times fine showing output and showing progress, but 
then at some point after a couple of seconds it just stops - no longer 
getting any output. It just sits there with no further output. I let it 
sit there for a whole minute and nothing happens. So I attach gdb to try 
to figure out what is happening. One thread seems to be stuck on a while 
loop, see lib/librte_distributor/rte_distributor.c:310.

I looked at the assembly code (layout asm, ni) and I saw these four 
lines below (which correspond to the while loop) being executed 
repeatedly and indefinitely. It looks like this thread is waiting for 
the variable bufptr64[0] to change state.

0xa064d0 <release+32>   pause
0xa064d2 <release+34>   mov    0x3840(%rdx),%rax
0xa064d9 <release+41>   test   $0x1,%al
0xa064db <release+43>   je     0xa064d0 <release+32>


While the first thread is waiting on bufptr64[0] to change state, there 
is another thread that is also stuck on another while loop on 
lib/librte_distributor/rte_distributor.c:53. It seems that this thread 
is stuck waiting for retptr64 to change state. Corresponding assembly 
being executed indefinitely:

0xa06de0 <rte_distributor_request_pkt_v1705+592> mov    0x38c0(%r8),%rax
0xa06de7 <rte_distributor_request_pkt_v1705+599> test   $0x1,%al
0xa06de9 <rte_distributor_request_pkt_v1705+601> je     0xa06bbd 
<rte_distributor_request_pkt_v1705+45>
0xa06def <rte_distributor_request_pkt_v1705+607>        nop
0xa06df0 <rte_distributor_request_pkt_v1705+608> pause
0xa06df2 <rte_distributor_request_pkt_v1705+610> rdtsc
0xa06df4 <rte_distributor_request_pkt_v1705+612> mov    %rdx,%r10
0xa06df7 <rte_distributor_request_pkt_v1705+615> shl    $0x20,%r10
0xa06dfb <rte_distributor_request_pkt_v1705+619> mov    %eax,%eax
0xa06dfd <rte_distributor_request_pkt_v1705+621> or     %r10,%rax
0xa06e00 <rte_distributor_request_pkt_v1705+624> lea    0x64(%rax),%r10
0xa06e04 <rte_distributor_request_pkt_v1705+628> jmp    0xa06e12 
<rte_distributor_request_pkt_v1705+642>
0xa06e06 <rte_distributor_request_pkt_v1705+630> nopw   %cs:0x0(%rax,%rax,1)
0xa06e10 <rte_distributor_request_pkt_v1705+640> pause
0xa06e12 <rte_distributor_request_pkt_v1705+642> rdtsc
0xa06e14 <rte_distributor_request_pkt_v1705+644> shl    $0x20,%rdx
0xa06e18 <rte_distributor_request_pkt_v1705+648> mov    %eax,%eax
0xa06e1a <rte_distributor_request_pkt_v1705+650> or     %rdx,%rax
0xa06e1d <rte_distributor_request_pkt_v1705+653> cmp    %rax,%r10
0xa06e20 <rte_distributor_request_pkt_v1705+656> ja     0xa06e10 
<rte_distributor_request_pkt_v1705+640>
0xa06e22 <rte_distributor_request_pkt_v1705+658> jmp    0xa06de0 
<rte_distributor_request_pkt_v1705+592>


My guess is that these threads are interdependent, so one thread is 
waiting for the other thread to change the state of the control 
variable. I can't say for sure if this is what is happening or why the 
these variables don't change state, so I would like ask someone who is 
more familiar with this particular code to take a look

>>
>> Yes, we need a variable to control this and select the targets that will do the tests and/or build the doc.
>> About the name, RUN_TESTS is ok for me.
>>
>> What do you want to make of this variable?
>> Have it as a simple boolean that enables everything? Or a selector with strings like unit-tests+doc+perf-tests?
>>
>>
>>>>> - librte_table unit test crashes on ipv6 [2],
>>> I guess we're waiting on a patch from Jananee (CC'd)?
>>
>> Yep.
>>
>>
>> --
>> David Marchand

Re: [dpdk-dev] [PATCH v2 00/15] Unit tests fixes for CI

[David Marchand]

On Tue, Jul 9, 2019 at 5:50 PM Michael Santana Francisco <
msantana@redhat.com> wrote:

> On 7/1/19 2:07 PM, Michael Santana Francisco wrote:
> >>
> >>
> >> On Mon, Jul 1, 2019 at 6:04 PM Aaron Conole <aconole@redhat.com> wrote:
> >>>>> - rwlock_autotest and hash_readwrite_lf_autotest are taking a little
> more
> >>>>>    than 10s,
> >>> Occasionally the distributor test times out as well.  I've moved them
> as
> >>> part of a separate patch, that I'll post along with a bigger series to
> >>> enable the unit tests under travis.  Michael and I are leaning toward
> >>> introducing a new variable called RUN_TESTS which will do the docs and
> >>> unit testing since those combined would add quite a bit to the
> execution
> >>> time of each job (and feel free to bike shed the name, since the
> patches
> >>> aren't final).
> >>
> >> Seeing how the distributor autotest usually takes less than a second to
> complete, this sounds like a bug.
> >> I don't think I caught this so far.
> > So I actually ran into the distributor test timing out. I agree with
> > David in that it is a bug with the test. Looking at the logs that test
> > normally finishes in less than 1/2 a second, so running to 10 seconds
> > and timing out is a big jump in run time. I ran into the issue where
> > it timedout, so I restarted the job and it finished no problem.
> > The test fails every so often for no good reason and the logs[1] dont
> > really say much. I speculate that it is waiting for a resource to
> > become available or in the worse case a deadlock. Seeing that it only
> > fails every so often and it passes when restarted I don't think it's a
> > big deal, nevertheless it's worth investing time figuring out what's
> > wrong
> >
> > [1] https://api.travis-ci.com/v3/job/212335916/log.txt
>
> I investigated a little bit on this this test. CC'd David Hunt,
>
> I was able to reproduce the problem on v19.08-rc1 with:
>
> `while sudo sh -c "echo 'distributor_autotest' |
> ./build/app/test/dpdk-test"; do :; done`
>
> It runs a couple of times fine showing output and showing progress, but
> then at some point after a couple of seconds it just stops - no longer
> getting any output. It just sits there with no further output. I let it
> sit there for a whole minute and nothing happens. So I attach gdb to try
> to figure out what is happening. One thread seems to be stuck on a while
> loop, see lib/librte_distributor/rte_distributor.c:310.
>
> I looked at the assembly code (layout asm, ni) and I saw these four
> lines below (which correspond to the while loop) being executed
> repeatedly and indefinitely. It looks like this thread is waiting for
> the variable bufptr64[0] to change state.
>
> 0xa064d0 <release+32>   pause
> 0xa064d2 <release+34>   mov    0x3840(%rdx),%rax
> 0xa064d9 <release+41>   test   $0x1,%al
> 0xa064db <release+43>   je     0xa064d0 <release+32>
>
>
> While the first thread is waiting on bufptr64[0] to change state, there
> is another thread that is also stuck on another while loop on
> lib/librte_distributor/rte_distributor.c:53. It seems that this thread
> is stuck waiting for retptr64 to change state. Corresponding assembly
> being executed indefinitely:
>
> 0xa06de0 <rte_distributor_request_pkt_v1705+592> mov    0x38c0(%r8),%rax
> 0xa06de7 <rte_distributor_request_pkt_v1705+599> test   $0x1,%al
> 0xa06de9 <rte_distributor_request_pkt_v1705+601> je     0xa06bbd
> <rte_distributor_request_pkt_v1705+45>
> 0xa06def <rte_distributor_request_pkt_v1705+607>        nop
> 0xa06df0 <rte_distributor_request_pkt_v1705+608> pause
> 0xa06df2 <rte_distributor_request_pkt_v1705+610> rdtsc
> 0xa06df4 <rte_distributor_request_pkt_v1705+612> mov    %rdx,%r10
> 0xa06df7 <rte_distributor_request_pkt_v1705+615> shl    $0x20,%r10
> 0xa06dfb <rte_distributor_request_pkt_v1705+619> mov    %eax,%eax
> 0xa06dfd <rte_distributor_request_pkt_v1705+621> or     %r10,%rax
> 0xa06e00 <rte_distributor_request_pkt_v1705+624> lea    0x64(%rax),%r10
> 0xa06e04 <rte_distributor_request_pkt_v1705+628> jmp    0xa06e12
> <rte_distributor_request_pkt_v1705+642>
> 0xa06e06 <rte_distributor_request_pkt_v1705+630> nopw
> %cs:0x0(%rax,%rax,1)
> 0xa06e10 <rte_distributor_request_pkt_v1705+640> pause
> 0xa06e12 <rte_distributor_request_pkt_v1705+642> rdtsc
> 0xa06e14 <rte_distributor_request_pkt_v1705+644> shl    $0x20,%rdx
> 0xa06e18 <rte_distributor_request_pkt_v1705+648> mov    %eax,%eax
> 0xa06e1a <rte_distributor_request_pkt_v1705+650> or     %rdx,%rax
> 0xa06e1d <rte_distributor_request_pkt_v1705+653> cmp    %rax,%r10
> 0xa06e20 <rte_distributor_request_pkt_v1705+656> ja     0xa06e10
> <rte_distributor_request_pkt_v1705+640>
> 0xa06e22 <rte_distributor_request_pkt_v1705+658> jmp    0xa06de0
> <rte_distributor_request_pkt_v1705+592>
>
>
> My guess is that these threads are interdependent, so one thread is
> waiting for the other thread to change the state of the control
> variable. I can't say for sure if this is what is happening or why the
> these variables don't change state, so I would like ask someone who is
> more familiar with this particular code to take a look
>

Ah cool, thanks for the analysis.
Can you create a bz with this description and assign it to the
librte_distributor maintainer?


-- 
David Marchand