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

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

[Abhinandan Gujjar]

Add crypto event adapter APIs to support packet transfer
mechanism between cryptodev and event device.

Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
Signed-off-by: Gage Eads <gage.eads@intel.com>
---
Notes:
	V2:
	1. Updated type as ENQ-DEQ in rte_event_crypto_adapter_type
	2. Removed enum rte_event_crypto_conf_type
	3. Updated struct rte_event_crypto_metadata
	4. Removed struct rte_event_crypto_queue_pair_conf
	5. Updated rte_event_crypto_adapter_queue_pair_add() API

 lib/librte_eventdev/Makefile                   |   1 +
 lib/librte_eventdev/rte_event_crypto_adapter.h | 452 +++++++++++++++++++++++++
 2 files changed, 453 insertions(+)
 create mode 100644 lib/librte_eventdev/rte_event_crypto_adapter.h

diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 7fd78c7..a5a6214 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -27,6 +27,7 @@ SYMLINK-y-include += rte_eventdev_pmd_pci.h
 SYMLINK-y-include += rte_eventdev_pmd_vdev.h
 SYMLINK-y-include += rte_event_ring.h
 SYMLINK-y-include += rte_event_eth_rx_adapter.h
+SYMLINK-y-include += rte_event_crypto_adapter.h
 
 # versioning export map
 EXPORT_MAP := rte_eventdev_version.map
