table: add dedicated params struct for cuckoo hash

Add dedicated parameter structure for cuckoo hash. The cuckoo hash from
librte_hash uses slightly different prototype for the hash function (no
key_mask parameter, 32-bit seed and return value) that require either
of the following approaches:
   1/ Function pointer conversion: gcc 8.1 warning [1], misleading [2]
   2/ Union within the parameter structure: pollutes a very generic API
      parameter structure with some implementation dependent detail
      (i.e. key mask not available for one of the available
      implementations)
   3/ Using opaque pointer for hash function: same issue from 2/
   4/ Different parameter structure: avoid issue from 2/; hopefully,
      it won't be long before librte_hash implements the key mask feature,
      so the generic API structure could be used.

[1] http://www.dpdk.org/ml/archives/dev/2018-April/094950.html
[2] http://www.dpdk.org/ml/archives/dev/2018-April/096250.html

Fixes: 5a80bf0ae613 ("table: add cuckoo hash")

Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

diff --git a/lib/librte_table/Makefile b/lib/librte_table/Makefile
index c4a9acb..276d476 100644 (file)
--- a/lib/librte_table/Makefile
+++ b/lib/librte_table/Makefile
@@ -45,6 +45,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
 SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include += rte_table_acl.h
 endif
 SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include += rte_table_hash.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include += rte_table_hash_cuckoo.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include += rte_lru.h
 ifeq ($(CONFIG_RTE_ARCH_X86),y)
 SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include += rte_lru_x86.h

diff --git a/lib/librte_table/meson.build b/lib/librte_table/meson.build
index 39ffaf1..8b2f841 100644 (file)
--- a/lib/librte_table/meson.build
+++ b/lib/librte_table/meson.build
@@ -18,6 +18,7 @@ headers = files('rte_table.h',
                'rte_table_lpm.h',
                'rte_table_lpm_ipv6.h',
                'rte_table_hash.h',
+               'rte_table_hash_cuckoo.h',
                'rte_lru.h',
                'rte_table_array.h',
                'rte_table_stub.h')

diff --git a/lib/librte_table/rte_table_hash.h b/lib/librte_table/rte_table_hash.h
index 7aad84f..6f55bd5 100644 (file)
--- a/lib/librte_table/rte_table_hash.h
+++ b/lib/librte_table/rte_table_hash.h
@@ -99,9 +99,6 @@ extern struct rte_table_ops rte_table_hash_key8_lru_ops;
 extern struct rte_table_ops rte_table_hash_key16_lru_ops;
 extern struct rte_table_ops rte_table_hash_key32_lru_ops;
 
-/** Cuckoo hash table operations */
-extern struct rte_table_ops rte_table_hash_cuckoo_ops;
-
 #ifdef __cplusplus
 }
 #endif

diff --git a/lib/librte_table/rte_table_hash_cuckoo.c b/lib/librte_table/rte_table_hash_cuckoo.c
index dcb4fe9..f024303 100644 (file)
--- a/lib/librte_table/rte_table_hash_cuckoo.c
+++ b/lib/librte_table/rte_table_hash_cuckoo.c
@@ -10,8 +10,7 @@
 #include <rte_malloc.h>
 #include <rte_log.h>
 
-#include <rte_hash.h>
-#include "rte_table_hash.h"
+#include "rte_table_hash_cuckoo.h"
 
 #ifdef RTE_TABLE_STATS_COLLECT
 
