Re: [dpdk-dev] [RFC v2, 2/2] eventdev: add crypto adapter API header

["Vangati, Narender" <narender.vangati@intel.com>]

Jerin,
I see the "ENQ" part of the adapter a little differently. I think there is value to offloading cryptodev_enqueue to an adapter service, even when the h/w natively supports DEQ. 
When the same queue pair is being used by the workers to enqueue requests (this could be where the pre crypto stage was ordered scheduled and the cryptodev enqueues need to be ordered), you would need some synchronization. Going thru eventdev to an adapter which enqueues on worker behalf provides that synchronization and ordering.

In that sense, the ENQ-DEQ mode is an application choice and defines its programming model. Based on that, a service would be created that does ENQ. For the DEQ part, perhaps the adapter can tell whether the h/w supports DEQ natively or needs to be done in s/w - that way you can have a single adapter.

Application model for DEQ mode - call cryptodev_enqueue directly. DEQ is h/w or s/w based on capability
Application model for ENQ-DEQ mode - use event_enqueue to offload cryptodev_enqueue to adapter. DEQ is h/w or s/w based on capability


vnr
---

-----Original Message-----
From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] 
Sent: Tuesday, February 20, 2018 7:59 AM
To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>; Eads, Gage <gage.eads@intel.com>; hemant.agrawal@nxp.com; akhil.goyal@nxp.com; narayanaprasad.athreya@cavium.com; nidadavolu.murthy@cavium.com; nithin.dabilpuram@cavium.com
Subject: Re: [RFC v2, 2/2] eventdev: add crypto adapter API header

-----Original Message-----
> Date: Mon, 19 Feb 2018 10:55:58 +0000
> From: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
>  <narender.vangati@intel.com>, "Rao, Nikhil" <nikhil.rao@intel.com>, 
> "Eads,  Gage" <gage.eads@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "akhil.goyal@nxp.com" 
> <akhil.goyal@nxp.com>,  "narayanaprasad.athreya@cavium.com" 
> <narayanaprasad.athreya@cavium.com>,
>  "nidadavolu.murthy@cavium.com" <nidadavolu.murthy@cavium.com>,  
> "nithin.dabilpuram@cavium.com" <nithin.dabilpuram@cavium.com>
> Subject: RE: [RFC v2, 2/2] eventdev: add crypto adapter API header
> 
> Hi Jerin,

Hi Abhinandan,

