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

[Eads, Gage]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Monday, December 18, 2017 12:30 AM
> To: Eads, Gage <gage.eads at intel.com>
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>; dev at dpdk.org;
> Vangati, Narender <narender.vangati at intel.com>; Rao, Nikhil
> <nikhil.rao at intel.com>; hemant.agrawal at nxp.com; Doherty, Declan
> <declan.doherty at intel.com>; nidadavolu.murthy at cavium.com;
> nithin.dabilpuram at cavium.com; narayanaprasad.athreya at cavium.com
> Subject: Re: [RFC] eventdev: add crypto adapter API header
> 
> -----Original Message-----
> > Date: Thu, 14 Dec 2017 18:52:02 +0000
> > From: "Eads, Gage" <gage.eads at intel.com>
> > To: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> > CC: "Gujjar, Abhinandan S" <abhinandan.gujjar at intel.com>, "dev at dpdk.org"
> >  <dev at dpdk.org>, "Vangati, Narender" <narender.vangati at intel.com>,
> > "Rao,  Nikhil" <nikhil.rao at intel.com>, "hemant.agrawal at nxp.com"
> >  <hemant.agrawal at nxp.com>, "Doherty, Declan"
> > <declan.doherty at intel.com>,  "nidadavolu.murthy at cavium.com"
> > <nidadavolu.murthy at cavium.com>,  "nithin.dabilpuram at cavium.com"
> > <nithin.dabilpuram at cavium.com>,  "narayanaprasad.athreya at cavium.com"
> > <narayanaprasad.athreya at cavium.com>
> > Subject: RE: [RFC] eventdev: add crypto adapter API header
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> > > Sent: Wednesday, December 13, 2017 8:49 PM
> > > To: Eads, Gage <gage.eads at intel.com>
> > > Cc: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>;
> > > dev at dpdk.org; Vangati, Narender <narender.vangati at intel.com>; Rao,
> > > Nikhil <nikhil.rao at intel.com>; hemant.agrawal at nxp.com; Doherty,
> > > Declan <declan.doherty at intel.com>; nidadavolu.murthy at cavium.com;
> > > nithin.dabilpuram at cavium.com; narayanaprasad.athreya at cavium.com
> > > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > >
> > > -----Original Message-----
> > > > Date: Wed, 13 Dec 2017 23:35:48 +0000
> > > > From: "Eads, Gage" <gage.eads at intel.com>
> > > > To: Jerin Jacob <jerin.jacob at caviumnetworks.com>, "Gujjar, Abhinandan
> S"
> > > >  <abhinandan.gujjar at intel.com>
> > > > CC: "dev at dpdk.org" <dev at dpdk.org>, "Vangati, Narender"
> > > >  <narender.vangati at intel.com>, "Rao, Nikhil"
> > > > <nikhil.rao at intel.com>, "hemant.agrawal at nxp.com"
> <hemant.agrawal at nxp.com>, "Doherty, Declan"
> > > >  <declan.doherty at intel.com>, "nidadavolu.murthy at cavium.com"
> > > >  <nidadavolu.murthy at cavium.com>, "nithin.dabilpuram at cavium.com"
> > > >  <nithin.dabilpuram at cavium.com>,
> "narayanaprasad.athreya at cavium.com"
> > > >  <narayanaprasad.athreya at cavium.com>
> > > > Subject: RE: [RFC] eventdev: add crypto adapter API header
> > > >
> > > > Hey Jerin,
> > >
> > > Hey Gage,
> > >
> > > >
> > > > </snip>
> > > >
> > > > > > +
> > > > > > + /**
> > > > > > + * @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?
> > > >
> > > > That could work, but perhaps the ev.op should be configured once
> > > > up front, as
> > > I see it being a function of the application architecture. A couple
> > > possible designs, for example:
> > > > - Worker enqueues into cryptodev, adapter polls for response: the
> > > > adapter
> > > port would always use OP_NEW here.
> > > > - Worker sends a crypto request event to the adapter, which gives
> > > > the request to the cryptodev and polls for response: the adapter
> > > > port would always use OP_FWD here. (This ties in with my implicit
> > > > release patch
> > > > (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> > > > - Etc.
> > >
> > > Yes. Semantically both approaches will work. I was trying to avoid
> > > extra clutter(enum rte_event_crypto_adapter_type) in adapter API.
> > > I don't see any problem in moving ev.op to adapter configuration
> > > time if it helps the SW driver.
> > >
> > > IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
> > > RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name,
> something
> > > like,
> RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTE
> > > R_TYPE_OP_FWD
> > > or something like that.
> > >
> >
> > I agree that the two naming schemes are equivalent, but since this option
> would control the adapter's behavior (Rx only vs. Rx + Tx), (IMO) I think
> Abhinandan's original names do a better job of conveying what effect these two
> options have on the adapter, compared to the op type names.
> 
> The only concern with Rx/Tx terminology was, It is mostly used in the ethdev
> domain.
> In crypto domain, typically, we use enqueue/dequeue.
> The only difference between two modes is if adapter enqueue the events with
> RTE_EVENT_OP_NEW vs RTE_EVENT_OP_FORWARD then (IMO) we can change
> something related to that name to avoid adding a new terminology.
> 

Oh, sure -- enqueue/dequeue makes sense here. I'd still prefer DEQ_ONLY or DEQ_ENQ, but the event_op names work just as well.

Speaking of the crypto domain, the cryptodev enqueue and dequeue operations both take crypto op pointers. The original RFC had the request event pointing to an mbuf (which had a crypto_op pointer in its private metadata), but with the suggested opaque eventdev metadata changes it makes more sense for the request event to point to a crypto op. And the RFC didn't specify what the response event would point to (mbuf or crypto op), but to match the cryptodev dequeue operation then a crypto op makes sense. Will this work with your hardware?

> BTW, Based on the earlier discussion, if we need to add opaque eventdev
> metadata to cryptodev then it may change ABI.If so, I think, we need to
> announce ABI change notice for cryptodev and plan cryptodev adapter for
> v18.05.

Personally I'd prefer to get this right/agreed-upon the first time around -- even if that means breaking ABI and pushing this adapter out to 18.05.