@@ -35,7 +34,7 @@ struct rte_table_hash {
        uint32_t key_size;
        uint32_t entry_size;
        uint32_t n_keys;
-       rte_table_hash_op_hash f_hash;
+       rte_hash_function f_hash;
        uint32_t seed;
        uint32_t key_offset;
 
@@ -47,7 +46,7 @@ struct rte_table_hash {
 };
 
 static int
-check_params_create_hash_cuckoo(struct rte_table_hash_params *params)
+check_params_create_hash_cuckoo(struct rte_table_hash_cuckoo_params *params)
 {
        if (params == NULL) {
                RTE_LOG(ERR, TABLE, "NULL Input Parameters.\n");
@@ -82,7 +81,7 @@ rte_table_hash_cuckoo_create(void *params,
                        int socket_id,
                        uint32_t entry_size)
 {
-       struct rte_table_hash_params *p = params;
+       struct rte_table_hash_cuckoo_params *p = params;
        struct rte_hash *h_table;
        struct rte_table_hash *t;
        uint32_t total_size;
@@ -107,7 +106,7 @@ rte_table_hash_cuckoo_create(void *params,
        struct rte_hash_parameters hash_cuckoo_params = {
                .entries = p->n_keys,
                .key_len = p->key_size,
-               .hash_func = (rte_hash_function)(p->f_hash),
+               .hash_func = p->f_hash,
                .hash_func_init_val = p->seed,
                .socket_id = socket_id,
                .name = p->name

diff --git a/lib/librte_table/rte_table_hash_cuckoo.h b/lib/librte_table/rte_table_hash_cuckoo.h
new file mode 100644 (file)
index 0000000..d9d4312
--- /dev/null
+++ b/lib/librte_table/rte_table_hash_cuckoo.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#ifndef __INCLUDE_RTE_TABLE_HASH_CUCKOO_H__
+#define __INCLUDE_RTE_TABLE_HASH_CUCKOO_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @file
+ * RTE Table Hash Cuckoo
+ *
+ ***/
+#include <stdint.h>
+
+#include <rte_hash.h>
+
+#include "rte_table.h"
+
+/** Hash table parameters */
+struct rte_table_hash_cuckoo_params {
+       /** Name */
+       const char *name;
+
+       /** Key size (number of bytes) */
+       uint32_t key_size;
+
+       /** Byte offset within packet meta-data where the key is located */
+       uint32_t key_offset;
+
+       /** Key mask */
+       uint8_t *key_mask;
+
+       /** Number of keys */
+       uint32_t n_keys;
+
+       /** Number of buckets */
+       uint32_t n_buckets;
+
+       /** Hash function */
+       rte_hash_function f_hash;
+
+       /** Seed value for the hash function */
+       uint32_t seed;
+};
+
+/** Cuckoo hash table operations */
+extern struct rte_table_ops rte_table_hash_cuckoo_ops;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

diff --git a/test/test-pipeline/main.h b/test/test-pipeline/main.h
index f844e94..59dcfdd 100644 (file)
--- a/test/test-pipeline/main.h
+++ b/test/test-pipeline/main.h
@@ -107,6 +107,10 @@ uint64_t test_hash(void *key,
        uint32_t key_size,
        uint64_t seed);
 
+uint32_t test_hash_cuckoo(const void *key,
+       uint32_t key_size,
+       uint32_t seed);
+
 void app_main_loop_worker(void);
 void app_main_loop_worker_pipeline_stub(void);
 void app_main_loop_worker_pipeline_hash(void);

diff --git a/test/test-pipeline/pipeline_hash.c b/test/test-pipeline/pipeline_hash.c
index 11e2402..c201472 100644 (file)
--- a/test/test-pipeline/pipeline_hash.c
+++ b/test/test-pipeline/pipeline_hash.c
@@ -15,6 +15,7 @@
 #include <rte_port_ring.h>
 #include <rte_table_hash.h>
 #include <rte_hash.h>
+#include <rte_table_hash_cuckoo.h>
 #include <rte_pipeline.h>
 
 #include "main.h"
@@ -151,6 +152,17 @@ app_main_loop_worker_pipeline_hash(void) {
                .seed = 0,
        };
 
+       struct rte_table_hash_cuckoo_params table_hash_cuckoo_params = {
+               .name = "TABLE",
+               .key_size = key_size,
+               .key_offset = APP_METADATA_OFFSET(32),
+               .key_mask = NULL,
+               .n_keys = 1 << 24,
+               .n_buckets = 1 << 22,
+               .f_hash = test_hash_cuckoo,
+               .seed = 0,
+       };
+
        /* Table configuration */
        switch (app.pipeline_type) {
        case e_APP_PIPELINE_HASH_KEY8_EXT:
@@ -298,7 +310,7 @@ app_main_loop_worker_pipeline_hash(void) {
        {
                struct rte_pipeline_table_params table_params = {
                        .ops = &rte_table_hash_cuckoo_ops,
-                       .arg_create = &table_hash_params,
+                       .arg_create = &table_hash_cuckoo_params,
                        .f_action_hit = NULL,
                        .f_action_miss = NULL,
                        .arg_ah = NULL,
@@ -379,6 +391,18 @@ uint64_t test_hash(
        return signature;
 }
 
+uint32_t test_hash_cuckoo(
+       const void *key,
+       __attribute__((unused)) uint32_t key_size,
+       __attribute__((unused)) uint32_t seed)
+{
+       const uint32_t *k32 = key;
+       uint32_t ip_dst = rte_be_to_cpu_32(k32[0]);
+       uint32_t signature = (ip_dst >> 2) | ((ip_dst & 0x3) << 30);
+
+       return signature;
+}
+
 void
 app_main_loop_rx_metadata(void) {
        uint32_t i, j;

diff --git a/test/test/test_table.c b/test/test/test_table.c
index f01652d..a4b0ed6 100644 (file)
--- a/test/test/test_table.c
+++ b/test/test/test_table.c
@@ -54,6 +54,17 @@ uint64_t pipeline_test_hash(void *key,
        return signature;
 }
 
+uint32_t pipeline_test_hash_cuckoo(const void *key,
+               __attribute__((unused)) uint32_t key_size,
+               __attribute__((unused)) uint32_t seed)
+{
+       const uint32_t *k32 = key;
+       uint32_t ip_dst = rte_be_to_cpu_32(k32[0]);
+       uint32_t signature = ip_dst;
+
+       return signature;
+}
+
 static void
 app_free_resources(void) {
        int i;

diff --git a/test/test/test_table.h b/test/test/test_table.h
index a4d3ca0..a66342c 100644 (file)
--- a/test/test/test_table.h
+++ b/test/test/test_table.h
@@ -6,6 +6,7 @@
 #include <rte_table_lpm.h>
 #include <rte_table_lpm_ipv6.h>
 #include <rte_table_hash.h>
+#include <rte_table_hash_cuckoo.h>
 #include <rte_table_array.h>
 #include <rte_pipeline.h>
 
@@ -106,6 +107,11 @@ uint64_t pipeline_test_hash(
        __attribute__((unused)) uint32_t key_size,
        __attribute__((unused)) uint64_t seed);
 
+uint32_t pipeline_test_hash_cuckoo(
+       const void *key,
+       __attribute__((unused)) uint32_t key_size,
+       __attribute__((unused)) uint32_t seed);
+
 /* Extern variables */
 extern struct rte_pipeline *p;
 extern struct rte_ring *rings_rx[N_PORTS];

diff --git a/test/test/test_table_combined.c b/test/test/test_table_combined.c
index 5e8e119..73ad015 100644 (file)
--- a/test/test/test_table_combined.c
+++ b/test/test/test_table_combined.c
@@ -778,14 +778,14 @@ test_table_hash_cuckoo_combined(void)
        int status, i;
 
        /* Traffic flow */
-       struct rte_table_hash_params cuckoo_params = {
+       struct rte_table_hash_cuckoo_params cuckoo_params = {
                .name = "TABLE",
                .key_size = 32,
                .key_offset = APP_METADATA_OFFSET(32),
                .key_mask = NULL,
                .n_keys = 1 << 16,
                .n_buckets = 1 << 16,
-               .f_hash = pipeline_test_hash,
+               .f_hash = pipeline_test_hash_cuckoo,
                .seed = 0,
        };
 

diff --git a/test/test/test_table_tables.c b/test/test/test_table_tables.c
index a7a69b8..20df2e9 100644 (file)
--- a/test/test/test_table_tables.c
+++ b/test/test/test_table_tables.c
@@ -903,14 +903,14 @@ test_table_hash_cuckoo(void)
        uint32_t entry_size = 1;
 
        /* Initialize params and create tables */
-       struct rte_table_hash_params cuckoo_params = {
+       struct rte_table_hash_cuckoo_params cuckoo_params = {
                .name = "TABLE",
                .key_size = 32,
                .key_offset = APP_METADATA_OFFSET(32),
                .key_mask = NULL,
                .n_keys = 1 << 16,
                .n_buckets = 1 << 16,
-               .f_hash = (rte_table_hash_op_hash)pipeline_test_hash,
+               .f_hash = pipeline_test_hash_cuckoo,
                .seed = 0, 
        };
 
@@ -941,7 +941,7 @@ test_table_hash_cuckoo(void)
        if (table != NULL)
                return -4;
 
-       cuckoo_params.f_hash = pipeline_test_hash;
+       cuckoo_params.f_hash = pipeline_test_hash_cuckoo;
        cuckoo_params.name = NULL;
 
        table = rte_table_hash_cuckoo_ops.f_create(&cuckoo_params,