DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev v1] crypto/ipsec_mb: multi-process IPC request handler
@ 2022-10-20 23:51 Kai Ji
  2022-10-24 23:38 ` [dpdk-dev v2] " Kai Ji
  0 siblings, 1 reply; 14+ messages in thread
From: Kai Ji @ 2022-10-20 23:51 UTC (permalink / raw)
  To: dev; +Cc: Kai Ji, Pablo de Lara, Anatoly Burakov

As the secondary process needs queue-pair to be configured by
the primary process before use. This patch adds an IPC register function
to allow secondary process to setup cryptodev queue-pair via IPC messages
during the runtime. A new "qp_in_used_pid" param stores the PID to provide
the ownership of the queue-pair so that only the PID matched queue-pair
can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 69 ++++++++++++++++++++++
 drivers/crypto/ipsec_mb/ipsec_mb_private.c | 22 ++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h | 33 +++++++++++
 3 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..abeef364d1 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -296,6 +296,75 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }
 
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg mp_res;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)mp_res.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+	struct rte_cryptodev_qp_conf *queue_conf =
+		(struct rte_cryptodev_qp_conf *)req_param->queue_conf;
+
+	resp_param->result = -EINVAL;
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_in_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_in_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_in_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&mp_res, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..3c5cf50673 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }
 
+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_REQ,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_REQ);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,9 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());
 
-	return 0;
+	retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }
 
 int
@@ -187,5 +205,7 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);
 
+	ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..4c894e6688 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -10,6 +10,7 @@
 #else
 #include <intel-ipsec-mb.h>
 #endif
+#include <rte_cryptodev.h>
 #include <cryptodev_pmd.h>
 #include <bus_vdev_driver.h>
 
@@ -25,6 +26,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32
 
+/* ipsec mbmulti-process queue pair config */
+#define IPSEC_MB_MP_REQ "ipsec_mb_mp_request"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -149,11 +153,40 @@ struct ipsec_mb_qp {
 	IMB_MGR *mb_mgr;
 	/* Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
+	/** Array of process id used for queue pairs **/
+	uint16_t qp_in_used_by_pid;
 	/* Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };
 
+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	void *queue_conf;
+	/**< A pointer of Crypto device queue pair configuration structure */
+	int result;
+	/**< The request result for response message */
+};
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v2] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-20 23:51 [dpdk-dev v1] crypto/ipsec_mb: multi-process IPC request handler Kai Ji
@ 2022-10-24 23:38 ` Kai Ji
  2022-10-25 21:48   ` [dpdk-dev v3] " Kai Ji
  0 siblings, 1 reply; 14+ messages in thread
From: Kai Ji @ 2022-10-24 23:38 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the secondary process needs queue-pair to be configured by
the primary process before use. This patch adds an IPC register function
to allow secondary process to setup cryptodev queue-pair via IPC messages
during the runtime. A new "qp_in_used_pid" param stores the PID to provide
the ownership of the queue-pair so that only the PID matched queue-pair
can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---

v2:
- add in share memzone for data exchange between multi-process

---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 135 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  40 +++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  44 +++++++
 3 files changed, 215 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..a0e696819b 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,45 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		struct rte_cryptodev_qp_conf *qp_conf, int socket_id, uint16_t pid,
+		enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+
+	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_REQ));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_REQ, sizeof(IPSEC_MB_MP_REQ));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = pid;
+	req_param->queue_conf = (void *)qp_conf;
+
+	qp_req_msg.num_fds = 0;
+	qp_resp.msgs = &qp_resp_msg;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+
+	resp_param = (struct ipsec_mb_mp_param *)&qp_resp_msg.param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +140,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +158,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, getpid(), RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +268,23 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			struct rte_cryptodev_qp_conf *shared_qp_conf;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			if (!mp_shared_data) {
+				IPSEC_MB_LOG(ERR, "no multi-process share mz allocated.");
+				return -EINVAL;
+			}
+			shared_qp_conf = &(mp_shared_data->qp_conf);
+
+			memcpy(shared_qp_conf, qp_conf,
+					sizeof(struct rte_cryptodev_qp_conf));
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						shared_qp_conf, socket_id, getpid(),
+						RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +356,75 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg mp_res;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)mp_res.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+	struct rte_cryptodev_qp_conf *queue_conf =
+		(struct rte_cryptodev_qp_conf *)req_param->queue_conf;
+
+	resp_param->result = -EINVAL;
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_in_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_in_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_in_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&mp_res, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..a02010881c 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,38 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	/* Setup memzone for shared data */
+	ipsec_mb_mp_mz = rte_memzone_reserve(
+			IPSEC_MB_MP_SHARED_MZ_NAME,
+			sizeof(struct ipsec_mb_mp_shared_data), 0, 0);
+	if (ipsec_mb_mp_mz == NULL) {
+		RTE_LOG(ERR, USER1,
+			"%s: cannot create multiprocess-shared memzone for ipsec_mb",
+			__func__);
+		return -1;
+	}
+
+	mp_shared_data = ipsec_mb_mp_mz->addr;
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_REQ,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	mp_shared_data = NULL;
+	if (ipsec_mb_mp_mz)
+		rte_memzone_free(ipsec_mb_mp_mz);
+
+	rte_mp_action_unregister(IPSEC_MB_MP_REQ);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +184,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +222,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..f27321b9c4 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -10,6 +10,7 @@
 #else
 #include <intel-ipsec-mb.h>
 #endif
+#include <rte_cryptodev.h>
 #include <cryptodev_pmd.h>
 #include <bus_vdev_driver.h>

@@ -25,6 +26,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_REQ "ipsec_mb_mp_request"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -63,6 +67,8 @@ extern int ipsec_mb_logtype_driver;
 	rte_log(RTE_LOG_##level, ipsec_mb_logtype_driver,                     \
 		"%s() line %u: " fmt "\n", __func__, __LINE__, ##__VA_ARGS__)

+#define IPSEC_MB_MP_SHARED_MZ_NAME		"IPSEC_MB_MP_SHARED_MZ"
+
 /** All supported device types */
 enum ipsec_mb_pmd_types {
 	IPSEC_MB_PMD_TYPE_AESNI_MB = 0,
@@ -149,11 +155,49 @@ struct ipsec_mb_qp {
 	IMB_MGR *mb_mgr;
 	/* Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
+	/** Array of process id used for queue pairs **/
+	uint16_t qp_in_used_by_pid;
 	/* Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/* multi-process shared data */
+struct ipsec_mb_mp_shared_data {
+	struct rte_cryptodev_qp_conf qp_conf;
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	void *queue_conf;
+	/**< A pointer of Crypto device queue pair configuration structure */
+	int result;
+	/**< The request result for response message */
+};
+
+/* memzone for multi-process shared data */
+const struct rte_memzone *ipsec_mb_mp_mz;
+struct ipsec_mb_mp_shared_data *mp_shared_data;
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v3] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-24 23:38 ` [dpdk-dev v2] " Kai Ji
@ 2022-10-25 21:48   ` Kai Ji
  2022-10-26  8:17     ` De Lara Guarch, Pablo
  2022-10-26 10:01     ` [dpdk-dev v4] " Kai Ji
  0 siblings, 2 replies; 14+ messages in thread
From: Kai Ji @ 2022-10-25 21:48 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the secondary process needs queue-pair to be configured by
the primary process before use. This patch adds an IPC register function
to allow secondary process to setup cryptodev queue-pair via IPC messages
during the runtime. A new "qp_in_used_pid" param stores the PID to provide
the ownership of the queue-pair so that only the PID matched queue-pair
can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---

v3:
- remove shared memzone as qp_conf params can be passed directly from
ipc message.

v2:
- add in shared memzone for data exchange between multi-process
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 130 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  46 ++++++++
 3 files changed, 196 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..c29656b746 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, uint16_t pid, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = pid;
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, getpid(), RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,14 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id, getpid(),
+						RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +348,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_in_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_in_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_in_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..a2b52f808e 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -10,6 +10,7 @@
 #else
 #include <intel-ipsec-mb.h>
 #endif
+#include <rte_cryptodev.h>
 #include <cryptodev_pmd.h>
 #include <bus_vdev_driver.h>

@@ -25,6 +26,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -63,6 +67,8 @@ extern int ipsec_mb_logtype_driver;
 	rte_log(RTE_LOG_##level, ipsec_mb_logtype_driver,                     \
 		"%s() line %u: " fmt "\n", __func__, __LINE__, ##__VA_ARGS__)

+#define IPSEC_MB_MP_SHARED_MZ_NAME		"IPSEC_MB_MP_SHARED_MZ"
+
 /** All supported device types */
 enum ipsec_mb_pmd_types {
 	IPSEC_MB_PMD_TYPE_AESNI_MB = 0,
@@ -149,11 +155,51 @@ struct ipsec_mb_qp {
 	IMB_MGR *mb_mgr;
 	/* Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
+	/** The process id used for queue pairs **/
+	uint16_t qp_in_used_by_pid;
 	/* Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/* multi-process shared data */
+struct ipsec_mb_mp_shared_data {
+	struct rte_cryptodev_qp_conf qp_conf;
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+/* memzone for multi-process shared data */
+const struct rte_memzone *ipsec_mb_mp_mz;
+struct ipsec_mb_mp_shared_data *mp_shared_data;
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [dpdk-dev v3] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-25 21:48   ` [dpdk-dev v3] " Kai Ji
@ 2022-10-26  8:17     ` De Lara Guarch, Pablo
  2022-10-26 10:01     ` [dpdk-dev v4] " Kai Ji
  1 sibling, 0 replies; 14+ messages in thread