> 
> Thanks for the review. Please find few comments inline.
> 
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Saturday, February 17, 2018 1:04 AM
> > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; 
> > Rao, Nikhil <nikhil.rao@intel.com>; Eads, Gage 
> > <gage.eads@intel.com>; hemant.agrawal@nxp.com; akhil.goyal@nxp.com; 
> > narayanaprasad.athreya@cavium.com; nidadavolu.murthy@cavium.com; 
> > nithin.dabilpuram@cavium.com
> > Subject: Re: [RFC v2, 2/2] eventdev: add crypto adapter API header
> > 
> > -----Original Message-----
> > > Date: Mon, 15 Jan 2018 16:23:50 +0530
> > > From: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > > To: jerin.jacob@caviumnetworks.com
> > > CC: dev@dpdk.org, narender.vangati@intel.com, Abhinandan Gujjar 
> > > <abhinandan.gujjar@intel.com>, Nikhil Rao <nikhil.rao@intel.com>, 
> > > Gage Eads <gage.eads@intel.com>
> > > Subject: [RFC v2, 2/2] eventdev: add crypto adapter API header
> > > X-Mailer: git-send-email 1.9.1
> > >
> > > +
> > > +/**
> > > + * This adapter adds support to enqueue crypto completions to event device.
> > > + * The packet flow from cryptodev to the event device can be 
> > > +accomplished
> > > + * using both SW and HW based transfer mechanisms.
> > > + * The adapter uses a EAL service core function for SW based 
> > > +packet transfer
> > > + * and uses the eventdev PMD functions to configure HW based 
> > > +packet transfer
> > > + * between the cryptodev and the event device.
> > > + *
> > > + * In the case of SW based transfers, application can choose to 
> > > +submit a
> > 
> > I think, we can remove "In the case of SW based transfers" as it 
> > should be applicable for HW case too
> Ok. In that case, adapter will detect the presence of HW connection 
> between cryptodev & eventdev and will not dequeue crypto completions.

I would say presence of "specific capability" instead of HW.

> 
> > 
> > > + * crypto operation directly to cryptodev or send it  to the 
> > > + cryptodev
> > > + * adapter via eventdev, the cryptodev adapter then submits the 
> > > + crypto
> > > + * operation to the crypto device. The first mode is known as the
> > 
> > The first mode (DEQ) is very clear. In the second mode(ENQ_DEQ),
> > - How does "worker" submits the crypto work through crypto-adapter?
> > If I understand it correctly, "workers" always deals with only 
> > cryptodev's
> > rte_cryptodev_enqueue_burst() API and "service" function in crypto 
> > adapter would be responsible for dequeue() from cryptodev and enqueue to eventdev?
> > 
> > I understand the need for OP_NEW vs OP_FWD mode difference in both modes.
> > Other than that, What makes ENQ_DEQ different? Could you share the 
> > flow for ENQ_DEQ mode with APIs.
> 
> /*
> Application changes for ENQ_DEQ mode:
> -------------------------------------------------
> 	/* In ENQ_DEQ mode, to enqueue to adapter app
> 	 * has to fill out following details.
> 	 */
> 	struct rte_event_crypto_request *req;
> 	struct rte_crypto_op *op = rte_crypto_op_alloc();
> 	
> 	/* fill request info */
> 	req = (void *)((char *)op + op.private_data_offset);
> 	req->cdev_id = 1;
> 	req->queue_pair_id = 1;
> 
> 	/* fill response info */
> 	...
> 
> 	/* send event to crypto adapter */
> 	ev->event_ptr = op;
> 	ev->queue_id = dst_event_qid;
> 	ev->priority = dst_priority;
> 	ev->sched_type = dst_sched_type;
> 	ev->event_type = RTE_EVENT_TYPE_CRYPTODEV;
> 	ev->sub_event_type = sub_event_type;
> 	ev->flow_id = dst_flow_id;
> 	ret = rte_event_enqueue_burst(event_dev_id, event_port_id, ev, 1);
> 
> 
> Adapter in ENQ_DEQ mode, submitting crypto ops to cryptodev:
> -----------------------------------------------------------------------------
> 	n = rte_event_dequeue_burst(event_dev_id, event_port_id, ev, BATCH_SIZE, time_out);
> 	struct rte_crypto_op *op = ev->event_ptr;
> 	struct rte_event_crypto_request *req = (void *)op + op.private_data_offset;
> 	cdev_id = req->cdev_id;
> 	qp_id = req->queue_pair_id
> 
> 	ret = rte_cryptodev_enqueue_burst(cdev_id, qp_id, op, 1);

This mode wont work for the HW implementations that I know. As in HW implementations, The Adapter is embedded in HW.
The DEQ mode works. But, This would call for to have two separate application logic for DEQ and ENQ_DEQ mode.
I think, it is unavoidable as SW scheme has better performance with ENQ_DEQ MODE.

If you think, there is no option other than introducing a capability in adapter then please create capability in Rx adapter to inform the adapter capability to the application. 

Do we think, it possible to have scheme with ENQ_DEQ mode, Where application still enqueue to cryptodev like DEQ mode but using cryptodev. ie. Adapter patches the cryptodev dev->enqueue_burst() to "eventdev enqueue burst" followed by "exiting dev->enqueue_burst".
Something like exiting ethdev rx_burst callback scheme.
This will enable application to have unified flow IMO.

Any thoughts from NXP folks?

> */
> > 
> > > + * dequeue only (DEQ) mode  and the second as the enqueue - 
> > > + dequeue
> > 
> > extra space between "mode" and "and"
> Ok
> > 
> > > + * (ENQ_DEQ) mode. The choice of mode can be specified when 
> > > + creating
> > > + * the adapter.
> > > + * In the latter choice, the cryptodev adapter is able to use
> > > + * RTE_OP_FORWARD as the event dev enqueue type, this has a 
> > > + performance
> > > + * advantage in "closed system" eventdevs like the eventdev SW 
> > > + PMD and
> >