From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 93B2E9AE8 for ; Tue, 24 Feb 2015 15:57:39 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 24 Feb 2015 06:57:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,639,1418112000"; d="scan'208";a="532092070" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.32]) by orsmga003.jf.intel.com with SMTP; 24 Feb 2015 06:48:32 -0800 Received: by (sSMTP sendmail emulation); Tue, 24 Feb 2015 14:57:34 +0025 Date: Tue, 24 Feb 2015 14:57:34 +0000 From: Bruce Richardson To: Yerden Zhumabekov Message-ID: <20150224145734.GA9192@bricha3-MOBL3> References: <1424781388-7485-1-git-send-email-e_zhumabekov@sts.kz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424781388-7485-1-git-send-email-e_zhumabekov@sts.kz> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] app/test: add crc32 algorithms equivalence check X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Feb 2015 14:57:40 -0000 On Tue, Feb 24, 2015 at 06:36:28PM +0600, Yerden Zhumabekov wrote: > New function test_crc32_hash_alg_equiv() checks whether software, > 4-byte operand and 8-byte operand versions of CRC32 hash function > implementations return the same result value. > > Signed-off-by: Yerden Zhumabekov Looks like what we want for this. In 2.1 we should perhaps look at splitting off unit tests for the hash algorithms from the tests for the hash tables themselves, since they are all in one file. However, I don't think such rework is feasible in 2.0 timeframe. Some comments inline. /Bruce > --- > app/test/test_hash.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/app/test/test_hash.c b/app/test/test_hash.c > index 76b1b8f..941dc69 100644 > --- a/app/test/test_hash.c > +++ b/app/test/test_hash.c > @@ -177,6 +177,56 @@ static struct rte_hash_parameters ut_params = { > .socket_id = 0, > }; > > +#define CRC32_ITERATIONS (1U << 16) This test takes almost no time at all, so maybe we want to do a few more iterations e.g. 2^18 - 2^20. > +#define CRC32_DWORDS (1U << 6) > +/* > + * Test if all CRC32 implementations yield the same hash value > + */ > +static int > +test_crc32_hash_alg_equiv(void) > +{ > + uint32_t hash_val; > + uint32_t init_val; > + uint64_t data64[CRC32_DWORDS]; > + unsigned i, j; > + size_t data_len; > + int res = 0; > + > + printf("# CRC32 implementations equivalence test\n"); > + for (i = 0; i < CRC32_ITERATIONS; i++) { > + /* Randomizing data_len of data set */ > + data_len = (size_t) (rte_rand() % sizeof(data64) + 1); I suggest parenthesis around the % operation for clarity. > + init_val = (uint32_t) rte_rand(); > + > + /* Fill the data set */ > + for (j = 0; j < CRC32_DWORDS; j++) { > + data64[j] = rte_rand(); > + } As a matter of style, we generally omit braces for single-statement loop bodies. > + > + /* Calculate software CRC32 */ > + rte_hash_crc_set_alg(CRC32_SW); > + hash_val = rte_hash_crc(data64, data_len, init_val); > + > + /* Check against 4-byte-operand sse4.2 CRC32 if available */ > + rte_hash_crc_set_alg(CRC32_SSE42); > + if (hash_val != rte_hash_crc(data64, data_len, init_val)) { > + res = -1; I think you need a print statement here, stating that the test failed, and why exactly it failed. Also, rather than setting res to -1, you can just do a print and break, and change "return res" below to "return i == CRC32_ITERATIONS ? 0 : -1", making use of the fact that you can check i to detect early termination on error. > + break; > + } > + > + /* Check against 8-byte-operand sse4.2 CRC32 if available */ > + rte_hash_crc_set_alg(CRC32_SSE42_x64); > + if (hash_val != rte_hash_crc(data64, data_len, init_val)) { > + res = -1; > + break; > + } > + } > + > + /* Resetting to best available algorithm */ > + rte_hash_crc_set_alg(CRC32_SSE42_x64); > + return res; > +} > + > /* > * Test a hash function. > */ > @@ -1356,6 +1406,9 @@ test_hash(void) > > run_hash_func_tests(); > > + if (test_crc32_hash_alg_equiv() < 0) > + return -1; > + > return 0; > } > > -- > 1.7.9.5 >