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

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

[Arek Kusztal]

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)
+{
+	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);
 
 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);
 
 
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 {
+	global:
+	rte_cryptodev_info_get;
+} DPDK_20.0;
+
+
 EXPERIMENTAL {
 	global:
 
-- 
2.1.0

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

[Akhil Goyal]

Hi Ray/Thomas/Bruce,

Could you please help review this patch wrt ABI policy?
> 
> 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>
> ---

Regards,
Akhil

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

[Thomas Monjalon]

14/04/2020 14:13, Akhil Goyal:
> Hi Ray/Thomas/Bruce,
> 
> Could you please help review this patch wrt ABI policy?
> > 
> > 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

What are Nodes 20.05 and 20.0?

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

rte_cryptodev_info is provided by the caller of the function.
I don't understand what you explained above, and the code is missing
some comments explaining the reason of the logic.

I still don't know whether this patch or not because it is missing
some clear explanations.

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

[Trahe, Fiona]

Hi Akhil, Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, April 14, 2020 2:04 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Cc: Ray Kinsella <mdr@ashroe.eu>; Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org;
> Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> 
> 14/04/2020 14:13, Akhil Goyal:
> > Hi Ray/Thomas/Bruce,
> >
> > Could you please help review this patch wrt ABI policy?
> > >
> > > 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
> 
> What are Nodes 20.05 and 20.0?
[Fiona] 20.0 is the designation for the API before 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.
> 
> rte_cryptodev_info is provided by the caller of the function.
> I don't understand what you explained above, and the code is missing
> some comments explaining the reason of the logic.
> 
> I still don't know whether this patch or not because it is missing
> some clear explanations.
[Fiona] We've identified that we also need to version the rte_cryptodev_capability_get() API
as it calls rte_cryptodev_info_get(). And the version handling oly works
on direct API calls, but not on calls within the API.
We will send a v2 to handle this.
And will add more explanation in the comments.  

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

[Ray Kinsella]

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

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

[Trahe, Fiona]

Hi Ray,

We're going to need hep to understand the numbering.
The examples here http://doc.dpdk.org/guides/contributing/abi_versioning.html#major-abi-versions
show only major numbers in the .map file.
Whereas the map files all have major.minor.

The example shows that to add a new version of an existing fn one should use the next major number, 
"When an ABI change is made between major ABI versions to a given library, a new section is added to
 that library’s version map describing the impending new ABI version"
so 
 - Old fn becomes fn_v20()
 - VERSION_SYMBOL(fn, _v20, 20); - maps node 20 to fn_v20() for dynamic linking
 - New fn becomes fn_v21()
 - BIND_DEFAULT_SYMBOL(fn, _v21, 21); - maps new builds to fn_v21 for dynamic linking and makes fn_v21 default
 - MAP_STATIC_SYMBOL(fn_proto, fn_v21); - for static linking
And the map file should have:

DPDK_21 {
	global:
	rte_cryptodev_info_get;
} DPDK_20;

When the ABI version moves to node 21 in DPDK20.11, the _v20 symbols can be removed.

You suggest using DPDK_20.0.2 and _v2002
Can you explain why? 
In 20.0.2 - which number is minor?
And why 3 numbers?

Regards,
Fiona


> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ray Kinsella
> Sent: Tuesday, April 14, 2020 2:54 PM
> To: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> 
> 
> 
> 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:
> >
> >

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

[Trahe, Fiona]

Hi Ray/Bruce/Thomas,  

We've been trying to make sense of the versioning scheme - but as the docs describe and give examples
 of a 2-number scheme and the code has a 3-number scheme it's pretty confusing.
If you can help me fill out the following table this may help:

VERSION | ABI_VERSION | LIBABIVER| stable soname | lib filename           | new section with ABI break in map file
19.11.0       |    20.0           |    1             | stable.so.20.0  | stable.so.20.0.0   |
20.02.0-rc0|    20.1           |     1             | n/a            
20.02.0       |    20.0.1         |    0.1          | stable.so.20.0  | stable.so.20.0.1  |
20.05.0       |    20.0.2         |    0.1         | stable.so.20.0  | stable.so.20.0.2   | 20.0.2           (or 21? use impending stable ABI_VERSION?) 
20.08.0       |    20.0.3         |    0.1         | stable.so.20.0  | stable.so.20.0.3   | 
20.11.0       |    21                |    0.1         | stable.so.21     | stable.so.21.0      |          (Can we move back to a 2 number ABI_VERSION at this stage? )
21.02.0       |    21.1             |   0.1          | stable.so.21     | stable.so.21.1      |

Note, first 2 ABI_VERSIONS above were mistakes, soname needs to be same as 19.11 to avoid ABI breakage, so it was changed to 3-part number, with first 2 used in soname.

Questions:
1. For this year, is major a 2-part num 20.0 (and there will never be a 20.1,20.2, etc) and minor is the 3rd number?
2. Can we move back to a 2-number scheme in 20.11?
3. What uses lib filename? (This is described in config/meson.build) Guessing stable soname is name on which apps depend, and is a sym link, pointing to lib filename?
4. What does LIBABIVER have to do with this ?  it was 1, changed to 0.1 in 20.2
5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0 
Or does it even matter, i.e. is the middle param to this macro arbitrary and just needs to match the fn name, e.g. if fn name was fn_vxyz could macro be: 
VERSION_SYMBOL(fn, _vxyz, 20.0); 
 5b. what should the name of the new fn be? fn_v2002 or fn_v21? Does it need to correspond to the new section in .map? If v2002, doesn't that imply it's compatible with 20.0 ABI
So shouldn't the new section be DPDK_21 and the new fn be fn_v21.
6. Assume we go with DPDK_21 and fn_v21. An app built against 20.05 has a dependency on fn_v21 (because of the BIND_DEFAULT) but on otherfn_v20.0 of all other fns?
If fn_v21 is changed again in 20.08, is there any expectation that the app built against 20.05 must work against the 20.08 fn of the same name?
i.e. is compatibility required on fn changes across minor nonstable releases? If not then jumping ahead to v21 seems the simplest option.
And the "final" fn_v21 in 20.11 is the one that "sticks" and belongs to the new ABI which then must remain stable til 21.11


> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Tuesday, April 14, 2020 7:27 PM
> To: Ray Kinsella <mdr@ashroe.eu>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> 
> Hi Ray,
> 
> We're going to need hep to understand the numbering.
> The examples here http://doc.dpdk.org/guides/contributing/abi_versioning.html#major-abi-versions
> show only major numbers in the .map file.
> Whereas the map files all have major.minor.
> 
> The example shows that to add a new version of an existing fn one should use the next major number,
> "When an ABI change is made between major ABI versions to a given library, a new section is added to
>  that library’s version map describing the impending new ABI version"
> so
>  - Old fn becomes fn_v20()
>  - VERSION_SYMBOL(fn, _v20, 20); - maps node 20 to fn_v20() for dynamic linking
>  - New fn becomes fn_v21()
>  - BIND_DEFAULT_SYMBOL(fn, _v21, 21); - maps new builds to fn_v21 for dynamic linking and makes
> fn_v21 default
>  - MAP_STATIC_SYMBOL(fn_proto, fn_v21); - for static linking
> And the map file should have:
> 
> DPDK_21 {
> 	global:
> 	rte_cryptodev_info_get;
> } DPDK_20;
> 
> When the ABI version moves to node 21 in DPDK20.11, the _v20 symbols can be removed.
> 
> You suggest using DPDK_20.0.2 and _v2002
> Can you explain why?
> In 20.0.2 - which number is minor?
> And why 3 numbers?
> 
> Regards,
> Fiona
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ray Kinsella
> > Sent: Tuesday, April 14, 2020 2:54 PM
> > To: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> >
> >
> >
> > 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:
> > >
> > >

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

[Bruce Richardson]

On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> Hi Ray/Bruce/Thomas,
> 
Hi Fiona,

answers as best I can give are inline below.

> We've been trying to make sense of the versioning scheme - but as the docs describe and give examples
>  of a 2-number scheme and the code has a 3-number scheme it's pretty confusing.
> If you can help me fill out the following table this may help:
> 
> VERSION | ABI_VERSION | LIBABIVER| stable soname | lib filename           | new section with ABI break in map file
> 19.11.0       |    20.0           |    1             | stable.so.20.0  | stable.so.20.0.0   |
> 20.02.0-rc0|    20.1           |     1             | n/a
> 20.02.0       |    20.0.1         |    0.1          | stable.so.20.0  | stable.so.20.0.1  |
> 20.05.0       |    20.0.2         |    0.1         | stable.so.20.0  | stable.so.20.0.2   | 20.0.2           (or 21? use impending stable ABI_VERSION?)
> 20.08.0       |    20.0.3         |    0.1         | stable.so.20.0  | stable.so.20.0.3   |
> 20.11.0       |    21                |    0.1         | stable.so.21     | stable.so.21.0      |          (Can we move back to a 2 number ABI_VERSION at this stage? )
> 21.02.0       |    21.1             |   0.1          | stable.so.21     | stable.so.21.1      |
> 
> Note, first 2 ABI_VERSIONS above were mistakes, soname needs to be same as 19.11 to avoid ABI breakage, so it was changed to 3-part number, with first 2 used in soname.
> 
> Questions:
> 1. For this year, is major a 2-part num 20.0 (and there will never be a 20.1,20.2, etc) and minor is the 3rd number?

Yes, for this year only.

> 2. Can we move back to a 2-number scheme in 20.11?

That is the plan, yes.

> 3. What uses lib filename? (This is described in config/meson.build) Guessing stable soname is name on which apps depend, and is a sym link, pointing to lib filename?

In general, yes.

> 4. What does LIBABIVER have to do with this ?  it was 1, changed to 0.1 in 20.2

Not sure about this one, what it refers to.

> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0

In technical terms it really doesn't matter, it's just a name that will be
looked up in a table. I don't think we strictly enforce the naming, so
whatever is clearest is best. I'd suggest the former.

> Or does it even matter, i.e. is the middle param to this macro arbitrary and just needs to match the fn name, e.g. if fn name was fn_vxyz could macro be:
> VERSION_SYMBOL(fn, _vxyz, 20.0);

Yes.

>  5b. what should the name of the new fn be? fn_v2002 or fn_v21? Does it need to correspond to the new section in .map? If v2002, doesn't that imply it's compatible with 20.0 ABI
> So shouldn't the new section be DPDK_21 and the new fn be fn_v21.
> 6. Assume we go with DPDK_21 and fn_v21. An app built against 20.05 has a dependency on fn_v21 (because of the BIND_DEFAULT) but on otherfn_v20.0 of all other fns?
> If fn_v21 is changed again in 20.08, is there any expectation that the app built against 20.05 must work against the 20.08 fn of the same name?
> i.e. is compatibility required on fn changes across minor nonstable releases? If not then jumping ahead to v21 seems the simplest option.
> And the "final" fn_v21 in 20.11 is the one that "sticks" and belongs to the new ABI which then must remain stable til 21.11

For functions that are part of the stable ABI, each change requires a new
version, since there is an expectation that 20.05 builds will also work
with 20.08.

Regards,
/Bruce

> 
> 
> > -----Original Message-----
> > From: Trahe, Fiona <fiona.trahe@intel.com>
> > Sent: Tuesday, April 14, 2020 7:27 PM
> > To: Ray Kinsella <mdr@ashroe.eu>; dev@dpdk.org
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> >
> > Hi Ray,
> >
> > We're going to need hep to understand the numbering.
> > The examples here http://doc.dpdk.org/guides/contributing/abi_versioning.html#major-abi-versions
> > show only major numbers in the .map file.
> > Whereas the map files all have major.minor.
> >
> > The example shows that to add a new version of an existing fn one should use the next major number,
> > "When an ABI change is made between major ABI versions to a given library, a new section is added to
> >  that library’s version map describing the impending new ABI version"
> > so
> >  - Old fn becomes fn_v20()
> >  - VERSION_SYMBOL(fn, _v20, 20); - maps node 20 to fn_v20() for dynamic linking
> >  - New fn becomes fn_v21()
> >  - BIND_DEFAULT_SYMBOL(fn, _v21, 21); - maps new builds to fn_v21 for dynamic linking and makes
> > fn_v21 default
> >  - MAP_STATIC_SYMBOL(fn_proto, fn_v21); - for static linking
> > And the map file should have:
> >
> > DPDK_21 {
> > global:
> > rte_cryptodev_info_get;
> > } DPDK_20;
> >
> > When the ABI version moves to node 21 in DPDK20.11, the _v20 symbols can be removed.
> >
> > You suggest using DPDK_20.0.2 and _v2002
> > Can you explain why?
> > In 20.0.2 - which number is minor?
> > And why 3 numbers?
> >
> > Regards,
> > Fiona
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ray Kinsella
> > > Sent: Tuesday, April 14, 2020 2:54 PM
> > > To: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> > >
> > >
> > >
> > > 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:
> > > >
> > > >

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

[Thomas Monjalon]

16/04/2020 11:51, Bruce Richardson:
> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> > 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
> 
> In technical terms it really doesn't matter, it's just a name that will be
> looked up in a table. I don't think we strictly enforce the naming, so
> whatever is clearest is best. I'd suggest the former.

Each release can have a new ABI.
The same function can have a different version in 20.02, 20.05 and 20.08.
If you name it fn_v20 in 20.05, what will be the name for the new version
in 20.08? I suggest using the release number when versioning a function.

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

[Ray Kinsella]

On 16/04/2020 11:01, Thomas Monjalon wrote:
> 16/04/2020 11:51, Bruce Richardson:
>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
>>
>> In technical terms it really doesn't matter, it's just a name that will be
>> looked up in a table. I don't think we strictly enforce the naming, so
>> whatever is clearest is best. I'd suggest the former.
> 
> Each release can have a new ABI.

How many ABI's do we want to support?

> The same function can have a different version in 20.02, 20.05 and 20.08.
> If you name it fn_v20 in 20.05, what will be the name for the new version
> in 20.08? I suggest using the release number when versioning a function.
> 

ok - so this is exactly _not_ what we wanted at the outset.

We committed to support a single ABI, that is the 19.11 (v20) ABI until the 20.11 (v21) ABI.
We do _not_ support ABI stability in quarterly releases, as the support overhead would balloon overtime. 

Really why would we do this - who would benefit?

Do we envisage a situation that someone who built against the say 20.02 shared libraries.
would expect their binaries to port to 20.05 without a rebuild?

It would also defeat the purpose of EXPERIMENTAL, if the community where willing to support any permutation of an API.
Why would ever bother to use experimental?

I have a bit more checking to do, but IMHO the following we should fix the following commits such that APIs are either EXPERIMENTAL or staged for v21. 

root@ashroe.eu:/build/dpdk# find . -name *.map | xargs grep 20.0.1
./lib/librte_meter/rte_meter_version.map:DPDK_20.0.1 {
./drivers/vdpa/mlx5/rte_pmd_mlx5_vdpa_version.map:DPDK_20.0.1 {
./drivers/net/ionic/rte_pmd_ionic_version.map:DPDK_20.0.1 {
./drivers/common/octeontx2/rte_common_octeontx2_version.map:DPDK_20.0.1 {
./drivers/common/mlx5/rte_common_mlx5_version.map:DPDK_20.0.1 {
./drivers/raw/octeontx2_ep/rte_rawdev_octeontx2_ep_version.map:DPDK_20.0.1 {

Thanks, 

Ray K

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

[Bruce Richardson]

On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
> 
> 
> On 16/04/2020 11:01, Thomas Monjalon wrote:
> > 16/04/2020 11:51, Bruce Richardson:
> >> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> >>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
> >>
> >> In technical terms it really doesn't matter, it's just a name that will be
> >> looked up in a table. I don't think we strictly enforce the naming, so
> >> whatever is clearest is best. I'd suggest the former.
> > 
> > Each release can have a new ABI.
> 
> How many ABI's do we want to support?
> 
It's not how many we want to support, but for me it's a matter of how many
do we need to support. If an API is part of the stable set, it can't just
drop to being experimental for one or two releases - it's always stable
until deprecated. We also shouldn't have a situation where release 20.08 is
ABI compatible with 19.11 but not 20.02 and 20.05.

> > The same function can have a different version in 20.02, 20.05 and 20.08.
> > If you name it fn_v20 in 20.05, what will be the name for the new version
> > in 20.08? I suggest using the release number when versioning a function.
> > 
> 
> ok - so this is exactly _not_ what we wanted at the outset.
> 
> We committed to support a single ABI, that is the 19.11 (v20) ABI until the 20.11 (v21) ABI.
> We do _not_ support ABI stability in quarterly releases, as the support overhead would balloon overtime. 
> 
> Really why would we do this - who would benefit?
> 
> Do we envisage a situation that someone who built against the say 20.02 shared libraries.
> would expect their binaries to port to 20.05 without a rebuild?
> 
I would have expected that, yes, as all have the v20 ABI.
Maybe I need to change my expectations, though.

/Bruce

> It would also defeat the purpose of EXPERIMENTAL, if the community where willing to support any permutation of an API.
> Why would ever bother to use experimental?
> 
> I have a bit more checking to do, but IMHO the following we should fix the following commits such that APIs are either EXPERIMENTAL or staged for v21. 
> 
> root@ashroe.eu:/build/dpdk# find . -name *.map | xargs grep 20.0.1
> ./lib/librte_meter/rte_meter_version.map:DPDK_20.0.1 {
> ./drivers/vdpa/mlx5/rte_pmd_mlx5_vdpa_version.map:DPDK_20.0.1 {
> ./drivers/net/ionic/rte_pmd_ionic_version.map:DPDK_20.0.1 {
> ./drivers/common/octeontx2/rte_common_octeontx2_version.map:DPDK_20.0.1 {
> ./drivers/common/mlx5/rte_common_mlx5_version.map:DPDK_20.0.1 {
> ./drivers/raw/octeontx2_ep/rte_rawdev_octeontx2_ep_version.map:DPDK_20.0.1 {
> 
> Thanks, 
> 
> Ray K
> 
> 

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

[Ray Kinsella]

On 17/04/2020 10:31, Bruce Richardson wrote:
> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
>>
>>
>> On 16/04/2020 11:01, Thomas Monjalon wrote:
>>> 16/04/2020 11:51, Bruce Richardson:
>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
>>>>
>>>> In technical terms it really doesn't matter, it's just a name that will be
>>>> looked up in a table. I don't think we strictly enforce the naming, so
>>>> whatever is clearest is best. I'd suggest the former.
>>>
>>> Each release can have a new ABI.
>>
>> How many ABI's do we want to support?
>>
> It's not how many we want to support, but for me it's a matter of how many
> do we need to support. If an API is part of the stable set, it can't just
> drop to being experimental for one or two releases - it's always stable
> until deprecated. We also shouldn't have a situation where release 20.08 is
> ABI compatible with 19.11 but not 20.02 and 20.05.

True. Let me say it differently.

Our only commitment is to support v20 - 19.11
However you are correct, if something gets committed as v21 in 20.02, in practise should also be there in 20.05+ also.
Because if it is committed as v21 and not as experimental, it should not be changing once committed.  

In answering Thomas, 
I was more commenting on the proliferation of ABI numbers & symbols we need to track in the build.
With v20, v21 & Experimental we need to keep track of 3.
If we start allowing quarterly builds to have managed ABI's, it will get confusing. 

> 
>>> The same function can have a different version in 20.02, 20.05 and 20.08.
>>> If you name it fn_v20 in 20.05, what will be the name for the new version
>>> in 20.08? I suggest using the release number when versioning a function.
>>>
>>
>> ok - so this is exactly _not_ what we wanted at the outset.
>>
>> We committed to support a single ABI, that is the 19.11 (v20) ABI until the 20.11 (v21) ABI.
>> We do _not_ support ABI stability in quarterly releases, as the support overhead would balloon overtime. 
>>
>> Really why would we do this - who would benefit?
>>
>> Do we envisage a situation that someone who built against the say 20.02 shared libraries.
>> would expect their binaries to port to 20.05 without a rebuild?
>>
> I would have expected that, yes, as all have the v20 ABI.
> Maybe I need to change my expectations, though.

You are correct.
However I guess, I would still see them as slightly different levels of commitment.

> 
> /Bruce
> 
>> It would also defeat the purpose of EXPERIMENTAL, if the community where willing to support any permutation of an API.
>> Why would ever bother to use experimental?
>>
>> I have a bit more checking to do, but IMHO the following we should fix the following commits such that APIs are either EXPERIMENTAL or staged for v21. 
>>
>> root@ashroe.eu:/build/dpdk# find . -name *.map | xargs grep 20.0.1
>> ./lib/librte_meter/rte_meter_version.map:DPDK_20.0.1 {
>> ./drivers/vdpa/mlx5/rte_pmd_mlx5_vdpa_version.map:DPDK_20.0.1 {
>> ./drivers/net/ionic/rte_pmd_ionic_version.map:DPDK_20.0.1 {
>> ./drivers/common/octeontx2/rte_common_octeontx2_version.map:DPDK_20.0.1 {
>> ./drivers/common/mlx5/rte_common_mlx5_version.map:DPDK_20.0.1 {
>> ./drivers/raw/octeontx2_ep/rte_rawdev_octeontx2_ep_version.map:DPDK_20.0.1 {
>>
>> Thanks, 
>>
>> Ray K
>>
>>

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

[Thomas Monjalon]

17/04/2020 11:42, Ray Kinsella:
> On 17/04/2020 10:31, Bruce Richardson wrote:
> > On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
> >> On 16/04/2020 11:01, Thomas Monjalon wrote:
> >>> 16/04/2020 11:51, Bruce Richardson:
> >>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> >>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
> >>>>
> >>>> In technical terms it really doesn't matter, it's just a name that will be
> >>>> looked up in a table. I don't think we strictly enforce the naming, so
> >>>> whatever is clearest is best. I'd suggest the former.
> >>>
> >>> Each release can have a new ABI.
> >>
> >> How many ABI's do we want to support?
> >>
> > It's not how many we want to support, but for me it's a matter of how many
> > do we need to support. If an API is part of the stable set, it can't just
> > drop to being experimental for one or two releases - it's always stable
> > until deprecated. We also shouldn't have a situation where release 20.08 is
> > ABI compatible with 19.11 but not 20.02 and 20.05.
> 
> True. Let me say it differently.
> 
> Our only commitment is to support v20 - 19.11
> However you are correct, if something gets committed as v21 in 20.02, in practise should also be there in 20.05+ also.
> Because if it is committed as v21 and not as experimental, it should not be changing once committed.  
> 
> In answering Thomas, 
> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the build.
> With v20, v21 & Experimental we need to keep track of 3.
> If we start allowing quarterly builds to have managed ABI's, it will get confusing. 

I don't remember why we are using intermediate ABI versions
between v20 and v21.
If we can use v21 for new ABI and make sure compatibility is maintained
between all versions from 19.11 to 20.08, I'm fine.

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

[Ray Kinsella]

On 17/04/2020 11:17, Thomas Monjalon wrote:
> 17/04/2020 11:42, Ray Kinsella:
>> On 17/04/2020 10:31, Bruce Richardson wrote:
>>> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
>>>> On 16/04/2020 11:01, Thomas Monjalon wrote:
>>>>> 16/04/2020 11:51, Bruce Richardson:
>>>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
>>>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the original function be? fn_v20, or fn_v20.0
>>>>>>
>>>>>> In technical terms it really doesn't matter, it's just a name that will be
>>>>>> looked up in a table. I don't think we strictly enforce the naming, so
>>>>>> whatever is clearest is best. I'd suggest the former.
>>>>>
>>>>> Each release can have a new ABI.
>>>>
>>>> How many ABI's do we want to support?
>>>>
>>> It's not how many we want to support, but for me it's a matter of how many
>>> do we need to support. If an API is part of the stable set, it can't just
>>> drop to being experimental for one or two releases - it's always stable
>>> until deprecated. We also shouldn't have a situation where release 20.08 is
>>> ABI compatible with 19.11 but not 20.02 and 20.05.
>>
>> True. Let me say it differently.
>>
>> Our only commitment is to support v20 - 19.11
>> However you are correct, if something gets committed as v21 in 20.02, in practise should also be there in 20.05+ also.
>> Because if it is committed as v21 and not as experimental, it should not be changing once committed.  
>>
>> In answering Thomas, 
>> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the build.
>> With v20, v21 & Experimental we need to keep track of 3.
>> If we start allowing quarterly builds to have managed ABI's, it will get confusing. 
> 
> I don't remember why we are using intermediate ABI versions
> between v20 and v21.
> If we can use v21 for new ABI and make sure compatibility is maintained
> between all versions from 19.11 to 20.08, I'm fine.
> 

Well I guess we missed this in 20.02, so I recommend that we fix in 20.05.

It will mean that a couple of symbols versioned 20.0.2 in DPDK 20.02, 
Will becomes 21.0 in DPDK 20.05 ... ABI checker will complain.

Apart from that I doubt anyone will notice?

Ray K

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

[Trahe, Fiona]

Hi all,

> -----Original Message-----
> From: Ray Kinsella <mdr@ashroe.eu>
> Sent: Friday, April 17, 2020 11:34 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce <bruce.richardson@intel.com>
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Luca Boccassi
> <bluca@debian.org>; Kevin Traynor <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> 
> 
> 
> On 17/04/2020 11:17, Thomas Monjalon wrote:
> > 17/04/2020 11:42, Ray Kinsella:
> >> On 17/04/2020 10:31, Bruce Richardson wrote:
> >>> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
> >>>> On 16/04/2020 11:01, Thomas Monjalon wrote:
> >>>>> 16/04/2020 11:51, Bruce Richardson:
> >>>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> >>>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the
> original function be? fn_v20, or fn_v20.0
> >>>>>>
> >>>>>> In technical terms it really doesn't matter, it's just a name that will be
> >>>>>> looked up in a table. I don't think we strictly enforce the naming, so
> >>>>>> whatever is clearest is best. I'd suggest the former.
> >>>>>
> >>>>> Each release can have a new ABI.
> >>>>
> >>>> How many ABI's do we want to support?
> >>>>
> >>> It's not how many we want to support, but for me it's a matter of how many
> >>> do we need to support. If an API is part of the stable set, it can't just
> >>> drop to being experimental for one or two releases - it's always stable
> >>> until deprecated. We also shouldn't have a situation where release 20.08 is
> >>> ABI compatible with 19.11 but not 20.02 and 20.05.
> >>
> >> True. Let me say it differently.
> >>
> >> Our only commitment is to support v20 - 19.11
> >> However you are correct, if something gets committed as v21 in 20.02, in practise should also be
> there in 20.05+ also.
> >> Because if it is committed as v21 and not as experimental, it should not be changing once
> committed.
> >>
> >> In answering Thomas,
> >> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the
> build.
> >> With v20, v21 & Experimental we need to keep track of 3.
> >> If we start allowing quarterly builds to have managed ABI's, it will get confusing.
> >
> > I don't remember why we are using intermediate ABI versions
> > between v20 and v21.
> > If we can use v21 for new ABI and make sure compatibility is maintained
> > between all versions from 19.11 to 20.08, I'm fine.
[Fiona] Here's a hypothetical case, but it illustrates why I don't think there 
should be an expectation to maintain ABI compatibility here.
Example: in 20.05 add a new info_get_v21() which includes ChaChaPoly.
In 20.08 add another new algorithm. info_get_v21() return now includes this. 
info_get_v21() will become stable in 20.11 and compatibility must be maintained from then on.
In the meantime, the fn is not experimental - that wouldn't be appropriate as it was a stable API.
But an app either wants stability and so should build against 19.11, or if prepared to move up to 
one non-stable-ABI quarterly release should be willing to rebuild for the next non-stable-ABI quarterly release.
I think it's an unnecessary burden to require ABI compatibility across quarterly releases.
And if required could end up with the version tracking hassle Ray referred to above with fn versions
of 20.0.1, 20.0.2, 20.0.3, v21, and potentially several versions of same fn.

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

[Ray Kinsella]

On 17/04/2020 12:46, Trahe, Fiona wrote:
> Hi all,
> 
>> -----Original Message-----
>> From: Ray Kinsella <mdr@ashroe.eu>
>> Sent: Friday, April 17, 2020 11:34 AM
>> To: Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce <bruce.richardson@intel.com>
>> Cc: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; Kusztal, ArkadiuszX
>> <arkadiuszx.kusztal@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Luca Boccassi
>> <bluca@debian.org>; Kevin Traynor <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
>>
>>
>>
>> On 17/04/2020 11:17, Thomas Monjalon wrote:
>>> 17/04/2020 11:42, Ray Kinsella:
>>>> On 17/04/2020 10:31, Bruce Richardson wrote:
>>>>> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
>>>>>> On 16/04/2020 11:01, Thomas Monjalon wrote:
>>>>>>> 16/04/2020 11:51, Bruce Richardson:
>>>>>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
>>>>>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the
>> original function be? fn_v20, or fn_v20.0
>>>>>>>>
>>>>>>>> In technical terms it really doesn't matter, it's just a name that will be
>>>>>>>> looked up in a table. I don't think we strictly enforce the naming, so
>>>>>>>> whatever is clearest is best. I'd suggest the former.
>>>>>>>
>>>>>>> Each release can have a new ABI.
>>>>>>
>>>>>> How many ABI's do we want to support?
>>>>>>
>>>>> It's not how many we want to support, but for me it's a matter of how many
>>>>> do we need to support. If an API is part of the stable set, it can't just
>>>>> drop to being experimental for one or two releases - it's always stable
>>>>> until deprecated. We also shouldn't have a situation where release 20.08 is
>>>>> ABI compatible with 19.11 but not 20.02 and 20.05.
>>>>
>>>> True. Let me say it differently.
>>>>
>>>> Our only commitment is to support v20 - 19.11
>>>> However you are correct, if something gets committed as v21 in 20.02, in practise should also be
>> there in 20.05+ also.
>>>> Because if it is committed as v21 and not as experimental, it should not be changing once
>> committed.
>>>>
>>>> In answering Thomas,
>>>> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the
>> build.
>>>> With v20, v21 & Experimental we need to keep track of 3.
>>>> If we start allowing quarterly builds to have managed ABI's, it will get confusing.
>>>
>>> I don't remember why we are using intermediate ABI versions
>>> between v20 and v21.
>>> If we can use v21 for new ABI and make sure compatibility is maintained
>>> between all versions from 19.11 to 20.08, I'm fine.
> [Fiona] Here's a hypothetical case, but it illustrates why I don't think there 
> should be an expectation to maintain ABI compatibility here.
> Example: in 20.05 add a new info_get_v21() which includes ChaChaPoly.
> In 20.08 add another new algorithm. info_get_v21() return now includes this. 
> info_get_v21() will become stable in 20.11 and compatibility must be maintained from then on.

Yes, and the v20 version gets removed in 20.11.

> In the meantime, the fn is not experimental - that wouldn't be appropriate as it was a stable API.

Well it could be ... these are just labels after all.
So it entirely possible and permitted to have a info_get@DPDK_20.0 and info_get@EXPERIMENTAL simultaneously. 

In fact, if it is your expectation that new version will change in successively quarterly releases.
That suggests it should be experimental. 

There in a gap here, we appear to be missing the required MACROS to support a v20.0 + EXPERIMENTAL simultaneously.

As I remember this api was particularly sensitive to size of an enumeration?
Has that been resolved to make the api less brittle?

> But an app either wants stability and so should build against 19.11, or if prepared to move up to 
> one non-stable-ABI quarterly release should be willing to rebuild for the next non-stable-ABI quarterly release.
> I think it's an unnecessary burden to require ABI compatibility across quarterly releases.

I generally agree with this point.
Our only cast iron solid commitment is compatibility with 19.11.

That said, once any API has dropped the EXPERIMENTAL label, it shouldn't be changing. 
All things being equal quarterly releases should be compatible with each other, as v21 APIs should not be changing. 

> And if required could end up with the version tracking hassle Ray referred to above with fn versions
> of 20.0.1, 20.0.2, 20.0.3, v21, and potentially several versions of same fn.
> 
> 

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

[Trahe, Fiona]

Gentle reminder that we still haven't come to a consensus about
whether ABI compatibility is required across quarterly releases or not.
See below.

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Friday, April 17, 2020 12:47 PM
> To: Ray Kinsella <mdr@ashroe.eu>; Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Luca Boccassi <bluca@debian.org>; Kevin Traynor
> <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> 
> Hi all,
> 
> > -----Original Message-----
> > From: Ray Kinsella <mdr@ashroe.eu>
> > Sent: Friday, April 17, 2020 11:34 AM
> > To: Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce <bruce.richardson@intel.com>
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Luca Boccassi
> > <bluca@debian.org>; Kevin Traynor <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
> >
> >
> >
> > On 17/04/2020 11:17, Thomas Monjalon wrote:
> > > 17/04/2020 11:42, Ray Kinsella:
> > >> On 17/04/2020 10:31, Bruce Richardson wrote:
> > >>> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
> > >>>> On 16/04/2020 11:01, Thomas Monjalon wrote:
> > >>>>> 16/04/2020 11:51, Bruce Richardson:
> > >>>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
> > >>>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the
> > original function be? fn_v20, or fn_v20.0
> > >>>>>>
> > >>>>>> In technical terms it really doesn't matter, it's just a name that will be
> > >>>>>> looked up in a table. I don't think we strictly enforce the naming, so
> > >>>>>> whatever is clearest is best. I'd suggest the former.
> > >>>>>
> > >>>>> Each release can have a new ABI.
> > >>>>
> > >>>> How many ABI's do we want to support?
> > >>>>
> > >>> It's not how many we want to support, but for me it's a matter of how many
> > >>> do we need to support. If an API is part of the stable set, it can't just
> > >>> drop to being experimental for one or two releases - it's always stable
> > >>> until deprecated. We also shouldn't have a situation where release 20.08 is
> > >>> ABI compatible with 19.11 but not 20.02 and 20.05.
> > >>
> > >> True. Let me say it differently.
> > >>
> > >> Our only commitment is to support v20 - 19.11
> > >> However you are correct, if something gets committed as v21 in 20.02, in practise should also be
> > there in 20.05+ also.
> > >> Because if it is committed as v21 and not as experimental, it should not be changing once
> > committed.
> > >>
> > >> In answering Thomas,
> > >> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the
> > build.
> > >> With v20, v21 & Experimental we need to keep track of 3.
> > >> If we start allowing quarterly builds to have managed ABI's, it will get confusing.
> > >
> > > I don't remember why we are using intermediate ABI versions
> > > between v20 and v21.
> > > If we can use v21 for new ABI and make sure compatibility is maintained
> > > between all versions from 19.11 to 20.08, I'm fine.
> [Fiona] Here's a hypothetical case, but it illustrates why I don't think there
> should be an expectation to maintain ABI compatibility here.
> Example: in 20.05 add a new info_get_v21() which includes ChaChaPoly.
> In 20.08 add another new algorithm. info_get_v21() return now includes this.
> info_get_v21() will become stable in 20.11 and compatibility must be maintained from then on.
> In the meantime, the fn is not experimental - that wouldn't be appropriate as it was a stable API.
> But an app either wants stability and so should build against 19.11, or if prepared to move up to
> one non-stable-ABI quarterly release should be willing to rebuild for the next non-stable-ABI quarterly
> release.
> I think it's an unnecessary burden to require ABI compatibility across quarterly releases.
> And if required could end up with the version tracking hassle Ray referred to above with fn versions
> of 20.0.1, 20.0.2, 20.0.3, v21, and potentially several versions of same fn.
> 

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

[Ray Kinsella]

Our only commitment is to the stability of the v19.11/v20 ABI, until v21. 

That said, once an ABI migrates from EXPERIMENTAL to v21, it _shouldn't_ be changing.
We don't have a strict commitment to the v21 ABI until v20.11.

However if v21 is changing across quarterlies (outside of additions) ... something else is wrong.

Ray K


On 20/04/2020 17:59, Trahe, Fiona wrote:
> Gentle reminder that we still haven't come to a consensus about
> whether ABI compatibility is required across quarterly releases or not.
> See below.
> 
>> -----Original Message-----
>> From: Trahe, Fiona <fiona.trahe@intel.com>
>> Sent: Friday, April 17, 2020 12:47 PM
>> To: Ray Kinsella <mdr@ashroe.eu>; Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce
>> <bruce.richardson@intel.com>
>> Cc: dev@dpdk.org; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Neil Horman
>> <nhorman@tuxdriver.com>; Luca Boccassi <bluca@debian.org>; Kevin Traynor
>> <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Trahe, Fiona
>> <fiona.trahe@intel.com>
>> Subject: RE: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
>>
>> Hi all,
>>
>>> -----Original Message-----
>>> From: Ray Kinsella <mdr@ashroe.eu>
>>> Sent: Friday, April 17, 2020 11:34 AM
>>> To: Thomas Monjalon <thomas@monjalon.net>; Richardson, Bruce <bruce.richardson@intel.com>
>>> Cc: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org; Kusztal, ArkadiuszX
>>> <arkadiuszx.kusztal@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Luca Boccassi
>>> <bluca@debian.org>; Kevin Traynor <ktraynor@redhat.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>>> Subject: Re: [dpdk-dev] [PATCH] cryptodev: version rte_cryptodev_info_get function
>>>
>>>
>>>
>>> On 17/04/2020 11:17, Thomas Monjalon wrote:
>>>> 17/04/2020 11:42, Ray Kinsella:
>>>>> On 17/04/2020 10:31, Bruce Richardson wrote:
>>>>>> On Fri, Apr 17, 2020 at 08:24:30AM +0100, Ray Kinsella wrote:
>>>>>>> On 16/04/2020 11:01, Thomas Monjalon wrote:
>>>>>>>> 16/04/2020 11:51, Bruce Richardson:
>>>>>>>>> On Wed, Apr 15, 2020 at 06:24:19PM +0100, Trahe, Fiona wrote:
>>>>>>>>>> 5a. If in 20.05 we add a version of a fn which breaks ABI 20.0, what should the name of the
>>> original function be? fn_v20, or fn_v20.0
>>>>>>>>>
>>>>>>>>> In technical terms it really doesn't matter, it's just a name that will be
>>>>>>>>> looked up in a table. I don't think we strictly enforce the naming, so
>>>>>>>>> whatever is clearest is best. I'd suggest the former.
>>>>>>>>
>>>>>>>> Each release can have a new ABI.
>>>>>>>
>>>>>>> How many ABI's do we want to support?
>>>>>>>
>>>>>> It's not how many we want to support, but for me it's a matter of how many
>>>>>> do we need to support. If an API is part of the stable set, it can't just
>>>>>> drop to being experimental for one or two releases - it's always stable
>>>>>> until deprecated. We also shouldn't have a situation where release 20.08 is
>>>>>> ABI compatible with 19.11 but not 20.02 and 20.05.
>>>>>
>>>>> True. Let me say it differently.
>>>>>
>>>>> Our only commitment is to support v20 - 19.11
>>>>> However you are correct, if something gets committed as v21 in 20.02, in practise should also be
>>> there in 20.05+ also.
>>>>> Because if it is committed as v21 and not as experimental, it should not be changing once
>>> committed.
>>>>>
>>>>> In answering Thomas,
>>>>> I was more commenting on the proliferation of ABI numbers & symbols we need to track in the
>>> build.
>>>>> With v20, v21 & Experimental we need to keep track of 3.
>>>>> If we start allowing quarterly builds to have managed ABI's, it will get confusing.
>>>>
>>>> I don't remember why we are using intermediate ABI versions
>>>> between v20 and v21.
>>>> If we can use v21 for new ABI and make sure compatibility is maintained
>>>> between all versions from 19.11 to 20.08, I'm fine.
>> [Fiona] Here's a hypothetical case, but it illustrates why I don't think there
>> should be an expectation to maintain ABI compatibility here.
>> Example: in 20.05 add a new info_get_v21() which includes ChaChaPoly.
>> In 20.08 add another new algorithm. info_get_v21() return now includes this.
>> info_get_v21() will become stable in 20.11 and compatibility must be maintained from then on.
>> In the meantime, the fn is not experimental - that wouldn't be appropriate as it was a stable API.
>> But an app either wants stability and so should build against 19.11, or if prepared to move up to
>> one non-stable-ABI quarterly release should be willing to rebuild for the next non-stable-ABI quarterly
>> release.
>> I think it's an unnecessary burden to require ABI compatibility across quarterly releases.
>> And if required could end up with the version tracking hassle Ray referred to above with fn versions
>> of 20.0.1, 20.0.2, 20.0.3, v21, and potentially several versions of same fn.
>>
> 

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

[Thomas Monjalon]

20/04/2020 19:31, Ray Kinsella:
> 
> Our only commitment is to the stability of the v19.11/v20 ABI, until v21. 
> 
> That said, once an ABI migrates from EXPERIMENTAL to v21, it _shouldn't_ be changing.
> We don't have a strict commitment to the v21 ABI until v20.11.
> 
> However if v21 is changing across quarterlies (outside of additions) ... something else is wrong.

The only way to check a symbol is not changing in a quarterly release
is to test it. That's what we wanted to enforce:
compare 20.02 ABI in 20.05 release.

What other process do you suggest?

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

[Ray Kinsella]

On 20/04/2020 18:37, Thomas Monjalon wrote:
> 20/04/2020 19:31, Ray Kinsella:
>>
>> Our only commitment is to the stability of the v19.11/v20 ABI, until v21. 
>>
>> That said, once an ABI migrates from EXPERIMENTAL to v21, it _shouldn't_ be changing.
>> We don't have a strict commitment to the v21 ABI until v20.11.
>>
>> However if v21 is changing across quarterlies (outside of additions) ... something else is wrong.
> 
> The only way to check a symbol is not changing in a quarterly release
> is to test it. That's what we wanted to enforce:
> compare 20.02 ABI in 20.05 release.
> 
> What other process do you suggest?
> 

Well I guess it's understanding the reason why you are doing something.
I can see reasons for wanting to test against both v19.11 and v20.02.

v19.11 because our strict commitment is to the v20 abi.
v20.02 to ensure that v21 symbols are not changing between quarterly releases.

On v20, since you tested v20.02 against v19.11 during the v20.02 release cycle.
The v20 symbols should not have changed during the v20.02 release cycle. 

I take your point, that then testing v20.05 against v20.02 would catch both v20 and v21 changes. 

Thanks,

Ray K

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

[Thomas Monjalon]

21/04/2020 08:01, Ray Kinsella:
> 
> On 20/04/2020 18:37, Thomas Monjalon wrote:
> > 20/04/2020 19:31, Ray Kinsella:
> >>
> >> Our only commitment is to the stability of the v19.11/v20 ABI, until v21. 
> >>
> >> That said, once an ABI migrates from EXPERIMENTAL to v21, it _shouldn't_ be changing.
> >> We don't have a strict commitment to the v21 ABI until v20.11.
> >>
> >> However if v21 is changing across quarterlies (outside of additions) ... something else is wrong.
> > 
> > The only way to check a symbol is not changing in a quarterly release
> > is to test it. That's what we wanted to enforce:
> > compare 20.02 ABI in 20.05 release.
> > 
> > What other process do you suggest?
> > 
> 
> Well I guess it's understanding the reason why you are doing something.
> I can see reasons for wanting to test against both v19.11 and v20.02.
> 
> v19.11 because our strict commitment is to the v20 abi.
> v20.02 to ensure that v21 symbols are not changing between quarterly releases.
> 
> On v20, since you tested v20.02 against v19.11 during the v20.02 release cycle.
> The v20 symbols should not have changed during the v20.02 release cycle. 
> 
> I take your point, that then testing v20.05 against v20.02 would catch both v20 and v21 changes. 

OK, so we need a policy or process update to make this conclusion clear to everybody.

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

[Ray Kinsella]

On 21/04/2020 10:36, Thomas Monjalon wrote:
> 21/04/2020 08:01, Ray Kinsella:
>>
>> On 20/04/2020 18:37, Thomas Monjalon wrote:
>>> 20/04/2020 19:31, Ray Kinsella:
>>>>
>>>> Our only commitment is to the stability of the v19.11/v20 ABI, until v21. 
>>>>
>>>> That said, once an ABI migrates from EXPERIMENTAL to v21, it _shouldn't_ be changing.
>>>> We don't have a strict commitment to the v21 ABI until v20.11.
>>>>
>>>> However if v21 is changing across quarterlies (outside of additions) ... something else is wrong.
>>>
>>> The only way to check a symbol is not changing in a quarterly release
>>> is to test it. That's what we wanted to enforce:
>>> compare 20.02 ABI in 20.05 release.
>>>
>>> What other process do you suggest?
>>>
>>
>> Well I guess it's understanding the reason why you are doing something.
>> I can see reasons for wanting to test against both v19.11 and v20.02.
>>
>> v19.11 because our strict commitment is to the v20 abi.
>> v20.02 to ensure that v21 symbols are not changing between quarterly releases.
>>
>> On v20, since you tested v20.02 against v19.11 during the v20.02 release cycle.
>> The v20 symbols should not have changed during the v20.02 release cycle. 
>>
>> I take your point, that then testing v20.05 against v20.02 would catch both v20 and v21 changes. 
> 
> OK, so we need a policy or process update to make this conclusion clear to everybody.
> 

yes, I replied to David Marchand on this separately. 

In the case the process is already in place ... Travis is routinely testing against v20.02.
If we wanted to make this abundantly clear we could add a note/section explaining the process to 
guides/contributing/abi_versioning.rst ?

So I think we need two other policy additions. 

1. Fiona is clear, that we need a note on the policy document to cover the version numbering error 
that was made in DPDK 19.11. Bruce's commit (f26c2b39b271cdcd857ba518c5e48c78cb1c30af) on the subject, 
explains it very well ... 

2. We also need to merge "doc: alias to experimental tag for stable apis"