Re: [dpdk-dev] [PATCH v4 1/3] eventdev: introduce crypto adapter enqueue API

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

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, April 8, 2021 8:27 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Shijith Thotton
> <sthotton@marvell.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> hemant.agrawal@nxp.com; nipun.gupta@nxp.com;
> sachin.saxena@oss.nxp.com; Anoob Joseph <anoobj@marvell.com>;
> matan@nvidia.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> g.singh@nxp.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Jayatheerthan,
> Jay <jay.jayatheerthan@intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> Subject: RE: [PATCH v4 1/3] eventdev: introduce crypto adapter enqueue API
> 
> Hi Abhinandan,
> > > > >
> > > > > In case an event from a previous stage is required to be
> > > > > forwarded to a crypto adapter and PMD supports internal event
> > > > > port in crypto adapter, exposed via capability
> > > > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD, we
> do
> > > not have a
> > > > > way to check in the API rte_event_enqueue_burst(), whether it is
> > > > > for crypto adapter or for eth tx adapter.
> > > > I may be missing something here. Crypto adapter is an atomic stage
> > > > has a port which is setup during the adapter configuration.
> > > > So, application enqueuing events will end up sending to the crypto
> > > > adapter (As the adapter dequeues from a specific port).
> > > > Still wondering why there is requirement for new API.
> > >
> > > While we do rte_event_enqueue_burst(), we do not have a way to
> > > identify whether The event is for crypto adapter or the eth adaptor.
> > > At the application layer, we know where to send the event, but in
> > > the event lib We do not know which port it need to be sent.
> > > IMO, event port is specifically designed to work for OP_NEW mode.
> > > I did not find a way to make it land into crypto adapter.
> > > Please let me know in case there is a better option other than
> > > adding a new API.
> > This looks like a hack as the new API does not actual enqueue events
> > to eventdev.
> > Rather it directly extracts the crypto info from each event and then
> > enqueue to cryptodev.
> >
> > How about doing this (No changes to rte_event_enqueue_burst(), PMD
> > takes care of things ):
> > uint16_t __rte_hot
> > ssows_enq_burst(void *port, const struct rte_event ev[], uint16_t
> > nb_events) {
> > +	struct otx2_ssogws *ws = port;
> > +
> > +	RTE_SET_USED(nb_events);
> > +
> > +	if (cap &
> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD)
> > +		return otx2_ca_enq(ws->tag_op, ev);
> >
> > 	return ssows_enq(port, ev);
> > }
> >
> > Everything will be hidden under the callback and application will not
> > have any changes.
> >
> You want to say we somehow save a flag in struct otx2_ssogws from the
> capability And check that flag here to enq to crypto. But that will not work, as
> the events coming In this API can be for both crypto as well as eth tx adapter.
> If this check is there, all events will go to crypto adapter.
> 
> In the library or the driver, we do not have a mechanism to determine the
> destination of the event (Note that event type is for source of event and not
> destination).
> Using some fields of rte_event will be a hack IMO.
> 
> The solution proposed in this patchset is not a hack. Here is a reasoning to it:
> - The application when dequeues an event from the previous stage, knows
> what to do with the event - send it to crypto or send to ethernet. Hence it
> makes sense to call the different API there itself as inside the
> rte_event_enqueue_burst() there is no way to determine if it is for crypto
> adapter or eth adapter.
> Moreover, the solution is very similar to what eth tx adapter already support
> (a new API to enqueue).
> 
> I hope this make things clearer now.
Yes Akhil. This makes it more clear. Thanks for clarifying.

> 
> Regards,
> Akhil
> > >
> > > >
> > > > >
> > > > > Hence we need a new API similar to
> > > > > rte_event_eth_tx_adapter_enqueue(),
> > > > > which can send to a crypto adapter.
> > > > >
> > > > > Note that RTE_EVENT_TYPE_* cannot be used to make that decision,
> > > > > as it is meant for event source and not event destination.
> > > > > And event port designated for crypto adapter is designed to be
> > > > > used for OP_NEW mode.
> > > > >
> > > > > Hence, in order to support an event PMD which has an internal
> > > > > event port
> > > > in
> > > > > crypto adapter (RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD
> mode),
> > > exposed
> > > > > via capability
> > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD,
> > > > > application should use rte_event_crypto_adapter_enqueue() API to
> > > > > enqueue events.
> > > > >
> > > > > When internal port is not
> > > > available(RTE_EVENT_CRYPTO_ADAPTER_OP_NEW
> > > > > mode), application can use API rte_event_enqueue_burst() as it
> > > > > was doing earlier, i.e. retrieve event port used by crypto
> > > > > adapter and bind its event queues to that port and enqueue
> > > > > events using the API rte_event_enqueue_burst().
> > > > >
> > > > > Signed-off-by: Akhil Goyal <gakhil@marvell.com>