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 0E4A4A00C2; Fri, 7 Oct 2022 00:43:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BA925400D6; Fri, 7 Oct 2022 00:42:59 +0200 (CEST) Received: from forward501j.mail.yandex.net (forward501j.mail.yandex.net [5.45.198.251]) by mails.dpdk.org (Postfix) with ESMTP id ADF9540042 for ; Fri, 7 Oct 2022 00:42:58 +0200 (CEST) Received: from myt5-cceafa914410.qloud-c.yandex.net (myt5-cceafa914410.qloud-c.yandex.net [IPv6:2a02:6b8:c12:3b23:0:640:ccea:fa91]) by forward501j.mail.yandex.net (Yandex) with ESMTP id 135B9623287; Fri, 7 Oct 2022 01:41:27 +0300 (MSK) Received: by myt5-cceafa914410.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id cmRfBg2BrQ-fPhKKV2O; Fri, 07 Oct 2022 01:41:26 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1665096086; bh=r/Qf5CGwOym+xtKoWcUmhMWzesHNNGdxiXFD9RIQKDo=; h=From:In-Reply-To:Cc:Date:References:To:Subject:Message-ID; b=cUtN9j/NWVuiwlF/cvRgrVdL1BB6jNx5X0DxGANAmiP33AaeGGd0mgOagGjSuE9j7 wwAxDgDyTRtKqeW6S+nufHEn2qJK1uVJaNgOHv+qHOwR5LZ04qGTz+C+P/bVFwSFuM Qv9EbvUXS5VeMuwRhDUOoJDrisRpBDNc3/l7cfZU= Authentication-Results: myt5-cceafa914410.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Message-ID: Date: Thu, 6 Oct 2022 23:41:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [dpdk-dev v5] lib/cryptodev: multi-process IPC request handler Content-Language: en-US To: Kai Ji , dev@dpdk.org Cc: Akhil Goyal , Fan Zhang , Ray Kinsella , Anatoly Burakov References: <20221006081646.81901-1-kai.ji@intel.com> <20221006170612.94392-1-kai.ji@intel.com> From: Konstantin Ananyev In-Reply-To: <20221006170612.94392-1-kai.ji@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 06/10/2022 18:06, Kai Ji пишет: > As some cryptode PMDs have multiprocess support, the secondary > process needs queue-pair to be configured by the primary process before > to use. This patch adds an IPC register function to help the primary > process to register IPC action that allow secondary process to configure > cryptodev queue-pair via IPC messages during the runtime. > After setup, 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 > free request is allowed in the future. A stupid question - how syncronisation is supposed to be guaranteed 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) desicdes to reconfigure same crypto-dev. Are there any way to prevent such race-conditions to happen? > > Signed-off-by: Kai Ji > Acked-by: Ciara Power > --- > v5: > - fix of unittest error > - cryptodev prog_guide updates > > v4: > - release note update > - Doxygen comments update > > v3: > - addin missing free function for qp_in_use_by_pid > > v2: > - code rework > --- > doc/guides/prog_guide/cryptodev_lib.rst | 21 ++++++ > doc/guides/rel_notes/release_22_11.rst | 8 ++- > lib/cryptodev/cryptodev_pmd.h | 3 +- > lib/cryptodev/rte_cryptodev.c | 93 +++++++++++++++++++++++++ > lib/cryptodev/rte_cryptodev.h | 46 ++++++++++++ > lib/cryptodev/version.map | 2 + > 6 files changed, 171 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst > index 01aad842a9..19bcda24bc 100644 > --- a/doc/guides/prog_guide/cryptodev_lib.rst > +++ b/doc/guides/prog_guide/cryptodev_lib.rst > @@ -1180,6 +1180,27 @@ Asymmetric Crypto Device API > The cryptodev Library API is described in the > `DPDK API Reference `_ > > +Multi-process IPC request register support > +------------------------------------------ > +As some cryptode PMDs have multiprocess support, the queue-pairs used in the > +secondary process are required to be configured by the primary process first. > +The cryptodev IPC request handler provide an alternative way to setup > +queue-pair from the secondary process via IPC messages in the runtime. > + > +The ``rte_cryptodev_mp_request_register`` function, called in primary process, > +register the ``ret_cryptodev_ipc_request`` function for IPC message request > +handling between primary and secondary process. > +The supported IPC request types are: > + > +.. code-block:: c > + RTE_CRYPTODEV_MP_REQ_QP_SET > + RTE_CRYPTODEV_MP_REQ_QP_FREE > + > +It up to the secondary process to fill-in required params in order to allow > +``rte_cryptodev_queue_pair_setup`` setup a queue-pair correctly. > +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 free request is allowed > +in the future. > > Device Statistics > ----------------- > diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst > index 4e64710a69..d60756318b 100644 > --- a/doc/guides/rel_notes/release_22_11.rst > +++ b/doc/guides/rel_notes/release_22_11.rst > @@ -87,6 +87,13 @@ New Features > Added MACsec transform for rte_security session and added new API > to configure security associations (SA) and secure channels (SC). > > +* **Added MP IPC request register function in cryptodev.** > + > + Added new functions ``rte_cryptodev_mp_request_register()`` and > + ``rte_cryptodev_mp_request_unregister()``. > + The function helps the primary process to register IPC action that allow > + secondary process to request cryptodev queue pairs setups via IPC messages. > + > * **Added new algorithms to cryptodev.** > > * Added symmetric hash algorithm ShangMi 3 (SM3). > @@ -123,7 +130,6 @@ New Features > into single event containing ``rte_event_vector`` > whose event type is ``RTE_EVENT_TYPE_CRYPTODEV_VECTOR``. > > - > Removed Items > ------------- > > diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h > index f27b3249ea..574aebe279 100644 > --- a/lib/cryptodev/cryptodev_pmd.h > +++ b/lib/cryptodev/cryptodev_pmd.h > @@ -78,7 +78,8 @@ struct rte_cryptodev_data { > void **queue_pairs; > /** Number of device queue pairs. */ > uint16_t nb_queue_pairs; > - > + /** Array of process id used for queue pairs **/ > + uint16_t *qp_in_use_by_pid; > /** PMD-specific private data */ > void *dev_private; > } __rte_cache_aligned; > diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c > index 2165a0688c..4c60aed481 100644 > --- a/lib/cryptodev/rte_cryptodev.c > +++ b/lib/cryptodev/rte_cryptodev.c > @@ -49,6 +49,9 @@ struct rte_crypto_fp_ops rte_crypto_fp_ops[RTE_CRYPTO_MAX_DEVS]; > /* spinlock for crypto device callbacks */ > static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER; > > +/* crypto queue pair config */ > +#define CRYPTODEV_MP_REQ "cryptodev_mp_request" > + > /** > * The user application callback description. > * > @@ -1047,6 +1050,9 @@ rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev) > return ret; > } > > + if (cryptodev->data->qp_in_use_by_pid) > + rte_free(cryptodev->data->qp_in_use_by_pid); > + > ret = rte_cryptodev_data_free(dev_id, &cryptodev_globals.data[dev_id]); > if (ret < 0) > return ret; > @@ -1135,6 +1141,21 @@ rte_cryptodev_queue_pairs_config(struct rte_cryptodev *dev, uint16_t nb_qpairs, > > } > dev->data->nb_queue_pairs = nb_qpairs; > + > + if (dev->data->qp_in_use_by_pid == NULL) { > + dev->data->qp_in_use_by_pid = rte_zmalloc_socket( > + "cryptodev->qp_in_use_by_pid", > + sizeof(dev->data->qp_in_use_by_pid[0]) * > + dev_info.max_nb_queue_pairs, > + RTE_CACHE_LINE_SIZE, socket_id); > + if (dev->data->qp_in_use_by_pid == NULL) { > + CDEV_LOG_ERR("failed to get memory for qp meta data, " > + "nb_queues %u", > + nb_qpairs); > + return -(ENOMEM); > + } > + } > + > return 0; > } > > @@ -1401,6 +1422,78 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id, > socket_id); > } > > +static int > +rte_cryptodev_ipc_request(const struct rte_mp_msg *mp_msg, const void *peer) > +{ > + struct rte_mp_msg mp_res; > + struct rte_cryptodev_mp_param *resp_param = > + (struct rte_cryptodev_mp_param *)mp_res.param; > + const struct rte_cryptodev_mp_param *req_param = > + (const struct rte_cryptodev_mp_param *)mp_msg->param; > + > + int ret; > + struct rte_cryptodev *dev; > + uint16_t *qp_in_used_by_pid; > + int dev_id = req_param->dev_id; > + int qp_id = req_param->qp_id; > + struct rte_cryptodev_qp_conf *queue_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; > + } > + > + if (!rte_cryptodev_get_qp_status(dev_id, qp_id)) > + goto out; > + > + dev = &rte_crypto_devices[dev_id]; > + qp_in_used_by_pid = dev->data->qp_in_use_by_pid; > + > + switch (req_param->type) { > + case RTE_CRYPTODEV_MP_REQ_QP_SET: > + ret = rte_cryptodev_queue_pair_setup(dev_id, qp_id, > + queue_conf, req_param->socket_id); > + if (!ret) > + qp_in_used_by_pid[qp_id] = req_param->process_id; > + resp_param->result = ret; > + break; > + case RTE_CRYPTODEV_MP_REQ_QP_FREE: > + if (qp_in_used_by_pid[qp_id] != req_param->process_id) { > + CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id); > + goto out; > + } > + > + ret = (*dev->dev_ops->queue_pair_release)(dev, qp_id); > + if (!ret) > + qp_in_used_by_pid[qp_id] = 0; > + > + resp_param->result = ret; > + break; > + default: > + CDEV_LOG_ERR("invalid mp request type\n"); > + } > + > +out: > + ret = rte_mp_reply(&mp_res, peer); > + return ret; > +} > + > +int > +rte_cryptodev_mp_request_register(void) > +{ > + RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY); > + return rte_mp_action_register(CRYPTODEV_MP_REQ, > + rte_cryptodev_ipc_request); > +} > + > +void > +rte_cryptodev_mp_request_unregister(void) > +{ > + RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY); > + rte_mp_action_unregister(CRYPTODEV_MP_REQ); > +} > + > struct rte_cryptodev_cb * > rte_cryptodev_add_enq_callback(uint8_t dev_id, > uint16_t qp_id, > diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h > index ece7157970..80076f487c 100644 > --- a/lib/cryptodev/rte_cryptodev.h > +++ b/lib/cryptodev/rte_cryptodev.h > @@ -539,6 +539,30 @@ enum rte_cryptodev_event_type { > RTE_CRYPTODEV_EVENT_MAX /**< max value of this enum */ > }; > > +/** Request types for IPC. */ > +enum rte_cryptodev_mp_req_type { > + RTE_CRYPTODEV_MP_REQ_NONE, /**< unknown event type */ > + RTE_CRYPTODEV_MP_REQ_QP_SET, /**< Queue pair setup request */ > + RTE_CRYPTODEV_MP_REQ_QP_FREE /**< Queue pair free request */ > +}; > + > +/** Parameters for IPC. */ > +struct rte_cryptodev_mp_param { > + enum rte_cryptodev_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 */ > + struct rte_cryptodev_qp_conf *queue_conf; > + /**< A pointer of Crypto device queue pair configuration structure */ > + int result; > + /**< The request result for response message */ > +}; > + > /** Crypto device queue pair configuration structure. */ > struct rte_cryptodev_qp_conf { > uint32_t nb_descriptors; /**< Number of descriptors per queue pair */ > @@ -767,6 +791,28 @@ extern int > rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id, > const struct rte_cryptodev_qp_conf *qp_conf, int socket_id); > > +/** > + * Register multi process request IPC handler > + * > + * Allow secondary process to send IPC request to setup queue pairs > + * once register function called in primary process. > + * > + * @return > + * - 0: Success registered > + * - 1: Failed registration failed > + * - EINVAL: device was not configured > + */ > +__rte_experimental > +int > +rte_cryptodev_mp_request_register(void); > + > +/** > + * Unregister multi process unrequest IPC handler > + */ > +__rte_experimental > +void > +rte_cryptodev_mp_request_unregister(void); > + > /** > * Get the status of queue pairs setup on a specific crypto device > * > diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map > index 00c99fb45c..e964a3d5ab 100644 > --- a/lib/cryptodev/version.map > +++ b/lib/cryptodev/version.map > @@ -150,6 +150,8 @@ EXPERIMENTAL { > __rte_cryptodev_trace_sym_session_get_user_data; > __rte_cryptodev_trace_sym_session_set_user_data; > __rte_cryptodev_trace_count; > + rte_cryptodev_mp_request_register; > + rte_cryptodev_mp_request_unregister; > }; > > INTERNAL { > -- > 2.17.1 >