From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 43CBC558E for ; Fri, 25 Mar 2016 16:08:40 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 25 Mar 2016 08:08:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,391,1455004800"; d="scan'208";a="675553038" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by FMSMGA003.fm.intel.com with ESMTP; 25 Mar 2016 08:08:27 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.13]) by IRSMSX153.ger.corp.intel.com ([169.254.9.60]) with mapi id 14.03.0248.002; Fri, 25 Mar 2016 15:08:25 +0000 From: "De Lara Guarch, Pablo" To: "Richardson, Bruce" CC: "dev@dpdk.org" , "edreddy@gmail.com" Thread-Topic: [PATCH] hash: fix to support multi process Thread-Index: AQHRhdShdAst8RCUNk+T+5gDsnNXqp9qPQ0AgAABYaA= Date: Fri, 25 Mar 2016 15:08:24 +0000 Message-ID: References: <1458827804-25496-1-git-send-email-pablo.de.lara.guarch@intel.com> <20160325144104.GB7916@bricha3-MOBL3> In-Reply-To: <20160325144104.GB7916@bricha3-MOBL3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTQxNzI3YjEtZjgzYi00MmM4LWE5N2ItMzUwNjYxODFlYTYwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ikd0Y1NPQWJyUE5MZVNZeWIrcHZTamdJUXpOTWs2VnU4MlRBdDZJQ1NyN289In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] hash: fix to support multi process 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: Fri, 25 Mar 2016 15:08:40 -0000 Hi, > -----Original Message----- > From: Richardson, Bruce > Sent: Friday, March 25, 2016 2:41 PM > To: De Lara Guarch, Pablo > Cc: dev@dpdk.org; edreddy@gmail.com > Subject: Re: [PATCH] hash: fix to support multi process >=20 > On Thu, Mar 24, 2016 at 01:56:44PM +0000, Pablo de Lara wrote: > > Hash library used a function pointer to choose a different > > key compare function, depending on the key size. > > As a result, multiple processes could not use the same hash table, > > as the function addresses vary from one process to another. > > > > Instead, a jump table is used, so each process has its own > > function addresses, accessing this table with an index stored > > in the hash table (note that using a custom key compare function > > is not supported in multi-process mode). > > > > Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation") > > > > Signed-off-by: Pablo de Lara >=20 > This is a hard problem to solve, but this probably looks the least-worst > solution > to it. Pablo, I assume the performance hit for lookups is pretty small he= re? Yes, performance degradation is negligible. >=20 > One small nit in the code below, and a comment on the doc change. Will send a v2 with the changes. Anyway, I was waiting for Djama and Zhang to give some feedback here and see if it works for them. Thanks, Pablo > Otherwise, >=20 > Acked-by: Bruce Richardson >=20 > > --- > > doc/guides/rel_notes/release_16_04.rst | 5 ++ > > lib/librte_hash/rte_cuckoo_hash.c | 85 ++++++++++++++++++++++++++= - > ------- > > 2 files changed, 71 insertions(+), 19 deletions(-) > > > > diff --git a/doc/guides/rel_notes/release_16_04.rst > b/doc/guides/rel_notes/release_16_04.rst > > index 2785b29..c4359dd 100644 > > --- a/doc/guides/rel_notes/release_16_04.rst > > +++ b/doc/guides/rel_notes/release_16_04.rst > > @@ -351,6 +351,11 @@ Libraries > > Fix crc32c hash functions to return a valid crc32c value for data le= ngths > > not multiple of 4 bytes. > > > > +* **hash: Fixed hash library to support multi-process mode.** > > + > > + Fix hash library to support multi-process mode, using a jump table, > > + instead of storing a function pointer to the key compare function. > > + > You probably need to clarify here and in the other docs (e.g. API doc) th= at: > * multi-process mode only works with the built-in functions and key sizes= . > * a custom function not in the jump table can be used but only in single- > process > mode. >=20 > > * **librte_port: Fixed segmentation fault for ring and ethdev writer > nodrop.** > > > > Fixed core dump issue on txq and swq when dropless is set to yes. > > diff --git a/lib/librte_hash/rte_cuckoo_hash.c > b/lib/librte_hash/rte_cuckoo_hash.c > > index 71b5b76..38c19ab 100644 > > --- a/lib/librte_hash/rte_cuckoo_hash.c > > +++ b/lib/librte_hash/rte_cuckoo_hash.c > > @@ -102,6 +102,41 @@ EAL_REGISTER_TAILQ(rte_hash_tailq) > > > > #define LCORE_CACHE_SIZE 8 > > > > +/* > > + * All different options to select a key compare function, > > + * based on the key size and custom function. > > + */ > > +enum cmp_jump_table_case { > > + KEY_CUSTOM =3D 0, > > + KEY_16_BYTES, > > + KEY_32_BYTES, > > + KEY_48_BYTES, > > + KEY_64_BYTES, > > + KEY_80_BYTES, > > + KEY_96_BYTES, > > + KEY_112_BYTES, > > + KEY_128_BYTES, > > + KEY_OTHER_BYTES, > > + NUM_KEY_CMP_CASES, > > +}; > > + > > +/* > > + * Table storing all different key compare functions > > + * (multi-process supported) > > + */ > > +rte_hash_cmp_eq_t cmp_jump_table[NUM_KEY_CMP_CASES] =3D { > > + NULL, > > + rte_hash_k16_cmp_eq, > > + rte_hash_k32_cmp_eq, > > + rte_hash_k48_cmp_eq, > > + rte_hash_k64_cmp_eq, > > + rte_hash_k80_cmp_eq, > > + rte_hash_k96_cmp_eq, > > + rte_hash_k112_cmp_eq, > > + rte_hash_k128_cmp_eq, > > + memcmp > > +}; >=20 > Array should be const. >=20 > /Bruce