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

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> Sent: Tuesday, April 10, 2018 4:07 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>;
> jerin.jacob at caviumnetworks.com; hemant.agrawal at nxp.com; dev at dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>; Doherty, Declan
> <declan.doherty at intel.com>; Vangati, Narender
> <narender.vangati at intel.com>; Rao, Nikhil <nikhil.rao at intel.com>
> Subject: Re: [dpdk-dev, v1, 1/5] eventdev: add caps API and PMD callbacks for
> crypto adapter
> 
> It would be good if we have a cover letter and description in the commit
> message.
Sure..
> 
> The patchset does not apply on eventdev subtree. Could you please rebase.
All the patches were based on " dpdk-next-eventdev ".
> 
> I think there is a need to add struct rte_crypto_op in the rte_event word 1 union
> as you are adding crypto event. All crypto drivers would process crypto op and
> not mbuf.
The " event_ptr" of *rte_event* will point to *rte_crypto_op* from application side.
From adapter side, the same is extracted using "crypto_op = ev[i].event_ptr".
Please refer adapter implementation and test/test/.c files & review them too.

> 
> Also, I guess the patchset is dependent on another patchset. Please mention
> that also in the cover letter.
You mean cryptodev? If yes, sure..

> 
> On 4/4/2018 12:26 PM, Abhinandan Gujjar wrote:
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar at 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;
> It is expected that caps memory is initialized by the caller. This should be
> mentioned in the description of the API.
Ok
> > +	*caps = 0;
> > +
> > +	return dev->dev_ops->crypto_adapter_caps_get ?
> > +		(*dev->dev_ops->crypto_adapter_caps_get)
> > +		(dev, cdev, caps) : 0;
> 
> It should be -ENOTSUP instead of 0. I think same needs to be corrected for eth
> rx adapter.
Ok. I will update this in the crypto caps API.
> 
> > +}
> > +
> >   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.
> > + */
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Retrieve the event device's crypto adapter capabilities for the
> > + * specified cryptodev device
> > + *
> > + * @param dev_id
> > + *   The identifier of the device.
> > + *
> > + * @param cdev_id
> > + *   The identifier of the cryptodev device.
> > + *
> > + * @param[out] caps
> > + *   A pointer to memory filled with event adapter capabilities.
> > + *
> > + * @return
> > + *   - 0: Success, driver provides event adapter capabilities for the
> > + *	cryptodev device.
> > + *   - <0: Error code returned by the driver function.
> > + *
> > + */
> > +int __rte_experimental
> > +rte_event_crypto_adapter_caps_get(uint8_t dev_id, uint8_t cdev_id,
> > +				  uint32_t *caps);
> > +
> >   struct rte_eventdev_driver;
> >   struct rte_eventdev_ops;
> >   struct rte_eventdev;
> > diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h
> > b/lib/librte_eventdev/rte_eventdev_pmd.h
> > index 3a8ddd7..656d160 100644
> > --- a/lib/librte_eventdev/rte_eventdev_pmd.h
> > +++ b/lib/librte_eventdev/rte_eventdev_pmd.h
> > @@ -585,6 +585,167 @@ typedef int
> (*eventdev_eth_rx_adapter_stats_reset)
> >    */
> >   typedef int (*eventdev_selftest)(void);
> >
> > +
> > +struct rte_cryptodev;
> > +/**
> > + * This API may change without prior notice
> > + *
> > + * Retrieve the event device's crypto adapter capabilities for the
> > + * specified cryptodev
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   cryptodev pointer
> > + *
> > + * @param[out] caps
> > + *   A pointer to memory filled with event adapter capabilities.
> > + *
> > + * @return
> > + *   - 0: Success, driver provides event adapter capabilities for the
> > + *	cryptodev.
> > + *   - <0: Error code returned by the driver function.
> > + *
> > + */
> > +typedef int (*eventdev_crypto_adapter_caps_get_t)
> > +					(const struct rte_eventdev *dev,
> > +					const struct rte_cryptodev *cdev,
> > +					uint32_t *caps);
> > +
> > +/**
> > + * This API may change without prior notice
> > + *
> > + * Add crypto queue pair to event device. This callback is invoked if
> > + * the caps returned from rte_event_crypto_adapter_caps_get(,
> > +cdev_id)
> > + * has RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT set.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   cryptodev pointer
> > + *
> > + * @param[out] queue_pair_id
> > + *   cryptodev queue pair indentifier.
> spell check.
Ok
> > + *
> > + * @return
> > + *   - 0: Success, cryptodev queue pair added successfully.
> > + *   - <0: Error code returned by the driver function.
> > + *
> > + */
> > +typedef int (*eventdev_crypto_adapter_queue_pair_add_t)
> > +					(const struct rte_eventdev *dev,
> > +					const struct rte_cryptodev *cdev,
> > +					int32_t queue_pair_id);
> > +
> > +
> > +/**
> > + * This API may change without prior notice
> > + *
> > + * Delete crypto queue pair to event device. This callback is invoked
> > +if
> > + * the caps returned from rte_event_crypto_adapter_caps_get(,
> > +cdev_id)
> > + * has RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT set.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   cryptodev pointer
> > + *
> > + * @param[out] queue_pair_id
> > + *   cryptodev queue pair indentifier.
> spell check
Ok
> > + *
> > + * @return
> > + *   - 0: Success, cryptodev queue pair deleted successfully.
> > + *   - <0: Error code returned by the driver function.
> > + *
> > + */
> > +typedef int (*eventdev_crypto_adapter_queue_pair_del_t)
> > +					(const struct rte_eventdev *dev,
> > +					const struct rte_cryptodev *cdev,
> > +					int32_t queue_pair_id);
> > +
> > +/**
> > + * Start crypto adapter. This callback is invoked if
> > + * the caps returned from rte_event_crypto_adapter_caps_get(..,
> > +cdev_id)
> > + * has RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT set and queue
> pairs
> > + * from cdev_id have been added to the event device.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   Crypto device pointer
> > + *
> > + * @return
> > + *   - 0: Success, crypto adapter started successfully.
> > + *   - <0: Error code returned by the driver function.
> > + */
> > +typedef int (*eventdev_crypto_adapter_start_t)
> > +					(const struct rte_eventdev *dev,
> > +					 const struct rte_cryptodev *cdev);
> > +
> > +/**
> > + * Stop crypto adapter. This callback is invoked if
> > + * the caps returned from rte_event_crypto_adapter_caps_get(..,
> > +cdev_id)
> > + * has RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT set and queue
> pairs
> > + * from cdev_id have been added to the event device.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   Crypto device pointer
> > + *
> > + * @return
> > + *   - 0: Success, crypto adapter stopped successfully.
> > + *   - <0: Error code returned by the driver function.
> > + */
> > +typedef int (*eventdev_crypto_adapter_stop_t)
> > +					(const struct rte_eventdev *dev,
> > +					 const struct rte_cryptodev *cdev);
> > +
> > +struct rte_event_crypto_adapter_stats;
> > +
> > +/**
> > + * Retrieve crypto adapter statistics.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   Crypto device pointer
> > + *
> > + * @param[out] stats
> > + *   Pointer to stats structure
> > + *
> > + * @return
> > + *   Return 0 on success.
> > + */
> > +
> > +typedef int (*eventdev_crypto_adapter_stats_get)
> > +			(const struct rte_eventdev *dev,
> > +			const struct rte_cryptodev *cdev,
> > +			struct rte_event_crypto_adapter_stats *stats);
> > +
> > +/**
> > + * Reset crypto adapter statistics.
> > + *
> > + * @param dev
> > + *   Event device pointer
> > + *
> > + * @param cdev
> > + *   Crypto device pointer
> > + *
> > + * @return
> > + *   Return 0 on success.
> > + */
> > +
> > +typedef int (*eventdev_crypto_adapter_stats_reset)
> > +			(const struct rte_eventdev *dev,
> > +			const struct rte_cryptodev *cdev);
> > +
> >   /** Event device operations function pointer table */
> >   struct rte_eventdev_ops {
> >   	eventdev_info_get_t dev_infos_get;	/**< Get device info. */
> > @@ -640,6 +801,21 @@ struct rte_eventdev_ops {
> >   	eventdev_eth_rx_adapter_stats_reset eth_rx_adapter_stats_reset;
> >   	/**< Reset ethernet Rx stats */
> >
> > +	eventdev_crypto_adapter_caps_get_t crypto_adapter_caps_get;
> > +	/**< Get crypto adapter capabilities */
> > +	eventdev_crypto_adapter_queue_pair_add_t
> crypto_adapter_queue_pair_add;
> > +	/**< Add queue pair to crypto adapter */
> > +	eventdev_crypto_adapter_queue_pair_del_t
> crypto_adapter_queue_pair_del;
> > +	/**< Delete queue pair from crypto adapter */
> > +	eventdev_crypto_adapter_start_t crypto_adapter_start;
> > +	/**< Start crypto adapter */
> > +	eventdev_crypto_adapter_stop_t crypto_adapter_stop;
> > +	/**< Stop crypto adapter */
> > +	eventdev_crypto_adapter_stats_get crypto_adapter_stats_get;
> > +	/**< Get crypto stats */
> > +	eventdev_crypto_adapter_stats_reset crypto_adapter_stats_reset;
> > +	/**< Reset crypto stats */
> > +
> >   	eventdev_selftest dev_selftest;
> >   	/**< Start eventdev Selftest */
> >
> >