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-boun...@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.kusz...@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:
> >
> >
