Re: [PATCH v7 20/22] hash: move rte_thash_gfni stubs out of header file

[Medvedkin, Vladimir]

Hi Stepthen,

There is a problem with the build on a machine that supports GFNI:
../lib/hash/rte_thash_gfni.c:15:1: error: redefinition of ‘rte_thash_gfni’
   15 | rte_thash_gfni(const uint64_t *mtrx __rte_unused,
      | ^~~~~~~~~~~~~~
In file included from ../lib/hash/rte_thash_gfni.h:13,
                 from ../lib/hash/rte_thash_gfni.c:9:
../lib/hash/rte_thash_x86_gfni.h:180:1: note: previous definition of 
‘rte_thash_gfni’ was here
  180 | rte_thash_gfni(const uint64_t *m, const uint8_t *tuple, int len)
      | ^~~~~~~~~~~~~~


I believe you forgot to frame dummy functions with #ifndef 
RTE_THASH_GFNI_DEFINED


On 15/02/2023 17:23, Stephen Hemminger wrote:
Having stubs in header file makes it harder to update
RTE_LOG(). Also modify to only print warning once.

Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
---
  lib/hash/meson.build      |  8 +++++++-
  lib/hash/rte_thash_gfni.c | 43 +++++++++++++++++++++++++++++++++++++++
  lib/hash/rte_thash_gfni.h | 28 +++++--------------------
  lib/hash/version.map      |  4 ++++
  4 files changed, 59 insertions(+), 24 deletions(-)
  create mode 100644 lib/hash/rte_thash_gfni.c

diff --git a/lib/hash/meson.build b/lib/hash/meson.build
index 2f757d45f9bc..e56ee8572564 100644
--- a/lib/hash/meson.build
+++ b/lib/hash/meson.build
@@ -17,7 +17,13 @@ indirect_headers += files(
          'rte_thash_x86_gfni.h',
  )
  
-sources = files('rte_cuckoo_hash.c', 'rte_fbk_hash.c', 'rte_thash.c')
+sources = files(
+    'rte_cuckoo_hash.c',
+    'rte_fbk_hash.c',
+    'rte_thash.c',
+    'rte_thash_gfni.c'
+)
+
  deps += ['net']
  deps += ['ring']
  deps += ['rcu']
diff --git a/lib/hash/rte_thash_gfni.c b/lib/hash/rte_thash_gfni.c
new file mode 100644
index 000000000000..7617b9d4f630
--- /dev/null
+++ b/lib/hash/rte_thash_gfni.c
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <stdbool.h>
+
+#include <rte_compat.h>
+#include <rte_log.h>
+#include <rte_thash_gfni.h>
+
+uint32_t
+rte_thash_gfni(const uint64_t *mtrx __rte_unused,
+              const uint8_t *key __rte_unused, int len __rte_unused)
+{
+       static bool warned;
+
+       if (!warned) {
+               warned = true;
+               RTE_LOG(ERR, HASH,
+                       "%s is undefined under given arch\n", __func__);
+       }
+
+       return 0;
+}
+
+void
+rte_thash_gfni_bulk(const uint64_t *mtrx __rte_unused,
+                   int len __rte_unused, uint8_t *tuple[] __rte_unused,
+                   uint32_t val[], uint32_t num)
+{
+       unsigned int i;
+
+       static bool warned;
+
+       if (!warned) {
+               warned = true;
+               RTE_LOG(ERR, HASH,
+                       "%s is undefined under given arch\n", __func__);
+       }
+
+       for (i = 0; i < num; i++)
+               val[i] = 0;
+}
diff --git a/lib/hash/rte_thash_gfni.h b/lib/hash/rte_thash_gfni.h
index ef90faa302d1..86208eb45ebb 100644
--- a/lib/hash/rte_thash_gfni.h
+++ b/lib/hash/rte_thash_gfni.h
@@ -9,13 +9,8 @@
  extern "C" {
  #endif
  
-#include <rte_compat.h>
-#include <rte_log.h>
-
  #ifdef RTE_ARCH_X86
-
  #include <rte_thash_x86_gfni.h>
-
  #endif
  
  #ifndef RTE_THASH_GFNI_DEFINED
@@ -38,13 +33,8 @@ extern "C" {
   *  Calculated Toeplitz hash value.
   */
  __rte_experimental
-static inline uint32_t
-rte_thash_gfni(const uint64_t *mtrx __rte_unused,
-       const uint8_t *key __rte_unused, int len __rte_unused)
-{
-       RTE_LOG(ERR, HASH, "%s is undefined under given arch\n", __func__);
-       return 0;
-}
+uint32_t
+rte_thash_gfni(const uint64_t *mtrx, const uint8_t *key, int len);
  
  /**
   * Bulk implementation for Toeplitz hash.
@@ -67,17 +57,9 @@ rte_thash_gfni(const uint64_t *mtrx __rte_unused,
   *  Number of tuples to hash.
   */
  __rte_experimental
-static inline void
-rte_thash_gfni_bulk(const uint64_t *mtrx __rte_unused,
-       int len __rte_unused, uint8_t *tuple[] __rte_unused,
-       uint32_t val[], uint32_t num)
-{
-       unsigned int i;
-
-       RTE_LOG(ERR, HASH, "%s is undefined under given arch\n", __func__);
-       for (i = 0; i < num; i++)
-               val[i] = 0;
-}
+void
+rte_thash_gfni_bulk(const uint64_t *mtrx, int len, uint8_t *tuple[],
+                   uint32_t val[], uint32_t num);
  
  #endif /* RTE_THASH_GFNI_DEFINED */
  
diff --git a/lib/hash/version.map b/lib/hash/version.map
index bdcebd19c29b..f03b047b2eec 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -51,4 +51,8 @@ EXPERIMENTAL {
        rte_thash_complete_matrix;
        rte_thash_get_gfni_matrices;
        rte_thash_gfni_supported;
+
+       # added in 22.07
+       rte_thash_gfni;
+       rte_thash_gfni_bulk;
  };

--
Regards,
Vladimir