[v2] hash: make GFNI stubs inline (again)
Checks
Commit Message
Tyler found build issues with MSVC and the thash gfni stubs.
The problem would be link errors from missing symbols.
This version puts back the rte_thash_gfni function stubs as
inlines, but instead of logging a message, they panic.
This is intentional because any application should be checking
with function rte_thash_gfni_supported() before calling the
hashing functions here. Better to panic then return zero
and put out log message which will be ignored...
Fixes: 07d836e5929d ("hash: uninline GFNI stubs")
Reported-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - uses panic instead of logging.
Please ignore the checkpatch nag about this.
lib/hash/meson.build | 1 -
lib/hash/rte_thash_gfni.c | 51 ---------------------------------------
lib/hash/rte_thash_gfni.h | 34 ++++++++++++++++++--------
lib/hash/version.map | 2 --
4 files changed, 24 insertions(+), 64 deletions(-)
delete mode 100644 lib/hash/rte_thash_gfni.c
Comments
On Mon, Mar 04, 2024 at 07:07:24PM -0800, Stephen Hemminger wrote:
> Tyler found build issues with MSVC and the thash gfni stubs.
> The problem would be link errors from missing symbols.
>
> This version puts back the rte_thash_gfni function stubs as
> inlines, but instead of logging a message, they panic.
> This is intentional because any application should be checking
> with function rte_thash_gfni_supported() before calling the
> hashing functions here. Better to panic then return zero
> and put out log message which will be ignored...
>
> Fixes: 07d836e5929d ("hash: uninline GFNI stubs")
> Reported-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
05/03/2024 04:07, Stephen Hemminger:
> Tyler found build issues with MSVC and the thash gfni stubs.
> The problem would be link errors from missing symbols.
>
> This version puts back the rte_thash_gfni function stubs as
> inlines, but instead of logging a message, they panic.
> This is intentional because any application should be checking
> with function rte_thash_gfni_supported() before calling the
> hashing functions here. Better to panic then return zero
> and put out log message which will be ignored...
I want to be able to grep rte_panic in the lib directory
to find those we should remove.
Having "legit" panic calls is a no-go for me.
@@ -22,7 +22,6 @@ sources = files(
'rte_hash_crc.c',
'rte_fbk_hash.c',
'rte_thash.c',
- 'rte_thash_gfni.c',
)
deps += ['net']
deleted file mode 100644
@@ -1,51 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2021 Intel Corporation
- */
-
-#include <stdbool.h>
-
-#include <rte_log.h>
-#include <rte_thash_gfni.h>
-
-#ifndef RTE_THASH_GFNI_DEFINED
-
-RTE_LOG_REGISTER_SUFFIX(hash_gfni_logtype, gfni, INFO);
-#define RTE_LOGTYPE_HASH hash_gfni_logtype
-#define HASH_LOG(level, ...) \
- RTE_LOG_LINE(level, HASH, "" __VA_ARGS__)
-
-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;
- HASH_LOG(ERR,
- "%s is undefined under given arch", __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;
- HASH_LOG(ERR,
- "%s is undefined under given arch", __func__);
- }
-
- for (i = 0; i < num; i++)
- val[i] = 0;
-}
-
-#endif
@@ -9,8 +9,6 @@
extern "C" {
#endif
-#include <rte_log.h>
-
#ifdef RTE_ARCH_X86
#include <rte_thash_x86_gfni.h>
@@ -19,9 +17,10 @@ extern "C" {
#ifndef RTE_THASH_GFNI_DEFINED
+#include <rte_debug.h>
+
/**
* Calculate Toeplitz hash.
- * Dummy implementation.
*
* @param m
* Pointer to the matrices generated from the corresponding
@@ -31,14 +30,21 @@ extern "C" {
* @param len
* Length of the data to be hashed.
* @return
- * Calculated Toeplitz hash value.
+ * Calculated hash value.
*/
-uint32_t
-rte_thash_gfni(const uint64_t *mtrx, const uint8_t *key, int len);
+static inline uint32_t
+rte_thash_gfni(const uint64_t *mtrx __rte_unused,
+ const uint8_t *key __rte_unused, int len __rte_unused)
+{
+ /*
+ * This stub always panics because the application should be calling
+ * rte_thash_gfni_supported() to check if the arch supports this.
+ */
+ rte_panic("%s is undefined under given arch\n", __func__);
+}
/**
* Bulk implementation for Toeplitz hash.
- * Dummy implementation.
*
* @param m
* Pointer to the matrices generated from the corresponding
@@ -53,9 +59,17 @@ rte_thash_gfni(const uint64_t *mtrx, const uint8_t *key, int len);
* @param num
* Number of tuples to hash.
*/
-void
-rte_thash_gfni_bulk(const uint64_t *mtrx, int len, uint8_t *tuple[],
- uint32_t val[], uint32_t num);
+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[] __rte_unused, uint32_t num __rte_unused)
+{
+ /*
+ * This stub always panics because the application should be calling
+ * rte_thash_gfni_supported() to check if the arch supports this.
+ */
+ rte_panic("%s is undefined under given arch\n", __func__);
+}
#endif /* RTE_THASH_GFNI_DEFINED */
@@ -41,8 +41,6 @@ DPDK_24 {
rte_thash_get_gfni_matrices;
rte_thash_get_helper;
rte_thash_get_key;
- rte_thash_gfni;
- rte_thash_gfni_bulk;
rte_thash_gfni_supported;
rte_thash_init_ctx;