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

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

Akhil,
I'm probably missing a point somewhere but I don't follow the suggestions. To me, ethdev, cryptodev, eventdev, etc. are device abstractions, whereas the proposed ENQ mode isn't at the same level.
The DEQ mode is a device abstraction for cryptodev->eventdev (whether h/w or s/w based), but the ENQ part of the adapter is purely a s/w programming model and optional to the application. It is independent of any device and it’s an application choice whether it wants to use this or not. Nothing prevents the application from calling cryptodev_enqueue_burst towards any device directly (whether it be soft crypto, NXP, Cavium, QAT, etc ) within an eventdev based environment. 
The ENQ mode allows an application programming model to be completely event based. If the application chooses, it enables the ENQ part where it enqueues an rte_event to the s/w adapter and the adapter then calls cryptodev_enqueue_burst on its behalf, towards any device PMD which was created. 
There are certain benefits to application architecture using this adapter where you can leverage the ordered scheduling within eventdev etc., (and certain cons where you need to run this service somewhere) but that’s up to the application to decide. 

In other words, I don’t consider ENQ mode as a device abstraction like cryptodev or ethdev where it needs to plug in to something transparently but a programming model that is provided as a choice, and that shouldn’t be tied up into a device abstraction layer.

vnr
---


-----Original Message-----
From: Akhil Goyal [mailto:akhil.goyal@nxp.com] 
Sent: Monday, February 26, 2018 7:52 AM
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; 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; narayanaprasad.athreya@cavium.com; nidadavolu.murthy@cavium.com; nithin.dabilpuram@cavium.com
Subject: Re: [RFC v2, 2/2] eventdev: add crypto adapter API header

Hi Jerin/Abhinandan,

On 2/20/2018 7:29 PM, Jerin Jacob wrote:
> -----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?

I see that there is performance gain in sw side while using ENQ_DEQ mode. But since we already have many modes in the application already, can we make this one with some callback to cryptodev.

So the application can call the rte_cryptodev_enqueue_burst() as it is doing, and if the ENQ_DEQ mode is supported by the underneath implementation then, it can register a callback to the implementation that is required in the driver layer itself.

In this way, the application will become less complex as compared to the
2 parallel implementations for SW and HW. It will also give more flexibility to the driver implementation as well.

-Akhil
> 
>> */
>>>
>>>> + * 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
>>>
>