From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A40F5A057C; Thu, 26 Mar 2020 13:54:37 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 26F692BAE; Thu, 26 Mar 2020 13:54:36 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id B01631AFF for ; Thu, 26 Mar 2020 13:54:33 +0100 (CET) IronPort-SDR: IkGah1Ff7w0ACJWoPdLbWyQzl5bzWTsUye0u2r3kajsCHV3h2Zx5WGWeXQGtEBaJu0rCxLU/mF vgXo9D97/Dbg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 05:54:32 -0700 IronPort-SDR: ZU+o9A2f9oU4pHcRGW5yMjB8N2DvX5d1wtfKAKFKTDDKkKfDElNAxAw03fyaXxcqnmGFOt6/Bb dmRCotz+3nJQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,308,1580803200"; d="scan'208";a="236269224" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.252.26.53]) ([10.252.26.53]) by orsmga007.jf.intel.com with ESMTP; 26 Mar 2020 05:54:28 -0700 To: "Ananyev, Konstantin" , "dev@dpdk.org" Cc: "akhil.goyal@nxp.com" References: <1583758545-375405-1-git-send-email-vladimir.medvedkin@intel.com> From: "Medvedkin, Vladimir" Message-ID: <1b991689-0cf4-15f0-9396-1a10642d77d1@intel.com> Date: Thu, 26 Mar 2020 12:54:22 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, On 24/03/2020 19:57, Ananyev, Konstantin wrote: > Hi Vladimir, > >> Change hash function from jhash to crc. >> Precalculate hash signatures for a bulk of keys and then >> use rte_hash_lookup_with_hash_bulk_data() to speed up sad lookup > Looks good in general. > Few thoughts below, nothing major. > >> Signed-off-by: Vladimir Medvedkin >> --- >> This patch depends on https://patches.dpdk.org/patch/66460/ >> >> lib/librte_ipsec/ipsec_sad.c | 38 +++++++++++++++++++++++++++++--------- >> 1 file changed, 29 insertions(+), 9 deletions(-) >> >> diff --git a/lib/librte_ipsec/ipsec_sad.c b/lib/librte_ipsec/ipsec_sad.c >> index 2c994ed..3292968 100644 >> --- a/lib/librte_ipsec/ipsec_sad.c >> +++ b/lib/librte_ipsec/ipsec_sad.c >> @@ -2,10 +2,12 @@ >> * Copyright(c) 2019 Intel Corporation >> */ >> >> +#include >> + >> #include >> #include >> #include >> -#include >> +#include >> #include >> #include >> #include >> @@ -24,7 +26,7 @@ >> /* "SAD_" */ >> #define SAD_FORMATSAD_PREFIX "%s" >> >> -#define DEFAULT_HASH_FUNCrte_jhash >> +#define DEFAULT_HASH_FUNCrte_hash_crc >> #define MIN_HASH_ENTRIES8U /* From rte_cuckoo_hash.h */ >> >> struct hash_cnt { >> @@ -35,6 +37,9 @@ struct hash_cnt { >> struct rte_ipsec_sad { >> char name[RTE_IPSEC_SAD_NAMESIZE]; >> struct rte_hash*hash[RTE_IPSEC_SAD_KEY_TYPE_MASK]; >> +uint32_t spi_dip_keysize; >> +uint32_t spi_dip_sip_keysize; > Pure stylish thing: might be > uint32_t keysize[RTE_IPSEC_SAD_KEY_TYPE_MASK]; > here? > >> +uint32_t init_val; >> /* Array to track number of more specific rules >> * (spi_dip or spi_dip_sip). Used only in add/delete >> * as a helper struct. >> @@ -272,6 +277,7 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf) >> >> hash_params.hash_func = DEFAULT_HASH_FUNC; > If we doing lookup_with_hash now, might be it makes sense to > do add_with_hash and del_with_hash too? > For code consistency. Generally I'd prefer to let rte_hash calculate signature inside, this makes code to be more laconic. But I'll provide a separate patch in v2 series with above mentioned changes. > >> hash_params.hash_func_init_val = rte_rand(); >> +sad->init_val = hash_params.hash_func_init_val; >> hash_params.socket_id = conf->socket_id; >> hash_params.name = hash_name; >> if (conf->flags & RTE_IPSEC_SAD_FLAG_RW_CONCURRENCY) >> @@ -295,6 +301,7 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf) >> else >> hash_params.key_len += >> sizeof(((struct rte_ipsec_sadv4_key *)0)->dip); >> +sad->spi_dip_keysize = hash_params.key_len; >> hash_params.entries = RTE_MAX(MIN_HASH_ENTRIES, >> conf->max_sa[RTE_IPSEC_SAD_SPI_DIP]); >> sad->hash[RTE_IPSEC_SAD_SPI_DIP] = rte_hash_create(&hash_params); >> @@ -311,6 +318,7 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf) >> else >> hash_params.key_len += >> sizeof(((struct rte_ipsec_sadv4_key *)0)->sip); >> +sad->spi_dip_sip_keysize = hash_params.key_len; >> hash_params.entries = RTE_MAX(MIN_HASH_ENTRIES, >> conf->max_sa[RTE_IPSEC_SAD_SPI_DIP_SIP]); >> sad->hash[RTE_IPSEC_SAD_SPI_DIP_SIP] = rte_hash_create(&hash_params); >> @@ -440,15 +448,21 @@ __ipsec_sad_lookup(const struct rte_ipsec_sad *sad, >> uint32_t n_3 = 0; >> uint32_t i; >> int found = 0; >> +hash_sig_t hash_sig[RTE_HASH_LOOKUP_BULK_MAX]; >> +hash_sig_t hash_sig_2[RTE_HASH_LOOKUP_BULK_MAX]; >> +hash_sig_t hash_sig_3[RTE_HASH_LOOKUP_BULK_MAX]; >> >> -for (i = 0; i < n; i++) >> +for (i = 0; i < n; i++) { >> sa[i] = NULL; >> +hash_sig[i] = rte_hash_crc_4byte(keys[i]->v4.spi, >> +sad->init_val); >> +} >> >> /* >> * Lookup keys in SPI only hash table first. >> */ >> -rte_hash_lookup_bulk_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> -(const void **)keys, n, &mask_1, sa); >> +rte_hash_lookup_with_hash_bulk_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> +(const void **)keys, hash_sig, n, &mask_1, sa); >> for (map = mask_1; map; map &= (map - 1)) { >> i = rte_bsf64(map); >> /* >> @@ -457,10 +471,14 @@ __ipsec_sad_lookup(const struct rte_ipsec_sad *sad, >> */ >> if ((uintptr_t)sa[i] & RTE_IPSEC_SAD_SPI_DIP_SIP) { >> idx_3[n_3] = i; >> +hash_sig_3[n_3] = rte_hash_crc(keys[i], >> +sad->spi_dip_sip_keysize, sad->init_val); >> keys_3[n_3++] = keys[i]; >> } >> if ((uintptr_t)sa[i] & RTE_IPSEC_SAD_SPI_DIP) { >> idx_2[n_2] = i; >> +hash_sig_2[n_2] = rte_hash_crc(keys[i], >> +sad->spi_dip_keysize, sad->init_val); >> keys_2[n_2++] = keys[i]; >> } >> /* clear 2 LSB's which indicate the presence >> @@ -471,8 +489,9 @@ __ipsec_sad_lookup(const struct rte_ipsec_sad *sad, >> >> /* Lookup for more specific rules in SPI_DIP table */ >> if (n_2 != 0) { >> -rte_hash_lookup_bulk_data(sad->hash[RTE_IPSEC_SAD_SPI_DIP], >> -keys_2, n_2, &mask_2, vals_2); >> +rte_hash_lookup_with_hash_bulk_data( >> +sad->hash[RTE_IPSEC_SAD_SPI_DIP], >> +keys_2, hash_sig_2, n_2, &mask_2, vals_2); >> for (map_spec = mask_2; map_spec; map_spec &= (map_spec - 1)) { >> i = rte_bsf64(map_spec); >> sa[idx_2[i]] = vals_2[i]; >> @@ -480,8 +499,9 @@ __ipsec_sad_lookup(const struct rte_ipsec_sad *sad, >> } >> /* Lookup for more specific rules in SPI_DIP_SIP table */ >> if (n_3 != 0) { >> -rte_hash_lookup_bulk_data(sad->hash[RTE_IPSEC_SAD_SPI_DIP_SIP], >> -keys_3, n_3, &mask_3, vals_3); >> +rte_hash_lookup_with_hash_bulk_data( >> +sad->hash[RTE_IPSEC_SAD_SPI_DIP_SIP], >> +keys_3, hash_sig_3, n_3, &mask_3, vals_3); >> for (map_spec = mask_3; map_spec; map_spec &= (map_spec - 1)) { >> i = rte_bsf64(map_spec); >> sa[idx_3[i]] = vals_3[i]; >> -- >> 2.7.4 -- Regards, Vladimir