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

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

[Arek Kusztal]

This patch adds versioned function rte_cryptodev_info_get.
Node 20.0.2 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.
Because rte_cryptodev_info_get is called by other API functions,
rte_cryptodev_sym_capability_get function was versioned the same way.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---

This patch depends on following patches:

[1] - "[v3] cryptodev: add chacha20-poly1305 aead algorithm"
(http://patchwork.dpdk.org/patch/64549/)

 lib/librte_cryptodev/rte_cryptodev.c           | 139 ++++++++++++++++++++++++-
 lib/librte_cryptodev/rte_cryptodev.h           |  38 ++++++-
 lib/librte_cryptodev/rte_cryptodev_version.map |   7 ++
 3 files changed, 180 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 6d1d0e9..158e7d6 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()
+};
+
+static struct rte_cryptodev_capabilities *capability_copies[RTE_CRYPTO_MAX_DEVS];
+static uint8_t is_capability_checked[RTE_CRYPTO_MAX_DEVS];
 
 /**
  * The user application callback description.
@@ -280,7 +290,43 @@ rte_crypto_auth_operation_strings[] = {
 };
 
 const struct rte_cryptodev_symmetric_capability *
-rte_cryptodev_sym_capability_get(uint8_t dev_id,
+rte_cryptodev_sym_capability_get_v20(uint8_t dev_id,
+		const struct rte_cryptodev_sym_capability_idx *idx)
+{
+	const struct rte_cryptodev_capabilities *capability;
+	struct rte_cryptodev_info dev_info;
+	int i = 0;
+
+	rte_cryptodev_info_get_v20(dev_id, &dev_info);
+
+	while ((capability = &dev_info.capabilities[i++])->op !=
+			RTE_CRYPTO_OP_TYPE_UNDEFINED) {
+		if (capability->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
+			continue;
+
+		if (capability->sym.xform_type != idx->type)
+			continue;
+
+		if (idx->type == RTE_CRYPTO_SYM_XFORM_AUTH &&
+			capability->sym.auth.algo == idx->algo.auth)
+			return &capability->sym;
+
+		if (idx->type == RTE_CRYPTO_SYM_XFORM_CIPHER &&
+			capability->sym.cipher.algo == idx->algo.cipher)
+			return &capability->sym;
+
+		if (idx->type == RTE_CRYPTO_SYM_XFORM_AEAD &&
+				capability->sym.aead.algo == idx->algo.aead)
+			return &capability->sym;
+	}
+
+	return NULL;
+
+}
+VERSION_SYMBOL(rte_cryptodev_sym_capability_get, _v20, 20.0);
+
+const struct rte_cryptodev_symmetric_capability *
+rte_cryptodev_sym_capability_get_v2002(uint8_t dev_id,
 		const struct rte_cryptodev_sym_capability_idx *idx)
 {
 	const struct rte_cryptodev_capabilities *capability;
@@ -313,6 +359,10 @@ rte_cryptodev_sym_capability_get(uint8_t dev_id,
 	return NULL;
 
 }
+MAP_STATIC_SYMBOL(const struct rte_cryptodev_symmetric_capability *
+		rte_cryptodev_sym_capability_get(uint8_t dev_id,
+				const struct rte_cryptodev_sym_capability_idx *idx),
+				rte_cryptodev_sym_capability_get_v2002);
 
 static int
 param_range_check(uint16_t size, const struct rte_crypto_param_range *range)
@@ -999,6 +1049,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 +1168,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 +1187,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_v2002(uint8_t dev_id, struct rte_cryptodev_info *dev_info)
+{
+	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_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..42b7b42 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 */
@@ -217,6 +220,15 @@ struct rte_cryptodev_asym_capability_idx {
  *   - Return NULL if the capability not exist.
  */
 const struct rte_cryptodev_symmetric_capability *
+rte_cryptodev_sym_capability_get_v20(uint8_t dev_id,
+		const struct rte_cryptodev_sym_capability_idx *idx);
+
+const struct rte_cryptodev_symmetric_capability *
+rte_cryptodev_sym_capability_get_v2002(uint8_t dev_id,
+		const struct rte_cryptodev_sym_capability_idx *idx);
+BIND_DEFAULT_SYMBOL(rte_cryptodev_sym_capability_get, _v2002, 20.0.2);
+
+const struct rte_cryptodev_symmetric_capability *
 rte_cryptodev_sym_capability_get(uint8_t dev_id,
 		const struct rte_cryptodev_sym_capability_idx *idx);
 
@@ -758,9 +770,33 @@ 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(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
 
+/* An extra element RTE_CRYPTO_AEAD_CHACHA20_POLY1305 is added
+ * to enum rte_crypto_aead_algorithm, also changing the value of
+ *  RTE_CRYPTO_AEAD_LIST_END. To maintain ABI compatibility with applications
+ * which linked against earlier versions, preventing them, for example, from
+ * picking up the new value and using it to index into an array sized too small
+ * for it, it is necessary to have two versions of rte_cryptodev_info_get()
+ * The latest version just returns directly the capabilities retrieved from the device.
+ * The compatible version inspects the capabilities retrieved from the device, but only
+ * returns them directly if the new value is not included. If the new value is
+ * included, it allocates space for a copy of the device capabilities,
+ * trims the new value from this and returns this copy. It only needs to do this
+ * once per device. For the corner case of a corner case when the alloc may fail,
+ * an empty capability list is returned, as there is no mechanism to return an error
+ * and adding such a mechanism would itself be an ABI breakage.
+ * The compatible version can be removed after the next major ABI release.
+ */
+
+void
+rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
+
+void
+rte_cryptodev_info_get_v2002(uint8_t dev_id, struct rte_cryptodev_info *dev_info);
+BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v2002, 20.0.2);
 
 /**
  * Register a callback function for specific device id.
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index 6e41b4b..1f382d1 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.0.2 {
+	global:
+	rte_cryptodev_info_get;
+	rte_cryptodev_sym_capability_get;
+} DPDK_20.0;
+
+
 EXPERIMENTAL {
 	global:
 
-- 
2.1.0

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

[Akhil Goyal]

++Ray and Thomas for review on ABI

> This patch adds versioned function rte_cryptodev_info_get.
> Node 20.0.2 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.
> Because rte_cryptodev_info_get is called by other API functions,
> rte_cryptodev_sym_capability_get function was versioned the same way.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> 
> This patch depends on following patches:
> 
> [1] - "[v3] cryptodev: add chacha20-poly1305 aead algorithm"

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

[Thomas Monjalon]

16/04/2020 10:18, Akhil Goyal:
> ++Ray and Thomas for review on ABI

Akhil, please work with Ray to understand what needs to be checked
in general for ABI. We need you to maintain ABI in crypto area.
First step: run the new ABI checking tool.


> > This patch adds versioned function rte_cryptodev_info_get.
> > Node 20.0.2 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.
> > Because rte_cryptodev_info_get is called by other API functions,
> > rte_cryptodev_sym_capability_get function was versioned the same way.
> > 
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> > 
> > This patch depends on following patches:
> > 
> > [1] - "[v3] cryptodev: add chacha20-poly1305 aead algorithm"

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

[Ray Kinsella]

FYI ... not ignoring this, by any means. 
Need to check a few points before I respond.

Ray K

On 16/04/2020 09:31, Thomas Monjalon wrote:
> 16/04/2020 10:18, Akhil Goyal:
>> ++Ray and Thomas for review on ABI
> 
> Akhil, please work with Ray to understand what needs to be checked
> in general for ABI. We need you to maintain ABI in crypto area.
> First step: run the new ABI checking tool.
> 
> 
>>> This patch adds versioned function rte_cryptodev_info_get.
>>> Node 20.0.2 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.
>>> Because rte_cryptodev_info_get is called by other API functions,
>>> rte_cryptodev_sym_capability_get function was versioned the same way.
>>>
>>> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
>>> ---
>>>
>>> This patch depends on following patches:
>>>
>>> [1] - "[v3] cryptodev: add chacha20-poly1305 aead algorithm"
> 
> 
>