DPDK patches and discussions
 help / color / mirror / Atom feed
From: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
To: dev@dpdk.org
Cc: ferruh.yigit@amd.com, kai.ji@intel.com, brian.dooley@intel.com,
	Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
Subject: [PATCH v2 1/3] net: add thread-safe crc api
Date: Tue,  1 Oct 2024 19:11:48 +0100	[thread overview]
Message-ID: <20241001181150.43506-2-arkadiuszx.kusztal@intel.com> (raw)
In-Reply-To: <20241001181150.43506-1-arkadiuszx.kusztal@intel.com>

The current net CRC API is not thread-safe, this patch
solves this by adding another, thread-safe API functions.
This API is also safe to use across multiple processes,
yet with limitations on max-simd-bitwidth, which will be checked only by
the process that created the CRC context; all other processes will use
the same CRC function when used with the same CRC context.
It is an undefined behavior when process binaries are compiled
with different SIMD capabilities when the same CRC context is used.

Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/net/net_crc.h     |  19 ++--
 lib/net/rte_net_crc.c | 309 +++++++++++++++-----------------------------------
 lib/net/rte_net_crc.h |  40 ++-----
 lib/net/version.map   |  18 ++-
 4 files changed, 124 insertions(+), 262 deletions(-)

diff --git a/lib/net/net_crc.h b/lib/net/net_crc.h
index 7a74d5406c..d220200685 100644
--- a/lib/net/net_crc.h
+++ b/lib/net/net_crc.h
@@ -5,40 +5,41 @@
 #ifndef _NET_CRC_H_
 #define _NET_CRC_H_
 
-/*
- * Different implementations of CRC
- */
-
-/* SSE4.2 */
+#include <rte_compat.h>
 
+__rte_internal
 void
 rte_net_crc_sse42_init(void);
 
+__rte_internal
 uint32_t
 rte_crc16_ccitt_sse42_handler(const uint8_t *data, uint32_t data_len);
 
+__rte_internal
 uint32_t
 rte_crc32_eth_sse42_handler(const uint8_t *data, uint32_t data_len);
 
-/* AVX512 */
-
+__rte_internal
 void
 rte_net_crc_avx512_init(void);
 
+__rte_internal
 uint32_t
 rte_crc16_ccitt_avx512_handler(const uint8_t *data, uint32_t data_len);
 
+__rte_internal
 uint32_t
 rte_crc32_eth_avx512_handler(const uint8_t *data, uint32_t data_len);
 
-/* NEON */
-
+__rte_internal
 void
 rte_net_crc_neon_init(void);
 
+__rte_internal
 uint32_t
 rte_crc16_ccitt_neon_handler(const uint8_t *data, uint32_t data_len);
 
+__rte_internal
 uint32_t
 rte_crc32_eth_neon_handler(const uint8_t *data, uint32_t data_len);
 
diff --git a/lib/net/rte_net_crc.c b/lib/net/rte_net_crc.c
index 346c285c15..da2f014d26 100644
--- a/lib/net/rte_net_crc.c
+++ b/lib/net/rte_net_crc.c
@@ -2,7 +2,6 @@
  * Copyright(c) 2017-2020 Intel Corporation
  */
 
-#include <stddef.h>
 #include <stdint.h>
 
 #include <rte_cpuflags.h>
@@ -13,81 +12,26 @@
 
 #include "net_crc.h"
 
-/** CRC polynomials */
+RTE_LOG_REGISTER_DEFAULT(libnet_logtype, INFO);
+#define RTE_LOGTYPE_NET libnet_logtype
+
+#define NET_LOG(level, ...) \
+	RTE_LOG_LINE_PREFIX(level, NET, "%s(): ", __func__, __VA_ARGS__)
+
 #define CRC32_ETH_POLYNOMIAL 0x04c11db7UL
 #define CRC16_CCITT_POLYNOMIAL 0x1021U
-
 #define CRC_LUT_SIZE 256
 
