From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yerden Zhumabekov <e_zhumabekov-8EHiFRVJVgQ@public.gmane.org>
Subject: Re: [PATCH 0/2] rewritten rte_hash_crc() call
Date: Fri, 14 Nov 2014 17:57:51 +0600
Message-ID: <5465EE3F.2010404@sts.kz>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Cc: dev-VfR2kkLFssw@public.gmane.org
To: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <20141114113327.GA19147-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>


14.11.2014 17:33, Neil Horman =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
> 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 <rte_hash_crc.h>
>> #define DEFAULT_HASH_FUNC       rte_hash_crc
>> #else
>> #include <rte_jhash.h>
>> #define DEFAULT_HASH_FUNC       rte_jhash
>> #endif
>>
>> In rte_fbk_hash.h
>> #ifdef RTE_MACHINE_CPUFLAG_SSE4_2
>> #include <rte_hash_crc.h>
>> /** Default four-byte key hash function if none is specified. */
>> #define RTE_FBK_HASH_FUNC_DEFAULT=C2=B7=C2=B7=C2=B7=C2=B7=C2=B7=C2=B7=C2=
=B7rte_hash_crc_4byte
>> #else
>> #include <rte_jhash.h>
>> #define RTE_FBK_HASH_FUNC_DEFAULT=C2=B7=C2=B7=C2=B7=C2=B7=C2=B7=C2=B7=C2=
=B7rte_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 fu=
nction
> 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 oste=
nsibly
> because of the behavior without SSE4.2 it defines these huge test table=
s twice
> based on the availability of SSE4.2.  It would be better if we could al=
low
> 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.

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.

--=20
Sincerely,

Yerden Zhumabekov
State Technical Service
Astana, KZ