From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 951F6A04AE; Tue, 5 May 2020 03:17:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 329011D52E; Tue, 5 May 2020 03:17:41 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 7D2491D52D for ; Tue, 5 May 2020 03:17:40 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200505011739euoutp02c4e61ff4bcc0f87b9e3133762131de86~L-P2WJQ2V0048600486euoutp02H for ; Tue, 5 May 2020 01:17:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200505011739euoutp02c4e61ff4bcc0f87b9e3133762131de86~L-P2WJQ2V0048600486euoutp02H DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1588641459; bh=0EC9fo++dZp7+LplbbtHxU6nREQFRhRuFuILMcPFNHg=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=p+c+hEyrMYmNa7Z2PFlUPTsw1sCjr0VkJAb5Nde6T82fX1wH0110cnTblEazPlGqI h/VS3TVGYZUFTuvayzOYuj7wp8EHJSsnpkg2PW3uqkMEtDnfigaQWNVPclijvYXy9+ Ur++108nLLHMqSdhRcnEcnQgnY3kRkQ5zE4Zm5XM= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200505011739eucas1p1ce625041e473ee5ab388626a479006af~L-P1_ZkJA2493124931eucas1p11; Tue, 5 May 2020 01:17:39 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 00.37.60679.3BEB0BE5; Tue, 5 May 2020 02:17:39 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20200505011738eucas1p19c6d38f38bc070d424e9b795f92f0464~L-P1ONGWe2490924909eucas1p1A; Tue, 5 May 2020 01:17:38 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200505011738eusmtrp106f9aaaf1972b0eae526892c7c7c1737~L-P1NqmDI2690026900eusmtrp1o; Tue, 5 May 2020 01:17:38 +0000 (GMT) X-AuditID: cbfec7f4-0cbff7000001ed07-21-5eb0beb365aa Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 2C.B8.07950.2BEB0BE5; Tue, 5 May 2020 02:17:38 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20200505011737eusmtip168d8be29a0c3ce47036eaacad4aa100d~L-P0yryhL0430704307eusmtip1b; Tue, 5 May 2020 01:17:37 +0000 (GMT) To: Akhil Goyal , dev@dpdk.org Cc: hemant.agrawal@nxp.com From: Lukasz Wojciechowski Message-ID: <2f74e9a3-e00a-8c64-75d8-858aecbac133@partner.samsung.com> Date: Tue, 5 May 2020 03:17:36 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200504213921.15449-1-akhil.goyal@nxp.com> Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmplleLIzCtJLcpLzFFi42LZduznOd3N+zbEGSxYzmux/sw8Rot3n7Yz Wax8vJHNgdnj14KlrB4b3+1gCmCK4rJJSc3JLEst0rdL4Mo49OEba0FbQsXaeUeZGhiXeHcx cnJICJhI/Fn9mqmLkYtDSGAFo8SRrVtYIZwvjBLb2vZDZT4zSvzfsZwVpmXZlhPMILaQwHJG idvP7SGK3jJKvNn/EaxIWCBQ4v+n3SwgtoiAscSGXTPB4swCUhLfzrQzgthsArYSR2Z+BYpz cPAKuEm8vu8OEmYRUJFYc+kTM0hYVCBWYvq1EJAwr4CgxMmZT1hAwpwClhLv+1MgBspLNG+d zQxhi0vcejIf7GQJgWZ2icurrrNDnOwicfn/bKjzhSVeHd8CFZeROD25hwWiYRujxNXfPxkh nP2MEtd7V0BVWUsc/vebDWQzs4CmxPpd+iCmhICjRONlRgiTT+LGW0GIG/gkJm2bzgwR5pXo aBOCmKEn8bRnKiPM1j9rn7BMYFSaheSxWUi+mYXkm1kIaxcwsqxiFE8tLc5NTy02ykst1ytO zC0uzUvXS87P3cQITByn/x3/soNx15+kQ4wCHIxKPLwRn9fHCbEmlhVX5h5ilOBgVhLhVTTb ECfEm5JYWZValB9fVJqTWnyIUZqDRUmc13jRy1ghgfTEktTs1NSC1CKYLBMHp1QDo7ruEQ/n 2x51DFvv/InuveFTnP/u+Q2p/XnzpJT5pSW/PVPrLasSMb3Da/HLb1ewzXypu7fbNi88fm9z wcIVYfdkermumeox/Mjz7N71je2zvMA/+cawSbPfuE3mrF/1RsyieUM3W7lRe9ysFIPDC3pE 1RYGBZz2mH3+LvMSnvYT0m1Jydf8lFiKMxINtZiLihMB0dOc3RgDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprOIsWRmVeSWpSXmKPExsVy+t/xu7qb9m2IM5i+XsJi/Zl5jBbvPm1n slj5eCObA7PHrwVLWT02vtvBFMAUpWdTlF9akqqQkV9cYqsUbWhhpGdoaaFnZGKpZ2hsHmtl ZKqkb2eTkpqTWZZapG+XoJdx6MM31oK2hIq1844yNTAu8e5i5OSQEDCRWLblBHMXIxeHkMBS RolrO2+wdTFyACVkJD5cEoCoEZb4c62LDaLmNaPEgSnfGEESwgKBEv8/7WYBsUUEjCU27JrJ CmIzC0hJfDvTDlYjJLCLUaKj1RzEZhOwlTgy8ysryHxeATeJ1/fdQcIsAioSay59YgYJiwrE SrRc1AQJ8woISpyc+YQFJMwpYCnxvj8FYriZxLzND5khbHmJ5q2zoWxxiVtP5jNNYBSahaR7 FpKWWUhaZiFpWcDIsopRJLW0ODc9t9hIrzgxt7g0L10vOT93EyMwUrYd+7llB2PXu+BDjAIc jEo8vBu+ro8TYk0sK67MPcQowcGsJMKraLYhTog3JbGyKrUoP76oNCe1+BCjKdBrE5mlRJPz gVGcVxJvaGpobmFpaG5sbmxmoSTO2yFwMEZIID2xJDU7NbUgtQimj4mDU6qB0eMUQ9f9utaM yRsmyPaKTFzFJTlBrzhrGe/fG/pRS/NC/0zSPn2gu4HnkpxBd+XsjvlfPqy5fLzXv8GV42jY 2v0hZzV7LX2YJmzSesVW9SSD3VzaQGXXnDcqt0KOXQnxZ29hZ7xfcfTHpk1Ckz5l+MXp7Toq 720mtdcnXZh515Sd4m9O7Qk/qMRSnJFoqMVcVJwIADOo9gaqAgAA X-CMS-MailID: 20200505011738eucas1p19c6d38f38bc070d424e9b795f92f0464 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200504220028eucas1p2206c59474f0aec1874032cc03843f101 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200504220028eucas1p2206c59474f0aec1874032cc03843f101 References: <20200504203915.9602-1-akhil.goyal@nxp.com> <20200504213921.15449-1-akhil.goyal@nxp.com> Subject: Re: [dpdk-dev] [PATCH v2 1/2] crypto/dpaa2_sec: improve error handling 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" W dniu 04.05.2020 o 23:39, Akhil Goyal pisze: > The return values in cases of errors were not > specified properly. With this patch appropriate > error numbers are returned. > > Signed-off-by: Akhil Goyal > --- > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 59 +++++++++++++-------- > 1 file changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > index 0919f3bf4..984c563e3 100644 > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > @@ -1814,7 +1814,7 @@ dpaa2_sec_cipher_init(struct rte_cryptodev *dev, > { > struct dpaa2_sec_dev_private *dev_priv = dev->data->dev_private; > struct alginfo cipherdata; > - int bufsize; > + int bufsize, ret = 0; > struct ctxt_priv *priv; > struct sec_flow_context *flc; > > @@ -1826,7 +1826,7 @@ dpaa2_sec_cipher_init(struct rte_cryptodev *dev, > RTE_CACHE_LINE_SIZE); > if (priv == NULL) { > DPAA2_SEC_ERR("No Memory for priv CTXT"); > - return -1; > + return -ENOMEM; > } > > priv->fle_pool = dev_priv->fle_pool; > @@ -1839,7 +1839,7 @@ dpaa2_sec_cipher_init(struct rte_cryptodev *dev, > if (session->cipher_key.data == NULL) { > DPAA2_SEC_ERR("No Memory for cipher key"); > rte_free(priv); > - return -1; > + return -ENOMEM; > } > session->cipher_key.length = xform->cipher.key.length; > > @@ -1916,15 +1916,18 @@ dpaa2_sec_cipher_init(struct rte_cryptodev *dev, > case RTE_CRYPTO_CIPHER_NULL: > DPAA2_SEC_ERR("Crypto: Unsupported Cipher alg %u", > xform->cipher.algo); > + ret = -ENOTSUP; > goto error_out; > default: > DPAA2_SEC_ERR("Crypto: Undefined Cipher specified %u", > xform->cipher.algo); > + ret = -ENOTSUP; > goto error_out; > } > > if (bufsize < 0) { > DPAA2_SEC_ERR("Crypto: Descriptor build failed"); > + ret = -EINVAL; > goto error_out; > } > > @@ -1936,12 +1939,12 @@ dpaa2_sec_cipher_init(struct rte_cryptodev *dev, > for (i = 0; i < bufsize; i++) > DPAA2_SEC_DEBUG("DESC[%d]:0x%x", i, priv->flc_desc[0].desc[i]); > #endif > - return 0; > + return ret; > > error_out: > rte_free(session->cipher_key.data); > rte_free(priv); > - return -1; > + return ret; > } > > static int > @@ -1951,7 +1954,7 @@ dpaa2_sec_auth_init(struct rte_cryptodev *dev, > { > struct dpaa2_sec_dev_private *dev_priv = dev->data->dev_private; > struct alginfo authdata; > - int bufsize; > + int bufsize, ret = 0; > struct ctxt_priv *priv; > struct sec_flow_context *flc; > > @@ -1964,7 +1967,7 @@ dpaa2_sec_auth_init(struct rte_cryptodev *dev, > RTE_CACHE_LINE_SIZE); > if (priv == NULL) { > DPAA2_SEC_ERR("No Memory for priv CTXT"); > - return -1; > + return -ENOMEM; > } > > priv->fle_pool = dev_priv->fle_pool; > @@ -1976,7 +1979,7 @@ dpaa2_sec_auth_init(struct rte_cryptodev *dev, > if (session->auth_key.data == NULL) { > DPAA2_SEC_ERR("Unable to allocate memory for auth key"); > rte_free(priv); > - return -1; > + return -ENOMEM; > } > session->auth_key.length = xform->auth.key.length; > > @@ -2082,15 +2085,18 @@ dpaa2_sec_auth_init(struct rte_cryptodev *dev, > case RTE_CRYPTO_AUTH_AES_CBC_MAC: > DPAA2_SEC_ERR("Crypto: Unsupported auth alg %un", > xform->auth.algo); > + ret = -ENOTSUP; > goto error_out; > default: > DPAA2_SEC_ERR("Crypto: Undefined Auth specified %u", > xform->auth.algo); > + ret = -ENOTSUP; > goto error_out; > } > > if (bufsize < 0) { > DPAA2_SEC_ERR("Crypto: Invalid buffer length"); > + ret = -EINVAL; > goto error_out; > } > > @@ -2103,12 +2109,12 @@ dpaa2_sec_auth_init(struct rte_cryptodev *dev, > i, priv->flc_desc[DESC_INITFINAL].desc[i]); > #endif > > - return 0; > + return ret; > > error_out: > rte_free(session->auth_key.data); > rte_free(priv); > - return -1; > + return ret; > } > > static int > @@ -2123,7 +2129,7 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > struct ctxt_priv *priv; > struct sec_flow_context *flc; > struct rte_crypto_aead_xform *aead_xform = &xform->aead; > - int err; > + int err, ret = 0; > > PMD_INIT_FUNC_TRACE(); > > @@ -2138,7 +2144,7 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > RTE_CACHE_LINE_SIZE); > if (priv == NULL) { > DPAA2_SEC_ERR("No Memory for priv CTXT"); > - return -1; > + return -ENOMEM; > } > > priv->fle_pool = dev_priv->fle_pool; > @@ -2149,7 +2155,7 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > if (session->aead_key.data == NULL && aead_xform->key.length > 0) { > DPAA2_SEC_ERR("No Memory for aead key"); > rte_free(priv); > - return -1; > + return -ENOMEM; > } > memcpy(session->aead_key.data, aead_xform->key.data, > aead_xform->key.length); > @@ -2172,10 +2178,12 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > case RTE_CRYPTO_AEAD_AES_CCM: > DPAA2_SEC_ERR("Crypto: Unsupported AEAD alg %u", > aead_xform->algo); > + ret = -ENOTSUP; > goto error_out; > default: > DPAA2_SEC_ERR("Crypto: Undefined AEAD specified %u", > aead_xform->algo); > + ret = -ENOTSUP; > goto error_out; > } > session->dir = (aead_xform->op == RTE_CRYPTO_AEAD_OP_ENCRYPT) ? > @@ -2189,6 +2197,7 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > > if (err < 0) { > DPAA2_SEC_ERR("Crypto: Incorrect key lengths"); > + ret = -EINVAL; > goto error_out; > } > if (priv->flc_desc[0].desc[1] & 1) { > @@ -2212,6 +2221,7 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > session->digest_length); > if (bufsize < 0) { > DPAA2_SEC_ERR("Crypto: Invalid buffer length"); > + ret = -EINVAL; > goto error_out; > } > > @@ -2223,12 +2233,12 @@ dpaa2_sec_aead_init(struct rte_cryptodev *dev, > DPAA2_SEC_DEBUG("DESC[%d]:0x%x\n", > i, priv->flc_desc[0].desc[i]); > #endif > - return 0; > + return ret; > > error_out: > rte_free(session->aead_key.data); > rte_free(priv); > - return -1; > + return ret; > } > > > @@ -2244,7 +2254,7 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > struct sec_flow_context *flc; > struct rte_crypto_cipher_xform *cipher_xform; > struct rte_crypto_auth_xform *auth_xform; > - int err; > + int err, ret = 0; > > PMD_INIT_FUNC_TRACE(); > > @@ -2272,7 +2282,7 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > RTE_CACHE_LINE_SIZE); > if (priv == NULL) { > DPAA2_SEC_ERR("No Memory for priv CTXT"); > - return -1; > + return -ENOMEM; > } > > priv->fle_pool = dev_priv->fle_pool; > @@ -2283,7 +2293,7 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > if (session->cipher_key.data == NULL && cipher_xform->key.length > 0) { > DPAA2_SEC_ERR("No Memory for cipher key"); > rte_free(priv); > - return -1; > + return -ENOMEM; > } > session->cipher_key.length = cipher_xform->key.length; > session->auth_key.data = rte_zmalloc(NULL, auth_xform->key.length, > @@ -2292,7 +2302,7 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > DPAA2_SEC_ERR("No Memory for auth key"); > rte_free(session->cipher_key.data); > rte_free(priv); > - return -1; > + return -ENOMEM; > } > session->auth_key.length = auth_xform->key.length; > memcpy(session->cipher_key.data, cipher_xform->key.data, > @@ -2354,10 +2364,12 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > case RTE_CRYPTO_AUTH_ZUC_EIA3: > DPAA2_SEC_ERR("Crypto: Unsupported auth alg %u", > auth_xform->algo); > + ret = -ENOTSUP; > goto error_out; > default: > DPAA2_SEC_ERR("Crypto: Undefined Auth specified %u", > auth_xform->algo); > + ret = -ENOTSUP; > goto error_out; > } > cipherdata.key = (size_t)session->cipher_key.data; > @@ -2389,10 +2401,12 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > case RTE_CRYPTO_CIPHER_KASUMI_F8: > DPAA2_SEC_ERR("Crypto: Unsupported Cipher alg %u", > cipher_xform->algo); > + ret = -ENOTSUP; > goto error_out; > default: > DPAA2_SEC_ERR("Crypto: Undefined Cipher specified %u", > cipher_xform->algo); > + ret = -ENOTSUP; > goto error_out; > } > session->dir = (cipher_xform->op == RTE_CRYPTO_CIPHER_OP_ENCRYPT) ? > @@ -2407,6 +2421,7 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > > if (err < 0) { > DPAA2_SEC_ERR("Crypto: Incorrect key lengths"); > + ret = -EINVAL; > goto error_out; > } > if (priv->flc_desc[0].desc[2] & 1) { > @@ -2434,10 +2449,12 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > session->dir); > if (bufsize < 0) { > DPAA2_SEC_ERR("Crypto: Invalid buffer length"); > + ret = -EINVAL; > goto error_out; > } > } else { > DPAA2_SEC_ERR("Hash before cipher not supported"); > + ret = -ENOTSUP; > goto error_out; > } > > @@ -2450,13 +2467,13 @@ dpaa2_sec_aead_chain_init(struct rte_cryptodev *dev, > i, priv->flc_desc[0].desc[i]); > #endif > > - return 0; > + return ret; > > error_out: > rte_free(session->cipher_key.data); > rte_free(session->auth_key.data); > rte_free(priv); > - return -1; > + return ret; > } > > static int Hi Akhil, There are still few places, where returned error code is not set. Maybe you can add those places to your patch? Here's the list: --- a/drivers/crypto/dpaa_sec/dpaa_sec.c +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c @@ -510,6 +510,7 @@ dpaa_sec_prep_cdb(dpaa_sec_session *ses)                         break;                 default:                         DPAA_SEC_ERR("unsupported auth alg %u", ses->auth_alg); +                       /* this path will return 0 */                 }                 break;         case DPAA_SEC_AEAD: @@ -2308,7 +2309,7 @@ dpaa_sec_attach_sess_q(struct dpaa_sec_qp *qp, dpaa_sec_session *sess)         ret = dpaa_sec_prep_cdb(sess);         if (ret) {                 DPAA_SEC_ERR("Unable to prepare sec cdb"); -               return -1; +               return -1; /* Why not return ret ? */         }         if (unlikely(!RTE_PER_LCORE(dpaa_io))) {                 ret = rte_dpaa_portal_init((void *)0); @@ -2826,7 +2827,7 @@ dpaa_sec_set_ipsec_session(__rte_unused struct rte_cryptodev *dev,                         }                 }         } else -               goto out; +               goto out; /* set ret before goto */         rte_spinlock_lock(&internals->lock);         for (i = 0; i < MAX_DPAA_CORES; i++) {                 session->inq[i] = dpaa_sec_attach_rxq(internals); @@ -2843,7 +2844,7 @@ dpaa_sec_set_ipsec_session(__rte_unused struct rte_cryptodev *dev,         rte_free(session->auth_key.data);         rte_free(session->cipher_key.data);         memset(session, 0, sizeof(dpaa_sec_session)); -       return -1; +       return -1; /* return ret ? */  }  static int @@ -2992,7 +2993,7 @@ dpaa_sec_set_pdcp_session(struct rte_cryptodev *dev,         rte_free(session->auth_key.data);         rte_free(session->cipher_key.data);         memset(session, 0, sizeof(dpaa_sec_session)); -       return -1; +       return -1; /* still returning generic -1 */  }  static int -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com