This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the rte_cryptodev_info_get function versioning was a temporary solution to maintain ABI compatibility for ChaCha20-Poly1305 and is not needed in 20.11. Fixes: a0f0de06d457 ("cryptodev: fix ABI compatibility for ChaCha20-Poly1305") Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com> Reviewed-by: Arek Kusztal <arkadiuszx.kusztal@intel.com> --- lib/librte_cryptodev/meson.build | 1 - lib/librte_cryptodev/rte_cryptodev.c | 147 +----------------- lib/librte_cryptodev/rte_cryptodev.h | 34 +--- .../rte_cryptodev_version.map | 6 - 4 files changed, 6 insertions(+), 182 deletions(-) diff --git a/lib/librte_cryptodev/meson.build b/lib/librte_cryptodev/meson.build index df1144058..c4c6b3b6a 100644 --- a/lib/librte_cryptodev/meson.build +++ b/lib/librte_cryptodev/meson.build @@ -1,7 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017-2019 Intel Corporation -use_function_versioning = true sources = files('rte_cryptodev.c', 'rte_cryptodev_pmd.c', 'cryptodev_trace_points.c') headers = files('rte_cryptodev.h', 'rte_cryptodev_pmd.h', diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c index 1dd795bcb..6c9a19f25 100644 --- a/lib/librte_cryptodev/rte_cryptodev.c +++ b/lib/librte_cryptodev/rte_cryptodev.c @@ -36,8 +36,6 @@ #include <rte_errno.h> #include <rte_spinlock.h> #include <rte_string_fns.h> -#include <rte_compat.h> -#include <rte_function_versioning.h> #include "rte_crypto.h" #include "rte_cryptodev.h" @@ -59,14 +57,6 @@ static struct rte_cryptodev_global cryptodev_globals = { /* spinlock for crypto device callbacks */ static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER; -static const struct rte_cryptodev_capabilities - cryptodev_undefined_capabilities[] = { - RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() -}; - -static struct rte_cryptodev_capabilities - *capability_copy[RTE_CRYPTO_MAX_DEVS]; -static uint8_t is_capability_checked[RTE_CRYPTO_MAX_DEVS]; /** * The user application callback description. @@ -291,43 +281,8 @@ rte_crypto_auth_operation_strings[] = { [RTE_CRYPTO_AUTH_OP_GENERATE] = "generate" }; -const struct rte_cryptodev_symmetric_capability __vsym * -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx) -{ - const struct rte_cryptodev_capabilities *capability; - struct rte_cryptodev_info dev_info; - int i = 0; - - rte_cryptodev_info_get_v20(dev_id, &dev_info); - - while ((capability = &dev_info.capabilities[i++])->op != - RTE_CRYPTO_OP_TYPE_UNDEFINED) { - if (capability->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC) - continue; - - if (capability->sym.xform_type != idx->type) - continue; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_AUTH && - capability->sym.auth.algo == idx->algo.auth) - return &capability->sym; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_CIPHER && - capability->sym.cipher.algo == idx->algo.cipher) - return &capability->sym; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_AEAD && - capability->sym.aead.algo == idx->algo.aead) - return &capability->sym; - } - - return NULL; -} -VERSION_SYMBOL(rte_cryptodev_sym_capability_get, _v20, 20.0); - -const struct rte_cryptodev_symmetric_capability __vsym * -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, +const struct rte_cryptodev_symmetric_capability * +rte_cryptodev_sym_capability_get(uint8_t dev_id, const struct rte_cryptodev_sym_capability_idx *idx) { const struct rte_cryptodev_capabilities *capability; @@ -358,12 +313,8 @@ rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, } return NULL; + } -MAP_STATIC_SYMBOL(const struct rte_cryptodev_symmetric_capability * - rte_cryptodev_sym_capability_get(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx), - rte_cryptodev_sym_capability_get_v21); -BIND_DEFAULT_SYMBOL(rte_cryptodev_sym_capability_get, _v21, 21); static int param_range_check(uint16_t size, const struct rte_crypto_param_range *range) @@ -1085,12 +1036,6 @@ rte_cryptodev_close(uint8_t dev_id) retval = (*dev->dev_ops->dev_close)(dev); rte_cryptodev_trace_close(dev_id, retval); - if (capability_copy[dev_id]) { - free(capability_copy[dev_id]); - capability_copy[dev_id] = NULL; - } - is_capability_checked[dev_id] = 0; - if (retval < 0) return retval; @@ -1233,61 +1178,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id) (*dev->dev_ops->stats_reset)(dev); } -static void -get_v20_capabilities(uint8_t dev_id, struct rte_cryptodev_info *dev_info) -{ - const struct rte_cryptodev_capabilities *capability; - uint8_t found_invalid_capa = 0; - uint8_t counter = 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) - return; - capability_copy[dev_id] = malloc(counter * - sizeof(struct rte_cryptodev_capabilities)); - if (capability_copy[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_copy[dev_id][counter++] = - *capability; - } - } - dev_info->capabilities = - capability_copy[dev_id]; - } -} -void __vsym -rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info) +void +rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info) { struct rte_cryptodev *dev; @@ -1303,40 +1196,10 @@ rte_cryptodev_info_get_v20(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_copy[dev_id] == NULL) { - if (!is_capability_checked[dev_id]) - get_v20_capabilities(dev_id, dev_info); - } else - dev_info->capabilities = capability_copy[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 __vsym -rte_cryptodev_info_get_v21(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_v21); -BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v21, 21); 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 7b3ebc20f..26abd0c52 100644 --- a/lib/librte_cryptodev/rte_cryptodev.h +++ b/lib/librte_cryptodev/rte_cryptodev.h @@ -219,14 +219,6 @@ struct rte_cryptodev_asym_capability_idx { * - Return NULL if the capability not exist. */ const struct rte_cryptodev_symmetric_capability * -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx); - -const struct rte_cryptodev_symmetric_capability * -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx); - -const struct rte_cryptodev_symmetric_capability * rte_cryptodev_sym_capability_get(uint8_t dev_id, const struct rte_cryptodev_sym_capability_idx *idx); @@ -789,33 +781,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id); * the last valid element has it's op field set to * RTE_CRYPTO_OP_TYPE_UNDEFINED. */ - -void +extern void rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info); -/* An extra element RTE_CRYPTO_AEAD_CHACHA20_POLY1305 is added - * to enum rte_crypto_aead_algorithm, also changing the value of - * RTE_CRYPTO_AEAD_LIST_END. To maintain ABI compatibility with applications - * which linked against earlier versions, preventing them, for example, from - * picking up the new value and using it to index into an array sized too small - * for it, it is necessary to have two versions of rte_cryptodev_info_get() - * The latest version just returns directly the capabilities retrieved from - * the device. The compatible version inspects the capabilities retrieved - * from the device, but only returns them directly if the new value - * is not included. If the new value is included, it allocates space - * for a copy of the device capabilities, trims the new value from this - * and returns this copy. It only needs to do this once per device. - * For the corner case of a corner case when the alloc may fail, - * an empty capability list is returned, as there is no mechanism to return - * an error and adding such a mechanism would itself be an ABI breakage. - * The compatible version can be removed after the next major ABI release. - */ - -void -rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info); - -void -rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info *dev_info); /** * Register a callback function for specific device id. diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map index 02f6dcf72..7727286ac 100644 --- a/lib/librte_cryptodev/rte_cryptodev_version.map +++ b/lib/librte_cryptodev/rte_cryptodev_version.map @@ -58,12 +58,6 @@ DPDK_21 { local: *; }; -DPDK_20.0 { - global: - rte_cryptodev_info_get; - rte_cryptodev_sym_capability_get; -}; - EXPERIMENTAL { global: -- 2.25.1
-----Original Message----- From: Dybkowski, AdamX <adamx.dybkowski@intel.com> Sent: piątek, 14 sierpnia 2020 12:00 To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com Cc: Dybkowski, AdamX <adamx.dybkowski@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com> Subject: [PATCH] cryptodev: revert ABI compatibility for ChaCha20-Poly1305 This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the rte_cryptodev_info_get function versioning was a temporary solution to maintain ABI compatibility for ChaCha20-Poly1305 and is not needed in 20.11. Fixes: a0f0de06d457 ("cryptodev: fix ABI compatibility for ChaCha20-Poly1305") Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com> Reviewed-by: Arek Kusztal <arkadiuszx.kusztal@intel.com> --- lib/librte_cryptodev/meson.build | 1 - lib/librte_cryptodev/rte_cryptodev.c | 147 +----------------- lib/librte_cryptodev/rte_cryptodev.h | 34 +--- .../rte_cryptodev_version.map | 6 - 4 files changed, 6 insertions(+), 182 deletions(-) Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
For the title, I would suggest: "cryptodev: remove v20 ABI compatibility" You did this change using a revert, but still, we can avoid restoring coding style issues, see nits below. On Fri, Aug 14, 2020 at 12:00 PM Adam Dybkowski <adamx.dybkowski@intel.com> wrote: > > This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the > rte_cryptodev_info_get function versioning was a temporary solution > to maintain ABI compatibility for ChaCha20-Poly1305 and is not > needed in 20.11. > > Fixes: a0f0de06d457 ("cryptodev: fix ABI compatibility for ChaCha20-Poly1305") > > Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com> > Reviewed-by: Arek Kusztal <arkadiuszx.kusztal@intel.com> > --- > lib/librte_cryptodev/meson.build | 1 - > lib/librte_cryptodev/rte_cryptodev.c | 147 +----------------- > lib/librte_cryptodev/rte_cryptodev.h | 34 +--- > .../rte_cryptodev_version.map | 6 - > 4 files changed, 6 insertions(+), 182 deletions(-) > > diff --git a/lib/librte_cryptodev/meson.build b/lib/librte_cryptodev/meson.build > index df1144058..c4c6b3b6a 100644 > --- a/lib/librte_cryptodev/meson.build > +++ b/lib/librte_cryptodev/meson.build > @@ -1,7 +1,6 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2017-2019 Intel Corporation > > -use_function_versioning = true > sources = files('rte_cryptodev.c', 'rte_cryptodev_pmd.c', 'cryptodev_trace_points.c') > headers = files('rte_cryptodev.h', > 'rte_cryptodev_pmd.h', > diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c > index 1dd795bcb..6c9a19f25 100644 > --- a/lib/librte_cryptodev/rte_cryptodev.c > +++ b/lib/librte_cryptodev/rte_cryptodev.c > @@ -36,8 +36,6 @@ > #include <rte_errno.h> > #include <rte_spinlock.h> > #include <rte_string_fns.h> > -#include <rte_compat.h> > -#include <rte_function_versioning.h> > > #include "rte_crypto.h" > #include "rte_cryptodev.h" > @@ -59,14 +57,6 @@ static struct rte_cryptodev_global cryptodev_globals = { > /* spinlock for crypto device callbacks */ > static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER; > > -static const struct rte_cryptodev_capabilities > - cryptodev_undefined_capabilities[] = { > - RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() > -}; > - > -static struct rte_cryptodev_capabilities > - *capability_copy[RTE_CRYPTO_MAX_DEVS]; > -static uint8_t is_capability_checked[RTE_CRYPTO_MAX_DEVS]; > Nit: remove empty line. > /** > * The user application callback description. > @@ -291,43 +281,8 @@ rte_crypto_auth_operation_strings[] = { > [RTE_CRYPTO_AUTH_OP_GENERATE] = "generate" > }; > > -const struct rte_cryptodev_symmetric_capability __vsym * > -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, > - const struct rte_cryptodev_sym_capability_idx *idx) > -{ > - const struct rte_cryptodev_capabilities *capability; > - struct rte_cryptodev_info dev_info; > - int i = 0; > - > - rte_cryptodev_info_get_v20(dev_id, &dev_info); > - > - while ((capability = &dev_info.capabilities[i++])->op != > - RTE_CRYPTO_OP_TYPE_UNDEFINED) { > - if (capability->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC) > - continue; > - > - if (capability->sym.xform_type != idx->type) > - continue; > - > - if (idx->type == RTE_CRYPTO_SYM_XFORM_AUTH && > - capability->sym.auth.algo == idx->algo.auth) > - return &capability->sym; > - > - if (idx->type == RTE_CRYPTO_SYM_XFORM_CIPHER && > - capability->sym.cipher.algo == idx->algo.cipher) > - return &capability->sym; > - > - if (idx->type == RTE_CRYPTO_SYM_XFORM_AEAD && > - capability->sym.aead.algo == idx->algo.aead) > - return &capability->sym; > - } > - > - return NULL; > -} > -VERSION_SYMBOL(rte_cryptodev_sym_capability_get, _v20, 20.0); > - > -const struct rte_cryptodev_symmetric_capability __vsym * > -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, > +const struct rte_cryptodev_symmetric_capability * > +rte_cryptodev_sym_capability_get(uint8_t dev_id, > const struct rte_cryptodev_sym_capability_idx *idx) > { > const struct rte_cryptodev_capabilities *capability; > @@ -358,12 +313,8 @@ rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, > } > > return NULL; > + Nit: remove unneeded extra line. > } > -MAP_STATIC_SYMBOL(const struct rte_cryptodev_symmetric_capability * > - rte_cryptodev_sym_capability_get(uint8_t dev_id, > - const struct rte_cryptodev_sym_capability_idx *idx), > - rte_cryptodev_sym_capability_get_v21); > -BIND_DEFAULT_SYMBOL(rte_cryptodev_sym_capability_get, _v21, 21); > > static int > param_range_check(uint16_t size, const struct rte_crypto_param_range *range) > @@ -1085,12 +1036,6 @@ rte_cryptodev_close(uint8_t dev_id) > retval = (*dev->dev_ops->dev_close)(dev); > rte_cryptodev_trace_close(dev_id, retval); > > - if (capability_copy[dev_id]) { > - free(capability_copy[dev_id]); > - capability_copy[dev_id] = NULL; > - } > - is_capability_checked[dev_id] = 0; > - > if (retval < 0) > return retval; > > @@ -1233,61 +1178,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id) > (*dev->dev_ops->stats_reset)(dev); > } > > -static void > -get_v20_capabilities(uint8_t dev_id, struct rte_cryptodev_info *dev_info) > -{ > - const struct rte_cryptodev_capabilities *capability; > - uint8_t found_invalid_capa = 0; > - uint8_t counter = 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) > - return; > - capability_copy[dev_id] = malloc(counter * > - sizeof(struct rte_cryptodev_capabilities)); > - if (capability_copy[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_copy[dev_id][counter++] = > - *capability; > - } > - } > - dev_info->capabilities = > - capability_copy[dev_id]; > - } > -} Nit: remove empty line. > > -void __vsym > -rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info) > +void > +rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info) > { > struct rte_cryptodev *dev; > > @@ -1303,40 +1196,10 @@ rte_cryptodev_info_get_v20(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_copy[dev_id] == NULL) { > - if (!is_capability_checked[dev_id]) > - get_v20_capabilities(dev_id, dev_info); > - } else > - dev_info->capabilities = capability_copy[dev_id]; > - > dev_info->driver_name = dev->device->driver->name; > dev_info->device = dev->device; > } > -VERSION_SYMBOL(rte_cryptodev_info_get, _v20, 20.0); > Nit: remove empty line. > -void __vsym > -rte_cryptodev_info_get_v21(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_v21); > -BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v21, 21); > > 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 7b3ebc20f..26abd0c52 100644 > --- a/lib/librte_cryptodev/rte_cryptodev.h > +++ b/lib/librte_cryptodev/rte_cryptodev.h > @@ -219,14 +219,6 @@ struct rte_cryptodev_asym_capability_idx { > * - Return NULL if the capability not exist. > */ > const struct rte_cryptodev_symmetric_capability * > -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, > - const struct rte_cryptodev_sym_capability_idx *idx); > - > -const struct rte_cryptodev_symmetric_capability * > -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, > - const struct rte_cryptodev_sym_capability_idx *idx); > - > -const struct rte_cryptodev_symmetric_capability * > rte_cryptodev_sym_capability_get(uint8_t dev_id, > const struct rte_cryptodev_sym_capability_idx *idx); > > @@ -789,33 +781,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id); > * the last valid element has it's op field set to > * RTE_CRYPTO_OP_TYPE_UNDEFINED. > */ > - > -void > +extern void Nit: no need for extern. > rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info); > > -/* An extra element RTE_CRYPTO_AEAD_CHACHA20_POLY1305 is added > - * to enum rte_crypto_aead_algorithm, also changing the value of > - * RTE_CRYPTO_AEAD_LIST_END. To maintain ABI compatibility with applications > - * which linked against earlier versions, preventing them, for example, from > - * picking up the new value and using it to index into an array sized too small > - * for it, it is necessary to have two versions of rte_cryptodev_info_get() > - * The latest version just returns directly the capabilities retrieved from > - * the device. The compatible version inspects the capabilities retrieved > - * from the device, but only returns them directly if the new value > - * is not included. If the new value is included, it allocates space > - * for a copy of the device capabilities, trims the new value from this > - * and returns this copy. It only needs to do this once per device. > - * For the corner case of a corner case when the alloc may fail, > - * an empty capability list is returned, as there is no mechanism to return > - * an error and adding such a mechanism would itself be an ABI breakage. > - * The compatible version can be removed after the next major ABI release. > - */ > - > -void > -rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info); > - > -void > -rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info *dev_info); > Nit: remove empty line. > /** > * Register a callback function for specific device id. > diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map > index 02f6dcf72..7727286ac 100644 > --- a/lib/librte_cryptodev/rte_cryptodev_version.map > +++ b/lib/librte_cryptodev/rte_cryptodev_version.map > @@ -58,12 +58,6 @@ DPDK_21 { > local: *; > }; > > -DPDK_20.0 { > - global: > - rte_cryptodev_info_get; > - rte_cryptodev_sym_capability_get; > -}; > - > EXPERIMENTAL { > global: > > -- > 2.25.1 > Thanks for working on this. Note to others watching ABI, with this, it should be the last patch about DPDK_20 ABI. -- David Marchand
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, 6 October, 2020 14:32
> To: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Cc: dev <dev@dpdk.org>; Trahe, Fiona <fiona.trahe@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: revert ABI compatibility for
> ChaCha20-Poly1305
>
> For the title, I would suggest: "cryptodev: remove v20 ABI compatibility"
>
> You did this change using a revert, but still, we can avoid restoring coding
> style issues, see nits below.
Thanks for the review, David.
I'll fix these styling issues and send v2 later this week.
Adam Dybkowski
On 06/10/2020 1:32 PM, David Marchand wrote: > For the title, I would suggest: "cryptodev: remove v20 ABI compatibility" > > You did this change using a revert, but still, we can avoid restoring > coding style issues, see nits below. > > > On Fri, Aug 14, 2020 at 12:00 PM Adam Dybkowski > <adamx.dybkowski@intel.com> wrote: >> This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the >> rte_cryptodev_info_get function versioning was a temporary solution >> to maintain ABI compatibility for ChaCha20-Poly1305 and is not >> needed in 20.11. >> ... >> >> 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 7b3ebc20f..26abd0c52 100644 >> --- a/lib/librte_cryptodev/rte_cryptodev.h >> +++ b/lib/librte_cryptodev/rte_cryptodev.h >> @@ -219,14 +219,6 @@ struct rte_cryptodev_asym_capability_idx { >> * - Return NULL if the capability not exist. >> */ >> const struct rte_cryptodev_symmetric_capability * >> -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, >> - const struct rte_cryptodev_sym_capability_idx *idx); >> - >> -const struct rte_cryptodev_symmetric_capability * >> -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, >> - const struct rte_cryptodev_sym_capability_idx *idx); >> - >> -const struct rte_cryptodev_symmetric_capability * >> rte_cryptodev_sym_capability_get(uint8_t dev_id, >> const struct rte_cryptodev_sym_capability_idx *idx); >> >> @@ -789,33 +781,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id); >> * the last valid element has it's op field set to >> * RTE_CRYPTO_OP_TYPE_UNDEFINED. >> */ >> - >> -void >> +extern void > Nit: no need for extern. Hey David, I think the cryptodev API consistently uses extern on nearly all it's function declarations. I'd proposed we do a separate patchset which removes extern on all function declarations to make it more consistent with the rest of DPDKs libraries. > /** > * Register a callback function for specific device id. ... > Thanks for working on this. > Note to others watching ABI, with this, it should be the last patch > about DPDK_20 ABI. > >
On Wed, Oct 7, 2020 at 12:41 PM Doherty, Declan
<declan.doherty@intel.com> wrote:
> >> @@ -789,33 +781,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id);
> >> * the last valid element has it's op field set to
> >> * RTE_CRYPTO_OP_TYPE_UNDEFINED.
> >> */
> >> -
> >> -void
> >> +extern void
> > Nit: no need for extern.
> Hey David, I think the cryptodev API consistently uses extern on nearly
> all it's function declarations. I'd proposed we do a separate patchset
> which removes extern on all function declarations to make it more
> consistent with the rest of DPDKs libraries.
Ok for me.
--
David Marchand
This reverts commit cryptodev: fix ABI compatibility for ChaCha20-Poly1305 as the rte_cryptodev_info_get function versioning was a temporary solution to maintain ABI compatibility for ChaCha20-Poly1305 and is not needed in 20.11. Adam Dybkowski (1): cryptodev: remove v20 ABI compatibility -- v2: * minor styling issues corrected (removed empty lines) -- lib/librte_cryptodev/meson.build | 1 - lib/librte_cryptodev/rte_cryptodev.c | 150 +----------------- lib/librte_cryptodev/rte_cryptodev.h | 34 +--- .../rte_cryptodev_version.map | 6 - 4 files changed, 5 insertions(+), 186 deletions(-) -- 2.25.1
This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the rte_cryptodev_info_get function versioning was a temporary solution to maintain ABI compatibility for ChaCha20-Poly1305 and is not needed in 20.11. Fixes: a0f0de06d457 ("cryptodev: fix ABI compatibility for ChaCha20-Poly1305") Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com> Reviewed-by: Arek Kusztal <arkadiuszx.kusztal@intel.com> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com> --- lib/librte_cryptodev/meson.build | 1 - lib/librte_cryptodev/rte_cryptodev.c | 150 +----------------- lib/librte_cryptodev/rte_cryptodev.h | 34 +--- .../rte_cryptodev_version.map | 6 - 4 files changed, 5 insertions(+), 186 deletions(-) diff --git a/lib/librte_cryptodev/meson.build b/lib/librte_cryptodev/meson.build index df1144058..c4c6b3b6a 100644 --- a/lib/librte_cryptodev/meson.build +++ b/lib/librte_cryptodev/meson.build @@ -1,7 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017-2019 Intel Corporation -use_function_versioning = true sources = files('rte_cryptodev.c', 'rte_cryptodev_pmd.c', 'cryptodev_trace_points.c') headers = files('rte_cryptodev.h', 'rte_cryptodev_pmd.h', diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c index 1dd795bcb..cda160f61 100644 --- a/lib/librte_cryptodev/rte_cryptodev.c +++ b/lib/librte_cryptodev/rte_cryptodev.c @@ -36,8 +36,6 @@ #include <rte_errno.h> #include <rte_spinlock.h> #include <rte_string_fns.h> -#include <rte_compat.h> -#include <rte_function_versioning.h> #include "rte_crypto.h" #include "rte_cryptodev.h" @@ -59,15 +57,6 @@ static struct rte_cryptodev_global cryptodev_globals = { /* spinlock for crypto device callbacks */ static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER; -static const struct rte_cryptodev_capabilities - cryptodev_undefined_capabilities[] = { - RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() -}; - -static struct rte_cryptodev_capabilities - *capability_copy[RTE_CRYPTO_MAX_DEVS]; -static uint8_t is_capability_checked[RTE_CRYPTO_MAX_DEVS]; - /** * The user application callback description. * @@ -291,43 +280,8 @@ rte_crypto_auth_operation_strings[] = { [RTE_CRYPTO_AUTH_OP_GENERATE] = "generate" }; -const struct rte_cryptodev_symmetric_capability __vsym * -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx) -{ - const struct rte_cryptodev_capabilities *capability; - struct rte_cryptodev_info dev_info; - int i = 0; - - rte_cryptodev_info_get_v20(dev_id, &dev_info); - - while ((capability = &dev_info.capabilities[i++])->op != - RTE_CRYPTO_OP_TYPE_UNDEFINED) { - if (capability->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC) - continue; - - if (capability->sym.xform_type != idx->type) - continue; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_AUTH && - capability->sym.auth.algo == idx->algo.auth) - return &capability->sym; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_CIPHER && - capability->sym.cipher.algo == idx->algo.cipher) - return &capability->sym; - - if (idx->type == RTE_CRYPTO_SYM_XFORM_AEAD && - capability->sym.aead.algo == idx->algo.aead) - return &capability->sym; - } - - return NULL; -} -VERSION_SYMBOL(rte_cryptodev_sym_capability_get, _v20, 20.0); - -const struct rte_cryptodev_symmetric_capability __vsym * -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, +const struct rte_cryptodev_symmetric_capability * +rte_cryptodev_sym_capability_get(uint8_t dev_id, const struct rte_cryptodev_sym_capability_idx *idx) { const struct rte_cryptodev_capabilities *capability; @@ -359,11 +313,6 @@ rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, return NULL; } -MAP_STATIC_SYMBOL(const struct rte_cryptodev_symmetric_capability * - rte_cryptodev_sym_capability_get(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx), - rte_cryptodev_sym_capability_get_v21); -BIND_DEFAULT_SYMBOL(rte_cryptodev_sym_capability_get, _v21, 21); static int param_range_check(uint16_t size, const struct rte_crypto_param_range *range) @@ -1085,12 +1034,6 @@ rte_cryptodev_close(uint8_t dev_id) retval = (*dev->dev_ops->dev_close)(dev); rte_cryptodev_trace_close(dev_id, retval); - if (capability_copy[dev_id]) { - free(capability_copy[dev_id]); - capability_copy[dev_id] = NULL; - } - is_capability_checked[dev_id] = 0; - if (retval < 0) return retval; @@ -1233,89 +1176,8 @@ rte_cryptodev_stats_reset(uint8_t dev_id) (*dev->dev_ops->stats_reset)(dev); } -static void -get_v20_capabilities(uint8_t dev_id, struct rte_cryptodev_info *dev_info) -{ - const struct rte_cryptodev_capabilities *capability; - uint8_t found_invalid_capa = 0; - uint8_t counter = 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) - return; - capability_copy[dev_id] = malloc(counter * - sizeof(struct rte_cryptodev_capabilities)); - if (capability_copy[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_copy[dev_id][counter++] = - *capability; - } - } - dev_info->capabilities = - capability_copy[dev_id]; - } -} - -void __vsym -rte_cryptodev_info_get_v20(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); - - if (capability_copy[dev_id] == NULL) { - if (!is_capability_checked[dev_id]) - get_v20_capabilities(dev_id, dev_info); - } else - dev_info->capabilities = capability_copy[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 __vsym -rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info *dev_info) +void +rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info) { struct rte_cryptodev *dev; @@ -1334,9 +1196,6 @@ rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info *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_v21); -BIND_DEFAULT_SYMBOL(rte_cryptodev_info_get, _v21, 21); int rte_cryptodev_callback_register(uint8_t dev_id, @@ -1449,7 +1308,6 @@ rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev, rte_spinlock_unlock(&rte_cryptodev_cb_lock); } - int rte_cryptodev_sym_session_init(uint8_t dev_id, struct rte_cryptodev_sym_session *sess, diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h index 7b3ebc20f..26abd0c52 100644 --- a/lib/librte_cryptodev/rte_cryptodev.h +++ b/lib/librte_cryptodev/rte_cryptodev.h @@ -219,14 +219,6 @@ struct rte_cryptodev_asym_capability_idx { * - Return NULL if the capability not exist. */ const struct rte_cryptodev_symmetric_capability * -rte_cryptodev_sym_capability_get_v20(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx); - -const struct rte_cryptodev_symmetric_capability * -rte_cryptodev_sym_capability_get_v21(uint8_t dev_id, - const struct rte_cryptodev_sym_capability_idx *idx); - -const struct rte_cryptodev_symmetric_capability * rte_cryptodev_sym_capability_get(uint8_t dev_id, const struct rte_cryptodev_sym_capability_idx *idx); @@ -789,33 +781,9 @@ rte_cryptodev_stats_reset(uint8_t dev_id); * the last valid element has it's op field set to * RTE_CRYPTO_OP_TYPE_UNDEFINED. */ - -void +extern void rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info); -/* An extra element RTE_CRYPTO_AEAD_CHACHA20_POLY1305 is added - * to enum rte_crypto_aead_algorithm, also changing the value of - * RTE_CRYPTO_AEAD_LIST_END. To maintain ABI compatibility with applications - * which linked against earlier versions, preventing them, for example, from - * picking up the new value and using it to index into an array sized too small - * for it, it is necessary to have two versions of rte_cryptodev_info_get() - * The latest version just returns directly the capabilities retrieved from - * the device. The compatible version inspects the capabilities retrieved - * from the device, but only returns them directly if the new value - * is not included. If the new value is included, it allocates space - * for a copy of the device capabilities, trims the new value from this - * and returns this copy. It only needs to do this once per device. - * For the corner case of a corner case when the alloc may fail, - * an empty capability list is returned, as there is no mechanism to return - * an error and adding such a mechanism would itself be an ABI breakage. - * The compatible version can be removed after the next major ABI release. - */ - -void -rte_cryptodev_info_get_v20(uint8_t dev_id, struct rte_cryptodev_info *dev_info); - -void -rte_cryptodev_info_get_v21(uint8_t dev_id, struct rte_cryptodev_info *dev_info); /** * Register a callback function for specific device id. diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map index 02f6dcf72..7727286ac 100644 --- a/lib/librte_cryptodev/rte_cryptodev_version.map +++ b/lib/librte_cryptodev/rte_cryptodev_version.map @@ -58,12 +58,6 @@ DPDK_21 { local: *; }; -DPDK_20.0 { - global: - rte_cryptodev_info_get; - rte_cryptodev_sym_capability_get; -}; - EXPERIMENTAL { global: -- 2.25.1
> This reverts commit a0f0de06d457753c94688d551a6e8659b4d4e041 as the
> rte_cryptodev_info_get function versioning was a temporary solution
> to maintain ABI compatibility for ChaCha20-Poly1305 and is not
> needed in 20.11.
>
> Fixes: a0f0de06d457 ("cryptodev: fix ABI compatibility for ChaCha20-Poly1305")
>
> Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> Reviewed-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Applied to dpdk-next-crypto
Thanks.