diff --git a/lib/librte_eventdev/rte_event_crypto_adapter.h b/lib/librte_eventdev/rte_event_crypto_adapter.h
new file mode 100644
index 0000000..e90b57e
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
@@ -0,0 +1,452 @@
+/*
+ *   Copyright(c) 2018 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_EVENT_CRYPTO_ADAPTER_
+#define _RTE_EVENT_CRYPTO_ADAPTER_
+
+/**
+ * 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
+ * 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
+ * dequeue only (DEQ) mode  and the second as the enqueue - dequeue
+ * (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
+ * also, the event cannot be dropped.
+ *
+ * In the ENQ_DEQ mode the application needs to specify the cryptodev ID
+ * and queue pair ID (request information) in addition to the event
+ * information (response information) needed to enqueue an event after
+ * the crypto operation has completed. The request and response information
+ * are specified in the rte_crypto_op private_data. In the DEQ mode the
+ * the application is required to provide only the response information.
+ *
+ * In the ENQ_DEQ mode, application sends crypto operations as events to
+ * the adapter which dequeues events and programs cryptodev operations.
+ * The adapter then dequeues crypto completions from cryptodev and
+ * enqueue events to the event device.
+ *
+ * In the case of HW based transfers, the cryptodev PMD callback invoked
+ * from rte_cryptodev_enqueue_burst() uses the response information to
+ * setup the event for the cryptodev completion.
+ *
+ * The event crypto adapter provides common APIs to configure the packet flow
+ * from the cryptodev to event devices across both SW and HW based transfers.
+ * The crypto event adapter's functions are:
+ *  - rte_event_crypto_adapter_create_ext()
+ *  - rte_event_crypto_adapter_create()
+ *  - rte_event_crypto_adapter_free()
+ *  - rte_event_crypto_adapter_queue_pair_add()
+ *  - rte_event_crypto_adapter_queue_pair_del()
+ *  - rte_event_crypto_adapter_start()
+ *  - rte_event_crypto_adapter_stop()
+ *  - rte_event_crypto_adapter_stats_get()
+ *  - rte_event_crypto_adapter_stats_reset()
+
+ * The applicaton creates an instance using rte_event_crypto_adapter_create()
+ * or rte_event_crypto_adapter_create_ext().
+ *
+ * Cryptodev queue pair addition/deletion is done
+ * using the rte_event_crypto_adapter_queue_pair_xxx() APIs.
+ *
+ * The SW adapter or HW PMD uses rte_crypto_op::private_data_type to decide
+ * whether request/response data is located in the crypto session/crypto
+ * security session or at an offset in the rte_crypto_op.
+ * rte_crypto_op::private_data_offset is used to locate the request/response
+ * in the rte_crypto_op. If the rte_crypto_op::private_data_type
+ * indicates that the data is in the crypto session/crypto security session
+ * then the rte_crypto_op::sess_type is used to decide whether the private
+ * data is in the session or security session.
+ *
+ * For session-less it is mandatory to place the request/response data with
+ * the rte_crypto_op where as with crypto session/security session it can be
+ * placed with the rte_crypto_op or in the session/security session.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <rte_service.h>
+
+#include "rte_eventdev.h"
+
+#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this enum may change without prior notice
+ *
+ * Crypto event adapter type
+ */
+enum rte_event_crypto_adapter_type {
+	RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY = 1,
+	/**< Start only dequeue part of crypto adapter.
+	 * Packets dequeued from cryptodev are enqueued to eventdev
+	 * as new events and events will be treated as RTE_EVENT_OP_NEW. */
+	RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ,
+	/**< Start both enqueue & dequeue part of crypto adapter.
+	 * Packet's event context will be retained and
+	 * event will be treated as RTE_EVENT_OP_FORWARD. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Crypto event request structure will be fill by application to
+ * provide event request information to the adapter.
+ */
+struct rte_event_crypto_request {
+	uint8_t resv[8];
+	/**< Overlaps with first 8 bytes of struct rte_event
+	 * that encode the response event information
+	 */
+	uint16_t cdev_id;
+	/**< cryptodev ID to be used */
+	uint16_t queue_pair_id;
+	/**< cryptodev queue pair ID to be used */
+	uint32_t resv1;
+	/**< Reserved bits */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Crypto event metadata structure will be filled by application
+ * to provide crypto request and event response information.
+ *
+ * If crypto events are enqueued using a HW mechanism, the cryptodev
+ * PMD will use the event response information to set up the event
+ * that is enqueued back to eventdev after completion of the crypto
+ * operation. If the transfer is done by SW, it will be used by the
+ * adapter.
+ */
+union rte_event_crypto_metadata {
+	struct rte_event_crypto_request request_info;
+	struct rte_event response_info;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Adapter configuration structure that the adapter configuration callback
+ * function is expected to fill out
+ * @see rte_event_crypto_adapter_conf_cb
+ */
+struct rte_event_crypto_adapter_conf {
+	uint8_t event_port_id;
+	/**< Event port identifier, the adapter enqueues mbuf events to this
+	 * port.
+	 */
+	uint32_t max_nb;
+	/**< The adapter can return early if it has processed at least
+	 * max_nb crypto ops. This isn't treated as a requirement; batching may
+	 * cause the adapter to process more than max_nb crypto ops.
+	 */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Function type used for adapter configuration callback. The callback is
+ * used to fill in members of the struct rte_event_crypto_adapter_conf, this
+ * callback is invoked when creating a SW service for packet transfer from
+ * cryptodev queue pair to the event device. The SW service is created within
+ * the rte_event_crypto_adapter_queue_add() function if SW based packet
+ * transfers from cryptodev queue pair to the event device are required.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param dev_id
+ *  Crypto device identifier.
+ *
+ * @param [out] conf
+ *  Structure that needs to be populated by this callback.
+ *
+ * @param arg
+ *  Argument to the callback. This is the same as the conf_arg passed to the
+ *  rte_event_crypto_adapter_create_ext().
+ */
+typedef int (*rte_event_crypto_adapter_conf_cb) (uint8_t id, uint8_t cdev_id,
+			struct rte_event_crypto_adapter_conf *conf,
+			void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * A structure used to retrieve statistics for an event crypto adapter
+ * instance.
+ */
+
+struct rte_event_crypto_adapter_stats {
+	uint64_t event_poll_count;
+	/**< Event port poll count */
+	uint64_t event_dequeue_count;
+	/**< Event dequeue count */
+	uint64_t crypto_enq_fail;
+	/**< Cryptodev enqueue failed count */
+	uint64_t crypto_deq_count;
+	/**< Cryptodev dequeue count */
+	uint64_t event_enq_retry_count;
+	/**< Event enqueue retry count */
+	uint64_t event_enq_fail_count;
+	/**< Event enqueue fail count */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Create a new event crypto adapter with the specified identifier.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param cdev_id
+ *  Crypto device identifier.
+ *
+ * @param conf_cb
+ *  Callback function that fills in members of a
+ *  struct rte_event_crypto_adapter_conf struct passed into
+ *  it.
+ *
+ * @param conf_arg
+ *  Argument that is passed to the conf_cb function.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure
+ */
+int rte_event_crypto_adapter_create_ext(uint8_t id, uint8_t cdev_id,
+				rte_event_crypto_adapter_conf_cb conf_cb,
+				void *conf_arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Create a new event crypto adapter with the specified identifier.
+ * This function uses an internal configuration function that creates an event
+ * port. This default function reconfigures the event device with an
+ * additional event port and setups up the event port using the port_config
+ * parameter passed into this function. In case the application needs more
+ * control in configuration of the service, it should use the
+ * rte_event_crypto_adapter_create_ext() version.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param cdev_id
+ *  Crypto device identifier.
+ *
+ * @param port_config
+ *  Argument of type *rte_event_port_conf* that is passed to the conf_cb
+ *  function.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure
+ */
+int rte_event_crypto_adapter_create(uint8_t id, uint8_t cdev_id,
+				    struct rte_event_port_conf *port_config);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Free an event crypto adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure, If the adapter still has queue pairs
+ *      added to it, the function returns -EBUSY.
+ */
+int rte_event_crypto_adapter_free(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add a queue pair to an event crypto adapter.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param cdev_id
+ *  Cryptodev identifier.
+ *
+ * @param queue_pair_id
+ *  Cryptodev queue pair identifier. If queue_pair_id is set -1,
+ *  adapter adds all the pre configured queue pairs to the instance.
+ *
+ *
+ * @return
+ *  - 0: Success, Receive queue pair added correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_queue_pair_add(uint8_t id,
+			uint8_t cdev_id,
+			int32_t queue_pair_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Delete a queue pair from an event crypto adapter.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param cdev_id
+ *  Cryptodev identifier.
+ *
+ * @param queue_pair_id
+ *  Cryptodev queue pair identifier.
+ *
+ * @return
+ *  - 0: Success, queue pair deleted successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_queue_pair_del(uint8_t id, uint8_t cdev_id,
+					    int32_t queue_pair_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Start event crypto adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param type
+ *  Flag to indicate to start dequeue only or both enqueue & dequeue.
+ *
+ * @return
+ *  - 0: Success, Adapter started successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_start(uint8_t id,
+				   enum rte_event_crypto_adapter_type type);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Stop event crypto adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *  - 0: Success, Adapter stopped successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_stop(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Retrieve statistics for an adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param [out] stats
+ *  A pointer to structure used to retrieve statistics for an adapter.
+ *
+ * @return
+ *  - 0: Success, retrieved successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_stats_get(uint8_t id,
+				struct rte_event_crypto_adapter_stats *stats);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Reset statistics for an adapter.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *  - 0: Success, statistics reset successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_stats_reset(uint8_t id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Retrieve the service ID of an adapter. If the adapter doesn't use
+ * a rte_service function, this function returns -ESRCH.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param [out] service_id
+ *  A pointer to a uint32_t, to be filled in with the service id.
+ *
+ * @return
+ *  - 0: Success
+ *  - <0: Error code on failure, if the adapter doesn't use a rte_service
+ * function, this function returns -ESRCH.
+ */
+int rte_event_crypto_adapter_service_id_get(uint8_t id, uint32_t *service_id);
+
+#ifdef __cplusplus
+}
+#endif
+#endif	/* _RTE_EVENT_CRYPTO_ADAPTER_ */
-- 
1.9.1

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

[Gujjar, Abhinandan S]

A gentle remainder for review :)

-Abhinandan

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Monday, January 15, 2018 4:24 PM
> To: jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>; Eads, Gage <gage.eads@intel.com>
> Subject: [RFC v2, 2/2] eventdev: add crypto adapter API header
> 
> Add crypto event adapter APIs to support packet transfer mechanism between
> cryptodev and event device.
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> ---
> Notes:
> 	V2:
> 	1. Updated type as ENQ-DEQ in rte_event_crypto_adapter_type
> 	2. Removed enum rte_event_crypto_conf_type
> 	3. Updated struct rte_event_crypto_metadata
> 	4. Removed struct rte_event_crypto_queue_pair_conf
> 	5. Updated rte_event_crypto_adapter_queue_pair_add() API
> 
>  lib/librte_eventdev/Makefile                   |   1 +
>  lib/librte_eventdev/rte_event_crypto_adapter.h | 452
> +++++++++++++++++++++++++
>  2 files changed, 453 insertions(+)
>  create mode 100644 lib/librte_eventdev/rte_event_crypto_adapter.h
> 
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile index
> 7fd78c7..a5a6214 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -27,6 +27,7 @@ SYMLINK-y-include += rte_eventdev_pmd_pci.h  SYMLINK-
> y-include += rte_eventdev_pmd_vdev.h  SYMLINK-y-include += rte_event_ring.h
> SYMLINK-y-include += rte_event_eth_rx_adapter.h
> +SYMLINK-y-include += rte_event_crypto_adapter.h
> 
>  # versioning export map
>  EXPORT_MAP := rte_eventdev_version.map
> diff --git a/lib/librte_eventdev/rte_event_crypto_adapter.h
> b/lib/librte_eventdev/rte_event_crypto_adapter.h
> new file mode 100644
> index 0000000..e90b57e
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
> @@ -0,0 +1,452 @@
> +/*
> + *   Copyright(c) 2018 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#ifndef _RTE_EVENT_CRYPTO_ADAPTER_
> +#define _RTE_EVENT_CRYPTO_ADAPTER_
> +
> +/**
> + * 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
> + * 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
> + * dequeue only (DEQ) mode  and the second as the enqueue - dequeue
> + * (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
> + * also, the event cannot be dropped.
> + *
> + * In the ENQ_DEQ mode the application needs to specify the cryptodev
> +ID
> + * and queue pair ID (request information) in addition to the event
> + * information (response information) needed to enqueue an event after
> + * the crypto operation has completed. The request and response
> +information
> + * are specified in the rte_crypto_op private_data. In the DEQ mode the
> + * the application is required to provide only the response information.
> + *
> + * In the ENQ_DEQ mode, application sends crypto operations as events
> +to
> + * the adapter which dequeues events and programs cryptodev operations.
> + * The adapter then dequeues crypto completions from cryptodev and
> + * enqueue events to the event device.
> + *
> + * In the case of HW based transfers, the cryptodev PMD callback
> +invoked
> + * from rte_cryptodev_enqueue_burst() uses the response information to
> + * setup the event for the cryptodev completion.
> + *
> + * The event crypto adapter provides common APIs to configure the
> +packet flow
> + * from the cryptodev to event devices across both SW and HW based
> transfers.
> + * The crypto event adapter's functions are:
> + *  - rte_event_crypto_adapter_create_ext()
> + *  - rte_event_crypto_adapter_create()
> + *  - rte_event_crypto_adapter_free()
> + *  - rte_event_crypto_adapter_queue_pair_add()
> + *  - rte_event_crypto_adapter_queue_pair_del()
> + *  - rte_event_crypto_adapter_start()
> + *  - rte_event_crypto_adapter_stop()
> + *  - rte_event_crypto_adapter_stats_get()
> + *  - rte_event_crypto_adapter_stats_reset()
> +
> + * The applicaton creates an instance using
> + rte_event_crypto_adapter_create()
> + * or rte_event_crypto_adapter_create_ext().
> + *
> + * Cryptodev queue pair addition/deletion is done
> + * using the rte_event_crypto_adapter_queue_pair_xxx() APIs.
> + *
> + * The SW adapter or HW PMD uses rte_crypto_op::private_data_type to
> + decide
> + * whether request/response data is located in the crypto
> + session/crypto
> + * security session or at an offset in the rte_crypto_op.
> + * rte_crypto_op::private_data_offset is used to locate the
> + request/response
> + * in the rte_crypto_op. If the rte_crypto_op::private_data_type
> + * indicates that the data is in the crypto session/crypto security
> + session
> + * then the rte_crypto_op::sess_type is used to decide whether the
> + private
> + * data is in the session or security session.
> + *
> + * For session-less it is mandatory to place the request/response data
> + with
> + * the rte_crypto_op where as with crypto session/security session it
> + can be
> + * placed with the rte_crypto_op or in the session/security session.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this enum may change without prior notice
> + *
> + * Crypto event adapter type
> + */
> +enum rte_event_crypto_adapter_type {
> +	RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY = 1,
> +	/**< Start only dequeue part of crypto adapter.
> +	 * Packets dequeued from cryptodev are enqueued to eventdev
> +	 * as new events and events will be treated as RTE_EVENT_OP_NEW. */
> +	RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ,
> +	/**< Start both enqueue & dequeue part of crypto adapter.
> +	 * Packet's event context will be retained and
> +	 * event will be treated as RTE_EVENT_OP_FORWARD. */ };
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event request structure will be fill by application to
> + * provide event request information to the adapter.
> + */
> +struct rte_event_crypto_request {
> +	uint8_t resv[8];
> +	/**< Overlaps with first 8 bytes of struct rte_event
> +	 * that encode the response event information
> +	 */
> +	uint16_t cdev_id;
> +	/**< cryptodev ID to be used */
> +	uint16_t queue_pair_id;
> +	/**< cryptodev queue pair ID to be used */
> +	uint32_t resv1;
> +	/**< Reserved bits */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event metadata structure will be filled by application
> + * to provide crypto request and event response information.
> + *
> + * If crypto events are enqueued using a HW mechanism, the cryptodev
> + * PMD will use the event response information to set up the event
> + * that is enqueued back to eventdev after completion of the crypto
> + * operation. If the transfer is done by SW, it will be used by the
> + * adapter.
> + */
> +union rte_event_crypto_metadata {
> +	struct rte_event_crypto_request request_info;
> +	struct rte_event response_info;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Adapter configuration structure that the adapter configuration
> +callback
> + * function is expected to fill out
> + * @see rte_event_crypto_adapter_conf_cb  */ struct
> +rte_event_crypto_adapter_conf {
> +	uint8_t event_port_id;
> +	/**< Event port identifier, the adapter enqueues mbuf events to this
> +	 * port.
> +	 */
> +	uint32_t max_nb;
> +	/**< The adapter can return early if it has processed at least
> +	 * max_nb crypto ops. This isn't treated as a requirement; batching may
> +	 * cause the adapter to process more than max_nb crypto ops.
> +	 */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Function type used for adapter configuration callback. The callback
> +is
> + * used to fill in members of the struct rte_event_crypto_adapter_conf,
> +this
> + * callback is invoked when creating a SW service for packet transfer
> +from
> + * cryptodev queue pair to the event device. The SW service is created
> +within
> + * the rte_event_crypto_adapter_queue_add() function if SW based packet
> + * transfers from cryptodev queue pair to the event device are required.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param dev_id
> + *  Crypto device identifier.
> + *
> + * @param [out] conf
> + *  Structure that needs to be populated by this callback.
> + *
> + * @param arg
> + *  Argument to the callback. This is the same as the conf_arg passed
> +to the
> + *  rte_event_crypto_adapter_create_ext().
> + */
> +typedef int (*rte_event_crypto_adapter_conf_cb) (uint8_t id, uint8_t cdev_id,
> +			struct rte_event_crypto_adapter_conf *conf,
> +			void *arg);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * A structure used to retrieve statistics for an event crypto adapter
> + * instance.
> + */
> +
> +struct rte_event_crypto_adapter_stats {
> +	uint64_t event_poll_count;
> +	/**< Event port poll count */
> +	uint64_t event_dequeue_count;
> +	/**< Event dequeue count */
> +	uint64_t crypto_enq_fail;
> +	/**< Cryptodev enqueue failed count */
> +	uint64_t crypto_deq_count;
> +	/**< Cryptodev dequeue count */
> +	uint64_t event_enq_retry_count;
> +	/**< Event enqueue retry count */
> +	uint64_t event_enq_fail_count;
> +	/**< Event enqueue fail count */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Create a new event crypto adapter with the specified identifier.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param cdev_id
> + *  Crypto device identifier.
> + *
> + * @param conf_cb
> + *  Callback function that fills in members of a
> + *  struct rte_event_crypto_adapter_conf struct passed into
> + *  it.
> + *
> + * @param conf_arg
> + *  Argument that is passed to the conf_cb function.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure
> + */
> +int rte_event_crypto_adapter_create_ext(uint8_t id, uint8_t cdev_id,
> +				rte_event_crypto_adapter_conf_cb conf_cb,
> +				void *conf_arg);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Create a new event crypto adapter with the specified identifier.
> + * This function uses an internal configuration function that creates
> +an event
> + * port. This default function reconfigures the event device with an
> + * additional event port and setups up the event port using the
> +port_config
> + * parameter passed into this function. In case the application needs
> +more
> + * control in configuration of the service, it should use the
> + * rte_event_crypto_adapter_create_ext() version.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param cdev_id
> + *  Crypto device identifier.
> + *
> + * @param port_config
> + *  Argument of type *rte_event_port_conf* that is passed to the
> +conf_cb
> + *  function.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure
> + */
> +int rte_event_crypto_adapter_create(uint8_t id, uint8_t cdev_id,
> +				    struct rte_event_port_conf *port_config);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Free an event crypto adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure, If the adapter still has queue pairs
> + *      added to it, the function returns -EBUSY.
> + */
> +int rte_event_crypto_adapter_free(uint8_t id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Add a queue pair to an event crypto adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param cdev_id
> + *  Cryptodev identifier.
> + *
> + * @param queue_pair_id
> + *  Cryptodev queue pair identifier. If queue_pair_id is set -1,
> + *  adapter adds all the pre configured queue pairs to the instance.
> + *
> + *
> + * @return
> + *  - 0: Success, Receive queue pair added correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_queue_pair_add(uint8_t id,
> +			uint8_t cdev_id,
> +			int32_t queue_pair_id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Delete a queue pair from an event crypto adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param cdev_id
> + *  Cryptodev identifier.
> + *
> + * @param queue_pair_id
> + *  Cryptodev queue pair identifier.
> + *
> + * @return
> + *  - 0: Success, queue pair deleted successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_queue_pair_del(uint8_t id, uint8_t cdev_id,
> +					    int32_t queue_pair_id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Start event crypto adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param type
> + *  Flag to indicate to start dequeue only or both enqueue & dequeue.
> + *
> + * @return
> + *  - 0: Success, Adapter started successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_start(uint8_t id,
> +				   enum rte_event_crypto_adapter_type type);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Stop event crypto adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, Adapter stopped successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_stop(uint8_t id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Retrieve statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param [out] stats
> + *  A pointer to structure used to retrieve statistics for an adapter.
> + *
> + * @return
> + *  - 0: Success, retrieved successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_stats_get(uint8_t id,
> +				struct rte_event_crypto_adapter_stats *stats);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Reset statistics for an adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_crypto_adapter_stats_reset(uint8_t id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Retrieve the service ID of an adapter. If the adapter doesn't use
> + * a rte_service function, this function returns -ESRCH.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param [out] service_id
> + *  A pointer to a uint32_t, to be filled in with the service id.
> + *
> + * @return
> + *  - 0: Success
> + *  - <0: Error code on failure, if the adapter doesn't use a
> +rte_service
> + * function, this function returns -ESRCH.
> + */
> +int rte_event_crypto_adapter_service_id_get(uint8_t id, uint32_t
> +*service_id);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif	/* _RTE_EVENT_CRYPTO_ADAPTER_ */
> --
> 1.9.1

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

[Jerin Jacob]

-----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
> 
> Add crypto event adapter APIs to support packet transfer
> mechanism between cryptodev and event device.
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> ---

Overall it looks good to me. Though I have some confusion over
ENQ-DEQ scheme. May be it will be get cleared along with implementation.

> Notes:
> 	V2:
> 	1. Updated type as ENQ-DEQ in rte_event_crypto_adapter_type
> 	2. Removed enum rte_event_crypto_conf_type
> 	3. Updated struct rte_event_crypto_metadata
> 	4. Removed struct rte_event_crypto_queue_pair_conf
> 	5. Updated rte_event_crypto_adapter_queue_pair_add() API
> 
>  lib/librte_eventdev/Makefile                   |   1 +
>  lib/librte_eventdev/rte_event_crypto_adapter.h | 452 +++++++++++++++++++++++++

1) Please update MAINTAINERS file
2) Add meson build support
3) Update doc/api/doxy-api-index.md file and check the doxygen output
using "make doc-api-html"
4) Please add the programmers guide in doc/guides/prog_guide/

>  
> +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
> @@ -0,0 +1,452 @@
> +/*
> + *   Copyright(c) 2018 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without

Use SPDX tag scheme

> +#ifndef _RTE_EVENT_CRYPTO_ADAPTER_
> +#define _RTE_EVENT_CRYPTO_ADAPTER_
> +
> +/**
> + * 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

> + * 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.

> + * dequeue only (DEQ) mode  and the second as the enqueue - dequeue

extra space between "mode" and "and"

> + * (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

s/eventdevs/eventdev ??

> + * also, the event cannot be dropped.
> + *
> + * In the ENQ_DEQ mode the application needs to specify the cryptodev ID
> + * and queue pair ID (request information) in addition to the event
> + * information (response information) needed to enqueue an event after
> + * the crypto operation has completed. The request and response information
> + * are specified in the rte_crypto_op private_data. In the DEQ mode the
> + * the application is required to provide only the response information.

Why ENQ_DEQ modes needs cryptodev ID and queue pair ID? Which is part of
rte_cryptodev_enqueue_burst(). May be I am missing something.

If ENQ_DEQ modes help in SW case then we can add it as capability.
I am just trying to see any scope of convergence.

> + *
> + * In the ENQ_DEQ mode, application sends crypto operations as events to
> + * the adapter which dequeues events and programs cryptodev operations.
> + * The adapter then dequeues crypto completions from cryptodev and
> + * enqueue events to the event device.
> + *
> + * In the case of HW based transfers, the cryptodev PMD callback invoked
> + * from rte_cryptodev_enqueue_burst() uses the response information to
> + * setup the event for the cryptodev completion.
> + *
> + * The event crypto adapter provides common APIs to configure the packet flow
> + * from the cryptodev to event devices across both SW and HW based transfers.
> + * The crypto event adapter's functions are:
> + *  - rte_event_crypto_adapter_create_ext()
> + *  - rte_event_crypto_adapter_create()
> + *  - rte_event_crypto_adapter_free()
> + *  - rte_event_crypto_adapter_queue_pair_add()
> + *  - rte_event_crypto_adapter_queue_pair_del()
> + *  - rte_event_crypto_adapter_start()
> + *  - rte_event_crypto_adapter_stop()
> + *  - rte_event_crypto_adapter_stats_get()
> + *  - rte_event_crypto_adapter_stats_reset()
> +
> + * The applicaton creates an instance using rte_event_crypto_adapter_create()

s/applicaton/application
 
> + * or rte_event_crypto_adapter_create_ext().
> + *
> + * Cryptodev queue pair addition/deletion is done
> + * using the rte_event_crypto_adapter_queue_pair_xxx() APIs.
> + *
> + * The SW adapter or HW PMD uses rte_crypto_op::private_data_type to decide
> + * whether request/response data is located in the crypto session/crypto
> + * security session or at an offset in the rte_crypto_op.
> + * rte_crypto_op::private_data_offset is used to locate the request/response
> + * in the rte_crypto_op. If the rte_crypto_op::private_data_type
> + * indicates that the data is in the crypto session/crypto security session
> + * then the rte_crypto_op::sess_type is used to decide whether the private
> + * data is in the session or security session.
> + *
> + * For session-less it is mandatory to place the request/response data with
> + * the rte_crypto_op where as with crypto session/security session it can be
> + * placed with the rte_crypto_op or in the session/security session.

Please update(if needed) above two paragraphs based on the conclusion
from cryptodev change

> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this enum may change without prior notice

Change to new EXPERIMENTAL tag scheme.

> + *
> + * Crypto event adapter type
> + */
> +enum rte_event_crypto_adapter_type {
> +	RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY = 1,
> +	/**< Start only dequeue part of crypto adapter.
> +	 * Packets dequeued from cryptodev are enqueued to eventdev
> +	 * as new events and events will be treated as RTE_EVENT_OP_NEW. */
> +	RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ,
> +	/**< Start both enqueue & dequeue part of crypto adapter.
> +	 * Packet's event context will be retained and
> +	 * event will be treated as RTE_EVENT_OP_FORWARD. */

This description makes sense.

> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event request structure will be fill by application to
> + * provide event request information to the adapter.
> + */
> +struct rte_event_crypto_request {
> +	uint8_t resv[8];
> +	/**< Overlaps with first 8 bytes of struct rte_event
> +	 * that encode the response event information
> +	 */
> +	uint16_t cdev_id;
> +	/**< cryptodev ID to be used */
> +	uint16_t queue_pair_id;
> +	/**< cryptodev queue pair ID to be used */

Shouldn't we say it is need only with ENQ_DEQ mode as described earlier.
adding "@see" section would help

> +	uint32_t resv1;
> +	/**< Reserved bits */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event metadata structure will be filled by application
> + * to provide crypto request and event response information.
> + *
> + * If crypto events are enqueued using a HW mechanism, the cryptodev
> + * PMD will use the event response information to set up the event
> + * that is enqueued back to eventdev after completion of the crypto
> + * operation. If the transfer is done by SW, it will be used by the
> + * adapter.
> + */
> +union rte_event_crypto_metadata {
> +	struct rte_event_crypto_request request_info;
> +	struct rte_event response_info;

Perfect.

Other than above comments, everything else looks OK to me.

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

[Gujjar, Abhinandan S]

Hi Jerin,

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
> >
> > Add crypto event adapter APIs to support packet transfer mechanism
> > between cryptodev and event device.
> >
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> > Signed-off-by: Gage Eads <gage.eads@intel.com>
> > ---
> 
> Overall it looks good to me. Though I have some confusion over ENQ-DEQ
> scheme. May be it will be get cleared along with implementation.
> 
> > Notes:
> > 	V2:
> > 	1. Updated type as ENQ-DEQ in rte_event_crypto_adapter_type
> > 	2. Removed enum rte_event_crypto_conf_type
> > 	3. Updated struct rte_event_crypto_metadata
> > 	4. Removed struct rte_event_crypto_queue_pair_conf
> > 	5. Updated rte_event_crypto_adapter_queue_pair_add() API
> >
> >  lib/librte_eventdev/Makefile                   |   1 +
> >  lib/librte_eventdev/rte_event_crypto_adapter.h | 452
> > +++++++++++++++++++++++++
> 
> 1) Please update MAINTAINERS file
> 2) Add meson build support
> 3) Update doc/api/doxy-api-index.md file and check the doxygen output using
> "make doc-api-html"
> 4) Please add the programmers guide in doc/guides/prog_guide/
Sure
> 
> >
> > +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
> > @@ -0,0 +1,452 @@
> > +/*
> > + *   Copyright(c) 2018 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> 
> Use SPDX tag scheme
Ok
> 
> > +#ifndef _RTE_EVENT_CRYPTO_ADAPTER_
> > +#define _RTE_EVENT_CRYPTO_ADAPTER_
> > +
> > +/**
> > + * 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.

> 
> > + * 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);
*/
> 
> > + * 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
> 
> s/eventdevs/eventdev ??
Ok
> 
> > + * also, the event cannot be dropped.
> > + *
> > + * In the ENQ_DEQ mode the application needs to specify the cryptodev
> > + ID
> > + * and queue pair ID (request information) in addition to the event
> > + * information (response information) needed to enqueue an event
> > + after
> > + * the crypto operation has completed. The request and response
> > + information
> > + * are specified in the rte_crypto_op private_data. In the DEQ mode
> > + the
> > + * the application is required to provide only the response information.
> 
> Why ENQ_DEQ modes needs cryptodev ID and queue pair ID? Which is part of
> rte_cryptodev_enqueue_burst(). May be I am missing something.
> 
> If ENQ_DEQ modes help in SW case then we can add it as capability.
> I am just trying to see any scope of convergence.
In ENQ_DEQ mode, adapter dequeues eventdev and dereferences cryptodev ID and queue pair ID
to enqueue the crypto operations to cryptodev using rte_cryptodev_enqueue_burst() API.
I hope, above code snippet would have made things more clear.
> 
> > + *
> > + * In the ENQ_DEQ mode, application sends crypto operations as events
> > + to
> > + * the adapter which dequeues events and programs cryptodev operations.
> > + * The adapter then dequeues crypto completions from cryptodev and
> > + * enqueue events to the event device.
> > + *
> > + * In the case of HW based transfers, the cryptodev PMD callback
> > + invoked
> > + * from rte_cryptodev_enqueue_burst() uses the response information
> > + to
> > + * setup the event for the cryptodev completion.
> > + *
> > + * The event crypto adapter provides common APIs to configure the
> > + packet flow
> > + * from the cryptodev to event devices across both SW and HW based
> transfers.
> > + * The crypto event adapter's functions are:
> > + *  - rte_event_crypto_adapter_create_ext()
> > + *  - rte_event_crypto_adapter_create()
> > + *  - rte_event_crypto_adapter_free()
> > + *  - rte_event_crypto_adapter_queue_pair_add()
> > + *  - rte_event_crypto_adapter_queue_pair_del()
> > + *  - rte_event_crypto_adapter_start()
> > + *  - rte_event_crypto_adapter_stop()
> > + *  - rte_event_crypto_adapter_stats_get()
> > + *  - rte_event_crypto_adapter_stats_reset()
> > +
> > + * The applicaton creates an instance using
> > + rte_event_crypto_adapter_create()
> 
> s/applicaton/application
Ok
> 
> > + * or rte_event_crypto_adapter_create_ext().
> > + *
> > + * Cryptodev queue pair addition/deletion is done
> > + * using the rte_event_crypto_adapter_queue_pair_xxx() APIs.
> > + *
> > + * The SW adapter or HW PMD uses rte_crypto_op::private_data_type to
> > + decide
> > + * whether request/response data is located in the crypto
> > + session/crypto
> > + * security session or at an offset in the rte_crypto_op.
> > + * rte_crypto_op::private_data_offset is used to locate the
> > + request/response
> > + * in the rte_crypto_op. If the rte_crypto_op::private_data_type
> > + * indicates that the data is in the crypto session/crypto security
> > + session
> > + * then the rte_crypto_op::sess_type is used to decide whether the
> > + private
> > + * data is in the session or security session.
> > + *
> > + * For session-less it is mandatory to place the request/response
> > + data with
> > + * the rte_crypto_op where as with crypto session/security session it
> > + can be
> > + * placed with the rte_crypto_op or in the session/security session.
> 
> Please update(if needed) above two paragraphs based on the conclusion from
> cryptodev change
Sure
> 
> > + */
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <stdint.h>
> > +#include <rte_service.h>
> > +
> > +#include "rte_eventdev.h"
> > +
> > +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this enum may change without prior notice
> 
> Change to new EXPERIMENTAL tag scheme.
Ok
> 
> > + *
> > + * Crypto event adapter type
> > + */
> > +enum rte_event_crypto_adapter_type {
> > +	RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY = 1,
> > +	/**< Start only dequeue part of crypto adapter.
> > +	 * Packets dequeued from cryptodev are enqueued to eventdev
> > +	 * as new events and events will be treated as RTE_EVENT_OP_NEW. */
> > +	RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ,
> > +	/**< Start both enqueue & dequeue part of crypto adapter.
> > +	 * Packet's event context will be retained and
> > +	 * event will be treated as RTE_EVENT_OP_FORWARD. */
> 
> This description makes sense.
> 
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * Crypto event request structure will be fill by application to
> > + * provide event request information to the adapter.
> > + */
> > +struct rte_event_crypto_request {
> > +	uint8_t resv[8];
> > +	/**< Overlaps with first 8 bytes of struct rte_event
> > +	 * that encode the response event information
> > +	 */
> > +	uint16_t cdev_id;
> > +	/**< cryptodev ID to be used */
> > +	uint16_t queue_pair_id;
> > +	/**< cryptodev queue pair ID to be used */
> 
> Shouldn't we say it is need only with ENQ_DEQ mode as described earlier.
Yes. I will update it.
> adding "@see" section would help
Ok
> 
> > +	uint32_t resv1;
> > +	/**< Reserved bits */
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * Crypto event metadata structure will be filled by application
> > + * to provide crypto request and event response information.
> > + *
> > + * If crypto events are enqueued using a HW mechanism, the cryptodev
> > + * PMD will use the event response information to set up the event
> > + * that is enqueued back to eventdev after completion of the crypto
> > + * operation. If the transfer is done by SW, it will be used by the
> > + * adapter.
> > + */
> > +union rte_event_crypto_metadata {
> > +	struct rte_event_crypto_request request_info;
> > +	struct rte_event response_info;
> 
> Perfect.
> 
> Other than above comments, everything else looks OK to me.

-Abhinandan

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

[Jerin Jacob]

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

> */
> > 
> > > + * 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
> > 

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

[Vangati, Narender]

Jerin,
I see the "ENQ" part of the adapter a little differently. I think there is value to offloading cryptodev_enqueue to an adapter service, even when the h/w natively supports DEQ. 
When the same queue pair is being used by the workers to enqueue requests (this could be where the pre crypto stage was ordered scheduled and the cryptodev enqueues need to be ordered), you would need some synchronization. Going thru eventdev to an adapter which enqueues on worker behalf provides that synchronization and ordering.

In that sense, the ENQ-DEQ mode is an application choice and defines its programming model. Based on that, a service would be created that does ENQ. For the DEQ part, perhaps the adapter can tell whether the h/w supports DEQ natively or needs to be done in s/w - that way you can have a single adapter.

Application model for DEQ mode - call cryptodev_enqueue directly. DEQ is h/w or s/w based on capability
Application model for ENQ-DEQ mode - use event_enqueue to offload cryptodev_enqueue to adapter. DEQ is h/w or s/w based on capability


vnr
---

-----Original Message-----
From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] 
Sent: Tuesday, February 20, 2018 7:59 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, 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?

> */
> > 
> > > + * 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
> > 

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

[Jerin Jacob]

-----Original Message-----
> Date: Tue, 20 Feb 2018 18:55:09 +0000
> From: "Vangati, Narender" <narender.vangati@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan S"
>  <abhinandan.gujjar@intel.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "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 Narender,

> Jerin,
> I see the "ENQ" part of the adapter a little differently. I think there is value to offloading cryptodev_enqueue to an adapter service, even when the h/w natively supports DEQ. 

But there is no element of _offload_ here. Right? meaning, We can express the ENQ-DEQ mode with existing cryptodev+eventdev normative API.
I think, Pushing to adapter will result in running ENQ-DEQ only on "service lcore" instead of "worker lcore".
How about making "ENQ-DEQ" scheme as set of helper C functions. This will enable,
1) To run the ENQ-DEQ logic in "service" or "worker" lcores
2) Remove the capability in adapter code. ie. If application wants to use ENQ-DEQ scheme
as a helper function it will work for both cases(HW and SW)

> When the same queue pair is being used by the workers to enqueue requests (this could be where the pre crypto stage was ordered scheduled and the cryptodev enqueues need to be ordered), you would need some synchronization. Going thru eventdev to an adapter which enqueues on worker behalf provides that synchronization and ordering.

Makes sense. Only question is that, do we need to expose as service function or generic helper C function?.
IMO, service functions makes sense when SW does something when there is no
HW support and to get the job done like schedule() function.

> 
> In that sense, the ENQ-DEQ mode is an application choice and defines its programming model. Based on that, a service would be created that does ENQ. For the DEQ part, perhaps the adapter can tell whether the h/w supports DEQ natively or needs to be done in s/w - that way you can have a single adapter.
> 
> Application model for DEQ mode - call cryptodev_enqueue directly. DEQ is h/w or s/w based on capability
> Application model for ENQ-DEQ mode - use event_enqueue to offload cryptodev_enqueue to adapter. DEQ is h/w or s/w based on capability
> 
> 
> vnr
> ---
> 
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] 
> Sent: Tuesday, February 20, 2018 7:59 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, 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?
> 
> > */
> > > 
> > > > + * 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
> > > 

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

[Akhil Goyal]

Hi Folks,


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 would be replying on this on Monday.
> 
>> */
>>>
>>>> + * 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
>>>
> 

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

[Akhil Goyal]

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

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

[Gujjar, Abhinandan S]

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

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

[Vangati, Narender]

Akhil,
I'm probably missing a point somewhere but I don't follow the suggestions. To me, ethdev, cryptodev, eventdev, etc. are device abstractions, whereas the proposed ENQ mode isn't at the same level.
The DEQ mode is a device abstraction for cryptodev->eventdev (whether h/w or s/w based), but the ENQ part of the adapter is purely a s/w programming model and optional to the application. It is independent of any device and it’s an application choice whether it wants to use this or not. Nothing prevents the application from calling cryptodev_enqueue_burst towards any device directly (whether it be soft crypto, NXP, Cavium, QAT, etc ) within an eventdev based environment. 
The ENQ mode allows an application programming model to be completely event based. If the application chooses, it enables the ENQ part where it enqueues an rte_event to the s/w adapter and the adapter then calls cryptodev_enqueue_burst on its behalf, towards any device PMD which was created. 
There are certain benefits to application architecture using this adapter where you can leverage the ordered scheduling within eventdev etc., (and certain cons where you need to run this service somewhere) but that’s up to the application to decide. 

In other words, I don’t consider ENQ mode as a device abstraction like cryptodev or ethdev where it needs to plug in to something transparently but a programming model that is provided as a choice, and that shouldn’t be tied up into a device abstraction layer.

vnr
---


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

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

[Akhil Goyal]

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

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

[Akhil Goyal]

Hi Narender,
On 3/4/2018 4:12 AM, Vangati, Narender wrote:
> Akhil,
> I'm probably missing a point somewhere but I don't follow the suggestions. To me, ethdev, cryptodev, eventdev, etc. are device abstractions, whereas the proposed ENQ mode isn't at the same level.
> The DEQ mode is a device abstraction for cryptodev->eventdev (whether h/w or s/w based), but the ENQ part of the adapter is purely a s/w programming model and optional to the application. It is independent of any device and it’s an application choice whether it wants to use this or not. Nothing prevents the application from calling cryptodev_enqueue_burst towards any device directly (whether it be soft crypto, NXP, Cavium, QAT, etc ) within an eventdev based environment.
> The ENQ mode allows an application programming model to be completely event based. If the application chooses, it enables the ENQ part where it enqueues an rte_event to the s/w adapter and the adapter then calls cryptodev_enqueue_burst on its behalf, towards any device PMD which was created.
> There are certain benefits to application architecture using this adapter where you can leverage the ordered scheduling within eventdev etc., (and certain cons where you need to run this service somewhere) but that’s up to the application to decide.
> 
> In other words, I don’t consider ENQ mode as a device abstraction like cryptodev or ethdev where it needs to plug in to something transparently but a programming model that is provided as a choice, and that shouldn’t be tied up into a device abstraction layer.
> 
> vnr

I am not against of eventdev enqueue API, or let the application decide 
to use it or not.
I am trying to limit more options in already multi-option IPSEC 
usecases. It is getting too confusing.

Also, my concern is that sw based crypto-event can also follow the same 
path as the hw based in this case. This will help the application to 
have a common code for both the cases.


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