(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 :-)
(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
@@ -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;
}
@@ -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",
@@ -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 */
@@ -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) {
@@ -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 */
@@ -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 */
@@ -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));
}
@@ -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");
@@ -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, "");
@@ -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) {
@@ -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, "");
@@ -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 */
@@ -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;
}
@@ -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;
@@ -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;
@@ -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;
@@ -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",
@@ -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));
@@ -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);