Re: [dpdk-dev] [EXT] [PATCH v3 02/10] crypto/ipsec_mb: add multiprocess support

[Akhil Goyal <gakhil@marvell.com>]

> The ipsec_mb SW PMD now has multiprocess support.
> The queue-pair IMB_MGR is stored in a memzone instead of being allocated
> externally by the Intel IPSec MB library, when v1.1 is used.
> If v1.0 is used, multi process is not supported, and allocation is
> done as before.
> The secondary process needs to reconfigure the queue-pair to allow for
> IMB_MGR function pointers be updated.
> 
> Intel IPsec MB library version 1.1 is required for this support.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst        |   7 ++
>  .../crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c    | 110 +++++++++++++++---
>  .../ipsec_mb/rte_ipsec_mb_pmd_private.h       |   5 +

Missed to comment on first patch.
rte_ may be skipped from the internal files of the PMD.
rte_ is normally added for files which are exposed to the user.

>  3 files changed, 106 insertions(+), 16 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index 43d367bcad..3c9d7e19cb 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -62,6 +62,13 @@ New Features
>    * Added bus-level parsing of the devargs syntax.
>    * Kept compatibility with the legacy syntax as parsing fallback.
> 
> +* **Added multi-process support for IPsec-mb PMD.**
> +
> +  Added multi-process support to IPsec-mb PMD, which will add support
> +  for PMDs that are moved to use this shared framework.
> +  This feature makes use of an intel-ipsec-mb API found in v1.1,
> +  which is the minimum required version to use this multi-process support.
> +

I believe first new PMD framework should be introduced in the release note and
a sub bullet may be added to specify multi process support.

>  * **Updated Marvell cnxk crypto PMD.**
> 
>    * Added AES-CBC SHA1-HMAC support in lookaside protocol (IPsec) for
> CN10K.
> diff --git a/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c
> b/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c
> index 1146297216..c7bcfd3dce 100644
> --- a/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c
> +++ b/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c
> @@ -9,6 +9,8 @@
> 
>  #include "rte_ipsec_mb_pmd_private.h"
> 
> +#define IMB_MP_REQ_VER_STR "1.1.0"
> +
>  /** Configure device */
>  int
>  ipsec_mb_pmd_config(__rte_unused struct rte_cryptodev *dev,
> @@ -98,10 +100,20 @@ ipsec_mb_pmd_qp_release(struct rte_cryptodev
> *dev, uint16_t qp_id)
>  	struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
>  	struct rte_ring *r = NULL;
> 
> -	if (qp != NULL) {
> +	if (qp != NULL && rte_eal_process_type() == RTE_PROC_PRIMARY) {
>  		r = rte_ring_lookup(qp->name);
>  		if (r)
>  			rte_ring_free(r);
> +
> +#if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
> +		if (qp->mb_mgr)
> +			free_mb_mgr(qp->mb_mgr);
> +#else
> +		if (qp->mb_mgr_mz) {
> +			rte_memzone_free(qp->mb_mgr_mz);
> +			qp->mb_mgr = NULL;
> +		}
> +#endif
>  		rte_free(qp);
>  		dev->data->queue_pairs[qp_id] = NULL;
>  	}
> @@ -154,6 +166,42 @@ static struct rte_ring
>  			       RING_F_SP_ENQ | RING_F_SC_DEQ);
>  }
> 
> +#if IMB_VERSION(1, 1, 0) <= IMB_VERSION_NUM
> +static IMB_MGR *
> +ipsec_mb_pmd_alloc_mb_from_memzone(const struct rte_memzone
> **mb_mgr_mz,
> +		const char *mb_mgr_mz_name)
> +{
> +	IMB_MGR *mb_mgr;
> +
> +	if (rte_eal_process_type() ==  RTE_PROC_PRIMARY) {
> +		*mb_mgr_mz = rte_memzone_lookup(mb_mgr_mz_name);
> +		if (*mb_mgr_mz == NULL) {
> +			*mb_mgr_mz =
> rte_memzone_reserve(mb_mgr_mz_name,
> +			imb_get_mb_mgr_size(),
> +			rte_socket_id(), 0);
> +		}
> +		if (*mb_mgr_mz == NULL) {
> +			IPSEC_MB_LOG(DEBUG, "Error allocating memzone
> for %s",
> +					mb_mgr_mz_name);
> +			return NULL;
> +		}
> +		mb_mgr = imb_set_pointers_mb_mgr((*mb_mgr_mz)->addr,
> 0, 1);
> +		init_mb_mgr_auto(mb_mgr, NULL);
> +	} else {
> +		*mb_mgr_mz = rte_memzone_lookup(mb_mgr_mz_name);
> +		if (*mb_mgr_mz == NULL) {
> +			IPSEC_MB_LOG(ERR,
> +				"Secondary can't find %s mz, did primary
> create it?",
> +				mb_mgr_mz_name);
> +			return NULL;
> +		}
> +		mb_mgr = imb_set_pointers_mb_mgr((*mb_mgr_mz)->addr,
> 0, 0);
> +		init_mb_mgr_auto(mb_mgr, NULL);
> +	}
> +	return mb_mgr;
> +}
> +#endif
> +
>  /** Setup a queue pair */
>  int
>  ipsec_mb_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
> @@ -167,16 +215,44 @@ ipsec_mb_pmd_qp_setup(struct rte_cryptodev
> *dev, uint16_t qp_id,
>  	uint32_t qp_size;
>  	int ret = -1;
> 
> -	/* Free memory prior to re-allocation if needed. */
> -	if (dev->data->queue_pairs[qp_id] != NULL)
> -		ipsec_mb_pmd_qp_release(dev, qp_id);
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +#if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
> +		IPSEC_MB_LOG(ERR, "The intel-ipsec-mb version (%s) does
> not support multiprocess,"
> +				"the minimum version required for this
> feature is %s.",
> +				IMB_VERSION_STR, IMB_MP_REQ_VER_STR);
> +		return -EINVAL;
> +#endif
> +		if (dev->data->queue_pairs[qp_id] != NULL)
> +			qp = dev->data->queue_pairs[qp_id];
> +	} else {
> +		/* Free memory prior to re-allocation if needed. */
> +		if (dev->data->queue_pairs[qp_id] != NULL)
> +			ipsec_mb_pmd_qp_release(dev, qp_id);
> +
> +		qp_size = sizeof(*qp) + pmd_data->qp_priv_size;
> +		/* Allocate the queue pair data structure. */
> +		qp = rte_zmalloc_socket("IPSEC PMD Queue Pair", qp_size,
> +					RTE_CACHE_LINE_SIZE, socket_id);
> +		if (qp == NULL)
> +			return -ENOMEM;
> +	}
> +
> +#if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
> +	qp->mb_mgr = alloc_init_mb_mgr();
> +#else
> +	char mz_name[IPSEC_MB_MAX_MZ_NAME];
> +	snprintf(mz_name, sizeof(mz_name), "IMB_MGR_DEV_%d_QP_%d",
> +			dev->data->dev_id, qp_id);
> +	qp->mb_mgr = ipsec_mb_pmd_alloc_mb_from_memzone(&(qp-
> >mb_mgr_mz),
> +			mz_name);

