From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [RFC] eventdev: add crypto adapter API header Date: Wed, 13 Dec 2017 16:56:07 +0530 Message-ID: <20171213112606.GA5166@jerin> References: <1510210453-61428-1-git-send-email-abhinandan.gujjar@intel.com> <20171129114153.GA16467@jerin> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Abhinandan Gujjar , dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao , Gage Eads , hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com, nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com To: "Doherty, Declan" Return-path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0053.outbound.protection.outlook.com [104.47.32.53]) by dpdk.org (Postfix) with ESMTP id A92DF293B for ; Wed, 13 Dec 2017 12:26:30 +0100 (CET) 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: Wed, 13 Dec 2017 11:03:06 +0000 > From: "Doherty, Declan" > To: Jerin Jacob , Abhinandan Gujjar > > CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao > , Gage Eads , > hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com, > nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com > Subject: Re: [RFC] eventdev: add crypto adapter API header > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 > Thunderbird/52.5.0 > > On 29/11/2017 11:41 AM, Jerin Jacob wrote: > > -----Original Message----- > > ... > > > > > Adding Declan and Hemant. > > > IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful > > from application perceptive as the scope is very limited. > > In real world use cases, we will be attaching destination event queue information > > to the session, not to the queue pair. > > > > > > IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very > > convenient for application writers as > > # it relies on mbuf private area memory. So it has additional memory alloc/free > > requirements. > > # additional overhead for application/PMD to write/read the event queue metadata > > information per packet. > > > > Since we already have meta data structure in the crypto > > area, How about adding the destination event queue attributes > > in the PMD crypto session area and for, _session less_, we can add it > > in rte_crypto_op stricture? This will help in: > > > > # Offloading HW specific meta data generation for event queue attributes > > to slow path. > > # From the application perspective, most likely the event queue parameters > > will be different for each session not per packet nor per event queue > > pair. > > > > Hey Jerin, Hey Declan, > > given my limited experience with eventdev, your proposed approach in general > makes sense to me, in that a packet flow having crypto processing done will > always be forwarded the same next stage event queue. So storing this state > in the crypto session, or crypto op in the case of sessionless ops, seems > sensible. > > > Something like below to share my view. Exact details may be we can work it out. > > > > I terms of your proposed changes below, my main concern would be introducing > dependencies on the eventdev library into cryptodev, as with this new crypto > adpater you will have a dependency on cryptodev in eventdev. I agree with your dependency concern. > > I think the best approach would be to support opaque metadata in both the > crypto op and crypto session structures, as this would allow other uses > cases to be supported which aren't specific to eventdev to also store > metadata across cryptodev processing. Make sense. Just to add, adding a pointer would be overhead. I think, we can reserve a few bytes as byte array and then later typecast with eventdev api in eventdev library. uint8_t eventdev_metadata[SOMEBYTES]; Thoughts? > > > ➜ [master][dpdk.org] $ git diff > > diff --git a/lib/librte_cryptodev/rte_crypto.h > > b/lib/librte_cryptodev/rte_crypto.h > > index 3d672fe7d..b44ef673b 100644 > > --- a/lib/librte_cryptodev/rte_crypto.h > > +++ b/lib/librte_cryptodev/rte_crypto.h > > @@ -115,6 +115,9 @@ struct rte_crypto_op { > > uint8_t reserved[5]; > > /**< Reserved bytes to fill 64 bits for future additions */ > > > > +#if 0 > > + Crypto completion event attribute. For _session less_ crypto enqueue operation, > > + The will field shall be used by application to post the crypto completion event > > + upon the crypto enqueue operation complete. > > > > + Note: In the case of session based crypto operation, SW based crypto adapter can use > > + this memory to store crypto event completion attributes from the PMD > > + specific session area. > > + > > + Note: ev.event_ptr will point to struct rte_crypto_op *op, So > > + that core can free the ops memory on event_dequeue(). > > +#endif > > + > > + struct rte_event ev; > > > > struct rte_mempool *mempool; > > /**< crypto operation mempool which operation is allocated from > > * */ > > diff --git a/lib/librte_cryptodev/rte_cryptodev.h > > b/lib/librte_cryptodev/rte_cryptodev.h > > index dade5548f..540b29e66 100644 > > --- a/lib/librte_cryptodev/rte_cryptodev.h > > +++ b/lib/librte_cryptodev/rte_cryptodev.h > > @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id, > > struct rte_crypto_sym_xform *xforms, > > struct rte_mempool *mempool); > > > > +#if 0 > > + Create PMD specific session meta data for the destination event queue > > + attribute to post the crypto completion event on crypto work complete. > > +#endif > > +int > > +rte_cryptodev_sym_session_event_init(uint8_t dev_id, > > + struct rte_cryptodev_sym_session *sess, > > + struct rte_crypto_sym_xform *xforms, > > + struct rte_mempool *mempool, > > + struct rte_event ev); > > + > > /** > > * Frees private data for the device id, based on its device type, > > * returning it to its mempool. > > > > > > > + * > > > + * The metadata offset is used to configure the location of the > > > + * rte_event_crypto_metadata structure within the mbuf's private metadata area. > > > + * > > > + * When the application sends crypto operations to the adapter, > > > + * the crypto queue pair identifier needs to be specified, similarly eventdev > > > + * parameters such as the flow id, scheduling type etc are needed by the > > > + * adapter when it enqueues mbufs from completed crypto operations to eventdev. > > > + */ > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +#include > > > +#include > > > + > > > +#include "rte_eventdev.h" > > > + > > > +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32 > > > + > > > + /** > > > + * @warning > > > + * @b EXPERIMENTAL: this enum may change without prior notice > > > + * > > > + * Crypto event queue conf type > > > + */ > > > +enum rte_event_crypto_conf_type { > > > + RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1, > > > + /**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */ > > > + RTE_EVENT_CRYPTO_CONF_TYPE_MBUF, > > > + /**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */ > > > + RTE_EVENT_CRYPTO_CONF_TYPE_MAX > > > +}; > > > > See above. > > > > > + > > > + /** > > > + * @warning > > > + * @b EXPERIMENTAL: this enum may change without prior notice > > > + * > > > + * Crypto event adapter type > > > + */ > > > +enum rte_event_crypto_adapter_type { > > > + RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1, > > > + /**< Start only Rx part of crypto adapter. > > > + * Packets dequeued from cryptodev are new to eventdev and > > > + * events will be treated as RTE_EVENT_OP_NEW */ > > > + RTE_EVENT_CRYPTO_ADAPTER_RX_TX, > > > + /**< Start both Rx & Tx part of crypto adapter. > > > + * Packet's event context will be retained and > > > + * event will be treated as RTE_EVENT_OP_FORWARD */ > > > +}; > > > > How about leveraging ev.op based schematics as mentioned above? > > >