diff mbox series

[1/2] net: add thread-safe crc api

Message ID 20240905150722.27789-1-arkadiuszx.kusztal@intel.com (mailing list archive)
State Superseded
Delegated to: Ferruh Yigit
Headers
Series [1/2] net: add thread-safe crc api |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Kusztal, ArkadiuszX Sept. 5, 2024, 3:07 p.m. UTC 
The current net CRC API is not thread-safe, this patch
solves this by adding another, thread-safe API functions.
These functions are not safe when using between different
processes, though.

Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/net/rte_net_crc.c | 40 +++++++++++++++++++++++++++++++++++++---
 lib/net/rte_net_crc.h | 14 ++++++++++++++
 lib/net/version.map   |  2 ++
 3 files changed, 53 insertions(+), 3 deletions(-)

Comments

Akhil Goyal Sept. 18, 2024, 5:57 a.m. UTC | #1 
> The current net CRC API is not thread-safe, this patch
> solves this by adding another, thread-safe API functions.
> These functions are not safe when using between different
> processes, though.
> 
> Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>

Added Jasvinder for review.

This patch is mainly related to net library. Delegated this patchset to Ferruh.

> ---
>  lib/net/rte_net_crc.c | 40 +++++++++++++++++++++++++++++++++++++---
>  lib/net/rte_net_crc.h | 14 ++++++++++++++
>  lib/net/version.map   |  2 ++
>  3 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/net/rte_net_crc.c b/lib/net/rte_net_crc.c
> index 346c285c15..87808a31dc 100644
> --- a/lib/net/rte_net_crc.c
> +++ b/lib/net/rte_net_crc.c
> @@ -35,9 +35,6 @@ 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,
> @@ -331,6 +328,43 @@ rte_net_crc_calc(const void *data,
>  	return ret;
>  }
> 
> +struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
> +	enum rte_net_crc_alg alg)
> +{
> +	const rte_net_crc_handler *handlers = NULL;
> +
> +	if (max_simd_bitwidth == 0)
> +		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;
> +		/* fall-through */
> +	case RTE_NET_CRC_SSE42:
> +		handlers = sse42_pclmulqdq_get_handlers();
> +		break;
> +	case RTE_NET_CRC_NEON:
> +		handlers = neon_pmull_get_handlers();
> +		/* fall-through */
> +	case RTE_NET_CRC_SCALAR:
> +		/* fall-through */
> +	default:
> +		break;
> +	}
> +	if (handlers == NULL)
> +		handlers = handlers_scalar;
> +
> +	return (struct rte_net_crc){ type, handlers[type] };
> +}
> +
> +uint32_t rte_net_crc(const struct rte_net_crc *ctx,
> +	const void *data, const uint32_t data_len)
> +{
> +	return ctx->crc(data, data_len);
> +}
> +
>  /* Call initialisation helpers for all crc algorithm handlers */
>  RTE_INIT(rte_net_crc_init)
>  {
> diff --git a/lib/net/rte_net_crc.h b/lib/net/rte_net_crc.h
> index 72d3e10ff6..f5c8f7173f 100644
> --- a/lib/net/rte_net_crc.h
> +++ b/lib/net/rte_net_crc.h
> @@ -11,6 +11,9 @@
>  extern "C" {
>  #endif
> 
> +typedef uint32_t
> +(*rte_net_crc_handler)(const uint8_t *data, uint32_t data_len);
> +
>  /** CRC types */
>  enum rte_net_crc_type {
>  	RTE_NET_CRC16_CCITT = 0,
> @@ -26,6 +29,11 @@ enum rte_net_crc_alg {
>  	RTE_NET_CRC_AVX512,
>  };
> 
> +struct rte_net_crc {
> +	enum rte_net_crc_type type;
> +	rte_net_crc_handler crc;
> +};
> +
>  /**
>   * This API set the CRC computation algorithm (i.e. scalar version,
>   * x86 64-bit sse4.2 intrinsic version, etc.) and internal data
> @@ -59,6 +67,12 @@ rte_net_crc_calc(const void *data,
>  	uint32_t data_len,
>  	enum rte_net_crc_type type);
> 
> +struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
> +	enum rte_net_crc_alg alg);
> +
> +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..5c3dbffba7 100644
> --- a/lib/net/version.map
> +++ b/lib/net/version.map
> @@ -4,6 +4,8 @@ DPDK_25 {
>  	rte_eth_random_addr;
>  	rte_ether_format_addr;
>  	rte_ether_unformat_addr;
> +	rte_net_crc;
> +	rte_net_crc_set;
>  	rte_net_crc_calc;
>  	rte_net_crc_set_alg;
>  	rte_net_get_ptype;
> --
> 2.13.6
Singh, Jasvinder Sept. 18, 2024, 9:26 a.m. UTC | #2 
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, September 18, 2024 6:58 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org;
> Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: Ji, Kai <kai.ji@intel.com>; Dooley, Brian <brian.dooley@intel.com>; Ferruh
> Yigit <ferruh.yigit@amd.com>
> Subject: RE: [EXTERNAL] [PATCH 1/2] net: add thread-safe crc api
> 
> > The current net CRC API is not thread-safe, this patch solves this by
> > adding another, thread-safe API functions.
> > These functions are not safe when using between different processes,
> > though.
> >
> > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> 
> Added Jasvinder for review.
> 
> This patch is mainly related to net library. Delegated this patchset to Ferruh.
> 
> > ---
> >  lib/net/rte_net_crc.c | 40 +++++++++++++++++++++++++++++++++++++-
> --
> >  lib/net/rte_net_crc.h | 14 ++++++++++++++
> >  lib/net/version.map   |  2 ++
> >  3 files changed, 53 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/net/rte_net_crc.c b/lib/net/rte_net_crc.c index
> > 346c285c15..87808a31dc 100644
> > --- a/lib/net/rte_net_crc.c
> > +++ b/lib/net/rte_net_crc.c
> > @@ -35,9 +35,6 @@ 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, @@ -331,6
> > +328,43 @@ rte_net_crc_calc(const void *data,
> >  	return ret;
> >  }
> >
> > +struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
> > +	enum rte_net_crc_alg alg)
> > +{
> > +	const rte_net_crc_handler *handlers = NULL;
> > +
> > +	if (max_simd_bitwidth == 0)
> > +		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;
> > +		/* fall-through */
> > +	case RTE_NET_CRC_SSE42:
> > +		handlers = sse42_pclmulqdq_get_handlers();
> > +		break;
> > +	case RTE_NET_CRC_NEON:
> > +		handlers = neon_pmull_get_handlers();
> > +		/* fall-through */
> > +	case RTE_NET_CRC_SCALAR:
> > +		/* fall-through */
> > +	default:
> > +		break;
> > +	}
> > +	if (handlers == NULL)
> > +		handlers = handlers_scalar;
> > +
> > +	return (struct rte_net_crc){ type, handlers[type] }; }
> > +
> > +uint32_t rte_net_crc(const struct rte_net_crc *ctx,
> > +	const void *data, const uint32_t data_len) {
> > +	return ctx->crc(data, data_len);
> > +}
> > +
> >  /* Call initialisation helpers for all crc algorithm handlers */
> >  RTE_INIT(rte_net_crc_init)
> >  {
> > diff --git a/lib/net/rte_net_crc.h b/lib/net/rte_net_crc.h index
> > 72d3e10ff6..f5c8f7173f 100644
> > --- a/lib/net/rte_net_crc.h
> > +++ b/lib/net/rte_net_crc.h
> > @@ -11,6 +11,9 @@
> >  extern "C" {
> >  #endif
> >
> > +typedef uint32_t
> > +(*rte_net_crc_handler)(const uint8_t *data, uint32_t data_len);
> > +
> >  /** CRC types */
> >  enum rte_net_crc_type {
> >  	RTE_NET_CRC16_CCITT = 0,
> > @@ -26,6 +29,11 @@ enum rte_net_crc_alg {
> >  	RTE_NET_CRC_AVX512,
> >  };
> >
> > +struct rte_net_crc {
> > +	enum rte_net_crc_type type;
> > +	rte_net_crc_handler crc;
> > +};
> > +
> >  /**
> >   * This API set the CRC computation algorithm (i.e. scalar version,
> >   * x86 64-bit sse4.2 intrinsic version, etc.) and internal data @@
> > -59,6 +67,12 @@ rte_net_crc_calc(const void *data,
> >  	uint32_t data_len,
> >  	enum rte_net_crc_type type);
> >
> > +struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
> > +	enum rte_net_crc_alg alg);
> > +
> > +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..5c3dbffba7 100644
> > --- a/lib/net/version.map
> > +++ b/lib/net/version.map
> > @@ -4,6 +4,8 @@ DPDK_25 {
> >  	rte_eth_random_addr;
> >  	rte_ether_format_addr;
> >  	rte_ether_unformat_addr;
> > +	rte_net_crc;
> > +	rte_net_crc_set;
> >  	rte_net_crc_calc;
> >  	rte_net_crc_set_alg;
> >  	rte_net_get_ptype;
> > --
> > 2.13.6

Hi Arkadiusz, 

Thanks for the patches. 

New api will make the existing ones obsolete, therefore would suggest removing them to avoid confusion as they implement similar functionality. Also, update the documentation accordingly.
diff mbox series

Patch

diff --git a/lib/net/rte_net_crc.c b/lib/net/rte_net_crc.c
index 346c285c15..87808a31dc 100644
--- a/lib/net/rte_net_crc.c
+++ b/lib/net/rte_net_crc.c
@@ -35,9 +35,6 @@  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,
@@ -331,6 +328,43 @@  rte_net_crc_calc(const void *data,
 	return ret;
 }
 
+struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
+	enum rte_net_crc_alg alg)
+{
+	const rte_net_crc_handler *handlers = NULL;
+
+	if (max_simd_bitwidth == 0)
+		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;
+		/* fall-through */
+	case RTE_NET_CRC_SSE42:
+		handlers = sse42_pclmulqdq_get_handlers();
+		break;
+	case RTE_NET_CRC_NEON:
+		handlers = neon_pmull_get_handlers();
+		/* fall-through */
+	case RTE_NET_CRC_SCALAR:
+		/* fall-through */
+	default:
+		break;
+	}
+	if (handlers == NULL)
+		handlers = handlers_scalar;
+
+	return (struct rte_net_crc){ type, handlers[type] };
+}
+
+uint32_t rte_net_crc(const struct rte_net_crc *ctx,
+	const void *data, const uint32_t data_len)
+{
+	return ctx->crc(data, data_len);
+}
+
 /* Call initialisation helpers for all crc algorithm handlers */
 RTE_INIT(rte_net_crc_init)
 {
diff --git a/lib/net/rte_net_crc.h b/lib/net/rte_net_crc.h
index 72d3e10ff6..f5c8f7173f 100644
--- a/lib/net/rte_net_crc.h
+++ b/lib/net/rte_net_crc.h
@@ -11,6 +11,9 @@ 
 extern "C" {
 #endif
 
+typedef uint32_t
+(*rte_net_crc_handler)(const uint8_t *data, uint32_t data_len);
+
 /** CRC types */
 enum rte_net_crc_type {
 	RTE_NET_CRC16_CCITT = 0,
@@ -26,6 +29,11 @@  enum rte_net_crc_alg {
 	RTE_NET_CRC_AVX512,
 };
 
+struct rte_net_crc {
+	enum rte_net_crc_type type;
+	rte_net_crc_handler crc;
+};
+
 /**
  * This API set the CRC computation algorithm (i.e. scalar version,
  * x86 64-bit sse4.2 intrinsic version, etc.) and internal data
@@ -59,6 +67,12 @@  rte_net_crc_calc(const void *data,
 	uint32_t data_len,
 	enum rte_net_crc_type type);
 
+struct rte_net_crc rte_net_crc_set(enum rte_net_crc_type type,
+	enum rte_net_crc_alg alg);
+
+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..5c3dbffba7 100644
--- a/lib/net/version.map
+++ b/lib/net/version.map
@@ -4,6 +4,8 @@  DPDK_25 {
 	rte_eth_random_addr;
 	rte_ether_format_addr;
 	rte_ether_unformat_addr;
+	rte_net_crc;
+	rte_net_crc_set;
 	rte_net_crc_calc;
 	rte_net_crc_set_alg;
 	rte_net_get_ptype;