[v2,17/17] net: add checks for max SIMD bitwidth

Message ID 20200827161304.32300-18-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series add max SIMD bitwidth to EAL |

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot warning Travis build: failed
ci/checkpatch success coding style OK

Commit Message

Power, Ciara Aug. 27, 2020, 4:13 p.m. UTC
  When choosing a vector path to take, an extra condition must be
satisfied to ensure the max SIMD bitwidth allows for the CPU enabled
path. This check is done just before the handler is called, it cannot
be done when setting the handlers initially as the EAL max simd bitwidth
value has not yet been set.

Cc: Jasvinder Singh <jasvinder.singh@intel.com>

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_net/rte_net_crc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Jasvinder Singh Sept. 2, 2020, 11:02 a.m. UTC | #1
> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Thursday, August 27, 2020 5:13 PM
> To: dev@dpdk.org
> Cc: Power, Ciara <ciara.power@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Olivier Matz <olivier.matz@6wind.com>
> Subject: [PATCH v2 17/17] net: add checks for max SIMD bitwidth
> 
> When choosing a vector path to take, an extra condition must be satisfied to
> ensure the max SIMD bitwidth allows for the CPU enabled path. This check is
> done just before the handler is called, it cannot be done when setting the
> handlers initially as the EAL max simd bitwidth value has not yet been set.
> 
> Cc: Jasvinder Singh <jasvinder.singh@intel.com>
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
>  lib/librte_net/rte_net_crc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/librte_net/rte_net_crc.c b/lib/librte_net/rte_net_crc.c index
> 9fd4794a9d..d3d3206919 100644
> --- a/lib/librte_net/rte_net_crc.c
> +++ b/lib/librte_net/rte_net_crc.c
> @@ -9,6 +9,7 @@
>  #include <rte_cpuflags.h>
>  #include <rte_common.h>
>  #include <rte_net_crc.h>
> +#include <rte_eal.h>
> 
>  #if defined(RTE_ARCH_X86_64) &&
> defined(RTE_MACHINE_CPUFLAG_PCLMULQDQ)
>  #define X86_64_SSE42_PCLMULQDQ     1
> @@ -60,6 +61,8 @@ static rte_net_crc_handler handlers_neon[] = {  };
> #endif
> 
> +static uint16_t max_simd_bitwidth;
> +
>  /**
>   * Reflect the bits about the middle
>   *
> @@ -175,6 +178,11 @@ rte_net_crc_calc(const void *data,
>  	uint32_t ret;
>  	rte_net_crc_handler f_handle;
> 
> +	if (max_simd_bitwidth == 0)
> +		max_simd_bitwidth = rte_get_max_simd_bitwidth();
> +	if (max_simd_bitwidth < RTE_MAX_128_SIMD &&
> +			handlers != handlers_scalar)
> +		rte_net_crc_set_alg(RTE_NET_CRC_SCALAR);


Above change doesn't seem right as rte_net_crc_set_alg () is invoked everytime when crc is computed. It potentially adds branches in runtime.  In my opinion,  bit width should be checked inside rte_net_crc_set_alg () function which is supposed to be used during initialization stage after eal sets the max simd bit width. 

>  	f_handle = handlers[type];
>  	ret = f_handle(data, data_len);
> 
> --
> 2.17.1
  

Patch

diff --git a/lib/librte_net/rte_net_crc.c b/lib/librte_net/rte_net_crc.c
index 9fd4794a9d..d3d3206919 100644
--- a/lib/librte_net/rte_net_crc.c
+++ b/lib/librte_net/rte_net_crc.c
@@ -9,6 +9,7 @@ 
 #include <rte_cpuflags.h>
 #include <rte_common.h>
 #include <rte_net_crc.h>
+#include <rte_eal.h>
 
 #if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_PCLMULQDQ)
 #define X86_64_SSE42_PCLMULQDQ     1
@@ -60,6 +61,8 @@  static rte_net_crc_handler handlers_neon[] = {
 };
 #endif
 
+static uint16_t max_simd_bitwidth;
+
 /**
  * Reflect the bits about the middle
  *
@@ -175,6 +178,11 @@  rte_net_crc_calc(const void *data,
 	uint32_t ret;
 	rte_net_crc_handler f_handle;
 
+	if (max_simd_bitwidth == 0)
+		max_simd_bitwidth = rte_get_max_simd_bitwidth();
+	if (max_simd_bitwidth < RTE_MAX_128_SIMD &&
+			handlers != handlers_scalar)
+		rte_net_crc_set_alg(RTE_NET_CRC_SCALAR);
 	f_handle = handlers[type];
 	ret = f_handle(data, data_len);