-/* crc tables */
 static uint32_t crc32_eth_lut[CRC_LUT_SIZE];
 static uint32_t crc16_ccitt_lut[CRC_LUT_SIZE];
 
-static uint32_t
-rte_crc16_ccitt_default_handler(const uint8_t *data, uint32_t data_len);
-
-static uint32_t
-rte_crc32_eth_default_handler(const uint8_t *data, uint32_t data_len);
-
-static uint32_t
-rte_crc16_ccitt_handler(const uint8_t *data, uint32_t data_len);
-
-static uint32_t
-rte_crc32_eth_handler(const uint8_t *data, uint32_t data_len);
-
-typedef uint32_t
-(*rte_net_crc_handler)(const uint8_t *data, uint32_t data_len);
-
-static rte_net_crc_handler handlers_default[] = {
-	[RTE_NET_CRC16_CCITT] = rte_crc16_ccitt_default_handler,
-	[RTE_NET_CRC32_ETH] = rte_crc32_eth_default_handler,
-};
-
-static const rte_net_crc_handler *handlers = handlers_default;
-
-static const rte_net_crc_handler handlers_scalar[] = {
-	[RTE_NET_CRC16_CCITT] = rte_crc16_ccitt_handler,
-	[RTE_NET_CRC32_ETH] = rte_crc32_eth_handler,
-};
-#ifdef CC_X86_64_AVX512_VPCLMULQDQ_SUPPORT
-static const rte_net_crc_handler handlers_avx512[] = {
-	[RTE_NET_CRC16_CCITT] = rte_crc16_ccitt_avx512_handler,
-	[RTE_NET_CRC32_ETH] = rte_crc32_eth_avx512_handler,
-};
-#endif
-#ifdef CC_X86_64_SSE42_PCLMULQDQ_SUPPORT
-static const rte_net_crc_handler handlers_sse42[] = {
-	[RTE_NET_CRC16_CCITT] = rte_crc16_ccitt_sse42_handler,
-	[RTE_NET_CRC32_ETH] = rte_crc32_eth_sse42_handler,
-};
-#endif
-#ifdef CC_ARM64_NEON_PMULL_SUPPORT
-static const rte_net_crc_handler handlers_neon[] = {
-	[RTE_NET_CRC16_CCITT] = rte_crc16_ccitt_neon_handler,
-	[RTE_NET_CRC32_ETH] = rte_crc32_eth_neon_handler,
-};
-#endif
-
-static uint16_t max_simd_bitwidth;
-
-RTE_LOG_REGISTER_DEFAULT(libnet_logtype, INFO);
-#define RTE_LOGTYPE_NET libnet_logtype
-
-#define NET_LOG(level, ...) \
-	RTE_LOG_LINE_PREFIX(level, NET, "%s(): ", __func__, __VA_ARGS__)
-
-/* Scalar handling */
+static struct
+{
+	uint32_t (*f[RTE_NET_CRC_REQS])
+		(const uint8_t *data, uint32_t data_len);
+} handlers[RTE_NET_CRC_AVX512 + 1];
 
-/**
- * Reflect the bits about the middle
- *
- * @param val
- *   value to be reflected
- *
- * @return
- *   reflected value
- */
-static uint32_t
+static inline uint32_t
 reflect_32bits(uint32_t val)
 {
 	uint32_t i, res = 0;
@@ -99,26 +43,7 @@ reflect_32bits(uint32_t val)
 	return res;
 }
 