From: De Lara Guarch, Pablo @ 2022-10-26  8:17 UTC (permalink / raw)
  To: Ji, Kai, dev; +Cc: gakhil, Burakov, Anatoly

Hi Kai,

A few comments below

Thanks,
Pablo

> -----Original Message-----
> From: Ji, Kai <kai.ji@intel.com>
> Sent: Tuesday, October 25, 2022 10:48 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: [dpdk-dev v3] crypto/ipsec_mb: multi-process IPC request handler
> 
> As the secondary process needs queue-pair to be configured by the primary
> process before use. 

This sentence looks incomplete: as the secondary process needs...., then what?

> This patch adds an IPC register function to allow
> secondary process to setup cryptodev queue-pair via IPC messages during
> the runtime. A new "qp_in_used_pid" param stores the PID to provide the
> ownership of the queue-pair so that only the PID matched queue-pair can be
> free'd in the request.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> ---
> 
> v3:
> - remove shared memzone as qp_conf params can be passed directly from
> ipc message.
> 
> v2:
> - add in shared memzone for data exchange between multi-process
> ---
>  drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 130
> ++++++++++++++++++++-
>  drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
> drivers/crypto/ipsec_mb/ipsec_mb_private.h |  46 ++++++++
>  3 files changed, 196 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index cedcaa2742..c29656b746 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -3,6 +3,7 @@
>   */
> 
>  #include <string.h>
> +#include <unistd.h>
> 
>  #include <rte_common.h>
>  #include <rte_malloc.h>
> @@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
>  	}
>  }
> 
> +static int
> +ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf,
> +		int socket_id, uint16_t pid, enum ipsec_mb_mp_req_type
> op_type) {
> +	int ret;
> +	struct rte_mp_msg qp_req_msg;
> +	struct rte_mp_msg *qp_resp_msg;
> +	struct rte_mp_reply qp_resp;
> +	struct ipsec_mb_mp_param *req_param;
> +	struct ipsec_mb_mp_param *resp_param;
> +	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
> +
> +	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
> +	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG,
> sizeof(IPSEC_MB_MP_MSG));
> +	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
> +
> +	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
> +	req_param->type = op_type;
> +	req_param->dev_id = dev_id;
> +	req_param->qp_id = qp_id;
> +	req_param->socket_id = socket_id;
> +	req_param->process_id = pid;

