Re: [dpdk-dev] [PATCH v2 17/17] net: add checks for max SIMD bitwidth

["Singh, Jasvinder" <jasvinder.singh@intel.com>]

> -----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