From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [v3,1/5] eventdev: introduce event crypto adapter Date: Mon, 7 May 2018 18:37:25 +0530 Message-ID: <20180507130724.GA18930@jerin> References: <1525546030-11204-1-git-send-email-abhinandan.gujjar@intel.com> <1525546030-11204-2-git-send-email-abhinandan.gujjar@intel.com> <20180507093516.GA8052@jerin> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Abhinandan Gujjar , hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com, nikhil.rao@intel.com, gage.eads@intel.com To: Akhil Goyal Return-path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0051.outbound.protection.outlook.com [104.47.38.51]) by dpdk.org (Postfix) with ESMTP id BAFB9239 for ; Mon, 7 May 2018 15:07:47 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Mon, 7 May 2018 18:02:04 +0530 > From: Akhil Goyal > To: Jerin Jacob , Abhinandan Gujjar > > CC: hemant.agrawal@nxp.com, akhil.goyal@nxp.com, dev@dpdk.org, > narender.vangati@intel.com, nikhil.rao@intel.com, gage.eads@intel.com > Subject: Re: [dpdk-dev] [v3,1/5] eventdev: introduce event crypto adapter > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 > Thunderbird/45.8.0 > > Hi Jerin, Abhinandan, > Overall the patch looks good. > But one comment on block diagram for OP_NEW mode functioning. > The comment was also made on previous version but it looks the intent was > misunderstood. > > > On 5/7/2018 3:05 PM, Jerin Jacob wrote: > > -----Original Message----- > > > Date: Sun, 6 May 2018 00:17:06 +0530 > > > From: Abhinandan Gujjar > > > To: jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com, > > > akhil.goyal@nxp.com, dev@dpdk.org > > > CC: narender.vangati@intel.com, abhinandan.gujjar@intel.com, > > > nikhil.rao@intel.com, gage.eads@intel.com > > > Subject: [v3,1/5] eventdev: introduce event crypto adapter > > > X-Mailer: git-send-email 1.9.1 > > > > > > Signed-off-by: Abhinandan Gujjar > > > Signed-off-by: Nikhil Rao > > > Signed-off-by: Gage Eads > > > --- > > > MAINTAINERS | 5 + > > > lib/librte_eventdev/rte_event_crypto_adapter.h | 554 +++++++++++++++++++++++++ > > > 2 files changed, 559 insertions(+) > > > create mode 100644 lib/librte_eventdev/rte_event_crypto_adapter.h > > > > > > > Overall it looks good. > > > > #1) > > > > Please fix the following ./devtools/checkpatches.sh warning. > > ➜ [master]laptop [dpdk.org] $ ./devtools/checkpatches.sh > > > > ### eventdev: add crypto adapter implementation > > > > WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier > > tag in line 1 > > #106: FILE: lib/librte_eventdev/rte_event_crypto_adapter.c:1: > > +/* SPDX-License-Identifier: BSD-3-Clause > > > > ### test: add event crypto adapter auto-test > > > > WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier > > tag in line 1 > > #38: FILE: test/test/test_event_crypto_adapter.c:1: > > +/* SPDX-License-Identifier: BSD-3-Clause > > > > total: 0 errors, 1 warnings, 927 lines checked > > > > ### doc: add event crypto adapter documentation > > > > WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier > > tag in line 1 > > #41: FILE: doc/guides/prog_guide/event_crypto_adapter.rst:1: > > +.. SPDX-License-Identifier: BSD-3-Clause > > > > * In the RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD mode, if HW supports > > * RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD capability the > > * application > > * can directly submit the crypto operations to the cryptodev. > > * If not, > > > > > > #2) I have added minor changes in description, Wherever it makes sense > > to you then please pull it for next revision. Else we can discuss more. > > > > a) I have uploaded the diff at https://ufile.io/247t9 for > > you convince. > > b) Please update the similar change in programmers guide too. > > > > > > diff --git a/lib/librte_eventdev/rte_event_crypto_adapter.h b/lib/librte_eventdev/rte_event_crypto_adapter.h > > index 2c1f54f76..55fbdc55e 100644 > > --- a/lib/librte_eventdev/rte_event_crypto_adapter.h > > +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h > > @@ -23,14 +23,17 @@ > > * between the crypto device and the event device. > > * > > * The application can choose to submit a crypto operation directly to > > - * crypto device or send it to the crypto adapter via eventdev, the crypto > > - * adapter then submits the crypto operation to the crypto device. > > - * The first mode is known as the event new (OP_NEW) mode and the > > - * second as the event forward (OP_FORWARD) mode. The choice of mode can > > - * be specified while creating the adapter. > > + * crypto device or send it to the crypto adapter via eventdev based on > > + * RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD capability. > > + * The first mode is known as the event new(RTE_EVENT_CRYPTO_ADAPTER_OP_NEW) > > + * mode and the second as the event forward(RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD) > > + * mode. The choice of mode can be specified while creating the adapter. > > + * In the former mode, it is an application responsibility to enable ingress packet > > + * ordering. In the latter mode, it is the adapter responsibility to enable > > + * the ingress packet ordering. > > * > > * > > - * Working model of OP_NEW mode: > > + * Working model of RTE_EVENT_CRYPTO_ADAPTER_OP_NEW mode: > > * > > * +--------------+ +--------------+ > > * --[1]-->| | | Crypto stage | > > @@ -47,25 +50,27 @@ > > * | | | | > > * +--------------+ +--------------+ > > * > > - * [1] Events from the previous stage. > > + * [1] Events from the previous stage and enqueue to crypto/atomic stage > > * [2] Application in atomic stage dequeues events from eventdev. > > - * [3] Crypto operations are submitted to cryptodev. > > + * [3] Crypto operations are submitted to cryptodev by application. > > * [4] Crypto adapter dequeues crypto completions from cryptodev. > > * [5] Crypto adapter enqueues events to the eventdev. > > * [6] Events to the next stage. > > I think the sequence should be as follows: > [1] Application dequeues from the previous stage. > [2] Application prepare for enqueue to cryptodev > [3] Application enqueues to cryptodev > [4] Crypto adapter dequeues crypto completions from cryptodev. > [5] Crypto adapter enqueues events to the eventdev. > [6] Application dequeues from eventdev and prepare for further processing. > > So the Block diagram should be something like > > + * +--------------+ +--------------+ > + * | | | Crypto stage | > + * | Application |---[2]-->| + enqueue to | > + * | | | cryptodev | > + * +--------------+ +--------------+ > + * ^ ^ | > + * | | [3] > + * [6] [1] | > + * | | | > + * +--------------+ | > + * | | | > + * | Event device | | > + * | | | > + * +--------------+ | > + * ^ | > + * | | > + * [5] | > + * | v > + * +--------------+ +--------------+ > + * | | | | > + * |Crypto adapter|<--[4]---| Cryptodev | > + * | | | | > + * +--------------+ +--------------+ > Please let me know if my understanding is not correct. Looks good to me. /Jerin