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 <vladimir.medved...@intel.com> > Acked-by: Konstantin Ananyev <konstantin.anan...@intel.com> > --- > 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 <string.h> > + > #include <rte_eal_memconfig.h> > #include <rte_errno.h> > #include <rte_hash.h> > -#include <rte_jhash.h> > +#include <rte_hash_crc.h> > #include <rte_malloc.h> > #include <rte_random.h> > #include <rte_rwlock.h> > @@ -24,7 +26,7 @@ > /* "SAD_<name>" */ > #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]; >
