From mboxrd@z Thu Jan 1 00:00:00 1970 From: Akhil Goyal Subject: Re: [RFC] eventdev: add crypto adapter API header Date: Wed, 13 Dec 2017 19:52:24 +0530 Message-ID: References: <1510210453-61428-1-git-send-email-abhinandan.gujjar@intel.com> <20171129114153.GA16467@jerin> <20171213112606.GA5166@jerin> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Cc: Abhinandan Gujjar , , , Nikhil Rao , Gage Eads , , , , To: Jerin Jacob , "Doherty, Declan" Return-path: Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0083.outbound.protection.outlook.com [104.47.41.83]) by dpdk.org (Postfix) with ESMTP id CB76B107A for ; Wed, 13 Dec 2017 15:22:31 +0100 (CET) In-Reply-To: <20171213112606.GA5166@jerin> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jerin, On 12/13/2017 4:56 PM, Jerin Jacob wrote: > -----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? I believe only 1 uint64 is sufficient. The metadata that we need here is rte_event which is 2 uint64 and the second one is mbuf. Since mbuf is already part of rte_crypto_sym_op, we do not need it. So only a pointer/uint64 is required. > >> >>> ➜ [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? >>> >> >