Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Wed, 13 Dec 2017 19:52:24 +0530
> From: Akhil Goyal <akhil.goyal@nxp.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Doherty, Declan"
>  <declan.doherty@intel.com>
> CC: Abhinandan Gujjar <abhinandan.gujjar@intel.com>, dev@dpdk.org,
>  narender.vangati@intel.com, Nikhil Rao <nikhil.rao@intel.com>, Gage Eads
>  <gage.eads@intel.com>, 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 6.1; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.5.0
> 
> Hi Jerin,

Hi Akhil,

> On 12/13/2017 4:56 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Wed, 13 Dec 2017 11:03:06 +0000
> > > From: "Doherty, Declan" <declan.doherty@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
> > >   <abhinandan.gujjar@intel.com>
> > > CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
> > >   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
> > >   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.

Yes.

> 
> So only a pointer/uint64 is required.

I would say uint64_t, as the pointer is 32bit in 32bit systems.IMO, We need
have reserved metadata(uint64_t) for eventdev(not generic).