[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 <stephen at 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