Re: [dpdk-dev] [dpdk-dev, v1, 1/5] eventdev: add caps API and PMD callbacks for crypto adapter

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

Hi Jerin,

Please find some comments inline

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Tuesday, April 10, 2018 11:38 AM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Cc: hemant.agrawal@nxp.com; akhil.goyal@nxp.com; dev@dpdk.org; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>;
> Nidadavolu.Murthy@cavium.com; NarayanaPrasad.Athreya@cavium.com
> Subject: Re: [dpdk-dev,v1,1/5] eventdev: add caps API and PMD callbacks for
> crypto adapter
> 
> -----Original Message-----
> > Date: Wed, 4 Apr 2018 12:26:18 +0530
> > From: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > To: jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com,
> > akhil.goyal@nxp.com, dev@dpdk.org
> > CC: pablo.de.lara.guarch@intel.com, declan.doherty@intel.com,
> > narender.vangati@intel.com, abhinandan.gujjar@intel.com,
> > nikhil.rao@intel.com
> > Subject: [dpdk-dev,v1,1/5] eventdev: add caps API and PMD callbacks
> > for  crypto adapter
> > X-Mailer: git-send-email 1.9.1
> >
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > ---
> >  lib/librte_eventdev/rte_eventdev.c     |  25 +++++
> >  lib/librte_eventdev/rte_eventdev.h     |  35 +++++++
> >  lib/librte_eventdev/rte_eventdev_pmd.h | 176
> > +++++++++++++++++++++++++++++++++
> >  3 files changed, 236 insertions(+)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.c
> > b/lib/librte_eventdev/rte_eventdev.c
> > index 2de8d9a..3d24e8f 100644
> > --- a/lib/librte_eventdev/rte_eventdev.c
> > +++ b/lib/librte_eventdev/rte_eventdev.c
> > @@ -29,6 +29,8 @@
> >  #include <rte_malloc.h>
> >  #include <rte_errno.h>
> >  #include <rte_ethdev.h>
> > +#include <rte_cryptodev.h>
> > +#include <rte_cryptodev_pmd.h>
> >
> >  #include "rte_eventdev.h"
> >  #include "rte_eventdev_pmd.h"
> > @@ -123,6 +125,29 @@
> >  				: 0;
> >  }
> >
> > +int __rte_experimental
> > +rte_event_crypto_adapter_caps_get(uint8_t dev_id, uint8_t cdev_id,
> > +				  uint32_t *caps)
> > +{
> > +	struct rte_eventdev *dev;
> > +	struct rte_cryptodev *cdev;
> > +
> > +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
> > +	if (!rte_cryptodev_pmd_is_valid_dev(cdev_id))
> > +		return -EINVAL;
> > +
> > +	dev = &rte_eventdevs[dev_id];
> > +	cdev = rte_cryptodev_pmd_get_dev(cdev_id);
> > +
> > +	if (caps == NULL)
> > +		return -EINVAL;
> > +	*caps = 0;
> > +
> > +	return dev->dev_ops->crypto_adapter_caps_get ?
> > +		(*dev->dev_ops->crypto_adapter_caps_get)
> > +		(dev, cdev, caps) : 0;
> > +}
> > +
> >  static inline int
> >  rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t
> > nb_queues)  { diff --git a/lib/librte_eventdev/rte_eventdev.h
> > b/lib/librte_eventdev/rte_eventdev.h
> > index a20077c..49a71d1 100644
> > --- a/lib/librte_eventdev/rte_eventdev.h
> > +++ b/lib/librte_eventdev/rte_eventdev.h
> > @@ -35,6 +35,8 @@
> >  #ifndef _RTE_EVENTDEV_H_
> >  #define _RTE_EVENTDEV_H_
> >
> > +#include <rte_compat.h>
> > +
> >  /**
> >   * @file
> >   *
> > @@ -1142,6 +1144,39 @@ struct rte_event {
> > rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
> >  				uint32_t *caps);
> >
> > +
> > +/* Crypto adapter capability bitmap flag */
> > +#define RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT	0x1
> > +/**< Flag indicates HW is capable of generating events.
> > + * Cryptodev can send packets to the event device using an internal event
> port.
> > + */
> 
> Some top level comments,
> 
> 1) Since we are not planning to abstract
> RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ mode inside the adapter, I think, it
> make sense to introduce new capability. ie. In some of the HW implementation,
> We could maintain the packet order even if source queue/scheded_type is
> ORDERED. ie. if the capability is set then application can use
> RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY to maintain the ingress order even
> if it is been called from multiple CPU in ORDERED/ATOMIC context instead of
> using RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ mode.
Sure. I will add ENQ-DEQ into caps.

> 
> 2) Please split the 2/5 to patch specification and implementation
Ok
> 
> 3) I think, we need to give a example code snippet in the programmer guide on
> how to use RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ mode in conjunction with
> rte_event_crypto_adapter_event_port_get().
I have added a test case in the test app to show usage of rte_event_crypto_adapter_event_port_get().
I will update the programmer guide too.

Regards
Abhinandan
> 
> Let me know your views?