From: Andy Green [mailto:a...@warmcat.com] Sent: Tuesday, May 8, 2018 3:35 PM To: dev@dpdk.org; Singh, Jasvinder <jasvinder.si...@intel.com> Cc: Dumitrescu, Cristian <cristian.dumitre...@intel.com> Subject: Re: [dpdk-dev] [PATCH] table: add dedicated params struct for cuckoo hash
On May 8, 2018 10:17:18 PM GMT+08:00, Jasvinder Singh <jasvinder.si...@intel.com<mailto:jasvinder.si...@intel.com>> wrote: >Add dedicated parameter structure for cuckoo hash. The cuckoo hash from >librte_hash uses slightly different prototype for the hash function (no >key_mask parameter, 32-bit seed and return value) that require either >of the following approaches: > 1/ Function pointer conversion: gcc 8.1 warning [1], misleading [2] As I wrote earlier this is broken on master currently... gcc 8.0.1, shipping on Fedora 28, is able to appreciate the existing cast is completely wrong and build errors out. It's not a compiler version quirk so much as what is in master is actually broken. [Cristian] Right, this is why we fixed it with this patch. > 2/ Union within the parameter structure: pollutes a very generic API > parameter structure with some implementation dependent detail > (i.e. key mask not available for one of the available > implementations) > 3/ Using opaque pointer for hash function: same issue from 2/ > 4/ Different parameter structure: avoid issue from 2/; hopefully, > it won't be long before librte_hash implements the key mask feature, > so the generic API structure could be used. Unifying them in a single function pointer type is obviously best since they're trying to do the same thing. [Cristian] It is not a bad solution, but we decided to go for a dedicated params structure for cuckoo hash for the above reasons. It is functionally equivalent and it removes the root cause of the problem (i.e. no function pointer conversion required). > static int >-check_params_create_hash_cuckoo(struct rte_table_hash_params *params) >+check_params_create_hash_cuckoo(struct rte_table_hash_cuckoo_params >*params) ... > { > if (params == NULL) { > RTE_LOG(ERR, TABLE, "NULL Input Parameters.\n"); >@@ -82,7 +81,7 @@ rte_table_hash_cuckoo_create(void *params, > int socket_id, > uint32_t entry_size) > { >- struct rte_table_hash_params *p = params; >+ struct rte_table_hash_cuckoo_params *p = params; I think a proper solution will have to get rid of the void * params... [Cristian] You should probably go and spend some time understand how the rte_table.h API works. >- .hash_func = (rte_hash_function)(p->f_hash), >+ .hash_func = p->f_hash, -Andy
