From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id EA96D3238 for ; Thu, 3 May 2018 08:03:05 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 May 2018 23:03:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,356,1520924400"; d="scan'208";a="52804426" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga001.jf.intel.com with ESMTP; 02 May 2018 23:03:02 -0700 Received: from pgsmsx102.gar.corp.intel.com ([169.254.6.245]) by PGSMSX104.gar.corp.intel.com ([169.254.3.13]) with mapi id 14.03.0319.002; Thu, 3 May 2018 14:03:01 +0800 From: "Gujjar, Abhinandan S" To: Jerin Jacob CC: "hemant.agrawal@nxp.com" , "akhil.goyal@nxp.com" , "dev@dpdk.org" , "Vangati, Narender" , "Rao, Nikhil" , "Eads, Gage" Thread-Topic: [v2,1/6] eventdev: introduce event crypto adapter Thread-Index: AQHT28nS3+7/tBq/xEKnOqyz+ojb3KQXavmAgAYebAA= Date: Thu, 3 May 2018 06:03:01 +0000 Message-ID: <5612CB344B05EE4F95FC5B729939F78070701D35@PGSMSX102.gar.corp.intel.com> References: <1524573807-168522-1-git-send-email-abhinandan.gujjar@intel.com> <1524573807-168522-2-git-send-email-abhinandan.gujjar@intel.com> <20180429160835.GA11546@jerin> In-Reply-To: <20180429160835.GA11546@jerin> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjdlNjc5Y2EtNjE4OS00MjQ3LWExYmUtNjEwMjc0Nzg2YjJmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImN5WVwvQ3o2eTYzZE5Ialk0a2gzK2VOS2g3eEhYM0ZwTEl4akZvNk1vNGprPSJ9 dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [172.30.20.205] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [v2,1/6] eventdev: introduce event crypto adapter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 May 2018 06:03:06 -0000 > -----Original Message----- > From: Jerin Jacob > Sent: Sunday, April 29, 2018 9:39 PM > To: Gujjar, Abhinandan S > Cc: hemant.agrawal@nxp.com; akhil.goyal@nxp.com; dev@dpdk.org; Vangati, > Narender ; Rao, Nikhil = ; > Eads, Gage > Subject: Re: [v2,1/6] eventdev: introduce event crypto adapter >=20 > -----Original Message----- > > Date: Tue, 24 Apr 2018 18:13:22 +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: [v2,1/6] 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 > > --- > > lib/librte_eventdev/rte_event_crypto_adapter.h | 532 > > +++++++++++++++++++++++++ > > 1 file changed, 532 insertions(+) > > create mode 100644 lib/librte_eventdev/rte_event_crypto_adapter.h > > > > diff --git a/lib/librte_eventdev/rte_event_crypto_adapter.h > > b/lib/librte_eventdev/rte_event_crypto_adapter.h > > new file mode 100644 > > index 0000000..aa4f32c > > --- /dev/null > > +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h > > @@ -0,0 +1,532 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2017-2018 Intel Corporation */ > > + > > +#ifndef _RTE_EVENT_CRYPTO_ADAPTER_ > > +#define _RTE_EVENT_CRYPTO_ADAPTER_ >=20 > 1) Please rebase to next-eventdev tree, If everything goes well, we will = try to > add it in RC2. Ok >=20 > 2) Please update MAINTAINERS section in this patch set with just one > rte_event_crypto_adapter.h file and as when you add new files update the = new > files in the MAINTAINERS file Ok >=20 > > + > > +/** > > + * @file > > + * > > + * RTE Event crypto adapter > > + * > > + * Eventdev library provides couple of adapters to bridge between > > +various > > + * components for providing new event source. The event crypto > > +adapter is > > + * one of those adapter which is intended to bridge between event > > +devices > > + * and crypto devices. > > + * > > + * The crypto adapter adds support to enqueue/dequeue crypto > > +operations to/ > > + * from event device. The packet flow between crypto device and the > > +event > > + * device can be accomplished using both SW and HW based transfer > mechanisms. > > + * The adapter uses an EAL service core function for SW based packet > > +transfer > > + * and uses the eventdev PMD functions to configure HW based packet > > +transfer > > + * 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 dequeue only (DEQ_ONLY) mode and > > +the > > + * second as the enqueue - dequeue (ENQ_DEQ) mode. The choice of mode > > +can > > + * be specified while creating the adapter. >=20 > We need to tell why these modes are required or use of it/when to use wha= t > like, OPS_NEW does not maintain ingress order. example, use of DEQ_ONLY > with FWD or ENQ_DEQ mode to use ingress order maintenance. Sure. I will add some more information for clarity. >=20 >=20 > > + * > > + * > > + * Working model of DEQ_ONLY mode: > > + * =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D >=20 > This diagram is not rendering correctly in doxygen html file. > check lib/librte_eventdev/rte_eventdev.h as reference. >=20 > invoke "make doc-api-html" to see the html output. >=20 >=20 > > + * > > + * +--------------+ +--------------+ > > + * Events | | | Crypto stage | > > + * <------>| Event device |-------->| + enqueue to | > > + * | | | cryptodev | > > + * +--------------+ +--------------+ > > + * event ^ | > > + * enqueue| | crypto > > + * | v enqueue > > + * +--------------+ +--------------+ > > + * | | | | > > + * |Crypto adapter|<--------| Cryptodev | > > + * | | | | > > + * +--------------+ +--------------+ > > + * >=20 > I think, we need to add sequence of numbers scheme to define the data flo= w for > this diagram as it is complex. >=20 > something like, ------[1]----> and describe the action means. Sure. I will add sequence number to the flow. Akhil also have some concern = on this. With the sequence number, it be will be more clear. >=20 > > + * In the DEQ_ONLY mode, application submits crypto operations > > + directly to > > + * crypto device. The adapter then dequeues crypto completions from > > + crypto > > + * device and enqueue events to the event device. > > + * In this mode, application needs to specify event information > > + (response > > + * information) which is needed to enqueue an event after the crypto > > + operation > > + * is completed. > > + * > > + * > > + * Working model of ENQ_DEQ mode: > > + * =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D > > + * > > + * +--------------+ +--------------+ > > + * Events | | | | > > + * <------>| Event device |-------->| Crypto stage | > > + * | | | | > > + * +--------------+ +--------------+ >=20 > I think, instead of "crypto stage", it is better to call it as "atomic s= tage" > something like that where the source queue's(ORDERED mode) ingress order = will > be updated in that stage and then enqueue to cryptodev happens for ingres= s > order maintenance. Ok. >=20 > > + * event ^ | > > + * enqueue| | event > > + * | v dequeue > > + * +---------------------------------------+ > > + * | | > > + * | Crypto adapter | > > + * | | > > + * +---------------------------------------+ > > + * ^ > > + * | crypto > > + * | enq/deq > > + * v > > + * +-------------+ > > + * | | > > + * | Cryptodev | > > + * | | > > + * +-------------+ >=20 > Same as above comments for the diagram(i.e adding sequence number) Ok >=20 > > + * > > + * In the ENQ_DEQ mode, application sends crypto operations as events > > + to > > + * the adapter which dequeues events and perform cryptodev operations. > ^^^^^^^ > Not to the adapter, right?. applications sends to the eventdev through po= rts > available through rte_event_crypto_adapter_event_port_get() eventdev port= . > Right? >=20 > Please reword if it makes sense. It is to the adapter through eventdev. May be elaborating little more something like, application gets crypto adap= ter's event port by rte_event_crypto_adapter_event_port_get() API. Application links it's event queue to this event port and starts enqueuing = crypto operations as events. Adapter dequeue these events and submit the crypto op= erations to the cryptodev. Does this make sense? >=20 > > + * The adapter dequeues crypto completions from cryptodev and enqueue > > + * events to the event device. > > + * In this mode, the application needs to specify the cryptodev ID > > + * and queue pair ID (request information) needed to enqueue a crypto > > + * operation in addition to the event information (response > > + information) > > + * needed to enqueue an event after the crypto operation has completed= . > > + * > > + * > > + * The event crypto adapter provides common APIs to configure the > > + packet flow > > + * from the crypto device to event devices for both SW and HW based > transfers. > > + * The crypto event adapter's functions are: > > + * - rte_event_crypto_adapter_create_ext() > > + * - rte_event_crypto_adapter_create() > > + * - rte_event_crypto_adapter_free() > > + * - rte_event_crypto_adapter_queue_pair_add() > > + * - rte_event_crypto_adapter_queue_pair_del() > > + * - rte_event_crypto_adapter_start() > > + * - rte_event_crypto_adapter_stop() > > + * - rte_event_crypto_adapter_stats_get() > > + * - rte_event_crypto_adapter_stats_reset() > > + > > + * The applicaton creates an instance using > > + rte_event_crypto_adapter_create() >=20 > s/application/application >=20 > > + * or rte_event_crypto_adapter_create_ext(). > > + * > > + * Cryptodev queue pair addition/deletion is done using the > > + * rte_event_crypto_adapter_queue_pair_xxx() APIs. >=20 > I think, we can mention the connection of > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND capability > here. Ok >=20 > > + * > > + * The SW adapter or HW PMD uses rte_crypto_op::sess_type to decide > > + whether > > + * request/response(private) data is located in the crypto/security > > + session > > + * or at an offset in the rte_crypto_op. > > + * The rte_crypto_op::private_data_offset provides an offset to > > + locate the > > + * request/response information in the rte_crypto_op. > > + * > > + * For session-based operations, the set and get API provides a > > + mechanism for > > + * an application to store and retrieve the data information stored > > + * along with the crypto session. >=20 > I think, we can mention the connection of > RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA capability here. Ok >=20 > > + > > + * For session-less mode, the adapter gets the private data > > + information placed > > + * along with the ``struct rte_crypto_op``. > > + * The ``rte_crypto_op::private_data_offset`` indicates the start of > > + private > > + * data information. The offset is counted from the start of the > > + rte_crypto_op > > + * including initialization vector (IV). > > + */ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include > > + > > +#include "rte_eventdev.h" > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this enum may change without prior notice > > + * > > + * Crypto event adapter mode > > + */ > > +enum rte_event_crypto_adapter_mode { > > + RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY =3D 1, >=20 > Why to mark it as explicit '1' ? Nothing specific. Have 0 & 1? >=20 >=20 > > + /**< Start only dequeue part of crypto adapter. > > + * Application submits crypto requests to the cryptodev. > > + * Adapter only dequeues the crypto completions from cryptodev > > + * and enqueue events to the eventdev. >=20 > I think, you can mention the connection with below capabilities here. > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_NEW > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD > and @see of those here. This enum can be renamed to OP_NEW & OP_FWD. So, the adapter will be configured with OP_NEW/OP_FWD mode. >=20 >=20 > > + */ > > + RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ, > > + /**< Start both enqueue & dequeue part of crypto adapter. > > + * Application submits crypto requests as events to the crypto > > + * adapter. Adapter submits crypto requests to the cryptodev > > + * and crypto completions are enqueued back to the eventdev. >=20 > IMO, You can add note when this mode will be used by application if devic= e is > not capable of RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD > and application need ingress order maintenance or something like that. Ok >=20 > > + */ > > +}; > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * Crypto event request structure will be filled by application to > > + * provide event request information to the adapter. > > + */ > > +struct rte_event_crypto_request { > > + uint8_t resv[8]; > > + /**< Overlaps with first 8 bytes of struct rte_event > > + * that encode the response event information >=20 > May be we could add more comments on application usage on updating struct > rte_event based field updating for response event information. Thought, struct name itself is self-explanatory. It will try to add some mo= re comments. >=20 > > + */ > > + uint16_t cdev_id; > > + /**< cryptodev ID to be used */ > > + uint16_t queue_pair_id; > > + /**< cryptodev queue pair ID to be used */ > > + uint32_t resv1; >=20 > How about rsvd ? No strong opinion though. resv is used as array. So resv1. >=20 > I think, you can add, Valid when it adapter is in ENQ_DEQ mode Ok >=20 > > + /**< Reserved bits */ > > +}; > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * Crypto event metadata structure will be filled by application > > + * to provide crypto request and event response information. > > + * > > + * If crypto events are enqueued using a HW mechanism, the cryptodev > > + * PMD will use the event response information to set up the event > > + * that is enqueued back to eventdev after completion of the crypto > > + * operation. If the transfer is done by SW, event response > > +information > > + * will be used by the adapter. > > + */ > > +union rte_event_crypto_metadata { > > + struct rte_event_crypto_request request_info; >=20 > I think, you can add, Provided by application for ENQ_DEQ mode Ok >=20 > > + struct rte_event response_info; >=20 > I think, you can add, Provided by application for ENQ_DEQ and DEQ_ONLY > mode. Ok >=20 > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * Queue pair configuration structure containing event information. > > + * @see > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND > > + */ > > +struct rte_event_crypto_queue_pair_conf { > > + struct rte_event ev; >=20 > MO, As Akhil said, we can pass struct rte_event directly. Yes. I will remove this in next patch. >=20 > > +}; > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Create a new event crypto adapter with the specified identifier. > > + * This function uses an internal configuration function that creates > > +an event > > + * port. This default function reconfigures the event device with an > > + * additional event port and setups up the event port using the > > +port_config > > + * parameter passed into this function. In case the application needs > > +more > > + * control in configuration of the service, it should use the > > + * rte_event_crypto_adapter_create_ext() version. > > + * > > + * @param id > > + * Adapter identifier. > > + * > > + * @param dev_id > > + * Event device identifier. > > + * > > + * @param port_config > > + * Argument of type *rte_event_port_conf* that is passed to the > > +conf_cb > > + * function. > > + * > > + * @param mode > > + * Flag to indicate to start dequeue only or both enqueue & dequeue. > > + * > > + * @return > > + * - 0: Success > > + * - <0: Error code on failure > > + */ > > +int __rte_experimental > > +rte_event_crypto_adapter_create(uint8_t id, uint8_t dev_id, > > + struct rte_event_port_conf *port_config, > > + enum rte_event_crypto_adapter_mode mode); >=20 > - Detecting what to pass (RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY or > RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ) > in application need a lot checks based on capabilities, instead, How abou= t > passing whats been desired by the application by adding a new > enum(RTE_EVENT_CRYPTO_ADAPTER_MODE_OP_NEW or > RTE_EVENT_CRYPTO_ADAPTER_MODE_OP_FWD). > and internally based on mode selected and the device capability, the comm= on > code can choose RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY in NEW or FWD > mode vs RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ. I will rename the mode to OP_NEW and OP_FWD. >=20 > - We could add common code based API to return the exiting configured mod= e. >=20 > enum rte_event_crypto_adapter_mode mode __rte_experimental > rte_event_crypto_adapter_mode_get(uint8_t id, uint8_t dev_id); Since the existing enum is renamed, this API will not be useful. >=20 > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Free an event crypto adapter > > + * > > + * @param id > > + * Adapter identifier. > > + * > > + * @return > > + * - 0: Success > > + * - <0: Error code on failure, If the adapter still has queue pairs > > + * added to it, the function returns -EBUSY. > > + */ > > +int __rte_experimental > > +rte_event_crypto_adapter_free(uint8_t id); > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Add a queue pair to an event crypto adapter. > > + * > > + * @param id > > + * Adapter identifier. > > + * > > + * @param cdev_id > > + * Cryptodev identifier. > > + * > > + * @param queue_pair_id > > + * Cryptodev queue pair identifier. If queue_pair_id is set -1, > > + * adapter adds all the pre configured queue pairs to the instance. > > + * > > + * @param conf > > + * Additional configuration structure of type > > + * *rte_event_crypto_queue_pair_conf* >=20 > @see RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND Ok