-static void
-crc32_eth_init_lut(uint32_t poly,
-	uint32_t *lut)
-{
-	uint32_t i, j;
-
-	for (i = 0; i < CRC_LUT_SIZE; i++) {
-		uint32_t crc = reflect_32bits(i);
-
-		for (j = 0; j < 8; j++) {
-			if (crc & 0x80000000L)
-				crc = (crc << 1) ^ poly;
-			else
-				crc <<= 1;
-		}
-		lut[i] = reflect_32bits(crc);
-	}
-}
-
-static __rte_always_inline uint32_t
+static inline uint32_t
 crc32_eth_calc_lut(const uint8_t *data,
 	uint32_t data_len,
 	uint32_t crc,
@@ -130,20 +55,9 @@ crc32_eth_calc_lut(const uint8_t *data,
 	return crc;
 }
 
-static void
-rte_net_crc_scalar_init(void)
-{
-	/* 32-bit crc init */
-	crc32_eth_init_lut(CRC32_ETH_POLYNOMIAL, crc32_eth_lut);
-
-	/* 16-bit CRC init */
-	crc32_eth_init_lut(CRC16_CCITT_POLYNOMIAL << 16, crc16_ccitt_lut);
-}
-
 static inline uint32_t
-rte_crc16_ccitt_handler(const uint8_t *data, uint32_t data_len)
+crc16_ccitt(const uint8_t *data, uint32_t data_len)
 {
-	/* return 16-bit CRC value */
 	return (uint16_t)~crc32_eth_calc_lut(data,
 		data_len,
 		0xffff,
@@ -151,16 +65,42 @@ rte_crc16_ccitt_handler(const uint8_t *data, uint32_t data_len)
 }
 
 static inline uint32_t
-rte_crc32_eth_handler(const uint8_t *data, uint32_t data_len)
+crc32_eth(const uint8_t *data, uint32_t data_len)
 {
-	/* return 32-bit CRC value */
 	return ~crc32_eth_calc_lut(data,
 		data_len,
 		0xffffffffUL,
 		crc32_eth_lut);
 }
 
-/* AVX512/VPCLMULQDQ handling */
+static void
+crc32_eth_init_lut(uint32_t poly,
+	uint32_t *lut)
+{
+	uint32_t i, j;
+
+	for (i = 0; i < CRC_LUT_SIZE; i++) {
+		uint32_t crc = reflect_32bits(i);
+
+		for (j = 0; j < 8; j++) {
+			if (crc & 0x80000000L)
+				crc = (crc << 1) ^ poly;
+			else
+				crc <<= 1;
+		}
+		lut[i] = reflect_32bits(crc);
+	}
+}
+
+static void
+crc_scalar_init(void)
+{
+	crc32_eth_init_lut(CRC32_ETH_POLYNOMIAL, crc32_eth_lut);
+	crc32_eth_init_lut(CRC16_CCITT_POLYNOMIAL << 16, crc16_ccitt_lut);
+
+	handlers[RTE_NET_CRC_SCALAR].f[RTE_NET_CRC16_CCITT] = crc16_ccitt;
+	handlers[RTE_NET_CRC_SCALAR].f[RTE_NET_CRC32_ETH] = crc32_eth;
+}
 
 #define AVX512_VPCLMULQDQ_CPU_SUPPORTED ( \
 	rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F) && \
@@ -171,170 +111,101 @@ rte_crc32_eth_handler(const uint8_t *data, uint32_t data_len)
 	rte_cpu_get_flag_enabled(RTE_CPUFLAG_VPCLMULQDQ) \
 )
 
-static const rte_net_crc_handler *
-avx512_vpclmulqdq_get_handlers(void)
-{
-#ifdef CC_X86_64_AVX512_VPCLMULQDQ_SUPPORT
-	if (AVX512_VPCLMULQDQ_CPU_SUPPORTED &&
-			max_simd_bitwidth >= RTE_VECT_SIMD_512)
-		return handlers_avx512;
-#endif
-	NET_LOG(INFO, "Requirements not met, can't use AVX512");
-	return NULL;
-}
-
 static void
 avx512_vpclmulqdq_init(void)
 {
 #ifdef CC_X86_64_AVX512_VPCLMULQDQ_SUPPORT
-	if (AVX512_VPCLMULQDQ_CPU_SUPPORTED)
+	if (AVX512_VPCLMULQDQ_CPU_SUPPORTED) {
 		rte_net_crc_avx512_init();
+		handlers[RTE_NET_CRC_AVX512].f[RTE_NET_CRC16_CCITT] =
+			rte_crc16_ccitt_avx512_handler;
+		handlers[RTE_NET_CRC_AVX512].f[RTE_NET_CRC32_ETH] =
+			rte_crc32_eth_avx512_handler;
+	}
 #endif
 }
 
