Hi Pablo,
> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Monday, July 24, 2017 9:54 AM
> To: zbigniew.bo...@caviumnetworks.com; jerin.ja...@caviumnetworks.com;
> akhil.go...@nxp.com;
> hemant.agra...@nxp.com; Trahe, Fiona <fiona.tr...@intel.com>; Jain, Deepak K
> <deepak.k.j...@intel.com>; Griffin, John <john.grif...@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>
> Subject: [PATCH] cryptodev: fix session init return value
>
> When calling rte_cryptodev_sym_session_init(),
> if there was an error, it returned -1, regardless
> the error.
> Instead, it should return the specific error code, which can
> be valuable for the application for error handling.
>
> Fixes: b3bbd9e5f265 ("cryptodev: support device independent sessions")
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.gua...@intel.com>
> ---
Trimmed to just the relevant QAT sections
> diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
> index d92de35..e115540 100644
> --- a/drivers/crypto/qat/qat_crypto.c
> +++ b/drivers/crypto/qat/qat_crypto.c
> @@ -171,16 +171,19 @@ bpi_cipher_decrypt(uint8_t *src, uint8_t *dst,
> /** Creates a context in either AES or DES in ECB mode
> * Depends on openssl libcrypto
> */
> -static void *
> +static int
> bpi_cipher_ctx_init(enum rte_crypto_cipher_algorithm cryptodev_algo,
> enum rte_crypto_cipher_operation direction __rte_unused,
> - uint8_t *key)
> + uint8_t *key, EVP_CIPHER_CTX **ctx)
My preference is to use a void ** ctx here. So all EVP refs are encapsulated
within these fns
and if we ever wanted to use a lib other than openssl the changes would be
confined to the
internals of these functions.
> @@ -499,44 +525,52 @@ qat_crypto_set_session_parameters(struct rte_cryptodev
> *dev,
> qat_cmd_id = qat_get_cmd_id(xform);
> if (qat_cmd_id < 0 || qat_cmd_id >= ICP_QAT_FW_LA_CMD_DELIMITER) {
> PMD_DRV_LOG(ERR, "Unsupported xform chain requested");
> - return -1;
> + return -ENOTSUP;
> }
> session->qat_cmd = (enum icp_qat_fw_la_cmd_id)qat_cmd_id;
> switch (session->qat_cmd) {
> case ICP_QAT_FW_LA_CMD_CIPHER:
> - if (qat_crypto_sym_configure_session_cipher(dev, xform,
> session) < 0)
> - return -1;
> + ret = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> + if (ret < 0)
> + return ret;
> break;
> case ICP_QAT_FW_LA_CMD_AUTH:
> - if (qat_crypto_sym_configure_session_auth(dev, xform, session)
> < 0)
> - return -1;
> + ret = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> + if (ret < 0)
> + return ret;
> break;
> case ICP_QAT_FW_LA_CMD_CIPHER_HASH:
> if (xform->type == RTE_CRYPTO_SYM_XFORM_AEAD) {
> - if (qat_crypto_sym_configure_session_aead(xform,
> - session) < 0)
> - return -1;
> + ret = qat_crypto_sym_configure_session_aead(xform,
> + session);
> + if (ret < 0)
> + return ret;
> } else {
> - if (qat_crypto_sym_configure_session_cipher(dev,
> - xform, session) < 0)
> - return -1;
> - if (qat_crypto_sym_configure_session_auth(dev,
> - xform, session) < 0)
> - return -1;
> + ret = qat_crypto_sym_configure_session_cipher(dev,
> + xform, session);
> + if (ret < 0)
> + return ret;
> + ret = qat_crypto_sym_configure_session_auth(dev,
> + xform, session);
> + if (ret < 0)
> + return ret;
In this case it is also necessary to undo what was done during the previous fn
qat_crypto_sym_configure_session_cipher(), i.e. add
if (session->bpi_ctx) {
bpi_cipher_ctx_free(session->bpi_ctx);
session->bpi_ctx = NULL;
}
OR encapsulate this in a new fn:
qat_crypto_sym_clear_session_cipher()
Note, this is a pre-existing bug, just highlighted by this change.
