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 99F54A0597; Tue, 21 Apr 2020 16:25:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 791631D429; Tue, 21 Apr 2020 16:25:25 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 60DFB1D41F for ; Tue, 21 Apr 2020 16:25:23 +0200 (CEST) IronPort-SDR: BvBfKVNDX3u0cK5lcQeu6yLoFx/uEurjc9XtO4R7NqDlkjVkDO5QjGpZ5emlugh2UvGjYV5JzV GdzsgVGOe9xQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Apr 2020 07:25:22 -0700 IronPort-SDR: Z1VUytZeMR26INFe58nvWoma9fgHBhmVW9EDt8QaXc9kRz8vegeS/ffC3eo55vF177GG7pfxv4 6rBYPeZXdztw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,410,1580803200"; d="scan'208";a="456119534" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.254.156.250]) ([10.254.156.250]) by fmsmga005.fm.intel.com with ESMTP; 21 Apr 2020 07:25:19 -0700 To: "Lukas Bartosik [C]" Cc: "konstantin.ananyev@intel.com" , "akhil.goyal@nxp.com" , Anoob Joseph , "dev@dpdk.org" References: <2811605d29b62740ba467527271c7a30fe75cf78.1585240684.git.vladimir.medvedkin@intel.com> <6590f0170bc4f55d9707ef15ee398e388802b012.1587406845.git.vladimir.medvedkin@intel.com> <347a1212-f3cb-4aa9-e551-c7779b7ac17c@marvell.com> From: "Medvedkin, Vladimir" Message-ID: Date: Tue, 21 Apr 2020 15:25:18 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <347a1212-f3cb-4aa9-e551-c7779b7ac17c@marvell.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v3] 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 Lukas, http://patches.dpdk.org/patch/66715/ is suspended due to new hash library it relates to will not be applied in 20.05. This patch replaces lookup in __ipsec_sad_lookup only for SPI_ONLY table. So yes, in the future both solutions will coexist (new rte_dwk_hash_lookup for SPI_ONLY and rte_hash_lookup_with_hash_bulk_data for SPI_DIP/SPI_DIP_SIP). On 21/04/2020 14:28, Lukas Bartosik [C] wrote: > Hi Vladimir, > > How does this patch relate to http://patches.dpdk.org/patch/66715/. > Both patches replace lookup function rte_hash_lookup_bulk_data used in __ipsec_sad_lookup with > rte_hash_lookup_with_hash_bulk_data or rte_dwk_hash_lookup. > > Is the plan for both solutions to coexist or will only one be selected ? > > Thanks, > Lukasz > > On 20.04.2020 20:27, Vladimir Medvedkin wrote: >> 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 >> Also use rte_hash_add_key_with_hash and _del_key_with_hash with >> precalculated hash signature for a key in rte_ipsec_sad_add and >> rte_ipsec_sad_del >> >> Signed-off-by: Vladimir Medvedkin >> Acked-by: Konstantin Ananyev >> --- >> This patch depends on https://urldefense.proofpoint.com/v2/url?u=https-3A__patches.dpdk.org_patch_68700_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SchRHhE7GLCjEY4i2a1byjC_FpWgRLtq4-kLvKp3_84&m=NIgAtdq8iGRrs8haKYwwa0eyciO0TMFWnUYisMoGD5o&s=IpnJ270RsaE8LIU06649hZvkyKeAaTwamVbIMX9AyUY&e= >> >> lib/librte_ipsec/ipsec_sad.c | 106 ++++++++++++++++++++++++++++++------------- >> 1 file changed, 74 insertions(+), 32 deletions(-) >> >> diff --git a/lib/librte_ipsec/ipsec_sad.c b/lib/librte_ipsec/ipsec_sad.c >> index 2c994ed..6c95240 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_FORMAT SAD_PREFIX "%s" >> >> -#define DEFAULT_HASH_FUNC rte_jhash >> +#define DEFAULT_HASH_FUNC rte_hash_crc >> #define MIN_HASH_ENTRIES 8U /* From rte_cuckoo_hash.h */ >> >> struct hash_cnt { >> @@ -35,6 +37,8 @@ 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 keysize[RTE_IPSEC_SAD_KEY_TYPE_MASK]; >> + 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. >> @@ -70,29 +74,36 @@ add_specific(struct rte_ipsec_sad *sad, const void *key, >> /* Check if the key is present in the table. >> * Need for further accaunting in cnt_arr >> */ >> - ret = rte_hash_lookup(sad->hash[key_type], key); >> + ret = rte_hash_lookup_with_hash(sad->hash[key_type], key, >> + rte_hash_crc(key, sad->keysize[key_type], sad->init_val)); >> notexist = (ret == -ENOENT); >> >> /* Add an SA to the corresponding table.*/ >> - ret = rte_hash_add_key_data(sad->hash[key_type], key, sa); >> + ret = rte_hash_add_key_with_hash_data(sad->hash[key_type], key, >> + rte_hash_crc(key, sad->keysize[key_type], sad->init_val), sa); >> if (ret != 0) >> return ret; >> >> /* Check if there is an entry in SPI only table with the same SPI */ >> - ret = rte_hash_lookup_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> - key, &tmp_val); >> + ret = rte_hash_lookup_with_hash_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> + key, rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val), &tmp_val); >> if (ret < 0) >> tmp_val = NULL; >> tmp_val = SET_BIT(tmp_val, key_type); >> >> /* Add an entry into SPI only table */ >> - ret = rte_hash_add_key_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> - key, tmp_val); >> + ret = rte_hash_add_key_with_hash_data( >> + sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key, >> + rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val), tmp_val); >> if (ret != 0) >> return ret; >> >> /* Update a counter for a given SPI */ >> - ret = rte_hash_lookup(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key); >> + ret = rte_hash_lookup_with_hash(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key, >> + rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val)); >> if (key_type == RTE_IPSEC_SAD_SPI_DIP) >> sad->cnt_arr[ret].cnt_dip += notexist; >> else >> @@ -122,15 +133,17 @@ rte_ipsec_sad_add(struct rte_ipsec_sad *sad, >> */ >> switch (key_type) { >> case(RTE_IPSEC_SAD_SPI_ONLY): >> - ret = rte_hash_lookup_data(sad->hash[key_type], >> - key, &tmp_val); >> + ret = rte_hash_lookup_with_hash_data(sad->hash[key_type], >> + key, rte_hash_crc(key, sad->keysize[key_type], >> + sad->init_val), &tmp_val); >> if (ret >= 0) >> tmp_val = SET_BIT(sa, GET_BIT(tmp_val, >> RTE_IPSEC_SAD_KEY_TYPE_MASK)); >> else >> tmp_val = sa; >> - ret = rte_hash_add_key_data(sad->hash[key_type], >> - key, tmp_val); >> + ret = rte_hash_add_key_with_hash_data(sad->hash[key_type], >> + key, rte_hash_crc(key, sad->keysize[key_type], >> + sad->init_val), tmp_val); >> return ret; >> case(RTE_IPSEC_SAD_SPI_DIP): >> case(RTE_IPSEC_SAD_SPI_DIP_SIP): >> @@ -158,13 +171,15 @@ del_specific(struct rte_ipsec_sad *sad, const void *key, int key_type) >> uint32_t *cnt; >> >> /* Remove an SA from the corresponding table.*/ >> - ret = rte_hash_del_key(sad->hash[key_type], key); >> + ret = rte_hash_del_key_with_hash(sad->hash[key_type], key, >> + rte_hash_crc(key, sad->keysize[key_type], sad->init_val)); >> if (ret < 0) >> return ret; >> >> /* Get an index of cnt_arr entry for a given SPI */ >> - ret = rte_hash_lookup_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> - key, &tmp_val); >> + ret = rte_hash_lookup_with_hash_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> + key, rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val), &tmp_val); >> if (ret < 0) >> return ret; >> cnt = (key_type == RTE_IPSEC_SAD_SPI_DIP) ? >> @@ -182,10 +197,15 @@ del_specific(struct rte_ipsec_sad *sad, const void *key, int key_type) >> * remove an entry from SPI_only table >> */ >> if (tmp_val == NULL) >> - ret = rte_hash_del_key(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key); >> + ret = rte_hash_del_key_with_hash( >> + sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key, >> + rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val)); >> else >> - ret = rte_hash_add_key_data(sad->hash[RTE_IPSEC_SAD_SPI_ONLY], >> - key, tmp_val); >> + ret = rte_hash_add_key_with_hash_data( >> + sad->hash[RTE_IPSEC_SAD_SPI_ONLY], key, >> + rte_hash_crc(key, sad->keysize[RTE_IPSEC_SAD_SPI_ONLY], >> + sad->init_val), tmp_val); >> if (ret < 0) >> return ret; >> return 0; >> @@ -203,19 +223,23 @@ rte_ipsec_sad_del(struct rte_ipsec_sad *sad, >> return -EINVAL; >> switch (key_type) { >> case(RTE_IPSEC_SAD_SPI_ONLY): >> - ret = rte_hash_lookup_data(sad->hash[key_type], >> - key, &tmp_val); >> + ret = rte_hash_lookup_with_hash_data(sad->hash[key_type], >> + key, rte_hash_crc(key, sad->keysize[key_type], >> + sad->init_val), &tmp_val); >> if (ret < 0) >> return ret; >> if (GET_BIT(tmp_val, RTE_IPSEC_SAD_KEY_TYPE_MASK) == 0) { >> - ret = rte_hash_del_key(sad->hash[key_type], >> - key); >> + ret = rte_hash_del_key_with_hash(sad->hash[key_type], >> + key, rte_hash_crc(key, sad->keysize[key_type], >> + sad->init_val)); >> ret = ret < 0 ? ret : 0; >> } else { >> tmp_val = GET_BIT(tmp_val, >> RTE_IPSEC_SAD_KEY_TYPE_MASK); >> - ret = rte_hash_add_key_data(sad->hash[key_type], >> - key, tmp_val); >> + ret = rte_hash_add_key_with_hash_data( >> + sad->hash[key_type], key, >> + rte_hash_crc(key, sad->keysize[key_type], >> + sad->init_val), tmp_val); >> } >> return ret; >> case(RTE_IPSEC_SAD_SPI_DIP): >> @@ -272,6 +296,7 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf) >> >> hash_params.hash_func = DEFAULT_HASH_FUNC; >> 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) >> @@ -280,6 +305,7 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf) >> /** Init hash[RTE_IPSEC_SAD_SPI_ONLY] for SPI only */ >> snprintf(hash_name, sizeof(hash_name), "sad_1_%p", sad); >> hash_params.key_len = sizeof(((struct rte_ipsec_sadv4_key *)0)->spi); >> + sad->keysize[RTE_IPSEC_SAD_SPI_ONLY] = hash_params.key_len; >> hash_params.entries = sa_sum; >> sad->hash[RTE_IPSEC_SAD_SPI_ONLY] = rte_hash_create(&hash_params); >> if (sad->hash[RTE_IPSEC_SAD_SPI_ONLY] == NULL) { >> @@ -295,6 +321,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->keysize[RTE_IPSEC_SAD_SPI_DIP] = 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 +338,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->keysize[RTE_IPSEC_SAD_SPI_DIP_SIP] = 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 +468,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 +491,16 @@ __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->keysize[RTE_IPSEC_SAD_SPI_DIP_SIP], >> + 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->keysize[RTE_IPSEC_SAD_SPI_DIP], >> + sad->init_val); >> keys_2[n_2++] = keys[i]; >> } >> /* clear 2 LSB's which indicate the presence >> @@ -471,8 +511,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 +521,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]; -- Regards, Vladimir