From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id 295E1AABC for ; Tue, 8 May 2018 16:34:43 +0200 (CEST) Date: Tue, 08 May 2018 22:34:49 +0800 User-Agent: K-9 Mail for Android In-Reply-To: <20180508141718.18706-1-jasvinder.singh@intel.com> References: <20180508141718.18706-1-jasvinder.singh@intel.com> To: dev@dpdk.org,Jasvinder Singh CC: cristian.dumitrescu@intel.com From: Andy Green Message-ID: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] table: add dedicated params struct for cuckoo hash 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: , X-List-Received-Date: Tue, 08 May 2018 14:34:43 -0000 On May 8, 2018 10:17:18 PM GMT+08:00, Jasvinder Singh wrote: >Add dedicated parameter structure for cuckoo hash=2E 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=2E1 warning [1], misleading [2] As I wrote earlier this is broken on master currently=2E=2E=2E gcc 8=2E0= =2E1, shipping on Fedora 28, is able to appreciate the existing cast is com= pletely wrong and build errors out=2E It's not a compiler version quirk so= much as what is in master is actually broken=2E > 2/ Union within the parameter structure: pollutes a very generic API > parameter structure with some implementation dependent detail > (i=2Ee=2E 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=2E Unifying them in a single function pointer type is obviously best since th= ey're trying to do the same thing=2E > 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) =2E=2E=2E > { > if (params =3D=3D NULL) { > RTE_LOG(ERR, TABLE, "NULL Input Parameters=2E\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 =3D params; >+ struct rte_table_hash_cuckoo_params *p =3D params; I think a proper solution will have to get rid of the void * params=2E=2E= =2E >- =2Ehash_func =3D (rte_hash_function)(p->f_hash), >+ =2Ehash_func =3D p->f_hash, -Andy