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

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Abhinandan,

Sorry for the delayed response, my office network had some issues wrt 
NNTP, so couldn't reply.

On 2/28/2018 2:31 PM, Gujjar, Abhinandan S wrote:
> Hi Akhil,
> 

...<snip>...

>>>>>
>>>>>> + * 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 ENQ-DEQ mode, crypto request are sent through the eventdev.
> With your proposal, it is not clear how crypto request can be hidden under rte_cryptodev_enqueue_burst()!
> Can you please share flow diagram or pseudo code?
> 
> -Abhinandan
> 
The code flow is what Jerin also suggested.

"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."

The suggestion was just to simplify the flow in the application.
My main concern is that the ipsec-secgw application is already having a 
lot of modes and we are about to add two more cases.

>>
>> 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
>>>>>
>>>
>