-----Original Message-----

Date: Tue, 20 Feb 2018 18:55:09 +0000
From: "Vangati, Narender" <redacted>
To: Jerin Jacob <redacted>, "Gujjar, Abhinandan S"
 [off-list ref]
CC: "dev@dpdk.org" <redacted>, "Rao, Nikhil" <redacted>,
 "Eads, Gage" [off-list ref], "hemant.agrawal@nxp.com"
 [off-list ref], "akhil.goyal@nxp.com" [off-list ref],
 "narayanaprasad.athreya@cavium.com" [off-list ref],
 "nidadavolu.murthy@cavium.com" [off-list ref],
 "nithin.dabilpuram@cavium.com" [off-list ref]
Subject: RE: [RFC v2, 2/2] eventdev: add crypto adapter API header

Hi Narender,

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. 

But there is no element of _offload_ here. Right? meaning, We can express the ENQ-DEQ mode with existing cryptodev+eventdev normative API.
I think, Pushing to adapter will result in running ENQ-DEQ only on "service lcore" instead of "worker lcore".
How about making "ENQ-DEQ" scheme as set of helper C functions. This will enable,
1) To run the ENQ-DEQ logic in "service" or "worker" lcores
2) Remove the capability in adapter code. ie. If application wants to use ENQ-DEQ scheme
as a helper function it will work for both cases(HW and SW)

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.

Makes sense. Only question is that, do we need to expose as service function or generic helper C function?.
IMO, service functions makes sense when SW does something when there is no
HW support and to get the job done like schedule() function.

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 <redacted>
Cc: dev@dpdk.org; Vangati, Narender <redacted>; Rao, Nikhil <redacted>; Eads, Gage <redacted>; 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-----

quoted

Date: Mon, 19 Feb 2018 10:55:58 +0000
From: "Gujjar, Abhinandan S" <redacted>
To: Jerin Jacob <redacted>
CC: "dev@dpdk.org" <redacted>, "Vangati, Narender"
 [off-list ref], "Rao, Nikhil" [off-list ref], 
"Eads,  Gage" [off-list ref], "hemant.agrawal@nxp.com"
 [off-list ref], "akhil.goyal@nxp.com" 
[off-list ref],  "narayanaprasad.athreya@cavium.com" 
[off-list ref],
 "nidadavolu.murthy@cavium.com" [off-list ref],  
"nithin.dabilpuram@cavium.com" [off-list ref]
Subject: RE: [RFC v2, 2/2] eventdev: add crypto adapter API header

Hi Jerin,

Hi Abhinandan,

quoted

Thanks for the review. Please find few comments inline.

quoted

-----Original Message-----
From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
Sent: Saturday, February 17, 2018 1:04 AM
To: Gujjar, Abhinandan S <redacted>
Cc: dev@dpdk.org; Vangati, Narender <redacted>; 
Rao, Nikhil [off-list ref]; Eads, Gage 
[off-list ref]; 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-----

quoted

Date: Mon, 15 Jan 2018 16:23:50 +0530
From: Abhinandan Gujjar <redacted>
To: jerin.jacob@caviumnetworks.com
CC: dev@dpdk.org, narender.vangati@intel.com, Abhinandan Gujjar 
[off-list ref], Nikhil Rao [off-list ref], 
Gage Eads [off-list ref]
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.

quoted

quoted

quoted

+ * 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?

quoted

*/

quoted

quoted

+ * dequeue only (DEQ) mode  and the second as the enqueue - 
+ dequeue

extra space between "mode" and "and"

Ok

quoted

quoted

+ * (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