[dpdk-dev] [RFC] hash: unify crc32 API header for x86 and ARM

[Pavan Nikhilesh Bhagavatula]

>> From: Pavan Nikhilesh <pbhagavatula at marvell.com>
>>
>> Merge crc32 hash calculation public API headers for x86 and ARM,
>> split implementations of x86 and ARM into their respective private
>> headers.
>> This reduces the ifdef code clutter while keeping current ABI intact.
>>
>> Although we install `rte_crc_arm64.h` it is not used in any of the lib or
>> drivers layers. All the libs and drivers use `rte_hash_crc.h` which falls
>> back to SW crc32 calculation for ARM platform.
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
>> ---
>>
>>  Currently, if application incorrectly sets CRC32_ARM64 as crc32
>algorithm
>>  through `rte_hash_crc_set_alg()` on x86 or vice-versa we fallback to
>algorithm
>>  set previously via `rte_hash_crc_set_alg()` instead of setting the best
>>  available.
>>  This behaviour should probably change to setting the best available
>algorithm
>>  and is up for discussion.
>>
>>  app/test/test_hash.c            |   6 +
>>  lib/librte_hash/Makefile        |   5 -
>>  lib/librte_hash/crc_arm64.h     |  67 +++++++++++
>>  lib/librte_hash/crc_x86.h       |  68 +++++++++++
>>  lib/librte_hash/meson.build     |   3 +-
>>  lib/librte_hash/rte_crc_arm64.h | 183 ------------------------------
>>  lib/librte_hash/rte_hash_crc.h  | 193 +++++++++++++------------------
>-
>>  7 files changed, 219 insertions(+), 306 deletions(-)
>>  create mode 100644 lib/librte_hash/crc_arm64.h
>>  create mode 100644 lib/librte_hash/crc_x86.h
>>  delete mode 100644 lib/librte_hash/rte_crc_arm64.h
>>
>> diff --git a/app/test/test_hash.c b/app/test/test_hash.c
>> index afa3a1a3c..7bd457dac 100644
>> --- a/app/test/test_hash.c
>> +++ b/app/test/test_hash.c
>> @@ -195,7 +195,13 @@ test_crc32_hash_alg_equiv(void)
>>  	}
>>
>>  	/* Resetting to best available algorithm */
>> +#if defined RTE_ARCH_X86
>>  	rte_hash_crc_set_alg(CRC32_SSE42_x64);
>> +#elif defined RTE_ARCH_ARM64
>> +	rte_hash_crc_set_alg(CRC32_ARM64);
>> +#else
>> +	rte_hash_crc_set_alg(CRC32_SW);
>> +#endif
>>
>>  	if (i == CRC32_ITERATIONS)
>>  		return 0;
>> diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile
>> index ec9f86499..f640afc42 100644
>> --- a/lib/librte_hash/Makefile
>> +++ b/lib/librte_hash/Makefile
>> @@ -19,11 +19,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) +=
>rte_fbk_hash.c
>>  # install this header file
>>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include := rte_hash.h
>>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_hash_crc.h
>> -ifeq ($(CONFIG_RTE_ARCH_ARM64),y)
>> -ifneq ($(findstring RTE_MACHINE_CPUFLAG_CRC32,$(CFLAGS)),)
>> -SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_crc_arm64.h
>> -endif
>> -endif
>>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_jhash.h
>>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_thash.h
>>  SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_fbk_hash.h
>> diff --git a/lib/librte_hash/crc_arm64.h b/lib/librte_hash/crc_arm64.h
>> new file mode 100644
>> index 000000000..8e75f8297
>
>Wouldn't that break 'make  install T=...'?

My bad I verified with meson and it was building fine.

>As now rte_hash_crc.h includes not public headers (crc_x86.h, etc.).
>Same question about external apps, where they would get from these
>headers?

I think in the next version we can directly have the arch specific functions
Implemented in rte_hash_crc.h. Since its pretty stable code and overhead of extra 
~120 lines.

>
>> --- /dev/null
>> +++ b/lib/librte_hash/crc_arm64.h
>> @@ -0,0 +1,67 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2015 Cavium, Inc
>> + */
>> +