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 <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];
--
Regards,
Vladimir

Reply via email to