[dpdk-dev] [PATCH] hash: rename unused field to "reserved"

[dpdk-dev] [PATCH] hash: rename unused field to "reserved"

[Bruce Richardson]

The cuckoo hash has a fixed number of entries per bucket, so the
configuration parameter for this is unused. We change this field in the
parameters struct to "reserved" to indicate that there is now no such
parameter value, while at the same time keeping ABI consistency.

Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")

Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_hash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index 68109d5..1cddc07 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -75,7 +75,7 @@ typedef uint32_t (*rte_hash_function)(const void *key, uint32_t key_len,
 struct rte_hash_parameters {
 	const char *name;		/**< Name of the hash. */
 	uint32_t entries;		/**< Total hash table entries. */
-	uint32_t bucket_entries;        /**< Bucket entries. */
+	uint32_t reserved;		/**< Unused field. Should be set to 0 */
 	uint32_t key_len;		/**< Length of hash key. */
 	rte_hash_function hash_func;	/**< Primary Hash function used to calculate hash. */
 	uint32_t hash_func_init_val;	/**< Init value used by hash_func. */
-- 
2.4.3

Re: [dpdk-dev] [PATCH] hash: rename unused field to "reserved"

[Bruce Richardson]

On Mon, Jul 13, 2015 at 05:25:51PM +0100, Bruce Richardson wrote:
> The cuckoo hash has a fixed number of entries per bucket, so the
> configuration parameter for this is unused. We change this field in the
> parameters struct to "reserved" to indicate that there is now no such
> parameter value, while at the same time keeping ABI consistency.
> 
> Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")
> 
> Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Self-NAK. 

Missed some extra code dependencies using this field. V2 to follow.

[dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

[Bruce Richardson]

The cuckoo hash has a fixed number of entries per bucket, so the
configuration parameter for this is unused. We change this field in the
parameters struct to "reserved" to indicate that there is now no such
parameter value, while at the same time keeping ABI consistency.

Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")

Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_func_reentrancy.c | 1 -
 app/test/test_hash_perf.c       | 1 -
 app/test/test_hash_scaling.c    | 1 -
 drivers/net/enic/enic_clsf.c    | 2 --
 examples/l3fwd-power/main.c     | 2 --
 examples/l3fwd-vf/main.c        | 1 -
 examples/l3fwd/main.c           | 2 --
 lib/librte_hash/rte_hash.h      | 2 +-
 8 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 85504c0..be61773 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -226,7 +226,6 @@ hash_create_free(__attribute__((unused)) void *arg)
 	struct rte_hash_parameters hash_params = {
 		.name = NULL,
 		.entries = 16,
-		.bucket_entries = 4,
 		.key_len = 4,
 		.hash_func = (rte_hash_function)rte_jhash_32b,
 		.hash_func_init_val = 0,
diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c
index e9a522b..a87fc80 100644
--- a/app/test/test_hash_perf.c
+++ b/app/test/test_hash_perf.c
@@ -100,7 +100,6 @@ int32_t positions[KEYS_TO_ADD];
 /* Parameters used for hash table in unit test functions. */
 static struct rte_hash_parameters ut_params = {
 	.entries = MAX_ENTRIES,
-	.bucket_entries = BUCKET_SIZE,
 	.hash_func = rte_jhash,
 	.hash_func_init_val = 0,
 };
diff --git a/app/test/test_hash_scaling.c b/app/test/test_hash_scaling.c
index 682ae94..39602cb 100644
--- a/app/test/test_hash_scaling.c
+++ b/app/test/test_hash_scaling.c
@@ -129,7 +129,6 @@ test_hash_scaling(int locking_mode)
 	uint64_t i, key;
 	struct rte_hash_parameters hash_params = {
 		.entries = num_iterations*2,
-		.bucket_entries = 16,
 		.key_len = sizeof(key),
 		.hash_func = rte_hash_crc,
 		.hash_func_init_val = 0,
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index ca12d2d..9c2abfb 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -63,7 +63,6 @@
 
 #define SOCKET_0                0
 #define ENICPMD_CLSF_HASH_ENTRIES       ENICPMD_FDIR_MAX
-#define ENICPMD_CLSF_BUCKET_ENTRIES     4
 
 void enic_fdir_stats_get(struct enic *enic, struct rte_eth_fdir_stats *stats)
 {
@@ -245,7 +244,6 @@ int enic_clsf_init(struct enic *enic)
 	struct rte_hash_parameters hash_params = {
 		.name = "enicpmd_clsf_hash",
 		.entries = ENICPMD_CLSF_HASH_ENTRIES,
-		.bucket_entries = ENICPMD_CLSF_BUCKET_ENTRIES,
 		.key_len = RTE_HASH_KEY_LENGTH_MAX,
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d4eba1a..6eb459d 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1247,7 +1247,6 @@ setup_hash(int socketid)
 	struct rte_hash_parameters ipv4_l3fwd_hash_params = {
 		.name = NULL,
 		.entries = L3FWD_HASH_ENTRIES,
-		.bucket_entries = 4,
 		.key_len = sizeof(struct ipv4_5tuple),
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
@@ -1256,7 +1255,6 @@ setup_hash(int socketid)
 	struct rte_hash_parameters ipv6_l3fwd_hash_params = {
 		.name = NULL,
 		.entries = L3FWD_HASH_ENTRIES,
-		.bucket_entries = 4,
 		.key_len = sizeof(struct ipv6_5tuple),
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
diff --git a/examples/l3fwd-vf/main.c b/examples/l3fwd-vf/main.c
index ccbb02f..01f610e 100644
--- a/examples/l3fwd-vf/main.c
+++ b/examples/l3fwd-vf/main.c
@@ -251,7 +251,6 @@ static lookup_struct_t *l3fwd_lookup_struct[NB_SOCKETS];
 struct rte_hash_parameters l3fwd_hash_params = {
 	.name = "l3fwd_hash_0",
 	.entries = L3FWD_HASH_ENTRIES,
-	.bucket_entries = 4,
 	.key_len = sizeof(struct ipv4_5tuple),
 	.hash_func = DEFAULT_HASH_FUNC,
 	.hash_func_init_val = 0,
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5c22ed1..def9594 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -2162,7 +2162,6 @@ setup_hash(int socketid)
     struct rte_hash_parameters ipv4_l3fwd_hash_params = {
         .name = NULL,
         .entries = L3FWD_HASH_ENTRIES,
-        .bucket_entries = 4,
         .key_len = sizeof(union ipv4_5tuple_host),
         .hash_func = ipv4_hash_crc,
         .hash_func_init_val = 0,
@@ -2171,7 +2170,6 @@ setup_hash(int socketid)
     struct rte_hash_parameters ipv6_l3fwd_hash_params = {
         .name = NULL,
         .entries = L3FWD_HASH_ENTRIES,
-        .bucket_entries = 4,
         .key_len = sizeof(union ipv6_5tuple_host),
         .hash_func = ipv6_hash_crc,
         .hash_func_init_val = 0,
diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index 68109d5..1cddc07 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -75,7 +75,7 @@ typedef uint32_t (*rte_hash_function)(const void *key, uint32_t key_len,
 struct rte_hash_parameters {
 	const char *name;		/**< Name of the hash. */
 	uint32_t entries;		/**< Total hash table entries. */
-	uint32_t bucket_entries;        /**< Bucket entries. */
+	uint32_t reserved;		/**< Unused field. Should be set to 0 */
 	uint32_t key_len;		/**< Length of hash key. */
 	rte_hash_function hash_func;	/**< Primary Hash function used to calculate hash. */
 	uint32_t hash_func_init_val;	/**< Init value used by hash_func. */
-- 
2.4.3

Re: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

[Thomas Monjalon]

2015-07-13 17:38, Bruce Richardson:
> The cuckoo hash has a fixed number of entries per bucket, so the
> configuration parameter for this is unused. We change this field in the
> parameters struct to "reserved" to indicate that there is now no such
> parameter value, while at the same time keeping ABI consistency.
> 
> Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")
> 
> Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks

Re: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

[Olga Shern]

Hi, 

I see the following compilation error :
dpdk/lib/librte_hash/rte_cuckoo_hash.c:145: error: flexible array member in otherwise empty struct
when compiling on RH6.5

Best Regards,
Olga

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
Sent: Monday, July 13, 2015 7:39 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

The cuckoo hash has a fixed number of entries per bucket, so the configuration parameter for this is unused. We change this field in the parameters struct to "reserved" to indicate that there is now no such parameter value, while at the same time keeping ABI consistency.

Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")

Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_func_reentrancy.c | 1 -
 app/test/test_hash_perf.c       | 1 -
 app/test/test_hash_scaling.c    | 1 -
 drivers/net/enic/enic_clsf.c    | 2 --
 examples/l3fwd-power/main.c     | 2 --
 examples/l3fwd-vf/main.c        | 1 -
 examples/l3fwd/main.c           | 2 --
 lib/librte_hash/rte_hash.h      | 2 +-
 8 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c index 85504c0..be61773 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -226,7 +226,6 @@ hash_create_free(__attribute__((unused)) void *arg)
 	struct rte_hash_parameters hash_params = {
 		.name = NULL,
 		.entries = 16,
-		.bucket_entries = 4,
 		.key_len = 4,
 		.hash_func = (rte_hash_function)rte_jhash_32b,
 		.hash_func_init_val = 0,
diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c index e9a522b..a87fc80 100644
--- a/app/test/test_hash_perf.c
+++ b/app/test/test_hash_perf.c
@@ -100,7 +100,6 @@ int32_t positions[KEYS_TO_ADD];
 /* Parameters used for hash table in unit test functions. */  static struct rte_hash_parameters ut_params = {
 	.entries = MAX_ENTRIES,
-	.bucket_entries = BUCKET_SIZE,
 	.hash_func = rte_jhash,
 	.hash_func_init_val = 0,
 };
diff --git a/app/test/test_hash_scaling.c b/app/test/test_hash_scaling.c index 682ae94..39602cb 100644
--- a/app/test/test_hash_scaling.c
+++ b/app/test/test_hash_scaling.c
@@ -129,7 +129,6 @@ test_hash_scaling(int locking_mode)
 	uint64_t i, key;
 	struct rte_hash_parameters hash_params = {
 		.entries = num_iterations*2,
-		.bucket_entries = 16,
 		.key_len = sizeof(key),
 		.hash_func = rte_hash_crc,
 		.hash_func_init_val = 0,
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c index ca12d2d..9c2abfb 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -63,7 +63,6 @@
 
 #define SOCKET_0                0
 #define ENICPMD_CLSF_HASH_ENTRIES       ENICPMD_FDIR_MAX
-#define ENICPMD_CLSF_BUCKET_ENTRIES     4
 
 void enic_fdir_stats_get(struct enic *enic, struct rte_eth_fdir_stats *stats)  { @@ -245,7 +244,6 @@ int enic_clsf_init(struct enic *enic)
 	struct rte_hash_parameters hash_params = {
 		.name = "enicpmd_clsf_hash",
 		.entries = ENICPMD_CLSF_HASH_ENTRIES,
-		.bucket_entries = ENICPMD_CLSF_BUCKET_ENTRIES,
 		.key_len = RTE_HASH_KEY_LENGTH_MAX,
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c index d4eba1a..6eb459d 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1247,7 +1247,6 @@ setup_hash(int socketid)
 	struct rte_hash_parameters ipv4_l3fwd_hash_params = {
 		.name = NULL,
 		.entries = L3FWD_HASH_ENTRIES,
-		.bucket_entries = 4,
 		.key_len = sizeof(struct ipv4_5tuple),
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
@@ -1256,7 +1255,6 @@ setup_hash(int socketid)
 	struct rte_hash_parameters ipv6_l3fwd_hash_params = {
 		.name = NULL,
 		.entries = L3FWD_HASH_ENTRIES,
-		.bucket_entries = 4,
 		.key_len = sizeof(struct ipv6_5tuple),
 		.hash_func = DEFAULT_HASH_FUNC,
 		.hash_func_init_val = 0,
diff --git a/examples/l3fwd-vf/main.c b/examples/l3fwd-vf/main.c index ccbb02f..01f610e 100644
--- a/examples/l3fwd-vf/main.c
+++ b/examples/l3fwd-vf/main.c
@@ -251,7 +251,6 @@ static lookup_struct_t *l3fwd_lookup_struct[NB_SOCKETS];  struct rte_hash_parameters l3fwd_hash_params = {
 	.name = "l3fwd_hash_0",
 	.entries = L3FWD_HASH_ENTRIES,
-	.bucket_entries = 4,
 	.key_len = sizeof(struct ipv4_5tuple),
 	.hash_func = DEFAULT_HASH_FUNC,
 	.hash_func_init_val = 0,
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c index 5c22ed1..def9594 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -2162,7 +2162,6 @@ setup_hash(int socketid)
     struct rte_hash_parameters ipv4_l3fwd_hash_params = {
         .name = NULL,
         .entries = L3FWD_HASH_ENTRIES,
-        .bucket_entries = 4,
         .key_len = sizeof(union ipv4_5tuple_host),
         .hash_func = ipv4_hash_crc,
         .hash_func_init_val = 0,
@@ -2171,7 +2170,6 @@ setup_hash(int socketid)
     struct rte_hash_parameters ipv6_l3fwd_hash_params = {
         .name = NULL,
         .entries = L3FWD_HASH_ENTRIES,
-        .bucket_entries = 4,
         .key_len = sizeof(union ipv6_5tuple_host),
         .hash_func = ipv6_hash_crc,
         .hash_func_init_val = 0,
diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 68109d5..1cddc07 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -75,7 +75,7 @@ typedef uint32_t (*rte_hash_function)(const void *key, uint32_t key_len,  struct rte_hash_parameters {
 	const char *name;		/**< Name of the hash. */
 	uint32_t entries;		/**< Total hash table entries. */
-	uint32_t bucket_entries;        /**< Bucket entries. */
+	uint32_t reserved;		/**< Unused field. Should be set to 0 */
 	uint32_t key_len;		/**< Length of hash key. */
 	rte_hash_function hash_func;	/**< Primary Hash function used to calculate hash. */
 	uint32_t hash_func_init_val;	/**< Init value used by hash_func. */
--
2.4.3

Re: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

[De Lara Guarch, Pablo]

Hi Olga,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olga Shern
> Sent: Wednesday, July 15, 2015 9:08 AM
> To: Richardson, Bruce; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"
> 
> Hi,
> 
> I see the following compilation error :
> dpdk/lib/librte_hash/rte_cuckoo_hash.c:145: error: flexible array member in
> otherwise empty struct
> when compiling on RH6.5

Thanks for letting us know. I am working on a fix, that I will send to the mailing list
today.

Thanks,
Pablo
> 
> Best Regards,
> Olga

Re: [dpdk-dev] [PATCH v2] hash: rename unused field to "reserved"

[Stephen Hemminger]

On Mon, 13 Jul 2015 17:38:45 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> The cuckoo hash has a fixed number of entries per bucket, so the
> configuration parameter for this is unused. We change this field in the
> parameters struct to "reserved" to indicate that there is now no such
> parameter value, while at the same time keeping ABI consistency.
> 
> Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")
> 
> Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

I am sorry this patch went into 2.1 because it broke source code
compatibility.  To me having source code recompile compatibility is MORE
important than ABI compatibility.