Re: [dpdk-dev] [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test

[Dharmik Thakkar <Dharmik.Thakkar@arm.com>]

Thank you Ferruh and Yipeng! I will make the recommended changes and update the version.

> On Jan 12, 2019, at 12:07 AM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
> 
> Thanks for the bug fix! Nice catch!
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, January 11, 2019 9:10 AM
>> To: Dharmik Thakkar <dharmik.thakkar@arm.com>; Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; stable@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
>>> 	uint32_t duplicated_keys = 0;
>>> @@ -536,6 +535,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
>>> 
>>> 		rte_eal_mp_wait_lcore();
>>> 
>>> +		uint32_t iter = 0;
>> 
>> Logically looks good. Only we don't tend to declare the variables in the middle
>> of the scope, you may prefer to keep deceleration at its place but set 'iter' to
>> zero here.
> [Wang, Yipeng] Agree.
>> 
>>> +		memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
> 
> [Wang, Yipeng] test failed because of this line I think, the found is uint32_t array, so should be TOTAL_ENTRY * 4, or just change found to be int8
> 
>>> 		while (rte_hash_iterate(tbl_rw_test_param.h,
>>> 				&next_key, &next_data, &iter) >= 0) {
>>> 			/* Search for the key in the list of keys added .*/
>>> @@ -619,7 +620,7 @@ test_hash_readwrite_main(void)
>>> 	if (rte_lcore_count() <= 2) {
>>> 		printf("More than two lcores are required "
>>> 			"to do read write test\n");
>>> -		return 0;
>>> +		return -1;
>> 
>> This is something not mentioned in the commit log, changes the default return
>> value of test when not enough resources provided, cc'ed Yipeng for comment.
>> 
>> If decided to keep this change, please update commit log to mention from it.
> [Wang, Yipeng] Yes it should be -1. Thanks for the fix. Please fix the commit msg as Ferruh suggested