[v8,21/22] hash: move rte_hash_set_alg out header

  The code for setting algorithm for hash is not at all perf sensitive,
and doing it inline has a couple of problems. First, it means that if
multiple files include the header, then the initialization gets done
multiple times. But also, it makes it harder to fix usage of RTE_LOG().

Despite what the checking script say. This is not an ABI change, the
previous version inlined the same code; therefore both old and new code
will work the same.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/hash/meson.build    |  1 +
 lib/hash/rte_hash_crc.c | 63 +++++++++++++++++++++++++++++++++++++++++
 lib/hash/rte_hash_crc.h | 46 ++----------------------------
 lib/hash/version.map    |  1 +
 4 files changed, 67 insertions(+), 44 deletions(-)
 create mode 100644 lib/hash/rte_hash_crc.c
  

On Tue, Feb 21, 2023 at 12:38 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The code for setting algorithm for hash is not at all perf sensitive,
> and doing it inline has a couple of problems. First, it means that if
> multiple files include the header, then the initialization gets done
> multiple times. But also, it makes it harder to fix usage of RTE_LOG().
>
> Despite what the checking script say. This is not an ABI change, the
> previous version inlined the same code; therefore both old and new code
> will work the same.

I suppose you are referring to:
http://mails.dpdk.org/archives/test-report/2023-February/356872.html
ERROR: symbol rte_hash_crc_set_alg is added in the DPDK_23 section,
but is expected to be added in the EXPERIMENTAL section of the version
map

I agree that this is irrelevant and can be ignored in this particular case.
  

On Tue, Feb 21, 2023 at 12:38 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> diff --git a/lib/hash/rte_hash_crc.c b/lib/hash/rte_hash_crc.c
> new file mode 100644
> index 000000000000..c59eebccb1eb
> --- /dev/null
> +++ b/lib/hash/rte_hash_crc.c
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2014 Intel Corporation
> + */
> +
> +#include <rte_cpuflags.h>
> +#include <rte_log.h>
> +
> +#include "rte_hash_crc.h"
> +
> +/**
> + * Allow or disallow use of SSE4.2/ARMv8 intrinsics for CRC32 hash
> + * calculation.
> + *
> + * @param alg
> + *   An OR of following flags:
> + *   - (CRC32_SW) Don't use SSE4.2/ARMv8 intrinsics (default non-[x86/ARMv8])
> + *   - (CRC32_SSE42) Use SSE4.2 intrinsics if available
> + *   - (CRC32_SSE42_x64) Use 64-bit SSE4.2 intrinsic if available (default x86)
> + *   - (CRC32_ARM64) Use ARMv8 CRC intrinsic if available (default ARMv8)
> + *
> + */
> +void
> +rte_hash_crc_set_alg(uint8_t alg)
> +{
> +       crc32_alg = CRC32_SW;
> +
> +       if (alg == CRC32_SW)
> +               return;
> +
> +#if defined RTE_ARCH_X86
> +       if (!(alg & CRC32_SSE42_x64))
> +               RTE_LOG(WARNING, HASH,
> +                       "Unsupported CRC32 algorithm requested using CRC32_x64/CRC32_SSE42\n");
> +       if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T) || alg == CRC32_SSE42)
> +               crc32_alg = CRC32_SSE42;
> +       else
> +               crc32_alg = CRC32_SSE42_x64;
> +#endif
> +
> +#if defined RTE_ARCH_ARM64
> +       if (!(alg & CRC32_ARM64))
> +               RTE_LOG(WARNING, HASH,
> +                       "Unsupported CRC32 algorithm requested using CRC32_ARM64\n");
> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_CRC32))
> +               crc32_alg = CRC32_ARM64;
> +#endif
> +
> +       if (crc32_alg == CRC32_SW)
> +               RTE_LOG(WARNING, HASH,
> +                       "Unsupported CRC32 algorithm requested using CRC32_SW\n");
> +}
> +
> +/* Setting the best available algorithm */
> +RTE_INIT(rte_hash_crc_init_alg)
> +{
> +#if defined(RTE_ARCH_X86)
> +       rte_hash_crc_set_alg(CRC32_SSE42_x64);
> +#elif defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_CRC32)
> +       rte_hash_crc_set_alg(CRC32_ARM64);
> +#else
> +       rte_hash_crc_set_alg(CRC32_SW);
> +#endif
> +}
> diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
> index 0249ad16c5b6..e4acd99a0c81 100644
> --- a/lib/hash/rte_hash_crc.h
> +++ b/lib/hash/rte_hash_crc.h
> @@ -20,8 +20,6 @@ extern "C" {
>  #include <rte_branch_prediction.h>
>  #include <rte_common.h>
>  #include <rte_config.h>
> -#include <rte_cpuflags.h>
> -#include <rte_log.h>
>
>  #include "rte_crc_sw.h"
>
> @@ -53,48 +51,8 @@ static uint8_t crc32_alg = CRC32_SW;
>   *   - (CRC32_ARM64) Use ARMv8 CRC intrinsic if available (default ARMv8)
>   *
>   */
> -static inline void
> -rte_hash_crc_set_alg(uint8_t alg)
> -{
> -       crc32_alg = CRC32_SW;
> -
> -       if (alg == CRC32_SW)
> -               return;
> -
> -#if defined RTE_ARCH_X86
> -       if (!(alg & CRC32_SSE42_x64))
> -               RTE_LOG(WARNING, HASH,
> -                       "Unsupported CRC32 algorithm requested using CRC32_x64/CRC32_SSE42\n");
> -       if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T) || alg == CRC32_SSE42)
> -               crc32_alg = CRC32_SSE42;
> -       else
> -               crc32_alg = CRC32_SSE42_x64;
> -#endif
> -
> -#if defined RTE_ARCH_ARM64
> -       if (!(alg & CRC32_ARM64))
> -               RTE_LOG(WARNING, HASH,
> -                       "Unsupported CRC32 algorithm requested using CRC32_ARM64\n");
> -       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_CRC32))
> -               crc32_alg = CRC32_ARM64;
> -#endif
> -
> -       if (crc32_alg == CRC32_SW)
> -               RTE_LOG(WARNING, HASH,
> -                       "Unsupported CRC32 algorithm requested using CRC32_SW\n");
> -}
> -
> -/* Setting the best available algorithm */
> -RTE_INIT(rte_hash_crc_init_alg)
> -{
> -#if defined(RTE_ARCH_X86)
> -       rte_hash_crc_set_alg(CRC32_SSE42_x64);
> -#elif defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_CRC32)
> -       rte_hash_crc_set_alg(CRC32_ARM64);
> -#else
> -       rte_hash_crc_set_alg(CRC32_SW);
> -#endif
> -}
> +void
> +rte_hash_crc_set_alg(uint8_t alg);