I think you can use "get_pid()" here and not have "pid" argument in the function, as I believe it is always called within the same process.

> +	if (qp_conf) {
> +		req_param->nb_descriptors = qp_conf->nb_descriptors;
> +		req_param->mp_session = (void *)qp_conf->mp_session;
> +	}
> +
> +	qp_req_msg.num_fds = 0;
> +	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
> +	if (ret) {
> +		RTE_LOG(ERR, USER1, "Create MR request to primary
> process failed.");
> +		return -1;
> +	}
> +	qp_resp_msg = &qp_resp.msgs[0];
> +	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
> +
> +	return resp_param->result;
> +}
> +

..

> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> index b56eaf061e..a2b52f808e 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h

...

> +
>  /** All supported device types */
>  enum ipsec_mb_pmd_types {
>  	IPSEC_MB_PMD_TYPE_AESNI_MB = 0,
> @@ -149,11 +155,51 @@ struct ipsec_mb_qp {
>  	IMB_MGR *mb_mgr;
>  	/* Multi buffer manager */
>  	const struct rte_memzone *mb_mgr_mz;
> +	/** The process id used for queue pairs **/
> +	uint16_t qp_in_used_by_pid;

I would rename this to "qp_in_use_by_pid" or "qp_used_by_pid".

>  	/* Shared memzone for storing mb_mgr */

Check these comments. This last one "Shared memzone..." applies to the field "mb_mgr_mz".
Also, they should use "/**< ".

>  	__extension__ uint8_t additional_data[];
>  	/**< Storing PMD specific additional data */  };
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-25 21:48   ` [dpdk-dev v3] " Kai Ji
  2022-10-26  8:17     ` De Lara Guarch, Pablo
@ 2022-10-26 10:01     ` Kai Ji
  2022-10-26 10:22       ` Kai Ji
  1 sibling, 1 reply; 14+ messages in thread
From: Kai Ji @ 2022-10-26 10:01 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the queue pair used in secondary process need to be setuped by
the primary process, this patch add an IPC register function to help
secondary process to send out queue-pair setup reguest to primary
process via IPC messages. A new "qp_in_used_pid" param stores the PID
to provide the ownership of the queue-pair so that only the PID matched
queue-pair can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
v4:
- review comments resolved

v3:
- remove shared memzone as qp_conf params can be passed directly from
ipc message.

v2:
- add in shared memzone for data exchange between multi-process
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  48 +++++++-
 3 files changed, 195 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..bf18d692bd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = getpid();
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..607eb3d973 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -10,6 +10,7 @@
 #else
 #include <intel-ipsec-mb.h>
 #endif
+#include <rte_cryptodev.h>
 #include <cryptodev_pmd.h>
 #include <bus_vdev_driver.h>

@@ -25,6 +26,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -142,18 +146,58 @@ struct ipsec_mb_qp {
 	enum ipsec_mb_pmd_types pmd_type;
 	/**< pmd type */
 	uint8_t digest_idx;
+	/**< The process id used for queue pairs **/
+	uint16_t qp_used_by_pid;
 	/**< Index of the next
 	 * slot to be used in temp_digests,
 	 * to store the digest for a given operation
 	 */
 	IMB_MGR *mb_mgr;
-	/* Multi buffer manager */
+	/**< Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
-	/* Shared memzone for storing mb_mgr */
+	/**< Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/* multi-process shared data */
+struct ipsec_mb_mp_shared_data {
+	struct rte_cryptodev_qp_conf qp_conf;
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+/* memzone for multi-process shared data */
+const struct rte_memzone *ipsec_mb_mp_mz;
+struct ipsec_mb_mp_shared_data *mp_shared_data;
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 10:01     ` [dpdk-dev v4] " Kai Ji
@ 2022-10-26 10:22       ` Kai Ji
  2022-10-26 10:27         ` Kai Ji
  0 siblings, 1 reply; 14+ messages in thread
From: Kai Ji @ 2022-10-26 10:22 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the queue pair used in secondary process need to be setuped by
the primary process, this patch add an IPC register function to help
secondary process to send out queue-pair setup reguest to primary
process via IPC messages. A new "qp_in_used_pid" param stores the PID
to provide the ownership of the queue-pair so that only the PID matched
queue-pair can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  38 +++++-
 3 files changed, 185 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..bf18d692bd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */
 
 #include <string.h>
+#include <unistd.h>
 
 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }
 
+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = getpid();
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);
 
@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }
 
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }
 
+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());
 
-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }
 
 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);
 
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..f670fd6f0a 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -25,6 +25,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32
 
+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -142,18 +145,49 @@ struct ipsec_mb_qp {
 	enum ipsec_mb_pmd_types pmd_type;
 	/**< pmd type */
 	uint8_t digest_idx;
+	/**< The process id used for queue pairs **/
+	uint16_t qp_used_by_pid;
 	/**< Index of the next
 	 * slot to be used in temp_digests,
 	 * to store the digest for a given operation
 	 */
 	IMB_MGR *mb_mgr;
-	/* Multi buffer manager */
+	/**< Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
-	/* Shared memzone for storing mb_mgr */
+	/**< Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };
 
+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 10:22       ` Kai Ji
@ 2022-10-26 10:27         ` Kai Ji
  2022-10-26 11:07           ` Kusztal, ArkadiuszX
                             ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Kai Ji @ 2022-10-26 10:27 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the queue pair used in secondary process need to be setuped by
the primary process, this patch add an IPC register function to help
secondary process to send out queue-pair setup reguest to primary
process via IPC messages. A new "qp_in_used_pid" param stores the PID
to provide the ownership of the queue-pair so that only the PID matched
queue-pair can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
v4:
- review comments resolved

v3:
- remove shared memzone as qp_conf params can be passed directly from
ipc message.

v2:
- add in shared memzone for data exchange between multi-process
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  38 +++++-
 3 files changed, 185 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..bf18d692bd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = getpid();
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..f670fd6f0a 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -25,6 +25,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -142,18 +145,49 @@ struct ipsec_mb_qp {
 	enum ipsec_mb_pmd_types pmd_type;
 	/**< pmd type */
 	uint8_t digest_idx;
+	/**< The process id used for queue pairs **/
+	uint16_t qp_used_by_pid;
 	/**< Index of the next
 	 * slot to be used in temp_digests,
 	 * to store the digest for a given operation
 	 */
 	IMB_MGR *mb_mgr;
-	/* Multi buffer manager */
+	/**< Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
-	/* Shared memzone for storing mb_mgr */
+	/**< Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 10:27         ` Kai Ji
@ 2022-10-26 11:07           ` Kusztal, ArkadiuszX
  2022-10-26 12:32           ` De Lara Guarch, Pablo
  2022-10-26 12:48           ` Kai Ji
  2 siblings, 0 replies; 14+ messages in thread
From: Kusztal, ArkadiuszX @ 2022-10-26 11:07 UTC (permalink / raw)
  To: Ji, Kai, dev; +Cc: gakhil, Ji, Kai, De Lara Guarch, Pablo, Burakov, Anatoly



> -----Original Message-----
> From: Kai Ji <kai.ji@intel.com>
> Sent: Wednesday, October 26, 2022 12:28 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
> 
> As the queue pair used in secondary process need to be setuped by the primary
> process, this patch add an IPC register function to help secondary process to
> send out queue-pair setup reguest to primary process via IPC messages. A new
> "qp_in_used_pid" param stores the PID to provide the ownership of the queue-
> pair so that only the PID matched queue-pair can be free'd in the request.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> 
> --
> 2.17.1
Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 10:27         ` Kai Ji
  2022-10-26 11:07           ` Kusztal, ArkadiuszX
@ 2022-10-26 12:32           ` De Lara Guarch, Pablo
  2022-10-26 12:48           ` Kai Ji
  2 siblings, 0 replies; 14+ messages in thread
From: De Lara Guarch, Pablo @ 2022-10-26 12:32 UTC (permalink / raw)
  To: Ji, Kai, dev; +Cc: gakhil, Burakov, Anatoly

Hi Kai,

A couple of minor bits left. 

> -----Original Message-----
> From: Ji, Kai <kai.ji@intel.com>
> Sent: Wednesday, October 26, 2022 11:28 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
> 
> As the queue pair used in secondary process need to be setuped by the
needs/needed to be set up by...

> primary process, this patch add an IPC register function to help secondary
This patch adds

> process to send out queue-pair setup reguest to primary process via IPC
request

> messages. A new "qp_in_used_pid" param stores the PID to provide the
> ownership of the queue-pair so that only the PID matched queue-pair can be
> free'd in the request.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>


> --- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> @@ -25,6 +25,9 @@
>  /* Maximum length for memzone name */
>  #define IPSEC_MB_MAX_MZ_NAME 32
> 
> +/* ipsec mb multi-process queue pair config */ #define
> IPSEC_MB_MP_MSG
> +"ipsec_mb_mp_msg"
> +
>  enum ipsec_mb_vector_mode {
>  	IPSEC_MB_NOT_SUPPORTED = 0,
>  	IPSEC_MB_SSE,
> @@ -142,18 +145,49 @@ struct ipsec_mb_qp {
>  	enum ipsec_mb_pmd_types pmd_type;
>  	/**< pmd type */
>  	uint8_t digest_idx;
> +	/**< The process id used for queue pairs **/
> +	uint16_t qp_used_by_pid;
>  	/**< Index of the next
>  	 * slot to be used in temp_digests,
>  	 * to store the digest for a given operation
>  	 */

Comments are mixed here (digest_idx and qp_used_by_pid).

>  	IMB_MGR *mb_mgr;
> -	/* Multi buffer manager */
> +	/**< Multi buffer manager */
>  	const struct rte_memzone *mb_mgr_mz;
> -	/* Shared memzone for storing mb_mgr */
> +	/**< Shared memzone for storing mb_mgr */
>  	__extension__ uint8_t additional_data[];
>  	/**< Storing PMD specific additional data */  };
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 10:27         ` Kai Ji
  2022-10-26 11:07           ` Kusztal, ArkadiuszX
  2022-10-26 12:32           ` De Lara Guarch, Pablo
@ 2022-10-26 12:48           ` Kai Ji
  2022-10-26 13:04             ` De Lara Guarch, Pablo
  2022-10-26 13:19             ` [dpdk-dev v5] " Kai Ji
  2 siblings, 2 replies; 14+ messages in thread
From: Kai Ji @ 2022-10-26 12:48 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the queue pair used in secondary process needs to be set up by
the primary process, this patch adds an IPC register function to help
secondary process to send out queue-pair setup reguest to primary
process via IPC request messages. A new "qp_in_used_pid" param stores
the PID to provide the ownership of the queue-pair so that only the PID
matched queue-pair can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
v4:
- review comments resolved

v3:
- remove shared memzone as qp_conf params can be passed directly from
ipc message.

v2:
- add in shared memzone for data exchange between multi-process
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  38 +++++-
 3 files changed, 185 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..bf18d692bd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = getpid();
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..ab27214f2c 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -25,6 +25,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -146,14 +149,45 @@ struct ipsec_mb_qp {
 	 * slot to be used in temp_digests,
 	 * to store the digest for a given operation
 	 */
+	uint16_t qp_used_by_pid;
+	/**< The process id used for queue pairs **/
 	IMB_MGR *mb_mgr;
-	/* Multi buffer manager */
+	/**< Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
-	/* Shared memzone for storing mb_mgr */
+	/**< Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 12:48           ` Kai Ji
@ 2022-10-26 13:04             ` De Lara Guarch, Pablo
  2022-10-26 13:19             ` [dpdk-dev v5] " Kai Ji
  1 sibling, 0 replies; 14+ messages in thread
From: De Lara Guarch, Pablo @ 2022-10-26 13:04 UTC (permalink / raw)
  To: Ji, Kai, dev; +Cc: gakhil, Burakov, Anatoly

Hi Kai,


> -----Original Message-----
> From: Ji, Kai <kai.ji@intel.com>
> Sent: Wednesday, October 26, 2022 1:49 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: [dpdk-dev v4] crypto/ipsec_mb: multi-process IPC request handler
> 
> As the queue pair used in secondary process needs to be set up by the
> primary process, this patch adds an IPC register function to help secondary
> process to send out queue-pair setup reguest to primary process via IPC

Still typo: reguest -> request.

> request messages. A new "qp_in_used_pid" param stores the PID to provide
> the ownership of the queue-pair so that only the PID matched queue-pair
> can be free'd in the request.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>

Just one missing typo to fix. Apart from that:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev v5] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 12:48           ` Kai Ji
  2022-10-26 13:04             ` De Lara Guarch, Pablo
@ 2022-10-26 13:19             ` Kai Ji
  2022-10-27  9:54               ` [EXT] " Akhil Goyal
  2022-10-27 17:57               ` Konstantin Ananyev
  1 sibling, 2 replies; 14+ messages in thread
From: Kai Ji @ 2022-10-26 13:19 UTC (permalink / raw)
  To: dev; +Cc: gakhil, Kai Ji, Pablo de Lara, Anatoly Burakov

As the queue pair used in secondary process needs to be set up by
the primary process, this patch adds an IPC register function to help
secondary process to send out queue-pair setup request to primary
process via IPC request messages. A new "qp_in_used_pid" param stores
the PID to provide the ownership of the queue-pair so that only the PID
matched queue-pair can be free'd in the request.

Signed-off-by: Kai Ji <kai.ji@intel.com>
Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
v5:
- minor updates and typo fix

v4:
- review comments resolved

v3:
- remove shared memzone as qp_conf params can be passed directly from
ipc message.

v2:
- add in shared memzone for data exchange between multi-process
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h |  38 +++++-
 3 files changed, 185 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index cedcaa2742..bf18d692bd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -3,6 +3,7 @@
  */

 #include <string.h>
+#include <unistd.h>

 #include <rte_common.h>
 #include <rte_malloc.h>
@@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
 	}
 }

+static int
+ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
+		const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id, enum ipsec_mb_mp_req_type op_type)
+{
+	int ret;
+	struct rte_mp_msg qp_req_msg;
+	struct rte_mp_msg *qp_resp_msg;
+	struct rte_mp_reply qp_resp;
+	struct ipsec_mb_mp_param *req_param;
+	struct ipsec_mb_mp_param *resp_param;
+	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
+
+	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
+	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
+
+	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
+	req_param->type = op_type;
+	req_param->dev_id = dev_id;
+	req_param->qp_id = qp_id;
+	req_param->socket_id = socket_id;
+	req_param->process_id = getpid();
+	if (qp_conf) {
+		req_param->nb_descriptors = qp_conf->nb_descriptors;
+		req_param->mp_session = (void *)qp_conf->mp_session;
+	}
+
+	qp_req_msg.num_fds = 0;
+	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
+	if (ret) {
+		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
+		return -1;
+	}
+	qp_resp_msg = &qp_resp.msgs[0];
+	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
+
+	return resp_param->result;
+}
+
 /** Release queue pair */
 int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
@@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
+	if (qp != NULL)
+		return 0;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		r = rte_ring_lookup(qp->name);
 		rte_ring_free(r);

@@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 #endif
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
+	} else { /* secondary process */
+		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
@@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 #endif
 		qp = dev->data->queue_pairs[qp_id];
 		if (qp == NULL) {
-			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
-			return -EINVAL;
+			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
+						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
 		}
+
+		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
+		return -EINVAL;
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
@@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }

+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
+{
+	struct rte_mp_msg ipc_resp;
+	struct ipsec_mb_mp_param *resp_param =
+		(struct ipsec_mb_mp_param *)ipc_resp.param;
+	const struct ipsec_mb_mp_param *req_param =
+		(const struct ipsec_mb_mp_param *)mp_msg->param;
+
+	int ret;
+	struct rte_cryptodev *dev;
+	struct ipsec_mb_qp *qp;
+	struct rte_cryptodev_qp_conf queue_conf;
+	int dev_id = req_param->dev_id;
+	int qp_id = req_param->qp_id;
+
+	queue_conf.nb_descriptors = req_param->nb_descriptors;
+	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
+	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
+	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
+		goto out;
+	}
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	switch (req_param->type) {
+	case RTE_IPSEC_MB_MP_REQ_QP_SET:
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp)	{
+			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
+			goto out;
+		}
+
+		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
+		if (!ret) {
+			qp = dev->data->queue_pairs[qp_id];
+			if (!qp) {
+				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+					qp_id, dev_id);
+				goto out;
+			}
+			qp->qp_used_by_pid = req_param->process_id;
+		}
+		resp_param->result = ret;
+		break;
+	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
+		qp = dev->data->queue_pairs[qp_id];
+		if (!qp) {
+			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
+				qp_id, dev_id);
+			goto out;
+		}
+
+		if (qp->qp_used_by_pid != req_param->process_id) {
+			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
+			goto out;
+		}
+
+		qp->qp_used_by_pid = 0;
+		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
+		break;
+	default:
+		CDEV_LOG_ERR("invalid mp request type\n");
+	}
+
+out:
+	ret = rte_mp_reply(&ipc_resp, peer);
+	return ret;
+}
+
 /** Return the size of the specific pmd session structure */
 unsigned
 ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
