From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id C641D7E75 for ; Fri, 14 Nov 2014 14:43:12 +0100 (CET) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XpHJF-0008L9-7X; Fri, 14 Nov 2014 08:53:11 -0500 Date: Fri, 14 Nov 2014 08:53:08 -0500 From: Neil Horman To: Yerden Zhumabekov Message-ID: <20141114135308.GD19147@hmsreliant.think-freely.org> References: <1409724351-23786-1-git-send-email-e_zhumabekov@sts.kz> <2916837.QJI9btJm0N@xps13> <20141114005211.GC14230@localhost.localdomain> <5465AC00.1070602@sts.kz> <20141114113327.GA19147@hmsreliant.think-freely.org> <5465EE3F.2010404@sts.kz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5465EE3F.2010404@sts.kz> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 0/2] rewritten rte_hash_crc() call 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, 14 Nov 2014 13:43:13 -0000 On Fri, Nov 14, 2014 at 05:57:51PM +0600, Yerden Zhumabekov wrote: > > 14.11.2014 17:33, Neil Horman пишет: > > On Fri, Nov 14, 2014 at 01:15:12PM +0600, Yerden Zhumabekov wrote: > >> > >> Hello, > >> > >> A quick grep on dpdk source shows that rte_hash_crc() is used in > >> librte_hash in following context: > >> > >> In rte_hash.c: > >> /* Hash function used if none is specified */ > >> #ifdef RTE_MACHINE_CPUFLAG_SSE4_2 > >> #include > >> #define DEFAULT_HASH_FUNC rte_hash_crc > >> #else > >> #include > >> #define DEFAULT_HASH_FUNC rte_jhash > >> #endif > >> > >> In rte_fbk_hash.h > >> #ifdef RTE_MACHINE_CPUFLAG_SSE4_2 > >> #include > >> /** Default four-byte key hash function if none is specified. */ > >> #define RTE_FBK_HASH_FUNC_DEFAULT·······rte_hash_crc_4byte > >> #else > >> #include > >> #define RTE_FBK_HASH_FUNC_DEFAULT·······rte_jhash_1word > >> #endif > >> #endif > >> > >> > >> I guess it covers the cpu flags check you're talking about. > >> > > Not really. That covers the case of applications selecting the hash function > > using the DEFUALT_HASH_FUNC macro, but doesn't nothing for applications using > > the function directly. Test_hash_perf is an example of this, and ostensibly > > because of the behavior without SSE4.2 it defines these huge test tables twice > > based on the availability of SSE4.2. It would be better if we could allow > > applications to use rte_hash_crc regardless, and make the code it uses at run > > time configurable. > > I see, then we have a problem here :) > > Actually, that was one of my concerns when developing these patches. I > looked through the source code of libs and examples and I saw the > '#ifdef..#include..#endif'-like appoach while selecting hash function > was common. So I organized patches to minimize the impact on API and not > to contradict this approach. > Thats a reasonable approach, but I really hate the idea of continuing this need to select cpu features at compile time if its not nececcesary. > If we prefer to change this approach then, I guess, we need to introduce > broader changes to rte_hash library and change other code which uses it. > If that's what's needed, then it'll take some time for me to rework > these patches. > Well, its possible you'll get lucky. crc is such a common operation, its entirely possible that the gcc intrinsic emits software based crc computation if the SSE4.2 instructions aren't enabled. I recommend modifying the test_hash_crc function to use rte_hash_crc with SSE4.2 disabled, and see if you get a crash. If you don't examine the disassembly of your new function and confirm that something reasonable that doesn't use SSE4.2 is emitted. If thats the case, your patch is fine, and we can focus on how to change the ifdefs in the existing code, as use of the rte_hash_crc functions should be safe. Best Neil > -- > Sincerely, > > Yerden Zhumabekov > State Technical Service > Astana, KZ > >