DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup
@ 2020-03-09 12:55 Vladimir Medvedkin
  2020-03-24 19:57 ` Ananyev, Konstantin
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Vladimir Medvedkin @ 2020-03-09 12:55 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev, akhil.goyal

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

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
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 <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,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;
+	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;
 	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup
  2020-03-09 12:55 [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup Vladimir Medvedkin
@ 2020-03-24 19:57 ` Ananyev, Konstantin
  2020-03-26 12:54   ` Medvedkin, Vladimir
  2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 1/2] " Vladimir Medvedkin
  2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 2/2] ipsec: use hash add and delete with precalculated signatures Vladimir Medvedkin
  2 siblings, 1 reply; 5+ messages in thread
From: Ananyev, Konstantin @ 2020-03-24 19:57 UTC (permalink / raw)
  To: Medvedkin, Vladimir, dev; +Cc: akhil.goyal

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 <vladimir.medvedkin@intel.com>
> ---
> 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 <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,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.

>  	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup
  2020-03-24 19:57 ` Ananyev, Konstantin
@ 2020-03-26 12:54   ` Medvedkin, Vladimir
  0 siblings, 0 replies; 5+ messages in thread
From: Medvedkin, Vladimir @ 2020-03-26 12:54 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev; +Cc: akhil.goyal

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 <vladimir.medvedkin@intel.com>
>> ---
>> 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 <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_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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] ipsec: use hash lookup with hash sigs in sad lookup
  2020-03-09 12:55 [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup Vladimir Medvedkin
  2020-03-24 19:57 ` Ananyev, Konstantin
@ 2020-03-26 16:45 ` " Vladimir Medvedkin
  2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 2/2] ipsec: use hash add and delete with precalculated signatures Vladimir Medvedkin
  2 siblings, 0 replies; 5+ messages in thread
From: Vladimir Medvedkin @ 2020-03-26 16:45 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev, akhil.goyal

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

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 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 <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,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;
+	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;
 	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] ipsec: use hash add and delete with precalculated signatures
  2020-03-09 12:55 [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup Vladimir Medvedkin
  2020-03-24 19:57 ` Ananyev, Konstantin
  2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 1/2] " Vladimir Medvedkin
@ 2020-03-26 16:45 ` Vladimir Medvedkin
  2 siblings, 0 replies; 5+ messages in thread
From: Vladimir Medvedkin @ 2020-03-26 16:45 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev, akhil.goyal

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.medvedkin@intel.com>
---
 lib/librte_ipsec/ipsec_sad.c | 80 ++++++++++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 29 deletions(-)

diff --git a/lib/librte_ipsec/ipsec_sad.c b/lib/librte_ipsec/ipsec_sad.c
index 3292968..6c95240 100644
--- a/lib/librte_ipsec/ipsec_sad.c
+++ b/lib/librte_ipsec/ipsec_sad.c
@@ -37,8 +37,7 @@ 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;
+	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
@@ -75,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
@@ -127,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):
@@ -163,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) ?
@@ -187,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;
@@ -208,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):
@@ -286,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) {
@@ -301,7 +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->spi_dip_keysize = hash_params.key_len;
+	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);
@@ -318,7 +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->spi_dip_sip_keysize = hash_params.key_len;
+	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);
@@ -472,13 +492,15 @@ __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);
+				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->spi_dip_keysize, sad->init_val);
+				sad->keysize[RTE_IPSEC_SAD_SPI_DIP],
+				sad->init_val);
 			keys_2[n_2++] = keys[i];
 		}
 		/* clear 2 LSB's which indicate the presence
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 12:55 [dpdk-dev] [PATCH] ipsec: use hash lookup with hash sigs in sad lookup Vladimir Medvedkin
2020-03-24 19:57 ` Ananyev, Konstantin
2020-03-26 12:54   ` Medvedkin, Vladimir
2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 1/2] " Vladimir Medvedkin
2020-03-26 16:45 ` [dpdk-dev] [PATCH v2 2/2] ipsec: use hash add and delete with precalculated signatures Vladimir Medvedkin

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox