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

["Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>]

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Monday, February 26, 2018 7:22 PM
> 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 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

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