There is likely something unfinished with this code move.
See test report from GHA.
http://mails.dpdk.org/archives/test-report/2023-February/356932.html

[174/3761] Compiling C object 'lib/76b5a35@@rte_hash at
sta/hash_rte_fbk_hash.c.o'.
FAILED: lib/76b5a35@@rte_hash at sta/hash_rte_fbk_hash.c.o
ccache powerpc64le-linux-gnu-gcc -Ilib/76b5a35@@rte_hash at sta -Ilib
-I../lib -Ilib/hash -I../lib/hash -I. -I../ -Iconfig -I../config
-Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include
-I../lib/eal/linux/include -Ilib/eal/ppc/include
-I../lib/eal/ppc/include -Ilib/eal/common -I../lib/eal/common
-Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs
-Ilib/telemetry/../metrics -I../lib/telemetry/../metrics
-Ilib/telemetry -I../lib/telemetry -Ilib/net -I../lib/net -Ilib/mbuf
-I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring
-Ilib/rcu -I../lib/rcu -fdiagnostics-color=always -pipe
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -O2 -g
-include rte_config.h -Wcast-qual -Wdeprecated -Wformat
-Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition
-Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef
-Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -mcpu=power8
-mtune=power8 -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API
-Wno-format-truncation -DRTE_LOG_DEFAULT_LOGTYPE=lib.hash -MD -MQ
'lib/76b5a35@@rte_hash at sta/hash_rte_fbk_hash.c.o' -MF
'lib/76b5a35@@rte_hash at sta/hash_rte_fbk_hash.c.o.d' -o
'lib/76b5a35@@rte_hash at sta/hash_rte_fbk_hash.c.o' -c
../lib/hash/rte_fbk_hash.c
In file included from ../lib/hash/rte_fbk_hash.h:27,
                 from ../lib/hash/rte_fbk_hash.c:21:
../lib/hash/rte_hash_crc.h:32:16: error: ‘crc32_alg’ defined but not
used [-Werror=unused-variable]
   32 | static uint8_t crc32_alg = CRC32_SW;
      |                ^~~~~~~~~
cc1: all warnings being treated as errors
[175/3761] Compiling C object 'lib/76b5a35@@rte_hash at
sta/hash_rte_cuckoo_hash.c.o'.
FAILED: lib/76b5a35@@rte_hash at sta/hash_rte_cuckoo_hash.c.o
ccache powerpc64le-linux-gnu-gcc -Ilib/76b5a35@@rte_hash at sta -Ilib
-I../lib -Ilib/hash -I../lib/hash -I. -I../ -Iconfig -I../config
-Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include
-I../lib/eal/linux/include -Ilib/eal/ppc/include
-I../lib/eal/ppc/include -Ilib/eal/common -I../lib/eal/common
-Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs
-Ilib/telemetry/../metrics -I../lib/telemetry/../metrics
-Ilib/telemetry -I../lib/telemetry -Ilib/net -I../lib/net -Ilib/mbuf
-I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring
-Ilib/rcu -I../lib/rcu -fdiagnostics-color=always -pipe
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -O2 -g
-include rte_config.h -Wcast-qual -Wdeprecated -Wformat
-Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition
-Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef
-Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -mcpu=power8
-mtune=power8 -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API
-Wno-format-truncation -DRTE_LOG_DEFAULT_LOGTYPE=lib.hash -MD -MQ
'lib/76b5a35@@rte_hash at sta/hash_rte_cuckoo_hash.c.o' -MF
'lib/76b5a35@@rte_hash at sta/hash_rte_cuckoo_hash.c.o.d' -o
'lib/76b5a35@@rte_hash at sta/hash_rte_cuckoo_hash.c.o' -c
../lib/hash/rte_cuckoo_hash.c
In file included from ../lib/hash/rte_cuckoo_hash.h:43,
                 from ../lib/hash/rte_cuckoo_hash.c:32:
../lib/hash/rte_hash_crc.h:32:16: error: ‘crc32_alg’ defined but not
used [-Werror=unused-variable]
   32 | static uint8_t crc32_alg = CRC32_SW;
      |                ^~~~~~~~~
cc1: all warnings being treated as errors
[176/3761] Generating ethdev.sym_chk with a meson_exe.py custom command.
ninja: build stopped: subcommand failed.
##[error]Process completed with exit code 1.