Hi Akhil,
> -----Original Message----- > From: Akhil Goyal <akhil.go...@nxp.com> > Sent: Thursday, April 26, 2018 12:47 PM > To: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; Akhil Goyal > <akhil.go...@nxp.com>; jerin.ja...@caviumnetworks.com; > hemant.agra...@nxp.com; dev@dpdk.org > Cc: Vangati, Narender <narender.vang...@intel.com>; Rao, Nikhil > <nikhil....@intel.com>; Eads, Gage <gage.e...@intel.com> > Subject: Re: [dpdk-dev] [v2,1/6] eventdev: introduce event crypto adapter > > Hi Abhinandan, > On 4/26/2018 11:37 AM, Gujjar, Abhinandan S wrote: > > Hi Akhil, > > > >> -----Original Message----- > >> From: Akhil Goyal [mailto:akhil.go...@nxp.com] > >> Sent: Wednesday, April 25, 2018 6:12 PM > >> To: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; > >> jerin.ja...@caviumnetworks.com; hemant.agra...@nxp.com; > >> akhil.go...@nxp.com; dev@dpdk.org > >> Cc: Vangati, Narender <narender.vang...@intel.com>; Rao, Nikhil > >> <nikhil....@intel.com>; Eads, Gage <gage.e...@intel.com> > >> Subject: Re: [dpdk-dev] [v2,1/6] eventdev: introduce event crypto > >> adapter > >> > >> Hi Abhinandan, > >> On 4/24/2018 6:13 PM, Abhinandan Gujjar wrote: > >>> Signed-off-by: Abhinandan Gujjar <abhinandan.guj...@intel.com> > >>> Signed-off-by: Nikhil Rao <nikhil....@intel.com> > >>> Signed-off-by: Gage Eads <gage.e...@intel.com> > >>> --- > >>> 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_ > >>> + > >>> +/** > >>> + * @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. > >>> + * > >>> + * > >>> + * Working model of DEQ_ONLY mode: > >>> + * =============================== > >>> + * > >>> + * +--------------+ +--------------+ > >>> + * Events | | | Crypto stage | > >>> + * <------>| Event device |-------->| + enqueue to | > >>> + * | | | cryptodev | > >>> + * +--------------+ +--------------+ > >>> + * event ^ | > >>> + * enqueue| | crypto > >>> + * | v enqueue > >>> + * +--------------+ +--------------+ > >>> + * | | | | > >>> + * |Crypto adapter|<--------| Cryptodev | > >>> + * | | | | > >>> + * +--------------+ +--------------+ > >>> + * > >> The diagram looks a bit cryptic. Enqueue to cryptodev will be done > >> from application and not from event device. The arrow from event > >> device to crypto stage is not clear. And application dequeues events > >> from event device. So that should not be bidirectional arrow. > > > > You are right, it is done from application. But the application will > > be in the crypto stage of pipeline. Since crypto is an intermediate > > stage, events are first dequeued from eventdev to find out it's a crypto > > stage. > Then application, in the crypto stage, enqueue "rte_crypto_ops" > > to cryptodev and finally adapter enqueue crypto completions as events to > eventdev. > > From this point, eventdev will pass events to the next stage (may be Tx). > > That's the reason behind bidirectional arrow to event device. > > > You are talking about a particular application, but the comments should be > generic. In DEQ ONLY mode, enqueue to cryptodev is responsibility of > application and should not be from event dev. Actually the application will > dequeue from event dev and decide that this event comes from NIC (say), and it > needs to be processed by cryptodev next. So in this case the application > decides > what will happen to the packet and not the event dev, so it cannot be bi- > directional arrow. I will update the diagram with sequence numbers providing more information on what's happening at each block. This will give more clarity. BTW, the arrow from eventdev to crypto is unidirectional. > > >> > >>> + * 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: > >>> + * ============================== > >>> + * > >>> + * +--------------+ +--------------+ > >>> + * Events | | | | > >>> + * <------>| Event device |-------->| Crypto stage | > >>> + * | | | | > >>> + * +--------------+ +--------------+ > >>> + * event ^ | > >>> + * enqueue| | event > >>> + * | v dequeue > >>> + * +---------------------------------------+ > >>> + * | | > >>> + * | Crypto adapter | > >>> + * | | > >>> + * +---------------------------------------+ > >>> + * ^ > >>> + * | crypto > >>> + * | enq/deq > >>> + * v > >>> + * +-------------+ > >>> + * | | > >>> + * | Cryptodev | > >>> + * | | > >>> + * +-------------+ > >>> + * > >>> + * In the ENQ_DEQ mode, application sends crypto operations as > >>> + events to > >>> + * the adapter which dequeues events and perform cryptodev operations. > >>> + * 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() > >> application spell check. > >> > >>> + * or rte_event_crypto_adapter_create_ext(). > >>> + * > >>> + * Cryptodev queue pair addition/deletion is done using the > >>> + * rte_event_crypto_adapter_queue_pair_xxx() APIs. > >>> + * > >>> + * 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. > >> Above line is repeated below. This one can be removed. > > Ok. I will combine them. > >> > >>> + * > >>> + * 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. > >>> + > >>> + * 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 <stdint.h> > >>> + > >>> +#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 = 1, > >>> + /**< 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. > >>> + */ > >>> + 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. > >>> + */ > >>> +}; > >>> + > >>> +/** > >>> + * @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 > >>> + */ > >> I think in case of ENQ_DEQ mode, both request and response info is > >> required. I think it would be better to have a single structure as > >> > >> struct rte_event_crypto_metadata { > >> struct rte_event ev; > >> uint16_t cdev_id; > >> uint16_t queue_pair_id; > >> uint32_t resv1; > >> }; > >> The last 3 fields need not be filled by application for DEQ_ONLY mode. > >> Application will not get confused with request/response structures > >> when we have response info already present in request structure. > >> Or if you still insist to have separate structure for request and > >> response then it would be better to have it as struct instead of > >> union for metadata and remove the resv[8]. > >> IMO, first one is better. > > > > rte_event structure itself has enough memory to hold *both request and > response information*. > > struct rte_event { > > /** WORD0 */ > > union { > > uint64_t event; > > . > > } > > /** WORD1 */ > > union { > > uint64_t u64; > > . > > } > > } > > > > For response only *WORD0* is used. Whereas *WORD1* is used for request! > > > > As proposed, > > +struct rte_event_crypto_request { > > + uint8_t resv[8]; > > + /**< Overlaps with first 8 bytes of struct rte_event > > + * that encode the response event information > > + */ > > + uint16_t cdev_id; > > + /**< cryptodev ID to be used */ > > + uint16_t queue_pair_id; > > + /**< cryptodev queue pair ID to be used */ > > + uint32_t resv1; > > + /**< Reserved bits */ > > +}; > > > > First 8 bytes are *WORD0* and rest of the information fits into *WORD1*. > > +union rte_event_crypto_metadata { > > + struct rte_event_crypto_request request_info; > > + struct rte_event response_info; > > +}; > > Request and response together into a union will allocate memory required for > (WORD0+WORD1). > > I hope, this clarifies all your doubt. > > > Ok I missed the WORD1 in my previous comment. But my intention was to have > a common structure of metadata. As in case of ENQ-DEQ mode, both request > and response will be filled. So having a structure with a union of request and > response will be misleading. > > >> > >>> + uint16_t cdev_id; > >>> + /**< cryptodev ID to be used */ > >>> + uint16_t queue_pair_id; > >>> + /**< cryptodev queue pair ID to be used */ > >>> + uint32_t resv1; > >>> + /**< 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; > >>> + struct rte_event response_info; > >>> +}; > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this structure may change without prior notice > >>> + * > >>> + * Adapter configuration structure that the adapter configuration > >>> +callback > >>> + * function is expected to fill out > >>> + * @see rte_event_crypto_adapter_conf_cb */ struct > >>> +rte_event_crypto_adapter_conf { > >>> + uint8_t event_port_id; > >>> + /**< Event port identifier, the adapter enqueues events to this > >>> + * port and also dequeues crypto request events in ENQ_DEQ mode. > >>> + */ > >>> + uint32_t max_nb; > >>> + /**< The adapter can return early if it has processed at least > >>> + * max_nb crypto ops. This isn't treated as a requirement; batching > >>> + * may cause the adapter to process more than max_nb crypto ops. > >>> + */ > >>> +}; > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice > >>> + * > >>> + * Function type used for adapter configuration callback. The > >>> +callback is > >>> + * used to fill in members of the struct > >>> +rte_event_crypto_adapter_conf, this > >>> + * callback is invoked when creating a SW service for packet transfer > >>> +from > >>> + * cryptodev queue pair to the event device. The SW service is > >>> +created within > >>> + * the rte_event_crypto_adapter_queue_pair_add() function if SW based > >>> +packet > >>> + * transfers from cryptodev queue pair to the event device are required. > >>> + * > >>> + * @param id > >>> + * Adapter identifier. > >>> + * > >>> + * @param dev_id > >>> + * Event device identifier. > >>> + * > >>> + * @param conf > >>> + * Structure that needs to be populated by this callback. > >>> + * > >>> + * @param arg > >>> + * Argument to the callback. This is the same as the conf_arg passed > >>> +to the > >>> + * rte_event_crypto_adapter_create_ext(). > >>> + */ > >>> +typedef int (*rte_event_crypto_adapter_conf_cb) (uint8_t id, uint8_t > dev_id, > >>> + struct rte_event_crypto_adapter_conf *conf, > >>> + void *arg); > >>> + > >>> +/** > >>> + * @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; > >>> +}; > >> We may not need this wrapper structure. We can directly use rte_event. > > This was done keep in mind to accommodate need for any future requirement > > of having a new field added to the structure along with the rte_event. > > > Do you see anything in the future for configuration. If not, we can > remove it for now and should add in future when it is required. Sure. I will remove it in next patch. > > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this structure may change without prior notice > >>> + * > >>> + * A structure used to retrieve statistics for an event crypto > >>> +adapter > >>> + * instance. > >>> + */ > >>> + > >>> +struct rte_event_crypto_adapter_stats { > >>> + uint64_t event_poll_count; > >>> + /**< Event port poll count */ > >>> + uint64_t event_dequeue_count; > >> better to use uniform naming. event_deq_count > > ok > >>> + /**< Event dequeue count */ > >>> + uint64_t crypto_enq_count; > >>> + /**< Cryptodev enqueue count */ > >>> + uint64_t crypto_enq_fail; > >>> + /**< Cryptodev enqueue failed count */ > >>> + uint64_t crypto_deq_count; > >>> + /**< Cryptodev dequeue count */ > >>> + uint64_t event_enqueue_count; > >> event_enq_count > > ok > >>> + /**< Event enqueue count */ > >>> + uint64_t event_enq_retry_count; > >>> + /**< Event enqueue retry count */ > >>> + uint64_t event_enq_fail_count; > >>> + /**< Event enqueue fail count */ > >>> +}; > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice > >>> + * > >>> + * Create a new event crypto adapter with the specified identifier. > >>> + * > >>> + * @param id > >>> + * Adapter identifier. > >>> + * > >>> + * @param dev_id > >>> + * Event device identifier. > >>> + * > >>> + * @param conf_cb > >>> + * Callback function that fills in members of a > >>> + * struct rte_event_crypto_adapter_conf struct passed into > >>> + * it. > >>> + * > >>> + * @param mode > >>> + * Flag to indicate to start dequeue only or both enqueue & dequeue. > >>> + * > >>> + * @param conf_arg > >>> + * Argument that is passed to the conf_cb function. > >>> + * > >>> + * @return > >>> + * - 0: Success > >>> + * - <0: Error code on failure > >>> + */ > >>> +int __rte_experimental > >>> +rte_event_crypto_adapter_create_ext(uint8_t id, uint8_t dev_id, > >>> + rte_event_crypto_adapter_conf_cb conf_cb, > >>> + enum rte_event_crypto_adapter_mode mode, > >>> + void *conf_arg); > >>> + > >>> +/** > >>> + * @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 > >> set up > >>> + * 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); > >>> + > >> [..snip..] > >> > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice > >>> + * > >>> + * Retrieve the event port of an adapter. > >>> + * > >>> + * @param id > >>> + * Adapter identifier. > >>> + * > >>> + * @param [out] event_port_id > >>> + * Event port identifier used to link to the queue used in ENQ_DEQ mode. > >>> + * > >>> + * @return > >>> + * - 0: Success > >>> + * - <0: Error code on failure. > >>> + */ > >>> +int __rte_experimental > >>> +rte_event_crypto_adapter_event_port_get(uint8_t id, uint8_t > >>> +*event_port_id); > >> IIUC, crypto adapter can give packets to multiple event ports. > > There could be multiple event ports from cryptodev to eventdev in the HW > > which you are referring, by looking at DEQ mode. This API is used only for > > ENQ-DEQ mode. The SW adapter is using only one event port to do the job. > A comment shall be added in the description if that is the case. I have added a comment just below " param [out] event_port_id". I will try to add some more information, if possible. > >> > >> Also, I don't see similar API in eth_rx_adapter. > > As eth_rx_adapter does not dequeue any events from application, > > this API is not present there. > > Regards, > Akhil