index e56579596f..c5540ac8dc 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
@@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
 	return nb_enqueued;
 }

+static int
+ipsec_mb_mp_request_register(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
+	return rte_mp_action_register(IPSEC_MB_MP_MSG,
+				ipsec_mb_ipc_request);
+}
+
+static void
+ipsec_mb_mp_request_unregister(void)
+{
+	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
+	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
+}
+
 int
 ipsec_mb_create(struct rte_vdev_device *vdev,
 	enum ipsec_mb_pmd_types pmd_type)
@@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
 	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
 		     imb_get_version_str());

-	return 0;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		retval = ipsec_mb_mp_request_register();
+
+	return retval;
 }

 int
@@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
 	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
 		ipsec_mb_qp_release(cryptodev, qp_id);

+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		ipsec_mb_mp_request_unregister();
+
 	return rte_cryptodev_pmd_destroy(cryptodev);
 }
diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
index b56eaf061e..ab27214f2c 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
@@ -25,6 +25,9 @@
 /* Maximum length for memzone name */
 #define IPSEC_MB_MAX_MZ_NAME 32

+/* ipsec mb multi-process queue pair config */
+#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
+
 enum ipsec_mb_vector_mode {
 	IPSEC_MB_NOT_SUPPORTED = 0,
 	IPSEC_MB_SSE,
@@ -146,14 +149,45 @@ struct ipsec_mb_qp {
 	 * slot to be used in temp_digests,
 	 * to store the digest for a given operation
 	 */
+	uint16_t qp_used_by_pid;
+	/**< The process id used for queue pairs **/
 	IMB_MGR *mb_mgr;
-	/* Multi buffer manager */
+	/**< Multi buffer manager */
 	const struct rte_memzone *mb_mgr_mz;
-	/* Shared memzone for storing mb_mgr */
+	/**< Shared memzone for storing mb_mgr */
 	__extension__ uint8_t additional_data[];
 	/**< Storing PMD specific additional data */
 };

+/** Request types for IPC. */
+enum ipsec_mb_mp_req_type {
+	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
+	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
+	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
+};
+
+/** Parameters for IPC. */
+struct ipsec_mb_mp_param {
+	enum ipsec_mb_mp_req_type type; /**< IPC request type */
+	int dev_id;
+	/**< The identifier of the device */
+	int qp_id;
+	/**< The index of the queue pair to be configured */
+	int socket_id;
+	/**< Socket to allocate resources on */
+	uint16_t process_id;
+	/**< The pid who send out the requested */
+	uint32_t nb_descriptors;
+	/**< Number of descriptors per queue pair */
+	void *mp_session;
+	/**< The mempool for creating session in sessionless mode */
+	int result;
+	/**< The request result for response message */
+};
+
+int
+ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
+
 static __rte_always_inline void *
 ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
 {
--
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [EXT] [dpdk-dev v5] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 13:19             ` [dpdk-dev v5] " Kai Ji
@ 2022-10-27  9:54               ` Akhil Goyal
  2022-10-27 17:57               ` Konstantin Ananyev
  1 sibling, 0 replies; 14+ messages in thread
From: Akhil Goyal @ 2022-10-27  9:54 UTC (permalink / raw)
  To: Kai Ji, dev; +Cc: Pablo de Lara, Anatoly Burakov

> As the queue pair used in secondary process needs to be set up by
> the primary process, this patch adds an IPC register function to help
> secondary process to send out queue-pair setup request to primary
> process via IPC request messages. A new "qp_in_used_pid" param stores
> the PID to provide the ownership of the queue-pair so that only the PID
> matched queue-pair can be free'd in the request.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> v5:
> - minor updates and typo fix
Applied to dpdk-next-crypto

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [dpdk-dev v5] crypto/ipsec_mb: multi-process IPC request handler
  2022-10-26 13:19             ` [dpdk-dev v5] " Kai Ji
  2022-10-27  9:54               ` [EXT] " Akhil Goyal
@ 2022-10-27 17:57               ` Konstantin Ananyev
  1 sibling, 0 replies; 14+ messages in thread
From: Konstantin Ananyev @ 2022-10-27 17:57 UTC (permalink / raw)
  To: Kai Ji, dev; +Cc: gakhil, Pablo de Lara, Anatoly Burakov



> 
> As the queue pair used in secondary process needs to be set up by
> the primary process, this patch adds an IPC register function to help
> secondary process to send out queue-pair setup request to primary
> process via IPC request messages. A new "qp_in_used_pid" param stores
> the PID to provide the ownership of the queue-pair so that only the PID
> matched queue-pair can be free'd in the request.

As I can see that patch was already merged, but still: 
How we suppose  to guarantee synchronization with such approach?
Let say secondary process sends an IPC message to configure 
crypto-queue, and at the same moment primary process (for whatever reason)
decides to reconfigure same crypto-dev.
Is there any way to prevent such race-conditions to happen?

> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> v5:
> - minor updates and typo fix
> 
> v4:
> - review comments resolved
> 
> v3:
> - remove shared memzone as qp_conf params can be passed directly from
> ipc message.
> 
> v2:
> - add in shared memzone for data exchange between multi-process
> ---
>  drivers/crypto/ipsec_mb/ipsec_mb_ops.c     | 129 ++++++++++++++++++++-
>  drivers/crypto/ipsec_mb/ipsec_mb_private.c |  24 +++-
>  drivers/crypto/ipsec_mb/ipsec_mb_private.h |  38 +++++-
>  3 files changed, 185 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index cedcaa2742..bf18d692bd 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -3,6 +3,7 @@
>   */
> 
>  #include <string.h>
> +#include <unistd.h>
> 
>  #include <rte_common.h>
>  #include <rte_malloc.h>
> @@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev,
>  	}
>  }
> 
> +static int
> +ipsec_mb_secondary_qp_op(int dev_id, int qp_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf,
> +		int socket_id, enum ipsec_mb_mp_req_type op_type)
> +{
> +	int ret;
> +	struct rte_mp_msg qp_req_msg;
> +	struct rte_mp_msg *qp_resp_msg;
> +	struct rte_mp_reply qp_resp;
> +	struct ipsec_mb_mp_param *req_param;
> +	struct ipsec_mb_mp_param *resp_param;
> +	struct timespec ts = {.tv_sec = 1, .tv_nsec = 0};
> +
> +	memset(&qp_req_msg, 0, sizeof(IPSEC_MB_MP_MSG));
> +	memcpy(qp_req_msg.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
> +	req_param = (struct ipsec_mb_mp_param *)&qp_req_msg.param;
> +
> +	qp_req_msg.len_param = sizeof(struct ipsec_mb_mp_param);
> +	req_param->type = op_type;
> +	req_param->dev_id = dev_id;
> +	req_param->qp_id = qp_id;
> +	req_param->socket_id = socket_id;
> +	req_param->process_id = getpid();
> +	if (qp_conf) {
> +		req_param->nb_descriptors = qp_conf->nb_descriptors;
> +		req_param->mp_session = (void *)qp_conf->mp_session;
> +	}
> +
> +	qp_req_msg.num_fds = 0;
> +	ret = rte_mp_request_sync(&qp_req_msg, &qp_resp, &ts);
> +	if (ret) {
> +		RTE_LOG(ERR, USER1, "Create MR request to primary process failed.");
> +		return -1;
> +	}
> +	qp_resp_msg = &qp_resp.msgs[0];
> +	resp_param = (struct ipsec_mb_mp_param *)qp_resp_msg->param;
> +
> +	return resp_param->result;
> +}
> +
>  /** Release queue pair */
>  int
>  ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
> @@ -100,7 +141,10 @@ ipsec_mb_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 && rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +	if (qp != NULL)
> +		return 0;
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>  		r = rte_ring_lookup(qp->name);
>  		rte_ring_free(r);
> 
> @@ -115,6 +159,9 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>  #endif
>  		rte_free(qp);
>  		dev->data->queue_pairs[qp_id] = NULL;
> +	} else { /* secondary process */
> +		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> +					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
>  	}
>  	return 0;
>  }
> @@ -222,9 +269,13 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
>  #endif
>  		qp = dev->data->queue_pairs[qp_id];
>  		if (qp == NULL) {
> -			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
> -			return -EINVAL;
> +			IPSEC_MB_LOG(DEBUG, "Secondary process setting up device qp.");
> +			return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> +						qp_conf, socket_id,	RTE_IPSEC_MB_MP_REQ_QP_SET);
>  		}
> +
> +		IPSEC_MB_LOG(ERR, "Queue pair already setup'ed.");
> +		return -EINVAL;
>  	} else {
>  		/* Free memory prior to re-allocation if needed. */
>  		if (dev->data->queue_pairs[qp_id] != NULL)
> @@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
>  	return ret;
>  }
> 
> +int
> +ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer)
> +{
> +	struct rte_mp_msg ipc_resp;
> +	struct ipsec_mb_mp_param *resp_param =
> +		(struct ipsec_mb_mp_param *)ipc_resp.param;
> +	const struct ipsec_mb_mp_param *req_param =
> +		(const struct ipsec_mb_mp_param *)mp_msg->param;
> +
> +	int ret;
> +	struct rte_cryptodev *dev;
> +	struct ipsec_mb_qp *qp;
> +	struct rte_cryptodev_qp_conf queue_conf;
> +	int dev_id = req_param->dev_id;
> +	int qp_id = req_param->qp_id;
> +
> +	queue_conf.nb_descriptors = req_param->nb_descriptors;
> +	queue_conf.mp_session = (struct rte_mempool *)req_param->mp_session;
> +	memset(resp_param, 0, sizeof(struct ipsec_mb_mp_param));
> +	memcpy(ipc_resp.name, IPSEC_MB_MP_MSG, sizeof(IPSEC_MB_MP_MSG));
> +
> +	if (!rte_cryptodev_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%d", dev_id);
> +		goto out;
> +	}
> +
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> +	switch (req_param->type) {
> +	case RTE_IPSEC_MB_MP_REQ_QP_SET:
> +		qp = dev->data->queue_pairs[qp_id];
> +		if (qp)	{
> +			CDEV_LOG_DEBUG("qp %d on dev %d is initialised", qp_id, dev_id);
> +			goto out;
> +		}
> +
> +		ret = ipsec_mb_qp_setup(dev, qp_id,	&queue_conf, req_param->socket_id);
> +		if (!ret) {
> +			qp = dev->data->queue_pairs[qp_id];
> +			if (!qp) {
> +				CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
> +					qp_id, dev_id);
> +				goto out;
> +			}
> +			qp->qp_used_by_pid = req_param->process_id;
> +		}
> +		resp_param->result = ret;
> +		break;
> +	case RTE_IPSEC_MB_MP_REQ_QP_FREE:
> +		qp = dev->data->queue_pairs[qp_id];
> +		if (!qp) {
> +			CDEV_LOG_DEBUG("qp %d on dev %d is not initialised",
> +				qp_id, dev_id);
> +			goto out;
> +		}
> +
> +		if (qp->qp_used_by_pid != req_param->process_id) {
> +			CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
> +			goto out;
> +		}
> +
> +		qp->qp_used_by_pid = 0;
> +		resp_param->result = ipsec_mb_qp_release(dev, qp_id);
> +		break;
> +	default:
> +		CDEV_LOG_ERR("invalid mp request type\n");
> +	}
> +
> +out:
> +	ret = rte_mp_reply(&ipc_resp, peer);
> +	return ret;
> +}
> +
>  /** Return the size of the specific pmd session structure */
>  unsigned
>  ipsec_mb_sym_session_get_size(struct rte_cryptodev *dev)
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> index e56579596f..c5540ac8dc 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> @@ -42,6 +42,22 @@ ipsec_mb_enqueue_burst(void *__qp, struct rte_crypto_op **ops,
>  	return nb_enqueued;
>  }
> 
> +static int
> +ipsec_mb_mp_request_register(void)
> +{
> +	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
> +	IPSEC_MB_LOG(INFO, "Starting register MP IPC request\n");
> +	return rte_mp_action_register(IPSEC_MB_MP_MSG,
> +				ipsec_mb_ipc_request);
> +}
> +
> +static void
> +ipsec_mb_mp_request_unregister(void)
> +{
> +	RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
> +	rte_mp_action_unregister(IPSEC_MB_MP_MSG);
> +}
> +
>  int
>  ipsec_mb_create(struct rte_vdev_device *vdev,
>  	enum ipsec_mb_pmd_types pmd_type)
> @@ -152,7 +168,10 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
>  	IPSEC_MB_LOG(INFO, "IPSec Multi-buffer library version used: %s\n",
>  		     imb_get_version_str());
> 
> -	return 0;
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		retval = ipsec_mb_mp_request_register();
> +
> +	return retval;
>  }
> 
>  int
> @@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev)
>  	for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++)
>  		ipsec_mb_qp_release(cryptodev, qp_id);
> 
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		ipsec_mb_mp_request_unregister();
> +
>  	return rte_cryptodev_pmd_destroy(cryptodev);
>  }
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.h b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> index b56eaf061e..ab27214f2c 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.h
> @@ -25,6 +25,9 @@
>  /* Maximum length for memzone name */
>  #define IPSEC_MB_MAX_MZ_NAME 32
> 
> +/* ipsec mb multi-process queue pair config */
> +#define IPSEC_MB_MP_MSG "ipsec_mb_mp_msg"
> +
>  enum ipsec_mb_vector_mode {
>  	IPSEC_MB_NOT_SUPPORTED = 0,
>  	IPSEC_MB_SSE,
> @@ -146,14 +149,45 @@ struct ipsec_mb_qp {
>  	 * slot to be used in temp_digests,
>  	 * to store the digest for a given operation
>  	 */
> +	uint16_t qp_used_by_pid;
> +	/**< The process id used for queue pairs **/
>  	IMB_MGR *mb_mgr;
> -	/* Multi buffer manager */
> +	/**< Multi buffer manager */
>  	const struct rte_memzone *mb_mgr_mz;
> -	/* Shared memzone for storing mb_mgr */
> +	/**< Shared memzone for storing mb_mgr */
>  	__extension__ uint8_t additional_data[];
>  	/**< Storing PMD specific additional data */
>  };
> 
> +/** Request types for IPC. */
> +enum ipsec_mb_mp_req_type {
> +	RTE_IPSEC_MB_MP_REQ_NONE, /**< unknown event type */
> +	RTE_IPSEC_MB_MP_REQ_QP_SET, /**< Queue pair setup request */
> +	RTE_IPSEC_MB_MP_REQ_QP_FREE /**< Queue pair free request */
> +};
> +
> +/** Parameters for IPC. */
> +struct ipsec_mb_mp_param {
> +	enum ipsec_mb_mp_req_type type; /**< IPC request type */
> +	int dev_id;
> +	/**< The identifier of the device */
> +	int qp_id;
> +	/**< The index of the queue pair to be configured */
> +	int socket_id;
> +	/**< Socket to allocate resources on */
> +	uint16_t process_id;
> +	/**< The pid who send out the requested */
> +	uint32_t nb_descriptors;
> +	/**< Number of descriptors per queue pair */
> +	void *mp_session;
> +	/**< The mempool for creating session in sessionless mode */
> +	int result;
> +	/**< The request result for response message */
> +};
> +
> +int
> +ipsec_mb_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer);
> +
>  static __rte_always_inline void *
>  ipsec_mb_get_qp_private_data(struct ipsec_mb_qp *qp)
>  {
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-10-27 17:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20 23:51 [dpdk-dev v1] crypto/ipsec_mb: multi-process IPC request handler Kai Ji
2022-10-24 23:38 ` [dpdk-dev v2] " Kai Ji
2022-10-25 21:48   ` [dpdk-dev v3] " Kai Ji
2022-10-26  8:17     ` De Lara Guarch, Pablo
2022-10-26 10:01     ` [dpdk-dev v4] " Kai Ji
2022-10-26 10:22       ` Kai Ji
2022-10-26 10:27         ` Kai Ji
2022-10-26 11:07           ` Kusztal, ArkadiuszX
2022-10-26 12:32           ` De Lara Guarch, Pablo
2022-10-26 12:48           ` Kai Ji
2022-10-26 13:04             ` De Lara Guarch, Pablo
2022-10-26 13:19             ` [dpdk-dev v5] " Kai Ji
2022-10-27  9:54               ` [EXT] " Akhil Goyal
2022-10-27 17:57               ` Konstantin Ananyev

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).