[dpdk-dev] [PATCH v2 1/3] lib/librte_table: add hash_func header files

[Gavin Hu (Arm Technology China)]

The code looks good, some comments to the commit message.

> -----Original Message-----
> From: Kevin Laatz <kevin.laatz at intel.com>
> Sent: Friday, September 7, 2018 4:05 PM
> To: dev at dpdk.org
> Cc: cristian.dumitrescu at intel.com; Jianbo Liu <Jianbo.Liu at arm.com>; Gavin
> Hu (Arm Technology China) <Gavin.Hu at arm.com>;
> jerin.jacob at caviumnetworks.com; Kevin Laatz <kevin.laatz at intel.com>
> Subject: [PATCH v2 1/3] lib/librte_table: add hash_func header files
>
> This commit adds rte_table_hash_func.h and rte_table_hash_func_arm64.h
> to librte_table. This will replace hash_func.h and hash_func_arm64.h in the
> IP Pipeline application and in SoftNIC. This also adds a scalar implementation
> of the x86_64 intrinsic for crc32 as a generic fallback.
It is better to mention this patch reduces code duplication by removing duplicate headers files
Within two folders and consolidating them into a single one.
>
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu at intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz at intel.com>
Acked-by: Gavin Hu <gavin.hu at arm.com>
> ---
> v2:
>    - Fixed typo in commit message
>    - Fixed typo in arm header that caused some compilations to fail for the
>      entire patchset
> ---
>  lib/librte_table/Makefile                    |   2 +
>  lib/librte_table/meson.build                 |   2 +
>  lib/librte_table/rte_table_hash_func.h       | 244
> +++++++++++++++++++++++++++
>  lib/librte_table/rte_table_hash_func_arm64.h |  21 +++
>  lib/librte_table/rte_table_version.map       |  14 ++
>  5 files changed, 283 insertions(+)
>  create mode 100644 lib/librte_table/rte_table_hash_func.h
>  create mode 100644 lib/librte_table/rte_table_hash_func_arm64.h
>
> diff --git a/lib/librte_table/Makefile b/lib/librte_table/Makefile index
> 276d476..f935678 100644
> --- a/lib/librte_table/Makefile
> +++ b/lib/librte_table/Makefile
> @@ -46,6 +46,8 @@ 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_table_hash_func.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_TABLE)-include +=
> +rte_table_hash_func_arm64.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 8b2f841..6ae3cd6 100644
> --- a/lib/librte_table/meson.build
> +++ b/lib/librte_table/meson.build
> @@ -19,6 +19,8 @@ headers = files('rte_table.h',
>  'rte_table_lpm_ipv6.h',
>  'rte_table_hash.h',
>  'rte_table_hash_cuckoo.h',
> +'rte_table_hash_func.h',
> +'rte_table_hash_func_arm64.h',
>  'rte_lru.h',
>  'rte_table_array.h',
>  'rte_table_stub.h')
> diff --git a/lib/librte_table/rte_table_hash_func.h
> b/lib/librte_table/rte_table_hash_func.h
> new file mode 100644
> index 0000000..4eadbfb
> --- /dev/null
> +++ b/lib/librte_table/rte_table_hash_func.h
> @@ -0,0 +1,244 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2018 Intel Corporation  */
> +
> +#ifndef __INCLUDE_RTE_TABLE_HASH_FUNC_H__ #define
> +__INCLUDE_RTE_TABLE_HASH_FUNC_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +
> +#include <rte_common.h>
> +
> +static inline uint64_t
> +rte_crc32_u64_generic(uint64_t crc, uint64_t value) {
> +int i;
> +
> +crc = (crc & 0xFFFFFFFFLLU) ^ value;
> +for (i = 63; i >= 0; i--) {
> +uint64_t mask;
> +
> +mask = -(crc & 1LLU);
> +crc = (crc >> 1LLU) ^ (0x82F63B78LLU & mask);
> +}
> +
> +return crc;
> +}
> +
> +#if defined(RTE_ARCH_X86_64)
> +
> +#include <x86intrin.h>
> +
> +static inline uint64_t
> +rte_crc32_u64(uint64_t crc, uint64_t v) {
> +return _mm_crc32_u64(crc, v);
> +}
> +
> +#elif defined(RTE_ARCH_ARM64)
> +#include "rte_table_hash_func_arm64.h"
> +#else
> +
> +static inline uin64_t
> +rte_crc32_u64(uint64_t crc, uint64_t v) {
> +return rte_crc32_u64_generic(crc, v);
> +}
> +
> +#endif
> +
> +static inline uint64_t
> +rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t crc0;
> +
> +crc0 = rte_crc32_u64(seed, k[0] & m[0]);
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key16(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, crc0, crc1;
> +
> +k0 = k[0] & m[0];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key24(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, crc0, crc1;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc0 = rte_crc32_u64(crc0, k2);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key32(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, crc0, crc1, crc2, crc3;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc2 = rte_crc32_u64(k2, k[3] & m[3]);
> +crc3 = k2 >> 32;
> +
> +crc0 = rte_crc32_u64(crc0, crc1);
> +crc1 = rte_crc32_u64(crc2, crc3);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key40(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, crc0, crc1, crc2, crc3;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc2 = rte_crc32_u64(k2, k[3] & m[3]);
> +crc3 = rte_crc32_u64(k2 >> 32, k[4] & m[4]);
> +
> +crc0 = rte_crc32_u64(crc0, crc1);
> +crc1 = rte_crc32_u64(crc2, crc3);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key48(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, k5, crc0, crc1, crc2, crc3;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +k5 = k[5] & m[5];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc2 = rte_crc32_u64(k2, k[3] & m[3]);
> +crc3 = rte_crc32_u64(k2 >> 32, k[4] & m[4]);
> +
> +crc0 = rte_crc32_u64(crc0, (crc1 << 32) ^ crc2);
> +crc1 = rte_crc32_u64(crc3, k5);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key56(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +k5 = k[5] & m[5];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc2 = rte_crc32_u64(k2, k[3] & m[3]);
> +crc3 = rte_crc32_u64(k2 >> 32, k[4] & m[4]);
> +
> +crc4 = rte_crc32_u64(k5, k[6] & m[6]);
> +crc5 = k5 >> 32;
> +
> +crc0 = rte_crc32_u64(crc0, (crc1 << 32) ^ crc2);
> +crc1 = rte_crc32_u64(crc3, (crc4 << 32) ^ crc5);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +static inline uint64_t
> +rte_table_hash_crc_key64(void *key, void *mask, __rte_unused uint32_t
> key_size,
> +uint64_t seed)
> +{
> +uint64_t *k = key;
> +uint64_t *m = mask;
> +uint64_t k0, k2, k5, crc0, crc1, crc2, crc3, crc4, crc5;
> +
> +k0 = k[0] & m[0];
> +k2 = k[2] & m[2];
> +k5 = k[5] & m[5];
> +
> +crc0 = rte_crc32_u64(k0, seed);
> +crc1 = rte_crc32_u64(k0 >> 32, k[1] & m[1]);
> +
> +crc2 = rte_crc32_u64(k2, k[3] & m[3]);
> +crc3 = rte_crc32_u64(k2 >> 32, k[4] & m[4]);
> +
> +crc4 = rte_crc32_u64(k5, k[6] & m[6]);
> +crc5 = rte_crc32_u64(k5 >> 32, k[7] & m[7]);
> +
> +crc0 = rte_crc32_u64(crc0, (crc1 << 32) ^ crc2);
> +crc1 = rte_crc32_u64(crc3, (crc4 << 32) ^ crc5);
> +
> +crc0 ^= crc1;
> +
> +return crc0;
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/lib/librte_table/rte_table_hash_func_arm64.h
> b/lib/librte_table/rte_table_hash_func_arm64.h
> new file mode 100644
> index 0000000..eb04c1f
> --- /dev/null
> +++ b/lib/librte_table/rte_table_hash_func_arm64.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2017-2018 Linaro Limited  */
> +
> +#ifndef __INCLUDE_RTE_TABLE_HASH_FUNC_ARM64_H__
> +#define __INCLUDE_RTE_TABLE_HASH_FUNC_ARM64_H__
> +
> +#define _CRC32CX(crc, val)\
> +__asm__("crc32cx %w[c], %w[c], %x[v]":[c] "+r" (crc):[v] "r" (val))
> +
> +static inline uint64_t
> +rte_crc32_u64(uint64_t crc, uint64_t v) {
> +uint32_t crc32 = crc;
> +
> +_CRC32CX(crc32, v);
> +
> +return crc32;
> +}
> +
> +#endif
> diff --git a/lib/librte_table/rte_table_version.map
> b/lib/librte_table/rte_table_version.map
> index 6237252..dab339e 100644
> --- a/lib/librte_table/rte_table_version.map
> +++ b/lib/librte_table/rte_table_version.map
> @@ -18,3 +18,17 @@ DPDK_17.11 {
>
>  local: *;
>  };
> +
> +DPDK_18.11 {
> +global:
> +
> +rte_crc32_u64_generic;
> +rte_table_hash_crc_key8;
> +rte_table_hash_crc_key16;
> +rte_table_hash_crc_key24;
> +rte_table_hash_crc_key32;
> +rte_table_hash_crc_key40;
> +rte_table_hash_crc_key48;
> +rte_table_hash_crc_key56;
> +rte_table_hash_crc_key64;
> +} DPDK_17.11;
> --
> 2.9.5

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.