-/* SSE4.2/PCLMULQDQ handling */
-
 #define SSE42_PCLMULQDQ_CPU_SUPPORTED \
 	rte_cpu_get_flag_enabled(RTE_CPUFLAG_PCLMULQDQ)
 
-static const rte_net_crc_handler *
-sse42_pclmulqdq_get_handlers(void)
-{
-#ifdef CC_X86_64_SSE42_PCLMULQDQ_SUPPORT
-	if (SSE42_PCLMULQDQ_CPU_SUPPORTED &&
-			max_simd_bitwidth >= RTE_VECT_SIMD_128)
-		return handlers_sse42;
-#endif
-	NET_LOG(INFO, "Requirements not met, can't use SSE");
-	return NULL;
-}
-
 static void
 sse42_pclmulqdq_init(void)
 {
 #ifdef CC_X86_64_SSE42_PCLMULQDQ_SUPPORT
-	if (SSE42_PCLMULQDQ_CPU_SUPPORTED)
+	if (SSE42_PCLMULQDQ_CPU_SUPPORTED) {
 		rte_net_crc_sse42_init();
+		handlers[RTE_NET_CRC_SSE42].f[RTE_NET_CRC16_CCITT] =
+			rte_crc16_ccitt_sse42_handler;
+		handlers[RTE_NET_CRC_SSE42].f[RTE_NET_CRC32_ETH] =
+			rte_crc32_eth_sse42_handler;
+	}
 #endif
 }
 
-/* NEON/PMULL handling */
-
 #define NEON_PMULL_CPU_SUPPORTED \
 	rte_cpu_get_flag_enabled(RTE_CPUFLAG_PMULL)
 
-static const rte_net_crc_handler *
-neon_pmull_get_handlers(void)
-{
-#ifdef CC_ARM64_NEON_PMULL_SUPPORT
-	if (NEON_PMULL_CPU_SUPPORTED &&
-			max_simd_bitwidth >= RTE_VECT_SIMD_128)
-		return handlers_neon;
-#endif
-	NET_LOG(INFO, "Requirements not met, can't use NEON");
-	return NULL;
-}
-
 static void
 neon_pmull_init(void)
 {
 #ifdef CC_ARM64_NEON_PMULL_SUPPORT
-	if (NEON_PMULL_CPU_SUPPORTED)
+	if (NEON_PMULL_CPU_SUPPORTED) {
 		rte_net_crc_neon_init();
+		handlers[RTE_NET_CRC_NEON].f[RTE_NET_CRC16_CCITT] =
+			rte_crc16_ccitt_neon_handler;
+		handlers[RTE_NET_CRC_NEON].f[RTE_NET_CRC32_ETH] =
+			rte_crc32_eth_neon_handler;
+	}
 #endif
 }
 
