Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function

[Ray Kinsella <mdr@ashroe.eu>]

On 18/03/2020 20:41, Arek Kusztal wrote:
> This patch adds versioned function rte_cryptodev_info_get.
> Node 20.05 function works the same way it was working before.
> Node 20.0 function strips capability added in 20.05 release
> to prevent some issues with ABI policy. To do that new capability
> array is allocated per device and returned to user instead of the
> original array passed by PMD.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_cryptodev.c           | 97 +++++++++++++++++++++++++-
>  lib/librte_cryptodev/rte_cryptodev.h           | 12 +++-
>  lib/librte_cryptodev/rte_cryptodev_version.map |  7 ++
>  3 files changed, 113 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> index 6d1d0e9..2d030ba 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -41,6 +41,9 @@
>  #include "rte_cryptodev.h"
>  #include "rte_cryptodev_pmd.h"
>  
> +#include <rte_compat.h>
> +#include <rte_function_versioning.h>
> +
>  static uint8_t nb_drivers;
>  
>  static struct rte_cryptodev rte_crypto_devices[RTE_CRYPTO_MAX_DEVS];
> @@ -56,6 +59,13 @@ static struct rte_cryptodev_global cryptodev_globals = {
>  /* spinlock for crypto device callbacks */
>  static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER;
>  
> +static const struct rte_cryptodev_capabilities
> +		cryptodev_undefined_capabilities[] = {
> +		RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> +};
> +
> +struct rte_cryptodev_capabilities *capability_copies[RTE_CRYPTO_MAX_DEVS];
> +uint8_t is_capability_checked[RTE_CRYPTO_MAX_DEVS];
>  
>  /**
>   * The user application callback description.
> @@ -999,6 +1009,13 @@ rte_cryptodev_close(uint8_t dev_id)
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
>  	retval = (*dev->dev_ops->dev_close)(dev);
>  
> +
> +	if (capability_copies[dev_id]) {
> +		free(capability_copies[dev_id]);
> +		capability_copies[dev_id] = NULL;
> +	}
> +	is_capability_checked[dev_id] = 0;
> +
>  	if (retval < 0)
>  		return retval;
>  
> @@ -1111,11 +1128,12 @@ rte_cryptodev_stats_reset(uint8_t dev_id)
>  	(*dev->dev_ops->stats_reset)(dev);
>  }
>  
> -
>  void
> -rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info)
> +rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info)
>  {
>  	struct rte_cryptodev *dev;
> +	const struct rte_cryptodev_capabilities *capability;
> +	uint8_t counter = 0;
>  
>  	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
>  		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
> @@ -1129,10 +1147,85 @@ rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info)
>  	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>  	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
>  
> +	if (capability_copies[dev_id] == NULL) {
> +		if (!is_capability_checked[dev_id]) {
> +			uint8_t found_invalid_capa = 0;
> +
> +			for (capability = dev_info->capabilities;
> +					capability->op != RTE_CRYPTO_OP_TYPE_UNDEFINED;
> +					++capability, ++counter) {
> +				if (capability->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> +						capability->sym.xform_type ==
> +							RTE_CRYPTO_SYM_XFORM_AEAD
> +						&& capability->sym.aead.algo >=
> +						RTE_CRYPTO_AEAD_CHACHA20_POLY1305) {
> +					found_invalid_capa = 1;
> +					counter--;
> +				}
> +			}
> +			is_capability_checked[dev_id] = 1;
> +			if (found_invalid_capa) {
> +				capability_copies[dev_id] = malloc(counter *
> +					sizeof(struct rte_cryptodev_capabilities));
> +				if (capability_copies[dev_id] == NULL) {
> +					 /*
> +					  * error case - no memory to store the trimmed list,
> +					  * so have to return an empty list
> +					  */
> +					dev_info->capabilities =
> +						cryptodev_undefined_capabilities;
> +					is_capability_checked[dev_id] = 0;
> +				} else {
> +					counter = 0;
> +					for (capability = dev_info->capabilities;
> +							capability->op !=
> +							RTE_CRYPTO_OP_TYPE_UNDEFINED;
> +							capability++) {
> +						if (!(capability->op ==
> +								RTE_CRYPTO_OP_TYPE_SYMMETRIC
> +								&& capability->sym.xform_type ==
> +								RTE_CRYPTO_SYM_XFORM_AEAD
> +								&& capability->sym.aead.algo >=
> +								RTE_CRYPTO_AEAD_CHACHA20_POLY1305)) {
> +							capability_copies[dev_id][counter++] =
> +									*capability;
> +						}
> +					}
> +					dev_info->capabilities =
> +							capability_copies[dev_id];
> +				}
> +			}
> +		}
> +	} else
> +		dev_info->capabilities = capability_copies[dev_id];
> +
>  	dev_info->driver_name = dev->device->driver->name;
>  	dev_info->device = dev->device;
>  }
> +VERSION_SYMBOL(rte_cryptodev_info_get, _v20, 20.0);
> +
> +void
> +rte_cryptodev_info_get_v2005(uint8_t dev_id, struct rte_cryptodev_info *dev_info)

should be rte_cryptodev_info_get_v2002

> +{
> +	struct rte_cryptodev *dev;
>  
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
> +		return;
> +	}
> +
> +	dev = &rte_crypto_devices[dev_id];
> +
> +	memset(dev_info, 0, sizeof(struct rte_cryptodev_info));
> +
> +	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
> +	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
> +
> +	dev_info->driver_name = dev->device->driver->name;
> +	dev_info->device = dev->device;
> +}
> +MAP_STATIC_SYMBOL(void rte_cryptodev_info_get(uint8_t dev_id,
> +		struct rte_cryptodev_info *dev_info), rte_cryptodev_info_get_v2005);

should be rte_cryptodev_info_get_v2002

>  
>  int
>  rte_cryptodev_callback_register(uint8_t dev_id,
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index 437b8a9..06ce2f2 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -24,6 +24,9 @@ extern "C" {
>  #include <rte_common.h>
>  #include <rte_config.h>
>  
> +#include <rte_compat.h>
> +#include <rte_function_versioning.h>
> +
>  extern const char **rte_cyptodev_names;
>  
>  /* Logging Macros */
> @@ -758,7 +761,14 @@ rte_cryptodev_stats_reset(uint8_t dev_id);
>   * the last valid element has it's op field set to
>   * RTE_CRYPTO_OP_TYPE_UNDEFINED.
>   */
> -extern void
> +void
> +rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
> +
> +void
> +rte_cryptodev_info_get_v2005(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
> +BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v2005, 20.05);
> +
> +void
>  rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
do we still need this forward declaration?

>  
>  
> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
> index 6e41b4b..d6c945a 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_version.map
> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> @@ -58,6 +58,13 @@ DPDK_20.0 {
>  	local: *;
>  };
>  
> +
> +DPDK_20.05 {

should be DPDK_20.0.2

> +	global:
> +	rte_cryptodev_info_get;
> +} DPDK_20.0;
> +
> +
>  EXPERIMENTAL {
>  	global:
>  
>