Hi Ciara,
Minor nits. Please see inline.
With the fixes,
Acked-by: Anoob Joseph <ano...@marvell.com>
Thanks,
Anoob
> -----Original Message-----
> From: Ciara Power <ciara.po...@intel.com>
> Sent: Monday, January 24, 2022 8:34 PM
> To: dev@dpdk.org
> Cc: roy.fan.zh...@intel.com; Akhil Goyal <gak...@marvell.com>; Anoob Joseph
> <ano...@marvell.com>; m...@ashroe.eu; Ciara Power
> <ciara.po...@intel.com>; Declan Doherty <declan.dohe...@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.po...@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