_pmd_ may be skipped from the internal APIs. The function names are pretty long.

> +#endif
> +	if (qp->mb_mgr == NULL) {
> +		ret = -ENOMEM;
> +		goto qp_setup_cleanup;
> +	}
> 
> -	qp_size = sizeof(*qp) + pmd_data->qp_priv_size;
> -	/* Allocate the queue pair data structure. */
> -	qp = rte_zmalloc_socket("IPSEC PMD Queue Pair", qp_size,
> -				RTE_CACHE_LINE_SIZE, socket_id);
> -	if (qp == NULL)
> -		return -ENOMEM;
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +		return 0;
> 
>  	qp->id = qp_id;
>  	dev->data->queue_pairs[qp_id] = qp;
> @@ -194,12 +270,6 @@ ipsec_mb_pmd_qp_setup(struct rte_cryptodev
> *dev, uint16_t qp_id,
>  		goto qp_setup_cleanup;
>  	}
> 
> -	qp->mb_mgr = alloc_init_mb_mgr();
> -	if (!qp->mb_mgr) {
> -		ret = -ENOMEM;
> -		goto qp_setup_cleanup;
> -	}
> -
>  	memset(&qp->stats, 0, sizeof(qp->stats));
> 
>  	if (pmd_data->queue_pair_configure) {
> @@ -211,8 +281,15 @@ ipsec_mb_pmd_qp_setup(struct rte_cryptodev
> *dev, uint16_t qp_id,
>  	return 0;
> 
>  qp_setup_cleanup:
> +#if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
>  	if (qp->mb_mgr)
>  		free_mb_mgr(qp->mb_mgr);
> +#else
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +		return ret;
> +	if (qp->mb_mgr_mz)
> +		rte_memzone_free(qp->mb_mgr_mz);
> +#endif
>  	if (qp)
>  		rte_free(qp);
>  	return ret;
> @@ -269,6 +346,7 @@ ipsec_mb_pmd_sym_session_configure(
> 
>  	set_sym_session_private_data(sess, dev->driver_id,
> sess_private_data);
> 
> +	free_mb_mgr(mb_mgr);
>  	return 0;
>  }
> 
> diff --git a/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_private.h
> b/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_private.h
> index 754259aa59..35860b1b10 100644
> --- a/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_private.h
> +++ b/drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_private.h
> @@ -18,6 +18,9 @@
>  /* Maximum length for digest */
>  #define DIGEST_LENGTH_MAX 64
> 
> +/* Maximum length for memzone name */
> +#define IPSEC_MB_MAX_MZ_NAME 32
> +
>  enum ipsec_mb_vector_mode {
>  	IPSEC_MB_NOT_SUPPORTED = 0,
>  	IPSEC_MB_SSE,
> @@ -95,6 +98,8 @@ struct ipsec_mb_qp {
>  	 */
>  	IMB_MGR *mb_mgr;
>  	/* Multi buffer manager */
> +	const struct rte_memzone *mb_mgr_mz;
> +	/* Shared memzone for storing mb_mgr */
>  	__extension__ uint8_t additional_data[0];
>  	/**< Storing PMD specific additional data */
>  };
> --
> 2.25.1