From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 543542A5E for ; Thu, 3 Mar 2016 06:45:02 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 02 Mar 2016 21:44:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,531,1449561600"; d="scan'208";a="58885188" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga004.fm.intel.com with ESMTP; 02 Mar 2016 21:44:59 -0800 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 2 Mar 2016 21:44:58 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 2 Mar 2016 21:44:58 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.132]) with mapi id 14.03.0248.002; Thu, 3 Mar 2016 13:44:50 +0800 From: "Qiu, Michael" To: Dhana Eadala , "Richardson, Bruce" , "De Lara Guarch, Pablo" Thread-Topic: [PATCH] hash: fix memcmp function pointer in multi-process environment Thread-Index: AQHRdP3kUC2ffg0ir0iRi7x/4gv9cg== Date: Thu, 3 Mar 2016 05:44:49 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E6028622F5BBE8@SHSMSX101.ccr.corp.intel.com> References: <1456976152-15640-1-git-send-email-edreddy@gmail.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] hash: fix memcmp function pointer in multi-process environment 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: Thu, 03 Mar 2016 05:45:03 -0000 On 3/3/2016 11:36 AM, Dhana Eadala wrote:=0A= > We found a problem in dpdk-2.2 using under multi-process environment.=0A= > Here is the brief description how we are using the dpdk:=0A= >=0A= > We have two processes proc1, proc2 using dpdk. These proc1 and proc2 are = two different compiled binaries.=0A= > proc1 is started as primary process and proc2 as secondary process.=0A= >=0A= > proc1:=0A= > Calls srcHash =3D rte_hash_create("src_hash_name") to create rte_hash str= ucture.=0A= > As part of this, this api initalized the rte_hash structure and set the s= rcHash->rte_hash_cmp_eq to the address of memcmp() from proc1 address space= .=0A= >=0A= > proc2:=0A= > calls srcHash =3D rte_hash_find_existing("src_hash_name"). This returns = the rte_hash created by proc1.=0A= > This srcHash->rte_hash_cmp_eq still points to the address of memcmp() fro= m proc1 address space.=0A= > Later proc2 calls rte_hash_lookup_with_hash(srcHash, (const void*) &key,= key.sig);=0A= > Under the hood, rte_hash_lookup_with_hash() invokes __rte_hash_lookup_wit= h_hash(), which in turn calls h->rte_hash_cmp_eq(key, k->key, h->key_len).= =0A= > This leads to a crash as h->rte_hash_cmp_eq is an address from proc1 addr= ess space and is invalid address in proc2 address space.=0A= >=0A= > We found, from dpdk documentation, that=0A= >=0A= > "=0A= > The use of function pointers between multiple processes running based of= different compiled=0A= > binaries is not supported, since the location of a given function in one= process may be different to=0A= > its location in a second. This prevents the librte_hash library from beh= aving properly as in a multi-=0A= > threaded instance, since it uses a pointer to the hash function internal= ly.=0A= >=0A= > To work around this issue, it is recommended that multi-process applicat= ions perform the hash=0A= > calculations by directly calling the hashing function from the code and = then using the=0A= > rte_hash_add_with_hash()/rte_hash_lookup_with_hash() functions instead o= f the functions which do=0A= > the hashing internally, such as rte_hash_add()/rte_hash_lookup().=0A= > "=0A= >=0A= > We did follow the recommended steps by invoking rte_hash_lookup_with_hash= ().=0A= > It was no issue up to and including dpdk-2.0. In later releases started c= rashing because rte_hash_cmp_eq is introduced in dpdk-2.1=0A= >=0A= > We fixed it with the following patch and would like to submit the patch t= o dpdk.org.=0A= > Patch is created such that, if anyone wanted to use dpdk in multi-process= environment with function pointers not shared, they need to=0A= > define RTE_LIB_MP_NO_FUNC_PTR in their Makefile. Without defining this fl= ag in Makefile, it works as it is now.=0A= >=0A= > Signed-off-by: Dhana Eadala =0A= > ---=0A= >=0A= =0A= Some comments:=0A= =0A= 1. your commit log need to refactor, better to limit every line less=0A= than 80 character.=0A= =0A= 2. I think you could add the ifdef here in=0A= lib/librte_hash/rte_cuckoo_hash.c :=0A= /*=0A= * If x86 architecture is used, select appropriate compare function,=0A= * which may use x86 instrinsics, otherwise use memcmp=0A= */=0A= #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||\=0A= defined(RTE_ARCH_X86_X32) || defined(RTE_ARCH_ARM64)=0A= /* Select function to compare keys */=0A= switch (params->key_len) {=0A= case 16:=0A= h->rte_hash_cmp_eq =3D rte_hash_k16_cmp_eq;=0A= break;=0A= [...]=0A= break;=0A= default:=0A= /* If key is not multiple of 16, use generic memcmp */=0A= h->rte_hash_cmp_eq =3D memcmp;=0A= }=0A= #else=0A= h->rte_hash_cmp_eq =3D memcmp;=0A= #endif=0A= =0A= So that could remove other #ifdef in those lines.=0A= =0A= 3. I don't think ask others to write RTE_LIB_MP_NO_FUNC_PTR in makefile=0A= is a good idea, if you really want to do that, please add a doc so that=0A= others could know it.=0A= =0A= Thanks,=0A= Michael=0A=