From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 4A05FA0096 for ; Sat, 8 Jun 2019 10:01:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 665821BCB6; Sat, 8 Jun 2019 10:01:33 +0200 (CEST) Received: from mail-vs1-f65.google.com (mail-vs1-f65.google.com [209.85.217.65]) by dpdk.org (Postfix) with ESMTP id E3B441BCB3 for ; Sat, 8 Jun 2019 10:01:31 +0200 (CEST) Received: by mail-vs1-f65.google.com with SMTP id d128so2530955vsc.10 for ; Sat, 08 Jun 2019 01:01:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uF9lTBho4xagWUAodz2+O5army35JtZzjY6DTQFBqHY=; b=qxL6xjdmmMLfFeoiGh35LLJMNAa7XbYGjGeyRsFM4/Jh+BKQ40pEmj8lILRoVKDS42 zBytRl2o+p1QravopRRHWOmZF7MWwY8zTRfoXcukIGrwYHBiFJT0YJ5g6zv1epsYi3UR tvz44V4y533XqKw2UZ8rQs3MMVO1P4YVdxMT7X75+hgvIcRAntyPmrQXh4Pdgg/iD00X vwI91tJjFTxto3N8v0svaiSE6TmkHmzjBGitXfZ4QzzlzRq1P0kSi1kbZRTSUjr/Fv/P KHjCXZTNulvkPT6kvVteSRsrHa3OYPlnw55Av+ZZ70IRVlteNPYBqHxPFI1SHB+yYQ63 Yv1g== X-Gm-Message-State: APjAAAWeFuFmHMUCSFJEmN4r8aVlH2H4ZxWBSlT6IaVQSSuOmG4e4cRr rIKBnm9M4Rgc3HKoZcp+L4WcCrNdkxaOWlqqLOqtbQ== X-Google-Smtp-Source: APXvYqzjiCZ8mVLDzyRDuttI+jv23FxRWZPIgEDWWwRxFihFFFSrkkj599TBBmPA9/jLZwYswNwKkJ5Lf0Ei6XhCsTg= X-Received: by 2002:a67:f998:: with SMTP id b24mr18825998vsq.180.1559980891100; Sat, 08 Jun 2019 01:01:31 -0700 (PDT) MIME-Version: 1.0 References: <1559638792-8608-1-git-send-email-david.marchand@redhat.com> <1559638792-8608-15-git-send-email-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Sat, 8 Jun 2019 10:01:20 +0200 Message-ID: To: Honnappa Nagarahalli Cc: "dev@dpdk.org" , "thomas@monjalon.net" , "aconole@redhat.com" , "msantana@redhat.com" , Yipeng Wang , nd Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 14/14] test: skip tests when missing requirements X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" (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 > > --- > > 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(-) > > > > > > > > > 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