-/* Default handling */
-
-static uint32_t
-rte_crc16_ccitt_default_handler(const uint8_t *data, uint32_t data_len)
-{
-	handlers = NULL;
-	if (max_simd_bitwidth == 0)
-		max_simd_bitwidth = rte_vect_get_max_simd_bitwidth();
-
-	handlers = avx512_vpclmulqdq_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC16_CCITT](data, data_len);
-	handlers = sse42_pclmulqdq_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC16_CCITT](data, data_len);
-	handlers = neon_pmull_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC16_CCITT](data, data_len);
-	handlers = handlers_scalar;
-	return handlers[RTE_NET_CRC16_CCITT](data, data_len);
-}
-
-static uint32_t
-rte_crc32_eth_default_handler(const uint8_t *data, uint32_t data_len)
+struct rte_net_crc rte_net_crc_set(enum rte_net_crc_alg alg,
+	enum rte_net_crc_type type)
 {
-	handlers = NULL;
-	if (max_simd_bitwidth == 0)
-		max_simd_bitwidth = rte_vect_get_max_simd_bitwidth();
-
-	handlers = avx512_vpclmulqdq_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC32_ETH](data, data_len);
-	handlers = sse42_pclmulqdq_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC32_ETH](data, data_len);
-	handlers = neon_pmull_get_handlers();
-	if (handlers != NULL)
-		return handlers[RTE_NET_CRC32_ETH](data, data_len);
-	handlers = handlers_scalar;
-	return handlers[RTE_NET_CRC32_ETH](data, data_len);
-}
+	uint16_t max_simd_bitwidth;
 
-/* Public API */
-
-void
-rte_net_crc_set_alg(enum rte_net_crc_alg alg)
-{
-	handlers = NULL;
-	if (max_simd_bitwidth == 0)
-		max_simd_bitwidth = rte_vect_get_max_simd_bitwidth();
+	max_simd_bitwidth = rte_vect_get_max_simd_bitwidth();
 
 	switch (alg) {
 	case RTE_NET_CRC_AVX512:
-		handlers = avx512_vpclmulqdq_get_handlers();
-		if (handlers != NULL)
-			break;
+#ifdef CC_X86_64_AVX512_VPCLMULQDQ_SUPPORT
+		if (AVX512_VPCLMULQDQ_CPU_SUPPORTED &&
+				max_simd_bitwidth >= RTE_VECT_SIMD_512) {
+			return (struct rte_net_crc){ RTE_NET_CRC_AVX512, type };
+		}
+#endif
 		/* fall-through */
 	case RTE_NET_CRC_SSE42:
-		handlers = sse42_pclmulqdq_get_handlers();
-		break; /* for x86, always break here */
+#ifdef CC_X86_64_SSE42_PCLMULQDQ_SUPPORT
+		if (SSE42_PCLMULQDQ_CPU_SUPPORTED &&
+				max_simd_bitwidth >= RTE_VECT_SIMD_128) {
+			return (struct rte_net_crc){ RTE_NET_CRC_SSE42, type };
+		}
+#endif
+		break;
 	case RTE_NET_CRC_NEON:
-		handlers = neon_pmull_get_handlers();
-		/* fall-through */
-	case RTE_NET_CRC_SCALAR:
-		/* fall-through */
+#ifdef CC_ARM64_NEON_PMULL_SUPPORT
+		if (NEON_PMULL_CPU_SUPPORTED &&
+				max_simd_bitwidth >= RTE_VECT_SIMD_128) {
+			return (struct rte_net_crc){ RTE_NET_CRC_NEON, type };
+		}
+#endif
+		break;
 	default:
 		break;
 	}
-
-	if (handlers == NULL)
-		handlers = handlers_scalar;
+	return (struct rte_net_crc){ RTE_NET_CRC_SCALAR, type };
 }
 
-uint32_t
-rte_net_crc_calc(const void *data,
-	uint32_t data_len,
-	enum rte_net_crc_type type)
+uint32_t rte_net_crc(const struct rte_net_crc *ctx,
+	const void *data, const uint32_t data_len)
 {
-	uint32_t ret;
-	rte_net_crc_handler f_handle;
-
-	f_handle = handlers[type];
-	ret = f_handle(data, data_len);
-
-	return ret;
+	return handlers[ctx->alg].f[ctx->type](data, data_len);
 }
 
