From: Anoob Joseph <anoobj@marvell.com>
To: Ciara Power <ciara.power@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "roy.fan.zhang@intel.com" <roy.fan.zhang@intel.com>,
Akhil Goyal <gakhil@marvell.com>, "mdr@ashroe.eu" <mdr@ashroe.eu>,
Declan Doherty <declan.doherty@intel.com>
Subject: RE: [EXT] [PATCH v2 4/4] crypto: modify return value for asym session create
Date: Mon, 31 Jan 2022 14:35:44 +0000 [thread overview]
Message-ID: <PH0PR18MB4672DE86C12D2410092A222DDF259@PH0PR18MB4672.namprd18.prod.outlook.com> (raw)
In-Reply-To: <20220124150339.280090-5-ciara.power@intel.com>
Hi Ciara,
Minor nits. Please see inline.
With the fixes,
Acked-by: Anoob Joseph <anoobj@marvell.com>
Thanks,
Anoob
> -----Original Message-----
> From: Ciara Power <ciara.power@intel.com>
> Sent: Monday, January 24, 2022 8:34 PM
> To: dev@dpdk.org
> Cc: roy.fan.zhang@intel.com; Akhil Goyal <gakhil@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; mdr@ashroe.eu; Ciara Power
> <ciara.power@intel.com>; Declan Doherty <declan.doherty@intel.com>
> Subject: [EXT] [PATCH v2 4/4] crypto: modify return value for asym session
> create
>
> External Email
>
> ----------------------------------------------------------------------
> Rather than the asym session create function returning a session on success, and
> a NULL value on error, it is modified to now return int values - 0 on success or -
> EINVAL/-ENOTSUP/-ENOMEM on failure.
> The session to be used is passed as input.
>
> This adds clarity on the failure of the create function, which enables treating the
> -ENOTSUP return as TEST_SKIPPED in test apps.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
[snip]
> @@ -744,11 +746,13 @@ cperf_create_session(struct rte_mempool *sess_mp,
> xform.modex.exponent.data = perf_mod_e;
> xform.modex.exponent.length = sizeof(perf_mod_e);
>
> - sess = (void *)rte_cryptodev_asym_session_create(sess_mp,
> dev_id, &xform);
> - if (sess == NULL)
> + ret = rte_cryptodev_asym_session_create(&asym_sess,
> + sess_mp, dev_id, &xform);
> + if (ret < 0) {
> + RTE_LOG(ERR, USER1, "Asym session create failed");
[Anoob] Don't we need \n at the end?
[snip]
> @@ -644,9 +645,9 @@ test_rsa_sign_verify(void)
> struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
> struct rte_mempool *sess_mpool = ts_params->session_mpool;
> uint8_t dev_id = ts_params->valid_devs[0];
> - void *sess;
> + void *sess = NULL;
> struct rte_cryptodev_info dev_info;
> - int status = TEST_SUCCESS;
> + int status = TEST_SUCCESS, ret;
[Anoob] May be move status to the end? Here and in other places.
https://doc.dpdk.org/guides/contributing/coding_style.html#local-variables
>
> /* Test case supports op with exponent key only,
> * Check in PMD feature flag for RSA exponent key type support.
> @@ -659,12 +660,12 @@ test_rsa_sign_verify(void)
> return TEST_SKIPPED;
> }
>
> - sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &rsa_xform);
> -
> - if (!sess) {
> + ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
> + dev_id, &rsa_xform);
> + if (ret < 0) {
> RTE_LOG(ERR, USER1, "Session creation failed for "
> "sign_verify\n");
> - status = TEST_FAILED;
> + status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
> goto error_exit;
> }
>
[snip]
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h index
> 6a4d6d9934..89739def91 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -990,18 +990,21 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mempool);
> /**
> * Create asymmetric crypto session header (generic with no private data)
> *
> + * @param session void ** for session to be used
> * @param mempool mempool to allocate asymmetric session
> * objects from
> * @param dev_id ID of device that we want the session to be used on
> * @param xforms Asymmetric crypto transform operations to apply on flow
> * processed with this session
> * @return
> - * - On success return pointer to asym-session
> - * - On failure returns NULL
> + * - 0 on success.
> + * - -EINVAL on invalid device ID, or invalid mempool.
[Anoob] PMD can also return an -EINVAL if some invalid configuration is requested. May be better to leave this open, like,
"- EINVAL Invalid arguments"
> + * - -ENOMEM on memory error for session allocation.
> + * - -ENOTSUP if device doesn't support session configuration.
> */
> __rte_experimental
> -void *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
> +int
> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
> +*mempool,
> uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
>
> /**
> --
> 2.25.1
prev parent reply other threads:[~2022-01-31 14:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-24 15:03 [PATCH v2 0/4] crypto: improve asym session usage Ciara Power
2022-01-24 15:03 ` [PATCH v2 1/4] crypto: use single buffer for asymmetric session Ciara Power
2022-01-27 11:03 ` Zhang, Roy Fan
2022-01-24 15:03 ` [PATCH v2 2/4] crypto: hide asym session structure Ciara Power
2022-01-27 11:04 ` Zhang, Roy Fan
2022-01-24 15:03 ` [PATCH v2 3/4] crypto: add asym session user data API Ciara Power
2022-01-27 11:05 ` Zhang, Roy Fan
2022-01-31 14:46 ` [EXT] " Anoob Joseph
2022-01-24 15:03 ` [PATCH v2 4/4] crypto: modify return value for asym session create Ciara Power
2022-01-27 11:06 ` Zhang, Roy Fan
2022-01-31 14:35 ` Anoob Joseph [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PH0PR18MB4672DE86C12D2410092A222DDF259@PH0PR18MB4672.namprd18.prod.outlook.com \
--to=anoobj@marvell.com \
--cc=ciara.power@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=gakhil@marvell.com \
--cc=mdr@ashroe.eu \
--cc=roy.fan.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).