From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9FDA0A04E7; Mon, 2 Nov 2020 18:17:59 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 17CEC2BFE; Mon, 2 Nov 2020 18:17:58 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 17D171E2B for ; Mon, 2 Nov 2020 18:17:55 +0100 (CET) IronPort-SDR: neVF0ZeLtatwBhZik1vYXA175vN6AT/l1+Py555kvUjAk4/Clx4Av2gbAs7iFH40VVWUoOD4J6 1TF/J5yG6Oug== X-IronPort-AV: E=McAfee;i="6000,8403,9793"; a="186755905" X-IronPort-AV: E=Sophos;i="5.77,445,1596524400"; d="scan'208";a="186755905" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Nov 2020 09:17:53 -0800 IronPort-SDR: LfId+R5QVGCzDk0jgnxFK7htdofOty6YQ+umrvo6iXgjJRo0PnKwPFAKl0KkGeENGQN8HUzPQU 0Lm+pxOBaL6w== X-IronPort-AV: E=Sophos;i="5.77,445,1596524400"; d="scan'208";a="470450089" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.252.22.151]) ([10.252.22.151]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Nov 2020 09:17:52 -0800 To: Dharmik Thakkar , Bruce Richardson Cc: dev@dpdk.org, nd@arm.com References: <20201029153634.10647-1-dharmik.thakkar@arm.com> <20201029153634.10647-4-dharmik.thakkar@arm.com> From: "Medvedkin, Vladimir" Message-ID: <60004746-5cdb-9f03-5392-ce43793280b6@intel.com> Date: Mon, 2 Nov 2020 17:17:50 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.3.3 MIME-Version: 1.0 In-Reply-To: <20201029153634.10647-4-dharmik.thakkar@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 4/4] test/lpm: avoid code duplication in rcu qsbr perf 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" Hi Dharmik, Thanks for the patches, see comments inlined On 29/10/2020 15:36, Dharmik Thakkar wrote: > Avoid code duplication by combining single and multi threaded tests > > Signed-off-by: Dharmik Thakkar > Reviewed-by: Ruifeng Wang > --- > app/test/test_lpm_perf.c | 338 +++++++++------------------------------ > 1 file changed, 73 insertions(+), 265 deletions(-) > > diff --git a/app/test/test_lpm_perf.c b/app/test/test_lpm_perf.c > index 4f15db4f85ee..08312023b661 100644 > --- a/app/test/test_lpm_perf.c > +++ b/app/test/test_lpm_perf.c > @@ -430,11 +430,16 @@ test_lpm_rcu_qsbr_writer(void *arg) > { > unsigned int i, j, si, ei; > uint64_t begin, total_cycles; > - uint8_t core_id = (uint8_t)((uintptr_t)arg); > + uint8_t writer_id = (uint8_t)((uintptr_t)arg); > uint32_t next_hop_add = 0xAA; > > - /* 2 writer threads are used */ > - if (core_id % 2 == 0) { > + /* Single writer (writer_id = 1) */ > + if (writer_id == 1) { Probably it would be better to use enum here instead of 1/2/3? > + si = 0; > + ei = NUM_LDEPTH_ROUTE_ENTRIES; > + } > + /* 2 Writers (writer_id = 2/3)*/ > + else if (writer_id == 2) { > si = 0; > ei = NUM_LDEPTH_ROUTE_ENTRIES / 2; > } else { > @@ -482,16 +487,17 @@ test_lpm_rcu_qsbr_writer(void *arg) > > /* > * Functional test: > - * 2 writers, rest are readers > + * 1/2 writers, rest are readers > */ > static int > -test_lpm_rcu_perf_multi_writer(void) > +test_lpm_rcu_perf_multi_writer(uint8_t use_rcu) > { > struct rte_lpm_config config; > size_t sz; > - unsigned int i; > + unsigned int i, j; > uint16_t core_id; > struct rte_lpm_rcu_config rcu_cfg = {0}; > + int (*reader_f)(void *arg) = NULL; > > if (rte_lcore_count() < 3) { > printf("Not enough cores for lpm_rcu_perf_autotest, expecting at least 3\n"); > @@ -504,273 +510,76 @@ test_lpm_rcu_perf_multi_writer(void) > num_cores++; > } > > - printf("\nPerf test: 2 writers, %d readers, RCU integration enabled\n", > - num_cores - 2); > - > - /* Create LPM table */ > - config.max_rules = NUM_LDEPTH_ROUTE_ENTRIES; > - config.number_tbl8s = NUM_LDEPTH_ROUTE_ENTRIES; > - config.flags = 0; > - lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config); > - TEST_LPM_ASSERT(lpm != NULL); > - > - /* Init RCU variable */ > - sz = rte_rcu_qsbr_get_memsize(num_cores); > - rv = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz, > - RTE_CACHE_LINE_SIZE); > - rte_rcu_qsbr_init(rv, num_cores); > - > - rcu_cfg.v = rv; > - /* Assign the RCU variable to LPM */ > - if (rte_lpm_rcu_qsbr_add(lpm, &rcu_cfg) != 0) { > - printf("RCU variable assignment failed\n"); > - goto error; > - } > - > - writer_done = 0; > - __atomic_store_n(&gwrite_cycles, 0, __ATOMIC_RELAXED); > - > - __atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST); > - > - /* Launch reader threads */ > - for (i = 2; i < num_cores; i++) > - rte_eal_remote_launch(test_lpm_rcu_qsbr_reader, NULL, > - enabled_core_ids[i]); > - > - /* Launch writer threads */ > - for (i = 0; i < 2; i++) > - rte_eal_remote_launch(test_lpm_rcu_qsbr_writer, > - (void *)(uintptr_t)i, > - enabled_core_ids[i]); > - > - /* Wait for writer threads */ > - for (i = 0; i < 2; i++) > - if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0) > - goto error; > - > - printf("Total LPM Adds: %d\n", TOTAL_WRITES); > - printf("Total LPM Deletes: %d\n", TOTAL_WRITES); > - printf("Average LPM Add/Del: %"PRIu64" cycles\n", > - __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) / TOTAL_WRITES > - ); > - > - writer_done = 1; > - /* Wait until all readers have exited */ > - for (i = 2; i < num_cores; i++) > - rte_eal_wait_lcore(enabled_core_ids[i]); > - > - rte_lpm_free(lpm); > - rte_free(rv); > - lpm = NULL; > - rv = NULL; > - > - /* Test without RCU integration */ > - printf("\nPerf test: 2 writers, %d readers, RCU integration disabled\n", > - num_cores - 2); > - > - /* Create LPM table */ > - config.max_rules = NUM_LDEPTH_ROUTE_ENTRIES; > - config.number_tbl8s = NUM_LDEPTH_ROUTE_ENTRIES; > - config.flags = 0; > - lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config); > - TEST_LPM_ASSERT(lpm != NULL); > - > - writer_done = 0; > - __atomic_store_n(&gwrite_cycles, 0, __ATOMIC_RELAXED); > - __atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST); > - > - /* Launch reader threads */ > - for (i = 2; i < num_cores; i++) > - rte_eal_remote_launch(test_lpm_reader, NULL, > - enabled_core_ids[i]); > - > - /* Launch writer threads */ > - for (i = 0; i < 2; i++) > - rte_eal_remote_launch(test_lpm_rcu_qsbr_writer, > - (void *)(uintptr_t)i, > - enabled_core_ids[i]); > - > - /* Wait for writer threads */ > - for (i = 0; i < 2; i++) > - if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0) > - goto error; > - > - printf("Total LPM Adds: %d\n", TOTAL_WRITES); > - printf("Total LPM Deletes: %d\n", TOTAL_WRITES); > - printf("Average LPM Add/Del: %"PRIu64" cycles\n", > - __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) > - / TOTAL_WRITES); > - > - writer_done = 1; > - /* Wait until all readers have exited */ > - for (i = 2; i < num_cores; i++) > - rte_eal_wait_lcore(enabled_core_ids[i]); > - > - rte_lpm_free(lpm); > - > - return 0; > - > -error: > - writer_done = 1; > - /* Wait until all readers have exited */ > - rte_eal_mp_wait_lcore(); > - > - rte_lpm_free(lpm); > - rte_free(rv); > - > - return -1; > -} > - > -/* > - * Functional test: > - * Single writer, rest are readers > - */ > -static int > -test_lpm_rcu_perf(void) > -{ > - struct rte_lpm_config config; > - uint64_t begin, total_cycles; > - size_t sz; > - unsigned int i, j; > - uint16_t core_id; > - uint32_t next_hop_add = 0xAA; > - struct rte_lpm_rcu_config rcu_cfg = {0}; > - > - if (rte_lcore_count() < 2) { > - printf("Not enough cores for lpm_rcu_perf_autotest, expecting at least 2\n"); > - return TEST_SKIPPED; > - } > - > - num_cores = 0; > - RTE_LCORE_FOREACH_WORKER(core_id) { > - enabled_core_ids[num_cores] = core_id; > - num_cores++; > - } > - > - printf("\nPerf test: 1 writer, %d readers, RCU integration enabled\n", > - num_cores); > - > - /* Create LPM table */ > - config.max_rules = NUM_LDEPTH_ROUTE_ENTRIES; > - config.number_tbl8s = NUM_LDEPTH_ROUTE_ENTRIES; > - config.flags = 0; > - lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config); > - TEST_LPM_ASSERT(lpm != NULL); > - > - /* Init RCU variable */ > - sz = rte_rcu_qsbr_get_memsize(num_cores); > - rv = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz, > - RTE_CACHE_LINE_SIZE); > - rte_rcu_qsbr_init(rv, num_cores); > - > - rcu_cfg.v = rv; > - /* Assign the RCU variable to LPM */ > - if (rte_lpm_rcu_qsbr_add(lpm, &rcu_cfg) != 0) { > - printf("RCU variable assignment failed\n"); > - goto error; > - } > - > - writer_done = 0; > - __atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST); > - > - /* Launch reader threads */ > - for (i = 0; i < num_cores; i++) > - rte_eal_remote_launch(test_lpm_rcu_qsbr_reader, NULL, > - enabled_core_ids[i]); > - > - /* Measure add/delete. */ > - begin = rte_rdtsc_precise(); > - for (i = 0; i < RCU_ITERATIONS; i++) { > - /* Add all the entries */ > - for (j = 0; j < NUM_LDEPTH_ROUTE_ENTRIES; j++) > - if (rte_lpm_add(lpm, large_ldepth_route_table[j].ip, > - large_ldepth_route_table[j].depth, > - next_hop_add) != 0) { > - printf("Failed to add iteration %d, route# %d\n", > - i, j); > + for (j = 1; j < 3; j++) { > + if (use_rcu) > + printf("\nPerf test: %d writer(s), %d reader(s)," > + " RCU integration enabled\n", j, num_cores - j); > + else > + printf("\nPerf test: %d writer(s), %d reader(s)," > + " RCU integration disabled\n", j, num_cores - j); > + > + /* Create LPM table */ > + config.max_rules = NUM_LDEPTH_ROUTE_ENTRIES; > + config.number_tbl8s = NUM_LDEPTH_ROUTE_ENTRIES; > + config.flags = 0; > + lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config); > + TEST_LPM_ASSERT(lpm != NULL); > + > + /* Init RCU variable */ > + if (use_rcu) { > + sz = rte_rcu_qsbr_get_memsize(num_cores); > + rv = (struct rte_rcu_qsbr *)rte_zmalloc("rcu0", sz, > + RTE_CACHE_LINE_SIZE); > + rte_rcu_qsbr_init(rv, num_cores); > + > + rcu_cfg.v = rv; > + /* Assign the RCU variable to LPM */ > + if (rte_lpm_rcu_qsbr_add(lpm, &rcu_cfg) != 0) { > + printf("RCU variable assignment failed\n"); > goto error; > } > > - /* Delete all the entries */ > - for (j = 0; j < NUM_LDEPTH_ROUTE_ENTRIES; j++) > - if (rte_lpm_delete(lpm, large_ldepth_route_table[j].ip, > - large_ldepth_route_table[j].depth) != 0) { > - printf("Failed to delete iteration %d, route# %d\n", > - i, j); > - goto error; > - } > - } > - total_cycles = rte_rdtsc_precise() - begin; > + reader_f = test_lpm_rcu_qsbr_reader; > + } else > + reader_f = test_lpm_reader; > > - printf("Total LPM Adds: %d\n", TOTAL_WRITES); > - printf("Total LPM Deletes: %d\n", TOTAL_WRITES); > - printf("Average LPM Add/Del: %g cycles\n", > - (double)total_cycles / TOTAL_WRITES); > + writer_done = 0; > + __atomic_store_n(&gwrite_cycles, 0, __ATOMIC_RELAXED); > > - writer_done = 1; > - /* Wait until all readers have exited */ > - for (i = 0; i < num_cores; i++) > - if (rte_eal_wait_lcore(enabled_core_ids[i]); > + __atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST); > > - rte_lpm_free(lpm); > - rte_free(rv); > - lpm = NULL; > - rv = NULL; > + /* Launch reader threads */ > + for (i = j; i < num_cores; i++) > + rte_eal_remote_launch(reader_f, NULL, > + enabled_core_ids[i]); > > - /* Test without RCU integration */ > - printf("\nPerf test: 1 writer, %d readers, RCU integration disabled\n", > - num_cores); > + /* Launch writer threads */ > + for (i = 0; i < j; i++) > + rte_eal_remote_launch(test_lpm_rcu_qsbr_writer, So now even single writer will acquire a lock for every _add/_delete operation. I don't think it is necessary. > + (void *)(uintptr_t)(i + j), > + enabled_core_ids[i]); > > - /* Create LPM table */ > - config.max_rules = NUM_LDEPTH_ROUTE_ENTRIES; > - config.number_tbl8s = NUM_LDEPTH_ROUTE_ENTRIES; > - config.flags = 0; > - lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config); > - TEST_LPM_ASSERT(lpm != NULL); > - > - writer_done = 0; > - __atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST); > - > - /* Launch reader threads */ > - for (i = 0; i < num_cores; i++) > - rte_eal_remote_launch(test_lpm_reader, NULL, > - enabled_core_ids[i]); > - > - /* Measure add/delete. */ > - begin = rte_rdtsc_precise(); > - for (i = 0; i < RCU_ITERATIONS; i++) { > - /* Add all the entries */ > - for (j = 0; j < NUM_LDEPTH_ROUTE_ENTRIES; j++) > - if (rte_lpm_add(lpm, large_ldepth_route_table[j].ip, > - large_ldepth_route_table[j].depth, > - next_hop_add) != 0) { > - printf("Failed to add iteration %d, route# %d\n", > - i, j); > + /* Wait for writer threads */ > + for (i = 0; i < j; i++) > + if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0) > goto error; > - } > > - /* Delete all the entries */ > - for (j = 0; j < NUM_LDEPTH_ROUTE_ENTRIES; j++) > - if (rte_lpm_delete(lpm, large_ldepth_route_table[j].ip, > - large_ldepth_route_table[j].depth) != 0) { > - printf("Failed to delete iteration %d, route# %d\n", > - i, j); > - goto error; > - } > + printf("Total LPM Adds: %d\n", TOTAL_WRITES); > + printf("Total LPM Deletes: %d\n", TOTAL_WRITES); > + printf("Average LPM Add/Del: %"PRIu64" cycles\n", > + __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) > + / TOTAL_WRITES); > + > + writer_done = 1; > + /* Wait until all readers have exited */ > + for (i = j; i < num_cores; i++) > + rte_eal_wait_lcore(enabled_core_ids[i]); > + > + rte_lpm_free(lpm); > + rte_free(rv); > + lpm = NULL; > + rv = NULL; > } > - total_cycles = rte_rdtsc_precise() - begin; > - > - printf("Total LPM Adds: %d\n", TOTAL_WRITES); > - printf("Total LPM Deletes: %d\n", TOTAL_WRITES); > - printf("Average LPM Add/Del: %g cycles\n", > - (double)total_cycles / TOTAL_WRITES); > - > - writer_done = 1; > - /* Wait until all readers have exited */ > - for (i = 0; i < num_cores; i++) > - rte_eal_wait_lcore(enabled_core_ids[i]); > - > - rte_lpm_free(lpm); > > return 0; > > @@ -946,9 +755,8 @@ test_lpm_perf(void) > rte_lpm_delete_all(lpm); > rte_lpm_free(lpm); > > - test_lpm_rcu_perf(); > - > - test_lpm_rcu_perf_multi_writer(); > + test_lpm_rcu_perf_multi_writer(0); > + test_lpm_rcu_perf_multi_writer(1); > > return 0; > } > -- Regards, Vladimir