From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54E93A00C5; Thu, 27 Oct 2022 19:57:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 13EDD41153; Thu, 27 Oct 2022 19:57:23 +0200 (CEST) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id AEA0740146 for ; Thu, 27 Oct 2022 19:57:21 +0200 (CEST) Received: from fraeml709-chm.china.huawei.com (unknown [172.18.147.206]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4MytZs6Qtdz67tWW; Fri, 28 Oct 2022 01:53:41 +0800 (CST) Received: from fraeml712-chm.china.huawei.com (10.206.15.61) by fraeml709-chm.china.huawei.com (10.206.15.37) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Thu, 27 Oct 2022 19:57:19 +0200 Received: from fraeml712-chm.china.huawei.com ([10.206.15.61]) by fraeml712-chm.china.huawei.com ([10.206.15.61]) with mapi id 15.01.2375.031; Thu, 27 Oct 2022 19:57:19 +0200 From: Konstantin Ananyev To: Kai Ji , "dev@dpdk.org" CC: "gakhil@marvell.com" , Pablo de Lara , Anatoly Burakov Subject: RE: [dpdk-dev v5] crypto/ipsec_mb: multi-process IPC request handler Thread-Topic: [dpdk-dev v5] crypto/ipsec_mb: multi-process IPC request handler Thread-Index: AQHY6T2ReNdFhkDEsEqflprU+EkQCq4iiAow Date: Thu, 27 Oct 2022 17:57:19 +0000 Message-ID: References: <20221026124854.28791-1-kai.ji@intel.com> <20221026131902.31884-1-kai.ji@intel.com> In-Reply-To: <20221026131902.31884-1-kai.ji@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.206.138.42] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org >=20 > 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:=20 How we suppose to guarantee synchronization with such approach? Let say secondary process sends an IPC message to configure=20 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? >=20 > Signed-off-by: Kai Ji > Acked-by: Arek Kusztal > Acked-by: Pablo de Lara > --- > v5: > - minor updates and typo fix >=20 > v4: > - review comments resolved >=20 > v3: > - remove shared memzone as qp_conf params can be passed directly from > ipc message. >=20 > 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(-) >=20 > diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipse= c_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 @@ > */ >=20 > #include > +#include >=20 > #include > #include > @@ -93,6 +94,46 @@ ipsec_mb_info_get(struct rte_cryptodev *dev, > } > } >=20 > +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 =3D {.tv_sec =3D 1, .tv_nsec =3D 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 =3D (struct ipsec_mb_mp_param *)&qp_req_msg.param; > + > + qp_req_msg.len_param =3D sizeof(struct ipsec_mb_mp_param); > + req_param->type =3D op_type; > + req_param->dev_id =3D dev_id; > + req_param->qp_id =3D qp_id; > + req_param->socket_id =3D socket_id; > + req_param->process_id =3D getpid(); > + if (qp_conf) { > + req_param->nb_descriptors =3D qp_conf->nb_descriptors; > + req_param->mp_session =3D (void *)qp_conf->mp_session; > + } > + > + qp_req_msg.num_fds =3D 0; > + ret =3D 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 =3D &qp_resp.msgs[0]; > + resp_param =3D (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, uint1= 6_t qp_id) > struct ipsec_mb_qp *qp =3D dev->data->queue_pairs[qp_id]; > struct rte_ring *r =3D NULL; >=20 > - if (qp !=3D NULL && rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) { > + if (qp !=3D NULL) > + return 0; > + > + if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) { > r =3D rte_ring_lookup(qp->name); > rte_ring_free(r); >=20 > @@ -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] =3D 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 =3D dev->data->queue_pairs[qp_id]; > if (qp =3D=3D 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] !=3D NULL) > @@ -296,6 +347,78 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_= t qp_id, > return ret; > } >=20 > +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 =3D > + (struct ipsec_mb_mp_param *)ipc_resp.param; > + const struct ipsec_mb_mp_param *req_param =3D > + (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 =3D req_param->dev_id; > + int qp_id =3D req_param->qp_id; > + > + queue_conf.nb_descriptors =3D req_param->nb_descriptors; > + queue_conf.mp_session =3D (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=3D%d", dev_id); > + goto out; > + } > + > + dev =3D rte_cryptodev_pmd_get_dev(dev_id); > + switch (req_param->type) { > + case RTE_IPSEC_MB_MP_REQ_QP_SET: > + qp =3D 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 =3D ipsec_mb_qp_setup(dev, qp_id, &queue_conf, req_param->socket_i= d); > + if (!ret) { > + qp =3D 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 =3D req_param->process_id; > + } > + resp_param->result =3D ret; > + break; > + case RTE_IPSEC_MB_MP_REQ_QP_FREE: > + qp =3D 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 !=3D req_param->process_id) { > + CDEV_LOG_ERR("Unable to release qp_id=3D%d", qp_id); > + goto out; > + } > + > + qp->qp_used_by_pid =3D 0; > + resp_param->result =3D ipsec_mb_qp_release(dev, qp_id); > + break; > + default: > + CDEV_LOG_ERR("invalid mp request type\n"); > + } > + > +out: > + ret =3D 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_o= p **ops, > return nb_enqueued; > } >=20 > +static int > +ipsec_mb_mp_request_register(void) > +{ > + RTE_ASSERT(rte_eal_process_type() =3D=3D 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() =3D=3D 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()); >=20 > - return 0; > + if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) > + retval =3D ipsec_mb_mp_request_register(); > + > + return retval; > } >=20 > int > @@ -187,5 +206,8 @@ ipsec_mb_remove(struct rte_vdev_device *vdev) > for (qp_id =3D 0; qp_id < cryptodev->data->nb_queue_pairs; qp_id++) > ipsec_mb_qp_release(cryptodev, qp_id); >=20 > + if (rte_eal_process_type() =3D=3D 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 >=20 > +/* 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 =3D 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 */ > }; >=20 > +/** 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