From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 83FBA2629 for ; Tue, 25 Jul 2017 13:55:24 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP; 25 Jul 2017 04:55:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,411,1496127600"; d="scan'208";a="129209999" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga005.jf.intel.com with ESMTP; 25 Jul 2017 04:55:22 -0700 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.242]) by IRSMSX153.ger.corp.intel.com ([169.254.9.74]) with mapi id 14.03.0319.002; Tue, 25 Jul 2017 12:55:21 +0100 From: "Trahe, Fiona" To: "De Lara Guarch, Pablo" , "zbigniew.bodek@caviumnetworks.com" , "jerin.jacob@caviumnetworks.com" , "akhil.goyal@nxp.com" , "hemant.agrawal@nxp.com" , "Jain, Deepak K" , "Griffin, John" CC: "dev@dpdk.org" , "Trahe, Fiona" Thread-Topic: [PATCH] cryptodev: fix session init return value Thread-Index: AQHTBJ2BeB6B6CD8k06xhCkFL9E0H6JkYmlg Date: Tue, 25 Jul 2017 11:55:20 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B4358923BC4B@IRSMSX101.ger.corp.intel.com> References: <20170724085428.51995-1-pablo.de.lara.guarch@intel.com> In-Reply-To: <20170724085428.51995-1-pablo.de.lara.guarch@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYWIxNjQ4ZjQtZGJiMy00OTZiLTgyMjMtZDJhZTk3Y2FjYWNlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkFuVlpEVU9Zb1wvWWFnZ3hibzlHREhGOFloQlhyQzdkcWRLU1RZeGNVRms4PSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] cryptodev: fix session init return value X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jul 2017 11:55:25 -0000 Hi Pablo, > -----Original Message----- > From: De Lara Guarch, Pablo > Sent: Monday, July 24, 2017 9:54 AM > To: zbigniew.bodek@caviumnetworks.com; jerin.jacob@caviumnetworks.com; ak= hil.goyal@nxp.com; > hemant.agrawal@nxp.com; Trahe, Fiona ; Jain, Deepa= k K > ; Griffin, John > Cc: dev@dpdk.org; De Lara Guarch, Pablo > Subject: [PATCH] cryptodev: fix session init return value >=20 > 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. >=20 > Fixes: b3bbd9e5f265 ("cryptodev: support device independent sessions") >=20 > Signed-off-by: Pablo de Lara > --- Trimmed to just the relevant QAT sections > diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_cry= pto.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 encapsulate= d 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_crypto= dev *dev, > qat_cmd_id =3D qat_get_cmd_id(xform); > if (qat_cmd_id < 0 || qat_cmd_id >=3D ICP_QAT_FW_LA_CMD_DELIMITER) { > PMD_DRV_LOG(ERR, "Unsupported xform chain requested"); > - return -1; > + return -ENOTSUP; > } > session->qat_cmd =3D (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 =3D 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 =3D 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 =3D=3D RTE_CRYPTO_SYM_XFORM_AEAD) { > - if (qat_crypto_sym_configure_session_aead(xform, > - session) < 0) > - return -1; > + ret =3D 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 =3D qat_crypto_sym_configure_session_cipher(dev, > + xform, session); > + if (ret < 0) > + return ret; > + ret =3D 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 =3D 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.