-/* Call initialisation helpers for all crc algorithm handlers */
 RTE_INIT(rte_net_crc_init)
 {
-	rte_net_crc_scalar_init();
+	crc_scalar_init();
 	sse42_pclmulqdq_init();
 	avx512_vpclmulqdq_init();
 	neon_pmull_init();
diff --git a/lib/net/rte_net_crc.h b/lib/net/rte_net_crc.h
index 72d3e10ff6..2ec8ebbb91 100644
--- a/lib/net/rte_net_crc.h
+++ b/lib/net/rte_net_crc.h
@@ -5,8 +5,6 @@
 #ifndef _RTE_NET_CRC_H_
 #define _RTE_NET_CRC_H_
 
-#include <stdint.h>
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -26,39 +24,17 @@ enum rte_net_crc_alg {
 	RTE_NET_CRC_AVX512,
 };
 
-/**
- * This API set the CRC computation algorithm (i.e. scalar version,
- * x86 64-bit sse4.2 intrinsic version, etc.) and internal data
- * structure.
- *
- * @param alg
- *   This parameter is used to select the CRC implementation version.
- *   - RTE_NET_CRC_SCALAR
- *   - RTE_NET_CRC_SSE42 (Use 64-bit SSE4.2 intrinsic)
- *   - RTE_NET_CRC_NEON (Use ARM Neon intrinsic)
- *   - RTE_NET_CRC_AVX512 (Use 512-bit AVX intrinsic)
- */
-void
-rte_net_crc_set_alg(enum rte_net_crc_alg alg);
+struct rte_net_crc {
+	enum rte_net_crc_alg alg;
+	enum rte_net_crc_type type;
+};
 
-/**
- * CRC compute API
- *
- * @param data
- *   Pointer to the packet data for CRC computation
- * @param data_len
- *   Data length for CRC computation
- * @param type
- *   CRC type (enum rte_net_crc_type)
- *
- * @return
- *   CRC value
- */
-uint32_t
-rte_net_crc_calc(const void *data,
-	uint32_t data_len,
+struct rte_net_crc rte_net_crc_set(enum rte_net_crc_alg alg,
 	enum rte_net_crc_type type);
 
+uint32_t rte_net_crc(const struct rte_net_crc *ctx,
+	const void *data, const uint32_t data_len);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/net/version.map b/lib/net/version.map
index bec4ce23ea..47daf1464a 100644
--- a/lib/net/version.map
+++ b/lib/net/version.map
@@ -4,11 +4,25 @@ DPDK_25 {
 	rte_eth_random_addr;
 	rte_ether_format_addr;
 	rte_ether_unformat_addr;
-	rte_net_crc_calc;
-	rte_net_crc_set_alg;
 	rte_net_get_ptype;
 	rte_net_make_rarp_packet;
 	rte_net_skip_ip6_ext;
+	rte_net_crc;
+	rte_net_crc_set;
 
 	local: *;
 };
+
+INTERNAL {
+	global:
+
+	rte_net_crc_sse42_init;
+	rte_crc16_ccitt_sse42_handler;
+	rte_crc32_eth_sse42_handler;
+	rte_net_crc_avx512_init;
+	rte_crc16_ccitt_avx512_handler;
+	rte_crc32_eth_avx512_handler;
+	rte_net_crc_neon_init;
+	rte_crc16_ccitt_neon_handler;
+	rte_crc32_eth_neon_handler;
+};
-- 
2.13.6


  reply	other threads:[~2024-10-01 19:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-01 18:11 [PATCH v2 0/3] " Arkadiusz Kusztal
2024-10-01 18:11 ` Arkadiusz Kusztal [this message]
2024-10-01 21:44   ` [PATCH v2 1/3] " Stephen Hemminger
2024-10-01 18:11 ` [PATCH v2 2/3] crypto/qat: use process safe " Arkadiusz Kusztal
2024-10-01 18:11 ` [PATCH v2 3/3] test/crc: replace thread-unsafe api functions Arkadiusz Kusztal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241001181150.43506-2-arkadiuszx.kusztal@intel.com \
    --to=arkadiuszx.kusztal@intel.com \
    --cc=brian.dooley@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=kai.ji@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).