RE: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session create

[Akhil Goyal <gakhil@marvell.com>]

> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> b/doc/guides/prog_guide/cryptodev_lib.rst
> index 62bd3577f5..8e16461dc6 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -1236,10 +1236,10 @@ crypto operations is similar except change to
> respective op and xform setup).
>       * Create asym crypto session and initialize it for the crypto device.
>       * The session structure is hidden from the app, so void * is used.
>       */
> -    void *asym_session;
> -    asym_session =
> rte_cryptodev_asym_session_create(asym_session_pool,
> +    void *asym_session = NULL;
> +    ret = rte_cryptodev_asym_session_create(&asym_session,
> asym_session_pool,
>              cdev_id, &modex_xform);
> -    if (asym_session == NULL)
> +    if (ret < 0)
>          rte_exit(EXIT_FAILURE, "Session could not be created\n");

Sample Code in the rst files is no more added. @Thomas: Could you please confirm?
Probably a separate patch is required to clean this up.

> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 0d816ed4a9..005f0e7952 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -1912,9 +1912,9 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mp)
>  	return sess;
>  }
> 
> -void *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t
> dev_id,
> -		struct rte_crypto_asym_xform *xforms)
> +int
> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
> *mp,
> +		uint8_t dev_id, struct rte_crypto_asym_xform *xforms)

Do you really need a double pointer for the session handle?

>  {
>  	struct rte_cryptodev_asym_session *sess;
>  	uint32_t session_priv_data_sz;
> @@ -1926,18 +1926,18 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
> 
>  	if (!rte_cryptodev_is_valid_dev(dev_id)) {
>  		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> -		return NULL;
> +		return -EINVAL;
>  	}
>  	session_priv_data_sz =
> rte_cryptodev_asym_get_private_session_size(
>  			dev_id);
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
> 
>  	if (dev == NULL)
> -		return NULL;
> +		return -EINVAL;
> 
>  	if (!mp) {
>  		CDEV_LOG_ERR("invalid mempool\n");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	pool_priv = rte_mempool_get_priv(mp);
> @@ -1945,22 +1945,23 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  	if (pool_priv->max_priv_session_sz < session_priv_data_sz) {
>  		CDEV_LOG_DEBUG(
>  			"The private session data size used when creating the
> mempool is smaller than this device's private session data.");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	/* Verify if provided mempool can hold elements big enough. */
>  	if (mp->elt_size < session_header_size + session_priv_data_sz) {
>  		CDEV_LOG_ERR(
>  			"mempool elements too small to hold session
> objects");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	/* Allocate a session structure from the session pool */
> -	if (rte_mempool_get(mp, (void **)&sess)) {
> +	if (rte_mempool_get(mp, session)) {
>  		CDEV_LOG_ERR("couldn't get object from session
> mempool");
> -		return NULL;
> +		return -ENOMEM;
>  	}
> 
> +	sess = *session;
>  	sess->driver_id = dev->driver_id;
>  	sess->user_data_sz = pool_priv->user_data_sz;
>  	sess->max_priv_session_sz = pool_priv->max_priv_session_sz;
> @@ -1970,7 +1971,7 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  	 */
>  	memset(sess->sess_private_data, 0, session_priv_data_sz + sess-
> >user_data_sz);
> 
> -	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure, NULL);
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure, -ENOTSUP);
> 
>  	if (sess->sess_private_data[0] == 0) {
>  		ret = dev->dev_ops->asym_session_configure(dev,
> @@ -1980,12 +1981,12 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  			CDEV_LOG_ERR(
>  				"dev_id %d failed to configure session
> details",
>  				dev_id);
> -			return NULL;
> +			return ret;
>  		}
>  	}
> 
>  	rte_cryptodev_trace_asym_session_create(mp, sess);
> -	return sess;
> +	return 0;
>  }
> 
>  int
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 6a4d6d9934..9a75936963 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 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);
> 
Session should be the last parameter.
First all in params and then out params.