From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nithin Dabilpuram Subject: Re: [RFC] eventdev: add crypto adapter API header Date: Wed, 13 Dec 2017 17:58:22 +0530 Message-ID: <780d2af1-88e7-196c-eec8-bb10b74d1563@cavium.com> 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 , dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao , Gage Eads , hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com, narayanaprasad.athreya@cavium.com To: Jerin Jacob , "Doherty, Declan" Return-path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0071.outbound.protection.outlook.com [104.47.36.71]) by dpdk.org (Postfix) with ESMTP id A27A2107A for ; Wed, 13 Dec 2017 13:28:33 +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, Declan, On Wednesday 13 December 2017 04: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? Can we have this info in structure rte_crypto_sym_xform instead of rte_crypto_op so that for session less or session full we have just one api say as below to update the event information. rte_cryptodev_sym_xform_event_init(struct rte_crypto_sym_xform *xforms,                                                             struct rte_event ev) IMO, this is better because for both session_less or session_full modes, app has to prepare sym_xform structure and in case of session_less make the struct rte_crypto_op point to sym_xform and in session_full pass it to rte_cryptodev_sym_session_create(). The same can be followed for asym/security sessions in future if needed. >>> ➜ [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? >>>