[dpdk-dev] [RFC] eventdev: add crypto adapter API header

[dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Abhinandan Gujjar]

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>
---
 lib/librte_eventdev/Makefile                   |   1 +
 lib/librte_eventdev/rte_event_crypto_adapter.h | 474 +++++++++++++++++++++++++
 2 files changed, 475 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 5ac22cd..9cbe1a6 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -53,6 +53,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..080c3ed
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
@@ -0,0 +1,474 @@
+/*
+ *   Copyright(c) 2017 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 completion to event device.
+ * The packet flow from cryptodev to the event device can be accomplished
+ * using either HW or SW 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.
+ *
+ * The event crypto adapter provides common APIs to configure the packet flow
+ * from the cryptodev to event devices on both HW and SW.
+ * 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 rte_event_crypto_adapter_queue_pair_xxx() API.
+ *
+ * Adapter uses rte_event_crypto_queue_pair_conf to decide whether the event
+ * enqueue is based on RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ or
+ * RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ.
+ * In case of RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ,
+ * rte_event_crypto_queue_pair_conf::ev will be used for event enqueue.
+ * In case of RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ,
+ * members of rte_event_crypto_metadata will be used for event enqueue.
+ *
+ * The metadata offset is used to configure the location of the
+ * rte_event_crypto_metadata structure within the mbuf's private metadata area.
+ *
+ * When the application sends crypto operations to the adapter,
+ * the crypto queue pair identifier needs to be specified, similarly eventdev
+ * parameters such as the flow id, scheduling type etc are needed by the
+ * adapter when it enqueues mbufs from completed crypto operations to eventdev.
+ */
+
+#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 queue conf type
+ */
+enum rte_event_crypto_conf_type {
+	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
+	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
+	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
+	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
+	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
+};
+
+ /**
+ * @warning
+ * @b EXPERIMENTAL: this enum may change without prior notice
+ *
+ * Crypto event adapter type
+ */
+enum rte_event_crypto_adapter_type {
+	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
+	/**< Start only Rx part of crypto adapter.
+	* Packets dequeued from cryptodev are new to eventdev and
+	* events will be treated as RTE_EVENT_OP_NEW */
+	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
+	/**< Start both Rx & Tx 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
+ *
+ * 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 mbufs. This isn't treated as a requirement; batching may
+	 * cause the adapter to process more than max_nb mbufs.
+	 */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Crypto event metadata structure
+ */
+struct rte_event_crypto_metadata {
+	union {
+		uint64_t u64;
+		/**< Opaque 64-bit value */
+		struct rte_crypto_op *crypto_op;
+		/**< pointer to struct rte_crypto_op */
+	}
+	uint32_t flow_id:20;
+	/**< eventdev flow identifier */
+	uint32_t sub_event_type:8;
+	/**< eventdev sub-event type */
+	uint32_t sched_type:2;
+	/**< eventdev scheduling type */
+	uint16_t event_qid;
+	/**< eventdev queue identifier */
+	uint16_t cryptodev_qp_id;
+	/**< crypto queue pair index */
+	uint8_t priority;
+	/**< eventdev priority */
+	uint8_t reserved[15];
+	/**< Bytes reserved for future extension */
+};
+
+/**
+ * @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
+ *  Event 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
+ *
+ * Queue pair configuration structure
+ */
+struct rte_event_crypto_queue_pair_conf {
+	enum rte_event_crypto_conf_type type;
+	/**< Flags for handling received packets */
+	union {
+		uint32_t mbuf_metadata_offset;
+		/**<
+		* The metadata offset indicates the location of the
+		* rte_event_crypto_metadata structure within the mbuf's
+		* private metadata area.
+		*/
+		struct rte_event ev;
+		/**<
+		* When queuing is set to RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ
+		* the values from the following event fields will be used for
+		*  queuing mbuf events:
+		*   - queue_id: Targeted event queue ID for received packets.
+		*   - priority: Event priority of packets from this queue in
+		*                the event queue relative to other events.
+		*   - sched_type: Scheduling type for packets from this queue
+		*                  pair.
+		*   - flow_id: Identifier indicating the packet flow.
+		*   - sub_event_type: Sub event type for received packets
+		*
+		* The event adapter sets ev.event_type to RTE_EVENT_TYPE_CRYPTO
+		* in the enqueued event.
+		*/
+	};
+};
+
+/**
+ * @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
+ *  The identifier of the event crypto adapter.
+ *
+ * @param cdev_id
+ *  The identifier of the cryptodev to configure.
+ *
+ * @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
+ *  The identifier of the event crypto adapter.
+ *
+ * @param cdev_id
+ *  The identifier of the cryptodev to configure.
+ *
+ * @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 dev_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.
+ *
+ * @param conf
+ *  Additional configuration structure of type
+ *  *rte_event_crypto_queue_pair_conf*
+ *
+ * @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,
+			const struct rte_event_crypto_queue_pair_conf *conf);
+
+/**
+ * @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
+ *  Identifier of Cryptodev.
+ *
+ * @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 Rx only or both Rx & Tx.
+ *
+ * @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.
+ *
+ * @param type
+ *  Flag to indicate to start Rx only or both Rx & Tx.
+ *
+ * @return
+ *  - 0: Success, Adapter stopped successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_crypto_adapter_stop(uint8_t id,
+				  enum rte_event_crypto_adapter_type type);
+
+/**
+ * @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] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Thu, 9 Nov 2017 12:24:13 +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] eventdev: add crypto adapter API header
> X-Mailer: git-send-email 1.9.1
> 
> 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>
> ---
>  lib/librte_eventdev/Makefile                   |   1 +
>  lib/librte_eventdev/rte_event_crypto_adapter.h | 474 +++++++++++++++++++++++++
>  2 files changed, 475 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 5ac22cd..9cbe1a6 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -53,6 +53,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..080c3ed
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
> @@ -0,0 +1,474 @@
> +/*
> + *   Copyright(c) 2017 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 completion to event device.
> + * The packet flow from cryptodev to the event device can be accomplished
> + * using either HW or SW 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.
> + *
> + * The event crypto adapter provides common APIs to configure the packet flow
> + * from the cryptodev to event devices on both HW and SW.
> + * 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 rte_event_crypto_adapter_queue_pair_xxx() API.
> + *
> + * Adapter uses rte_event_crypto_queue_pair_conf to decide whether the event
> + * enqueue is based on RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ or
> + * RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ.
> + * In case of RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ,
> + * rte_event_crypto_queue_pair_conf::ev will be used for event enqueue.
> + * In case of RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ,
> + * members of rte_event_crypto_metadata will be used for event enqueue.

Adding Declan and Hemant.

IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
from application perceptive as the scope is very limited.
In real world use cases, we will be attaching destination event queue information
to the session, not to the queue pair.


IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
convenient for application writers as
# it relies on mbuf private area memory. So it has additional memory alloc/free
requirements.
# additional overhead for application/PMD to write/read the event queue metadata
information per packet.

Since we already have meta data structure in the crypto
area, How about adding the destination event queue attributes
in the PMD crypto session area and for, _session less_, we can add it
in rte_crypto_op stricture? This will help in:

# Offloading HW specific meta data generation for event queue attributes
to slow path.
# From the application perspective, most likely the event queue parameters
will be different for each session not per packet nor per event queue
pair.

Something like below to share my view. Exact details may be we can work it out.

➜ [master][dpdk.org] $ git diff
diff --git a/lib/librte_cryptodev/rte_crypto.h
b/lib/librte_cryptodev/rte_crypto.h
index 3d672fe7d..b44ef673b 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -115,6 +115,9 @@ struct rte_crypto_op {
 
        uint8_t reserved[5];
        /**< Reserved bytes to fill 64 bits for future additions */

+#if 0
+ Crypto completion event attribute. For _session less_ crypto enqueue operation,
+ The will field shall be used by application to post the crypto completion event
+ upon the crypto enqueue operation complete.

+ Note: In the case of session based crypto operation, SW based crypto adapter can use
+ this memory to store crypto event completion attributes from the PMD
+ specific session area.
+
+ Note: ev.event_ptr will point to struct rte_crypto_op *op, So
+ that core can free the ops memory on event_dequeue().
+#endif
+
+       struct rte_event ev;

        struct rte_mempool *mempool;
        /**< crypto operation mempool which operation is allocated from
 * */
 
diff --git a/lib/librte_cryptodev/rte_cryptodev.h
b/lib/librte_cryptodev/rte_cryptodev.h
index dade5548f..540b29e66 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
                        struct rte_crypto_sym_xform *xforms,
                        struct rte_mempool *mempool);

+#if 0
+ Create PMD specific session meta data for the destination event queue
+ attribute to post the crypto completion event on crypto work complete.
+#endif
+int
+rte_cryptodev_sym_session_event_init(uint8_t dev_id,
+                       struct rte_cryptodev_sym_session *sess,
+                       struct rte_crypto_sym_xform *xforms,
+                       struct rte_mempool *mempool,
+                       struct rte_event ev);
+
 /**
  * Frees private data for the device id, based on its device type,
  * returning it to its mempool.


> + *
> + * The metadata offset is used to configure the location of the
> + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
> + *
> + * When the application sends crypto operations to the adapter,
> + * the crypto queue pair identifier needs to be specified, similarly eventdev
> + * parameters such as the flow id, scheduling type etc are needed by the
> + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
> + */
> +
> +#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 queue conf type
> + */
> +enum rte_event_crypto_conf_type {
> +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
> +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
> +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
> +};

See above.

> +
> + /**
> + * @warning
> + * @b EXPERIMENTAL: this enum may change without prior notice
> + *
> + * Crypto event adapter type
> + */
> +enum rte_event_crypto_adapter_type {
> +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> +	/**< Start only Rx part of crypto adapter.
> +	* Packets dequeued from cryptodev are new to eventdev and
> +	* events will be treated as RTE_EVENT_OP_NEW */
> +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> +	/**< Start both Rx & Tx part of crypto adapter.
> +	* Packet's event context will be retained and
> +	* event will be treated as RTE_EVENT_OP_FORWARD */
> +};

How about leveraging ev.op based schematics as mentioned above?

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Doherty, Declan]

On 29/11/2017 11:41 AM, Jerin Jacob wrote:
> -----Original Message-----

...

> 
> Adding Declan and Hemant.
> > IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
> from application perceptive as the scope is very limited.
> In real world use cases, we will be attaching destination event queue information
> to the session, not to the queue pair.
> 
> 
> IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
> convenient for application writers as
> # it relies on mbuf private area memory. So it has additional memory alloc/free
> requirements.
> # additional overhead for application/PMD to write/read the event queue metadata
> information per packet.
> 
> Since we already have meta data structure in the crypto
> area, How about adding the destination event queue attributes
> in the PMD crypto session area and for, _session less_, we can add it
> in rte_crypto_op stricture? This will help in:
> 
> # Offloading HW specific meta data generation for event queue attributes
> to slow path.
> # From the application perspective, most likely the event queue parameters
> will be different for each session not per packet nor per event queue
> pair.
> 

Hey Jerin,

given my limited experience with eventdev, your proposed approach in 
general makes sense to me, in that a packet flow having crypto 
processing done will always be forwarded the same next stage event 
queue. So storing this state in the crypto session, or crypto op in the 
case of sessionless ops, seems sensible.

> Something like below to share my view. Exact details may be we can work it out.
> 

I terms of your proposed changes below, my main concern would be 
introducing dependencies on the eventdev library into cryptodev, as with 
this new crypto adpater you will have a dependency on cryptodev in 
eventdev.

I think the best approach would be to support opaque metadata in both 
the crypto op and crypto session structures, as this would allow other 
uses cases to be supported which aren't specific to eventdev to also 
store metadata across cryptodev processing.

> ➜ [master][dpdk.org] $ git diff
> diff --git a/lib/librte_cryptodev/rte_crypto.h
> b/lib/librte_cryptodev/rte_crypto.h
> index 3d672fe7d..b44ef673b 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -115,6 +115,9 @@ struct rte_crypto_op {
>   
>          uint8_t reserved[5];
>          /**< Reserved bytes to fill 64 bits for future additions */
> 
> +#if 0
> + Crypto completion event attribute. For _session less_ crypto enqueue operation,
> + The will field shall be used by application to post the crypto completion event
> + upon the crypto enqueue operation complete.
> 
> + Note: In the case of session based crypto operation, SW based crypto adapter can use
> + this memory to store crypto event completion attributes from the PMD
> + specific session area.
> +
> + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
> + that core can free the ops memory on event_dequeue().
> +#endif
> +
> +       struct rte_event ev;
> 
>          struct rte_mempool *mempool;
>          /**< crypto operation mempool which operation is allocated from
>   * */
>   
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> b/lib/librte_cryptodev/rte_cryptodev.h
> index dade5548f..540b29e66 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>                          struct rte_crypto_sym_xform *xforms,
>                          struct rte_mempool *mempool);
> 
> +#if 0
> + Create PMD specific session meta data for the destination event queue
> + attribute to post the crypto completion event on crypto work complete.
> +#endif
> +int
> +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
> +                       struct rte_cryptodev_sym_session *sess,
> +                       struct rte_crypto_sym_xform *xforms,
> +                       struct rte_mempool *mempool,
> +                       struct rte_event ev);
> +
>   /**
>    * Frees private data for the device id, based on its device type,
>    * returning it to its mempool.
> 
> 
>> + *
>> + * The metadata offset is used to configure the location of the
>> + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
>> + *
>> + * When the application sends crypto operations to the adapter,
>> + * the crypto queue pair identifier needs to be specified, similarly eventdev
>> + * parameters such as the flow id, scheduling type etc are needed by the
>> + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
>> + */
>> +
>> +#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 queue conf type
>> + */
>> +enum rte_event_crypto_conf_type {
>> +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
>> +};
> 
> See above.
> 
>> +
>> + /**
>> + * @warning
>> + * @b EXPERIMENTAL: this enum may change without prior notice
>> + *
>> + * Crypto event adapter type
>> + */
>> +enum rte_event_crypto_adapter_type {
>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
>> +	/**< Start only Rx part of crypto adapter.
>> +	* Packets dequeued from cryptodev are new to eventdev and
>> +	* events will be treated as RTE_EVENT_OP_NEW */
>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
>> +	/**< Start both Rx & Tx part of crypto adapter.
>> +	* Packet's event context will be retained and
>> +	* event will be treated as RTE_EVENT_OP_FORWARD */
>> +};
> 
> How about leveraging ev.op based schematics as mentioned above?
> 

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 13 Dec 2017 11:03:06 +0000
> From: "Doherty, Declan" <declan.doherty@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
>  <abhinandan.gujjar@intel.com>
> CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
>  <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
>  hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
>  nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
> Subject: Re: [RFC] eventdev: add crypto adapter API header
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.5.0
> 
> On 29/11/2017 11:41 AM, Jerin Jacob wrote:
> > -----Original Message-----
> 
> ...
> 
> > 
> > Adding Declan and Hemant.
> > > IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
> > from application perceptive as the scope is very limited.
> > In real world use cases, we will be attaching destination event queue information
> > to the session, not to the queue pair.
> > 
> > 
> > IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
> > convenient for application writers as
> > # it relies on mbuf private area memory. So it has additional memory alloc/free
> > requirements.
> > # additional overhead for application/PMD to write/read the event queue metadata
> > information per packet.
> > 
> > Since we already have meta data structure in the crypto
> > area, How about adding the destination event queue attributes
> > in the PMD crypto session area and for, _session less_, we can add it
> > in rte_crypto_op stricture? This will help in:
> > 
> > # Offloading HW specific meta data generation for event queue attributes
> > to slow path.
> > # From the application perspective, most likely the event queue parameters
> > will be different for each session not per packet nor per event queue
> > pair.
> > 
> 
> Hey Jerin,

Hey Declan,

> 
> given my limited experience with eventdev, your proposed approach in general
> makes sense to me, in that a packet flow having crypto processing done will
> always be forwarded the same next stage event queue. So storing this state
> in the crypto session, or crypto op in the case of sessionless ops, seems
> sensible.
> 
> > Something like below to share my view. Exact details may be we can work it out.
> > 
> 
> I terms of your proposed changes below, my main concern would be introducing
> dependencies on the eventdev library into cryptodev, as with this new crypto
> adpater you will have a dependency on cryptodev in eventdev.

I agree with your dependency concern.

> 
> I think the best approach would be to support opaque metadata in both the
> crypto op and crypto session structures, as this would allow other uses
> cases to be supported which aren't specific to eventdev to also store
> metadata across cryptodev processing.

Make sense. Just to add, adding a pointer would be overhead. I think, we
can reserve a few bytes as byte array and then later typecast with
eventdev api in eventdev library.

uint8_t eventdev_metadata[SOMEBYTES];

Thoughts?

> 
> > ➜ [master][dpdk.org] $ git diff
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index 3d672fe7d..b44ef673b 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -115,6 +115,9 @@ struct rte_crypto_op {
> >          uint8_t reserved[5];
> >          /**< Reserved bytes to fill 64 bits for future additions */
> > 
> > +#if 0
> > + Crypto completion event attribute. For _session less_ crypto enqueue operation,
> > + The will field shall be used by application to post the crypto completion event
> > + upon the crypto enqueue operation complete.
> > 
> > + Note: In the case of session based crypto operation, SW based crypto adapter can use
> > + this memory to store crypto event completion attributes from the PMD
> > + specific session area.
> > +
> > + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
> > + that core can free the ops memory on event_dequeue().
> > +#endif
> > +
> > +       struct rte_event ev;
> > 
> >          struct rte_mempool *mempool;
> >          /**< crypto operation mempool which operation is allocated from
> >   * */
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> > b/lib/librte_cryptodev/rte_cryptodev.h
> > index dade5548f..540b29e66 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.h
> > +++ b/lib/librte_cryptodev/rte_cryptodev.h
> > @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
> >                          struct rte_crypto_sym_xform *xforms,
> >                          struct rte_mempool *mempool);
> > 
> > +#if 0
> > + Create PMD specific session meta data for the destination event queue
> > + attribute to post the crypto completion event on crypto work complete.
> > +#endif
> > +int
> > +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
> > +                       struct rte_cryptodev_sym_session *sess,
> > +                       struct rte_crypto_sym_xform *xforms,
> > +                       struct rte_mempool *mempool,
> > +                       struct rte_event ev);
> > +
> >   /**
> >    * Frees private data for the device id, based on its device type,
> >    * returning it to its mempool.
> > 
> > 
> > > + *
> > > + * The metadata offset is used to configure the location of the
> > > + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
> > > + *
> > > + * When the application sends crypto operations to the adapter,
> > > + * the crypto queue pair identifier needs to be specified, similarly eventdev
> > > + * parameters such as the flow id, scheduling type etc are needed by the
> > > + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
> > > + */
> > > +
> > > +#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 queue conf type
> > > + */
> > > +enum rte_event_crypto_conf_type {
> > > +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
> > > +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
> > > +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
> > > +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
> > > +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
> > > +};
> > 
> > See above.
> > 
> > > +
> > > + /**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > + *
> > > + * Crypto event adapter type
> > > + */
> > > +enum rte_event_crypto_adapter_type {
> > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > +	/**< Start only Rx part of crypto adapter.
> > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > +	* Packet's event context will be retained and
> > > +	* event will be treated as RTE_EVENT_OP_FORWARD */
> > > +};
> > 
> > How about leveraging ev.op based schematics as mentioned above?
> > 
> 

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Nithin Dabilpuram]

Hi Jerin, Declan,

On Wednesday 13 December 2017 04:56 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 13 Dec 2017 11:03:06 +0000
>> From: "Doherty, Declan" <declan.doherty@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
>>   <abhinandan.gujjar@intel.com>
>> CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
>>   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
>>   hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
>>   nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
>> Subject: Re: [RFC] eventdev: add crypto adapter API header
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.5.0
>>
>> On 29/11/2017 11:41 AM, Jerin Jacob wrote:
>>> -----Original Message-----
>> ...
>>
>>> Adding Declan and Hemant.
>>>> IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
>>> from application perceptive as the scope is very limited.
>>> In real world use cases, we will be attaching destination event queue information
>>> to the session, not to the queue pair.
>>>
>>>
>>> IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
>>> convenient for application writers as
>>> # it relies on mbuf private area memory. So it has additional memory alloc/free
>>> requirements.
>>> # additional overhead for application/PMD to write/read the event queue metadata
>>> information per packet.
>>>
>>> Since we already have meta data structure in the crypto
>>> area, How about adding the destination event queue attributes
>>> in the PMD crypto session area and for, _session less_, we can add it
>>> in rte_crypto_op stricture? This will help in:
>>>
>>> # Offloading HW specific meta data generation for event queue attributes
>>> to slow path.
>>> # From the application perspective, most likely the event queue parameters
>>> will be different for each session not per packet nor per event queue
>>> pair.
>>>
>> Hey Jerin,
> Hey Declan,
>
>> given my limited experience with eventdev, your proposed approach in general
>> makes sense to me, in that a packet flow having crypto processing done will
>> always be forwarded the same next stage event queue. So storing this state
>> in the crypto session, or crypto op in the case of sessionless ops, seems
>> sensible.
>>
>>> Something like below to share my view. Exact details may be we can work it out.
>>>
>> I terms of your proposed changes below, my main concern would be introducing
>> dependencies on the eventdev library into cryptodev, as with this new crypto
>> adpater you will have a dependency on cryptodev in eventdev.
> I agree with your dependency concern.
>
>> I think the best approach would be to support opaque metadata in both the
>> crypto op and crypto session structures, as this would allow other uses
>> cases to be supported which aren't specific to eventdev to also store
>> metadata across cryptodev processing.
> Make sense. Just to add, adding a pointer would be overhead. I think, we
> can reserve a few bytes as byte array and then later typecast with
> eventdev api in eventdev library.
>
> uint8_t eventdev_metadata[SOMEBYTES];
>
> Thoughts?
Can we have this info in structure rte_crypto_sym_xform instead of 
rte_crypto_op
so that for session less or session full we have just one api say as below
to update the event information.

rte_cryptodev_sym_xform_event_init(struct rte_crypto_sym_xform *xforms,
                                                             struct 
rte_event ev)

IMO, this is better because for both session_less or session_full modes, 
app has to
prepare sym_xform structure and in case of session_less make the struct 
rte_crypto_op
point to sym_xform and in session_full pass it to 
rte_cryptodev_sym_session_create().

The same can be followed for asym/security sessions in future if needed.

>>> ➜ [master][dpdk.org] $ git diff
>>> diff --git a/lib/librte_cryptodev/rte_crypto.h
>>> b/lib/librte_cryptodev/rte_crypto.h
>>> index 3d672fe7d..b44ef673b 100644
>>> --- a/lib/librte_cryptodev/rte_crypto.h
>>> +++ b/lib/librte_cryptodev/rte_crypto.h
>>> @@ -115,6 +115,9 @@ struct rte_crypto_op {
>>>           uint8_t reserved[5];
>>>           /**< Reserved bytes to fill 64 bits for future additions */
>>>
>>> +#if 0
>>> + Crypto completion event attribute. For _session less_ crypto enqueue operation,
>>> + The will field shall be used by application to post the crypto completion event
>>> + upon the crypto enqueue operation complete.
>>>
>>> + Note: In the case of session based crypto operation, SW based crypto adapter can use
>>> + this memory to store crypto event completion attributes from the PMD
>>> + specific session area.
>>> +
>>> + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
>>> + that core can free the ops memory on event_dequeue().
>>> +#endif
>>> +
>>> +       struct rte_event ev;
>>>
>>>           struct rte_mempool *mempool;
>>>           /**< crypto operation mempool which operation is allocated from
>>>    * */
>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
>>> b/lib/librte_cryptodev/rte_cryptodev.h
>>> index dade5548f..540b29e66 100644
>>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>>> @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>>>                           struct rte_crypto_sym_xform *xforms,
>>>                           struct rte_mempool *mempool);
>>>
>>> +#if 0
>>> + Create PMD specific session meta data for the destination event queue
>>> + attribute to post the crypto completion event on crypto work complete.
>>> +#endif
>>> +int
>>> +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
>>> +                       struct rte_cryptodev_sym_session *sess,
>>> +                       struct rte_crypto_sym_xform *xforms,
>>> +                       struct rte_mempool *mempool,
>>> +                       struct rte_event ev);
>>> +
>>>    /**
>>>     * Frees private data for the device id, based on its device type,
>>>     * returning it to its mempool.
>>>
>>>
>>>> + *
>>>> + * The metadata offset is used to configure the location of the
>>>> + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
>>>> + *
>>>> + * When the application sends crypto operations to the adapter,
>>>> + * the crypto queue pair identifier needs to be specified, similarly eventdev
>>>> + * parameters such as the flow id, scheduling type etc are needed by the
>>>> + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
>>>> + */
>>>> +
>>>> +#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 queue conf type
>>>> + */
>>>> +enum rte_event_crypto_conf_type {
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
>>>> +};
>>> See above.
>>>
>>>> +
>>>> + /**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this enum may change without prior notice
>>>> + *
>>>> + * Crypto event adapter type
>>>> + */
>>>> +enum rte_event_crypto_adapter_type {
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
>>>> +	/**< Start only Rx part of crypto adapter.
>>>> +	* Packets dequeued from cryptodev are new to eventdev and
>>>> +	* events will be treated as RTE_EVENT_OP_NEW */
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
>>>> +	/**< Start both Rx & Tx part of crypto adapter.
>>>> +	* Packet's event context will be retained and
>>>> +	* event will be treated as RTE_EVENT_OP_FORWARD */
>>>> +};
>>> How about leveraging ev.op based schematics as mentioned above?
>>>

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Akhil Goyal]

Hi Jerin,
On 12/13/2017 4:56 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 13 Dec 2017 11:03:06 +0000
>> From: "Doherty, Declan" <declan.doherty@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
>>   <abhinandan.gujjar@intel.com>
>> CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
>>   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
>>   hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
>>   nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
>> Subject: Re: [RFC] eventdev: add crypto adapter API header
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.5.0
>>
>> On 29/11/2017 11:41 AM, Jerin Jacob wrote:
>>> -----Original Message-----
>>
>> ...
>>
>>>
>>> Adding Declan and Hemant.
>>>> IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
>>> from application perceptive as the scope is very limited.
>>> In real world use cases, we will be attaching destination event queue information
>>> to the session, not to the queue pair.
>>>
>>>
>>> IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
>>> convenient for application writers as
>>> # it relies on mbuf private area memory. So it has additional memory alloc/free
>>> requirements.
>>> # additional overhead for application/PMD to write/read the event queue metadata
>>> information per packet.
>>>
>>> Since we already have meta data structure in the crypto
>>> area, How about adding the destination event queue attributes
>>> in the PMD crypto session area and for, _session less_, we can add it
>>> in rte_crypto_op stricture? This will help in:
>>>
>>> # Offloading HW specific meta data generation for event queue attributes
>>> to slow path.
>>> # From the application perspective, most likely the event queue parameters
>>> will be different for each session not per packet nor per event queue
>>> pair.
>>>
>>
>> Hey Jerin,
> 
> Hey Declan,
> 
>>
>> given my limited experience with eventdev, your proposed approach in general
>> makes sense to me, in that a packet flow having crypto processing done will
>> always be forwarded the same next stage event queue. So storing this state
>> in the crypto session, or crypto op in the case of sessionless ops, seems
>> sensible.
>>
>>> Something like below to share my view. Exact details may be we can work it out.
>>>
>>
>> I terms of your proposed changes below, my main concern would be introducing
>> dependencies on the eventdev library into cryptodev, as with this new crypto
>> adpater you will have a dependency on cryptodev in eventdev.
> 
> I agree with your dependency concern.
> 
>>
>> I think the best approach would be to support opaque metadata in both the
>> crypto op and crypto session structures, as this would allow other uses
>> cases to be supported which aren't specific to eventdev to also store
>> metadata across cryptodev processing.
> 
> Make sense. Just to add, adding a pointer would be overhead. I think, we
> can reserve a few bytes as byte array and then later typecast with
> eventdev api in eventdev library.
> 
> uint8_t eventdev_metadata[SOMEBYTES];
> 
> Thoughts?
I believe only 1 uint64 is sufficient. The metadata that we need here is 
rte_event which is 2 uint64 and the second one is mbuf. Since mbuf is 
already part of rte_crypto_sym_op, we do not need it.

So only a pointer/uint64 is required.

> 
>>
>>> ➜ [master][dpdk.org] $ git diff
>>> diff --git a/lib/librte_cryptodev/rte_crypto.h
>>> b/lib/librte_cryptodev/rte_crypto.h
>>> index 3d672fe7d..b44ef673b 100644
>>> --- a/lib/librte_cryptodev/rte_crypto.h
>>> +++ b/lib/librte_cryptodev/rte_crypto.h
>>> @@ -115,6 +115,9 @@ struct rte_crypto_op {
>>>           uint8_t reserved[5];
>>>           /**< Reserved bytes to fill 64 bits for future additions */
>>>
>>> +#if 0
>>> + Crypto completion event attribute. For _session less_ crypto enqueue operation,
>>> + The will field shall be used by application to post the crypto completion event
>>> + upon the crypto enqueue operation complete.
>>>
>>> + Note: In the case of session based crypto operation, SW based crypto adapter can use
>>> + this memory to store crypto event completion attributes from the PMD
>>> + specific session area.
>>> +
>>> + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
>>> + that core can free the ops memory on event_dequeue().
>>> +#endif
>>> +
>>> +       struct rte_event ev;
>>>
>>>           struct rte_mempool *mempool;
>>>           /**< crypto operation mempool which operation is allocated from
>>>    * */
>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
>>> b/lib/librte_cryptodev/rte_cryptodev.h
>>> index dade5548f..540b29e66 100644
>>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>>> @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>>>                           struct rte_crypto_sym_xform *xforms,
>>>                           struct rte_mempool *mempool);
>>>
>>> +#if 0
>>> + Create PMD specific session meta data for the destination event queue
>>> + attribute to post the crypto completion event on crypto work complete.
>>> +#endif
>>> +int
>>> +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
>>> +                       struct rte_cryptodev_sym_session *sess,
>>> +                       struct rte_crypto_sym_xform *xforms,
>>> +                       struct rte_mempool *mempool,
>>> +                       struct rte_event ev);
>>> +
>>>    /**
>>>     * Frees private data for the device id, based on its device type,
>>>     * returning it to its mempool.
>>>
>>>
>>>> + *
>>>> + * The metadata offset is used to configure the location of the
>>>> + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
>>>> + *
>>>> + * When the application sends crypto operations to the adapter,
>>>> + * the crypto queue pair identifier needs to be specified, similarly eventdev
>>>> + * parameters such as the flow id, scheduling type etc are needed by the
>>>> + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
>>>> + */
>>>> +
>>>> +#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 queue conf type
>>>> + */
>>>> +enum rte_event_crypto_conf_type {
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
>>>> +};
>>>
>>> See above.
>>>
>>>> +
>>>> + /**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this enum may change without prior notice
>>>> + *
>>>> + * Crypto event adapter type
>>>> + */
>>>> +enum rte_event_crypto_adapter_type {
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
>>>> +	/**< Start only Rx part of crypto adapter.
>>>> +	* Packets dequeued from cryptodev are new to eventdev and
>>>> +	* events will be treated as RTE_EVENT_OP_NEW */
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
>>>> +	/**< Start both Rx & Tx part of crypto adapter.
>>>> +	* Packet's event context will be retained and
>>>> +	* event will be treated as RTE_EVENT_OP_FORWARD */
>>>> +};
>>>
>>> How about leveraging ev.op based schematics as mentioned above?
>>>
>>
> 

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Akhil Goyal]

Hi Nithin,

On 12/13/2017 5:58 PM, Nithin Dabilpuram wrote:
> Hi Jerin, Declan,
> 
> On Wednesday 13 December 2017 04:56 PM, Jerin Jacob wrote:
>> -----Original Message-----
>>> Date: Wed, 13 Dec 2017 11:03:06 +0000
>>> From: "Doherty, Declan" <declan.doherty@intel.com>
>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
>>>   <abhinandan.gujjar@intel.com>
>>> CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
>>>   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
>>>   hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
>>>   nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
>>> Subject: Re: [RFC] eventdev: add crypto adapter API header
>>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>>   Thunderbird/52.5.0
>>>
>>> On 29/11/2017 11:41 AM, Jerin Jacob wrote:
>>>> -----Original Message-----
>>> ...
>>>
>>>> Adding Declan and Hemant.
>>>>> IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
>>>> from application perceptive as the scope is very limited.
>>>> In real world use cases, we will be attaching destination event 
>>>> queue information
>>>> to the session, not to the queue pair.
>>>>
>>>>
>>>> IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
>>>> convenient for application writers as
>>>> # it relies on mbuf private area memory. So it has additional memory 
>>>> alloc/free
>>>> requirements.
>>>> # additional overhead for application/PMD to write/read the event 
>>>> queue metadata
>>>> information per packet.
>>>>
>>>> Since we already have meta data structure in the crypto
>>>> area, How about adding the destination event queue attributes
>>>> in the PMD crypto session area and for, _session less_, we can add it
>>>> in rte_crypto_op stricture? This will help in:
>>>>
>>>> # Offloading HW specific meta data generation for event queue 
>>>> attributes
>>>> to slow path.
>>>> # From the application perspective, most likely the event queue 
>>>> parameters
>>>> will be different for each session not per packet nor per event queue
>>>> pair.
>>>>
>>> Hey Jerin,
>> Hey Declan,
>>
>>> given my limited experience with eventdev, your proposed approach in 
>>> general
>>> makes sense to me, in that a packet flow having crypto processing 
>>> done will
>>> always be forwarded the same next stage event queue. So storing this 
>>> state
>>> in the crypto session, or crypto op in the case of sessionless ops, 
>>> seems
>>> sensible.
>>>
>>>> Something like below to share my view. Exact details may be we can 
>>>> work it out.
>>>>
>>> I terms of your proposed changes below, my main concern would be 
>>> introducing
>>> dependencies on the eventdev library into cryptodev, as with this new 
>>> crypto
>>> adpater you will have a dependency on cryptodev in eventdev.
>> I agree with your dependency concern.
>>
>>> I think the best approach would be to support opaque metadata in both 
>>> the
>>> crypto op and crypto session structures, as this would allow other uses
>>> cases to be supported which aren't specific to eventdev to also store
>>> metadata across cryptodev processing.
>> Make sense. Just to add, adding a pointer would be overhead. I think, we
>> can reserve a few bytes as byte array and then later typecast with
>> eventdev api in eventdev library.
>>
>> uint8_t eventdev_metadata[SOMEBYTES];
>>
>> Thoughts?
> Can we have this info in structure rte_crypto_sym_xform instead of 
> rte_crypto_op
> so that for session less or session full we have just one api say as below
> to update the event information.
> 
> rte_cryptodev_sym_xform_event_init(struct rte_crypto_sym_xform *xforms,
>                                                              struct 
> rte_event ev)
> 
> IMO, this is better because for both session_less or session_full modes, 
> app has to
> prepare sym_xform structure and in case of session_less make the struct 
> rte_crypto_op
> point to sym_xform and in session_full pass it to 
> rte_cryptodev_sym_session_create().
> 
> The same can be followed for asym/security sessions in future if needed.
> 
IMO, the metadata that we are talking here is per packet and not per 
session. So moving it to xform will not serve the purpose as it is raw 
information which fills the session parameters of the driver.

And yes security sessions will surely support eventdev crypto adapter APIs.

>>>> ➜ [master][dpdk.org] $ git diff
>>>> diff --git a/lib/librte_cryptodev/rte_crypto.h
>>>> b/lib/librte_cryptodev/rte_crypto.h
>>>> index 3d672fe7d..b44ef673b 100644
>>>> --- a/lib/librte_cryptodev/rte_crypto.h
>>>> +++ b/lib/librte_cryptodev/rte_crypto.h
>>>> @@ -115,6 +115,9 @@ struct rte_crypto_op {
>>>>           uint8_t reserved[5];
>>>>           /**< Reserved bytes to fill 64 bits for future additions */
>>>>
>>>> +#if 0
>>>> + Crypto completion event attribute. For _session less_ crypto 
>>>> enqueue operation,
>>>> + The will field shall be used by application to post the crypto 
>>>> completion event
>>>> + upon the crypto enqueue operation complete.
>>>>
>>>> + Note: In the case of session based crypto operation, SW based 
>>>> crypto adapter can use
>>>> + this memory to store crypto event completion attributes from the PMD
>>>> + specific session area.
>>>> +
>>>> + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
>>>> + that core can free the ops memory on event_dequeue().
>>>> +#endif
>>>> +
>>>> +       struct rte_event ev;
>>>>
>>>>           struct rte_mempool *mempool;
>>>>           /**< crypto operation mempool which operation is allocated 
>>>> from
>>>>    * */
>>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
>>>> b/lib/librte_cryptodev/rte_cryptodev.h
>>>> index dade5548f..540b29e66 100644
>>>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>>>> @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>>>>                           struct rte_crypto_sym_xform *xforms,
>>>>                           struct rte_mempool *mempool);
>>>>
>>>> +#if 0
>>>> + Create PMD specific session meta data for the destination event queue
>>>> + attribute to post the crypto completion event on crypto work 
>>>> complete.
>>>> +#endif
>>>> +int
>>>> +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
>>>> +                       struct rte_cryptodev_sym_session *sess,
>>>> +                       struct rte_crypto_sym_xform *xforms,
>>>> +                       struct rte_mempool *mempool,
>>>> +                       struct rte_event ev);
>>>> +
>>>>    /**
>>>>     * Frees private data for the device id, based on its device type,
>>>>     * returning it to its mempool.
>>>>
>>>>
>>>>> + *
>>>>> + * The metadata offset is used to configure the location of the
>>>>> + * rte_event_crypto_metadata structure within the mbuf's private 
>>>>> metadata area.
>>>>> + *
>>>>> + * When the application sends crypto operations to the adapter,
>>>>> + * the crypto queue pair identifier needs to be specified, 
>>>>> similarly eventdev
>>>>> + * parameters such as the flow id, scheduling type etc are needed 
>>>>> by the
>>>>> + * adapter when it enqueues mbufs from completed crypto operations 
>>>>> to eventdev.
>>>>> + */
>>>>> +
>>>>> +#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 queue conf type
>>>>> + */
>>>>> +enum rte_event_crypto_conf_type {
>>>>> +    RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
>>>>> +    /**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
>>>>> +    RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
>>>>> +    /**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
>>>>> +    RTE_EVENT_CRYPTO_CONF_TYPE_MAX
>>>>> +};
>>>> See above.
>>>>
>>>>> +
>>>>> + /**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this enum may change without prior notice
>>>>> + *
>>>>> + * Crypto event adapter type
>>>>> + */
>>>>> +enum rte_event_crypto_adapter_type {
>>>>> +    RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
>>>>> +    /**< Start only Rx part of crypto adapter.
>>>>> +    * Packets dequeued from cryptodev are new to eventdev and
>>>>> +    * events will be treated as RTE_EVENT_OP_NEW */
>>>>> +    RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
>>>>> +    /**< Start both Rx & Tx part of crypto adapter.
>>>>> +    * Packet's event context will be retained and
>>>>> +    * event will be treated as RTE_EVENT_OP_FORWARD */
>>>>> +};
>>>> How about leveraging ev.op based schematics as mentioned above?
>>>>
> 
> 

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 13 Dec 2017 19:52:24 +0530
> From: Akhil Goyal <akhil.goyal@nxp.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Doherty, Declan"
>  <declan.doherty@intel.com>
> CC: Abhinandan Gujjar <abhinandan.gujjar@intel.com>, dev@dpdk.org,
>  narender.vangati@intel.com, Nikhil Rao <nikhil.rao@intel.com>, Gage Eads
>  <gage.eads@intel.com>, hemant.agrawal@nxp.com,
>  nidadavolu.murthy@cavium.com, nithin.dabilpuram@cavium.com,
>  narayanaprasad.athreya@cavium.com
> Subject: Re: [RFC] eventdev: add crypto adapter API header
> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.5.0
> 
> Hi Jerin,

Hi Akhil,

> On 12/13/2017 4:56 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Wed, 13 Dec 2017 11:03:06 +0000
> > > From: "Doherty, Declan" <declan.doherty@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
> > >   <abhinandan.gujjar@intel.com>
> > > CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
> > >   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
> > >   hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
> > >   nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
> > > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
> > >   Thunderbird/52.5.0
> > > 
> > > On 29/11/2017 11:41 AM, Jerin Jacob wrote:
> > > > -----Original Message-----
> > > 
> > > ...
> > > 
> > > > 
> > > > Adding Declan and Hemant.
> > > > > IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
> > > > from application perceptive as the scope is very limited.
> > > > In real world use cases, we will be attaching destination event queue information
> > > > to the session, not to the queue pair.
> > > > 
> > > > 
> > > > IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
> > > > convenient for application writers as
> > > > # it relies on mbuf private area memory. So it has additional memory alloc/free
> > > > requirements.
> > > > # additional overhead for application/PMD to write/read the event queue metadata
> > > > information per packet.
> > > > 
> > > > Since we already have meta data structure in the crypto
> > > > area, How about adding the destination event queue attributes
> > > > in the PMD crypto session area and for, _session less_, we can add it
> > > > in rte_crypto_op stricture? This will help in:
> > > > 
> > > > # Offloading HW specific meta data generation for event queue attributes
> > > > to slow path.
> > > > # From the application perspective, most likely the event queue parameters
> > > > will be different for each session not per packet nor per event queue
> > > > pair.
> > > > 
> > > 
> > > Hey Jerin,
> > 
> > Hey Declan,
> > 
> > > 
> > > given my limited experience with eventdev, your proposed approach in general
> > > makes sense to me, in that a packet flow having crypto processing done will
> > > always be forwarded the same next stage event queue. So storing this state
> > > in the crypto session, or crypto op in the case of sessionless ops, seems
> > > sensible.
> > > 
> > > > Something like below to share my view. Exact details may be we can work it out.
> > > > 
> > > 
> > > I terms of your proposed changes below, my main concern would be introducing
> > > dependencies on the eventdev library into cryptodev, as with this new crypto
> > > adpater you will have a dependency on cryptodev in eventdev.
> > 
> > I agree with your dependency concern.
> > 
> > > 
> > > I think the best approach would be to support opaque metadata in both the
> > > crypto op and crypto session structures, as this would allow other uses
> > > cases to be supported which aren't specific to eventdev to also store
> > > metadata across cryptodev processing.
> > 
> > Make sense. Just to add, adding a pointer would be overhead. I think, we
> > can reserve a few bytes as byte array and then later typecast with
> > eventdev api in eventdev library.
> > 
> > uint8_t eventdev_metadata[SOMEBYTES];
> > 
> > Thoughts?
> I believe only 1 uint64 is sufficient. The metadata that we need here is
> rte_event which is 2 uint64 and the second one is mbuf. Since mbuf is
> already part of rte_crypto_sym_op, we do not need it.

Yes.

> 
> So only a pointer/uint64 is required.

I would say uint64_t, as the pointer is 32bit in 32bit systems.IMO, We need
have reserved metadata(uint64_t) for eventdev(not generic). 

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Eads, Gage]

Hey Jerin,

</snip>

> > +
> > + /**
> > + * @warning
> > + * @b EXPERIMENTAL: this enum may change without prior notice
> > + *
> > + * Crypto event adapter type
> > + */
> > +enum rte_event_crypto_adapter_type {
> > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > +	/**< Start only Rx part of crypto adapter.
> > +	* Packets dequeued from cryptodev are new to eventdev and
> > +	* events will be treated as RTE_EVENT_OP_NEW */
> > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > +	/**< Start both Rx & Tx part of crypto adapter.
> > +	* Packet's event context will be retained and
> > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> 
> How about leveraging ev.op based schematics as mentioned above?

That could work, but perhaps the ev.op should be configured once up front, as I see it being a function of the application architecture. A couple possible designs, for example:
- Worker enqueues into cryptodev, adapter polls for response: the adapter port would always use OP_NEW here.
- Worker sends a crypto request event to the adapter, which gives the request to the cryptodev and polls for response: the adapter port would always use OP_FWD here. (This ties in with my implicit release patch (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
- Etc.

So I think it makes sense to specify the op once at adapter configuration time, rather than repeatedly in the datapath. This allows for a cleaner separation of configuration and datapath code, and specifying it just once means fewer chances to accidentally set the wrong op value.

Thanks,
Gage

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 13 Dec 2017 23:35:48 +0000
> From: "Eads, Gage" <gage.eads@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan S"
>  <abhinandan.gujjar@intel.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
>  <narender.vangati@intel.com>, "Rao, Nikhil" <nikhil.rao@intel.com>,
>  "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>, "Doherty, Declan"
>  <declan.doherty@intel.com>, "nidadavolu.murthy@cavium.com"
>  <nidadavolu.murthy@cavium.com>, "nithin.dabilpuram@cavium.com"
>  <nithin.dabilpuram@cavium.com>, "narayanaprasad.athreya@cavium.com"
>  <narayanaprasad.athreya@cavium.com>
> Subject: RE: [RFC] eventdev: add crypto adapter API header
> 
> Hey Jerin,

Hey Gage,

> 
> </snip>
> 
> > > +
> > > + /**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > + *
> > > + * Crypto event adapter type
> > > + */
> > > +enum rte_event_crypto_adapter_type {
> > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > +	/**< Start only Rx part of crypto adapter.
> > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > +	* Packet's event context will be retained and
> > > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> > 
> > How about leveraging ev.op based schematics as mentioned above?
> 
> That could work, but perhaps the ev.op should be configured once up front, as I see it being a function of the application architecture. A couple possible designs, for example:
> - Worker enqueues into cryptodev, adapter polls for response: the adapter port would always use OP_NEW here.
> - Worker sends a crypto request event to the adapter, which gives the request to the cryptodev and polls for response: the adapter port would always use OP_FWD here. (This ties in with my implicit release patch (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> - Etc.

Yes. Semantically both approaches will work. I was trying to avoid extra
clutter(enum rte_event_crypto_adapter_type) in adapter API.
I don't see any problem in moving ev.op to adapter configuration time if it
helps the SW driver.

IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name, something like,
RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_FWD
or something like that.


> 
> So I think it makes sense to specify the op once at adapter configuration time, rather than repeatedly in the datapath. This allows for a cleaner separation of configuration and datapath code, and specifying it just once means fewer chances to accidentally set the wrong op value.
> 
> Thanks,
> Gage

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Eads, Gage]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Wednesday, December 13, 2017 8:49 PM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty, Declan
> <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> Subject: Re: [RFC] eventdev: add crypto adapter API header
> 
> -----Original Message-----
> > Date: Wed, 13 Dec 2017 23:35:48 +0000
> > From: "Eads, Gage" <gage.eads@intel.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan S"
> >  <abhinandan.gujjar@intel.com>
> > CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
> >  <narender.vangati@intel.com>, "Rao, Nikhil" <nikhil.rao@intel.com>,
> > "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>, "Doherty, Declan"
> >  <declan.doherty@intel.com>, "nidadavolu.murthy@cavium.com"
> >  <nidadavolu.murthy@cavium.com>, "nithin.dabilpuram@cavium.com"
> >  <nithin.dabilpuram@cavium.com>, "narayanaprasad.athreya@cavium.com"
> >  <narayanaprasad.athreya@cavium.com>
> > Subject: RE: [RFC] eventdev: add crypto adapter API header
> >
> > Hey Jerin,
> 
> Hey Gage,
> 
> >
> > </snip>
> >
> > > > +
> > > > + /**
> > > > + * @warning
> > > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > > + *
> > > > + * Crypto event adapter type
> > > > + */
> > > > +enum rte_event_crypto_adapter_type {
> > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > > +	/**< Start only Rx part of crypto adapter.
> > > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > > +	* Packet's event context will be retained and
> > > > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> > >
> > > How about leveraging ev.op based schematics as mentioned above?
> >
> > That could work, but perhaps the ev.op should be configured once up front, as
> I see it being a function of the application architecture. A couple possible
> designs, for example:
> > - Worker enqueues into cryptodev, adapter polls for response: the adapter
> port would always use OP_NEW here.
> > - Worker sends a crypto request event to the adapter, which gives the
> > request to the cryptodev and polls for response: the adapter port
> > would always use OP_FWD here. (This ties in with my implicit release
> > patch (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> > - Etc.
> 
> Yes. Semantically both approaches will work. I was trying to avoid extra
> clutter(enum rte_event_crypto_adapter_type) in adapter API.
> I don't see any problem in moving ev.op to adapter configuration time if it helps
> the SW driver.
> 
> IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
> RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name, something
> like,
> RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTE
> R_TYPE_OP_FWD
> or something like that.
> 

I agree that the two naming schemes are equivalent, but since this option would control the adapter's behavior (Rx only vs. Rx + Tx), (IMO) I think Abhinandan's original names do a better job of conveying what effect these two options have on the adapter, compared to the op type names.

> 
> >
> > So I think it makes sense to specify the op once at adapter configuration time,
> rather than repeatedly in the datapath. This allows for a cleaner separation of
> configuration and datapath code, and specifying it just once means fewer
> chances to accidentally set the wrong op value.
> >
> > Thanks,
> > Gage

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Thu, 14 Dec 2017 18:52:02 +0000
> From: "Eads, Gage" <gage.eads@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>, "Vangati, Narender" <narender.vangati@intel.com>, "Rao,
>  Nikhil" <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "Doherty, Declan" <declan.doherty@intel.com>,
>  "nidadavolu.murthy@cavium.com" <nidadavolu.murthy@cavium.com>,
>  "nithin.dabilpuram@cavium.com" <nithin.dabilpuram@cavium.com>,
>  "narayanaprasad.athreya@cavium.com" <narayanaprasad.athreya@cavium.com>
> Subject: RE: [RFC] eventdev: add crypto adapter API header
> 
> 
> 
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Wednesday, December 13, 2017 8:49 PM
> > To: Eads, Gage <gage.eads@intel.com>
> > Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> > <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty, Declan
> > <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> > nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > 
> > -----Original Message-----
> > > Date: Wed, 13 Dec 2017 23:35:48 +0000
> > > From: "Eads, Gage" <gage.eads@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan S"
> > >  <abhinandan.gujjar@intel.com>
> > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
> > >  <narender.vangati@intel.com>, "Rao, Nikhil" <nikhil.rao@intel.com>,
> > > "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>, "Doherty, Declan"
> > >  <declan.doherty@intel.com>, "nidadavolu.murthy@cavium.com"
> > >  <nidadavolu.murthy@cavium.com>, "nithin.dabilpuram@cavium.com"
> > >  <nithin.dabilpuram@cavium.com>, "narayanaprasad.athreya@cavium.com"
> > >  <narayanaprasad.athreya@cavium.com>
> > > Subject: RE: [RFC] eventdev: add crypto adapter API header
> > >
> > > Hey Jerin,
> > 
> > Hey Gage,
> > 
> > >
> > > </snip>
> > >
> > > > > +
> > > > > + /**
> > > > > + * @warning
> > > > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > > > + *
> > > > > + * Crypto event adapter type
> > > > > + */
> > > > > +enum rte_event_crypto_adapter_type {
> > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > > > +	/**< Start only Rx part of crypto adapter.
> > > > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > > > +	* Packet's event context will be retained and
> > > > > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> > > >
> > > > How about leveraging ev.op based schematics as mentioned above?
> > >
> > > That could work, but perhaps the ev.op should be configured once up front, as
> > I see it being a function of the application architecture. A couple possible
> > designs, for example:
> > > - Worker enqueues into cryptodev, adapter polls for response: the adapter
> > port would always use OP_NEW here.
> > > - Worker sends a crypto request event to the adapter, which gives the
> > > request to the cryptodev and polls for response: the adapter port
> > > would always use OP_FWD here. (This ties in with my implicit release
> > > patch (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> > > - Etc.
> > 
> > Yes. Semantically both approaches will work. I was trying to avoid extra
> > clutter(enum rte_event_crypto_adapter_type) in adapter API.
> > I don't see any problem in moving ev.op to adapter configuration time if it helps
> > the SW driver.
> > 
> > IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
> > RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name, something
> > like,
> > RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTE
> > R_TYPE_OP_FWD
> > or something like that.
> > 
> 
> I agree that the two naming schemes are equivalent, but since this option would control the adapter's behavior (Rx only vs. Rx + Tx), (IMO) I think Abhinandan's original names do a better job of conveying what effect these two options have on the adapter, compared to the op type names.

The only concern with Rx/Tx terminology was, It is mostly used in the ethdev domain.
In crypto domain, typically, we use enqueue/dequeue.
The only difference between two modes is if adapter enqueue the events with
RTE_EVENT_OP_NEW vs RTE_EVENT_OP_FORWARD then (IMO) we can change
something related to that name to avoid adding a new terminology.

BTW, Based on the earlier discussion, if we need to add opaque eventdev metadata
to cryptodev then it may change ABI.If so, I think, we need to announce
ABI change notice for cryptodev and plan cryptodev adapter for v18.05.

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Eads, Gage]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, December 18, 2017 12:30 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty, Declan
> <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> Subject: Re: [RFC] eventdev: add crypto adapter API header
> 
> -----Original Message-----
> > Date: Thu, 14 Dec 2017 18:52:02 +0000
> > From: "Eads, Gage" <gage.eads@intel.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>, "dev@dpdk.org"
> >  <dev@dpdk.org>, "Vangati, Narender" <narender.vangati@intel.com>,
> > "Rao,  Nikhil" <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
> >  <hemant.agrawal@nxp.com>, "Doherty, Declan"
> > <declan.doherty@intel.com>,  "nidadavolu.murthy@cavium.com"
> > <nidadavolu.murthy@cavium.com>,  "nithin.dabilpuram@cavium.com"
> > <nithin.dabilpuram@cavium.com>,  "narayanaprasad.athreya@cavium.com"
> > <narayanaprasad.athreya@cavium.com>
> > Subject: RE: [RFC] eventdev: add crypto adapter API header
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > Sent: Wednesday, December 13, 2017 8:49 PM
> > > To: Eads, Gage <gage.eads@intel.com>
> > > Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> > > dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Rao,
> > > Nikhil <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty,
> > > Declan <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> > > nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> > > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > >
> > > -----Original Message-----
> > > > Date: Wed, 13 Dec 2017 23:35:48 +0000
> > > > From: "Eads, Gage" <gage.eads@intel.com>
> > > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan
> S"
> > > >  <abhinandan.gujjar@intel.com>
> > > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
> > > >  <narender.vangati@intel.com>, "Rao, Nikhil"
> > > > <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
> <hemant.agrawal@nxp.com>, "Doherty, Declan"
> > > >  <declan.doherty@intel.com>, "nidadavolu.murthy@cavium.com"
> > > >  <nidadavolu.murthy@cavium.com>, "nithin.dabilpuram@cavium.com"
> > > >  <nithin.dabilpuram@cavium.com>,
> "narayanaprasad.athreya@cavium.com"
> > > >  <narayanaprasad.athreya@cavium.com>
> > > > Subject: RE: [RFC] eventdev: add crypto adapter API header
> > > >
> > > > Hey Jerin,
> > >
> > > Hey Gage,
> > >
> > > >
> > > > </snip>
> > > >
> > > > > > +
> > > > > > + /**
> > > > > > + * @warning
> > > > > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > > > > + *
> > > > > > + * Crypto event adapter type
> > > > > > + */
> > > > > > +enum rte_event_crypto_adapter_type {
> > > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > > > > +	/**< Start only Rx part of crypto adapter.
> > > > > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > > > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > > > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > > > > +	* Packet's event context will be retained and
> > > > > > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> > > > >
> > > > > How about leveraging ev.op based schematics as mentioned above?
> > > >
> > > > That could work, but perhaps the ev.op should be configured once
> > > > up front, as
> > > I see it being a function of the application architecture. A couple
> > > possible designs, for example:
> > > > - Worker enqueues into cryptodev, adapter polls for response: the
> > > > adapter
> > > port would always use OP_NEW here.
> > > > - Worker sends a crypto request event to the adapter, which gives
> > > > the request to the cryptodev and polls for response: the adapter
> > > > port would always use OP_FWD here. (This ties in with my implicit
> > > > release patch
> > > > (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> > > > - Etc.
> > >
> > > Yes. Semantically both approaches will work. I was trying to avoid
> > > extra clutter(enum rte_event_crypto_adapter_type) in adapter API.
> > > I don't see any problem in moving ev.op to adapter configuration
> > > time if it helps the SW driver.
> > >
> > > IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
> > > RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name,
> something
> > > like,
> RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTE
> > > R_TYPE_OP_FWD
> > > or something like that.
> > >
> >
> > I agree that the two naming schemes are equivalent, but since this option
> would control the adapter's behavior (Rx only vs. Rx + Tx), (IMO) I think
> Abhinandan's original names do a better job of conveying what effect these two
> options have on the adapter, compared to the op type names.
> 
> The only concern with Rx/Tx terminology was, It is mostly used in the ethdev
> domain.
> In crypto domain, typically, we use enqueue/dequeue.
> The only difference between two modes is if adapter enqueue the events with
> RTE_EVENT_OP_NEW vs RTE_EVENT_OP_FORWARD then (IMO) we can change
> something related to that name to avoid adding a new terminology.
> 

Oh, sure -- enqueue/dequeue makes sense here. I'd still prefer DEQ_ONLY or DEQ_ENQ, but the event_op names work just as well.

Speaking of the crypto domain, the cryptodev enqueue and dequeue operations both take crypto op pointers. The original RFC had the request event pointing to an mbuf (which had a crypto_op pointer in its private metadata), but with the suggested opaque eventdev metadata changes it makes more sense for the request event to point to a crypto op. And the RFC didn't specify what the response event would point to (mbuf or crypto op), but to match the cryptodev dequeue operation then a crypto op makes sense. Will this work with your hardware?

> BTW, Based on the earlier discussion, if we need to add opaque eventdev
> metadata to cryptodev then it may change ABI.If so, I think, we need to
> announce ABI change notice for cryptodev and plan cryptodev adapter for
> v18.05.

Personally I'd prefer to get this right/agreed-upon the first time around -- even if that means breaking ABI and pushing this adapter out to 18.05.

Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 18 Dec 2017 16:33:53 +0000
> From: "Eads, Gage" <gage.eads@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>, "Vangati, Narender" <narender.vangati@intel.com>, "Rao,
>  Nikhil" <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "Doherty, Declan" <declan.doherty@intel.com>,
>  "nidadavolu.murthy@cavium.com" <nidadavolu.murthy@cavium.com>,
>  "nithin.dabilpuram@cavium.com" <nithin.dabilpuram@cavium.com>,
>  "narayanaprasad.athreya@cavium.com" <narayanaprasad.athreya@cavium.com>
> Subject: RE: [RFC] eventdev: add crypto adapter API header
> 
> 
> 
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, December 18, 2017 12:30 AM
> > To: Eads, Gage <gage.eads@intel.com>
> > Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> > <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty, Declan
> > <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> > nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > 
> > -----Original Message-----
> > > Date: Thu, 14 Dec 2017 18:52:02 +0000
> > > From: "Eads, Gage" <gage.eads@intel.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > CC: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>, "dev@dpdk.org"
> > >  <dev@dpdk.org>, "Vangati, Narender" <narender.vangati@intel.com>,
> > > "Rao,  Nikhil" <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
> > >  <hemant.agrawal@nxp.com>, "Doherty, Declan"
> > > <declan.doherty@intel.com>,  "nidadavolu.murthy@cavium.com"
> > > <nidadavolu.murthy@cavium.com>,  "nithin.dabilpuram@cavium.com"
> > > <nithin.dabilpuram@cavium.com>,  "narayanaprasad.athreya@cavium.com"
> > > <narayanaprasad.athreya@cavium.com>
> > > Subject: RE: [RFC] eventdev: add crypto adapter API header
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > > Sent: Wednesday, December 13, 2017 8:49 PM
> > > > To: Eads, Gage <gage.eads@intel.com>
> > > > Cc: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> > > > dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Rao,
> > > > Nikhil <nikhil.rao@intel.com>; hemant.agrawal@nxp.com; Doherty,
> > > > Declan <declan.doherty@intel.com>; nidadavolu.murthy@cavium.com;
> > > > nithin.dabilpuram@cavium.com; narayanaprasad.athreya@cavium.com
> > > > Subject: Re: [RFC] eventdev: add crypto adapter API header
> > > >
> > > > -----Original Message-----
> > > > > Date: Wed, 13 Dec 2017 23:35:48 +0000
> > > > > From: "Eads, Gage" <gage.eads@intel.com>
> > > > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Gujjar, Abhinandan
> > S"
> > > > >  <abhinandan.gujjar@intel.com>
> > > > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Vangati, Narender"
> > > > >  <narender.vangati@intel.com>, "Rao, Nikhil"
> > > > > <nikhil.rao@intel.com>, "hemant.agrawal@nxp.com"
> > <hemant.agrawal@nxp.com>, "Doherty, Declan"
> > > > >  <declan.doherty@intel.com>, "nidadavolu.murthy@cavium.com"
> > > > >  <nidadavolu.murthy@cavium.com>, "nithin.dabilpuram@cavium.com"
> > > > >  <nithin.dabilpuram@cavium.com>,
> > "narayanaprasad.athreya@cavium.com"
> > > > >  <narayanaprasad.athreya@cavium.com>
> > > > > Subject: RE: [RFC] eventdev: add crypto adapter API header
> > > > >
> > > > > Hey Jerin,
> > > >
> > > > Hey Gage,
> > > >
> > > > >
> > > > > </snip>
> > > > >
> > > > > > > +
> > > > > > > + /**
> > > > > > > + * @warning
> > > > > > > + * @b EXPERIMENTAL: this enum may change without prior notice
> > > > > > > + *
> > > > > > > + * Crypto event adapter type
> > > > > > > + */
> > > > > > > +enum rte_event_crypto_adapter_type {
> > > > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
> > > > > > > +	/**< Start only Rx part of crypto adapter.
> > > > > > > +	* Packets dequeued from cryptodev are new to eventdev and
> > > > > > > +	* events will be treated as RTE_EVENT_OP_NEW */
> > > > > > > +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
> > > > > > > +	/**< Start both Rx & Tx part of crypto adapter.
> > > > > > > +	* Packet's event context will be retained and
> > > > > > > +	* event will be treated as RTE_EVENT_OP_FORWARD */ };
> > > > > >
> > > > > > How about leveraging ev.op based schematics as mentioned above?
> > > > >
> > > > > That could work, but perhaps the ev.op should be configured once
> > > > > up front, as
> > > > I see it being a function of the application architecture. A couple
> > > > possible designs, for example:
> > > > > - Worker enqueues into cryptodev, adapter polls for response: the
> > > > > adapter
> > > > port would always use OP_NEW here.
> > > > > - Worker sends a crypto request event to the adapter, which gives
> > > > > the request to the cryptodev and polls for response: the adapter
> > > > > port would always use OP_FWD here. (This ties in with my implicit
> > > > > release patch
> > > > > (http://dpdk.org/ml/archives/dev/2017-December/083535.html))
> > > > > - Etc.
> > > >
> > > > Yes. Semantically both approaches will work. I was trying to avoid
> > > > extra clutter(enum rte_event_crypto_adapter_type) in adapter API.
> > > > I don't see any problem in moving ev.op to adapter configuration
> > > > time if it helps the SW driver.
> > > >
> > > > IMO, We can change RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY and
> > > > RTE_EVENT_CRYPTO_ADAPTER_RX_TX to more appropriate name,
> > something
> > > > like,
> > RTE_EVENT_CRYPTO_ADAPTER_TYPE_OP_NEW/RTE_EVENT_CRYPTO_ADAPTE
> > > > R_TYPE_OP_FWD
> > > > or something like that.
> > > >
> > >
> > > I agree that the two naming schemes are equivalent, but since this option
> > would control the adapter's behavior (Rx only vs. Rx + Tx), (IMO) I think
> > Abhinandan's original names do a better job of conveying what effect these two
> > options have on the adapter, compared to the op type names.
> > 
> > The only concern with Rx/Tx terminology was, It is mostly used in the ethdev
> > domain.
> > In crypto domain, typically, we use enqueue/dequeue.
> > The only difference between two modes is if adapter enqueue the events with
> > RTE_EVENT_OP_NEW vs RTE_EVENT_OP_FORWARD then (IMO) we can change
> > something related to that name to avoid adding a new terminology.
> > 
> 
> Oh, sure -- enqueue/dequeue makes sense here. I'd still prefer DEQ_ONLY or DEQ_ENQ, but the event_op names work just as well.

I prefer event_op name but enqueue/dequeue name work as well.

> 
> Speaking of the crypto domain, the cryptodev enqueue and dequeue operations both take crypto op pointers. The original RFC had the request event pointing to an mbuf (which had a crypto_op pointer in its private metadata), but with the suggested opaque eventdev metadata changes it makes more sense for the request event to point to a crypto op. And the RFC didn't specify what the response event would point to (mbuf or crypto op), but to match the cryptodev dequeue operation then a crypto op makes sense. Will this work with your hardware?

Yes. crypto op will work with Cavium HW. NXP guys can comment on their HW.
We are treating rte_event.event_ptr as opaque event pointer so it can carry
crypto_op or mbuf pointer.I think, For crypto operation, rte_crypto_op make sense as it has "status" etc.
May be for inline ipsec, mbuf would make sense. So I think, we can support both options by
reserving size of struct rte_event as eventdev metadata in crypto area.

> 
> > BTW, Based on the earlier discussion, if we need to add opaque eventdev
> > metadata to cryptodev then it may change ABI.If so, I think, we need to
> > announce ABI change notice for cryptodev and plan cryptodev adapter for
> > v18.05.
> 
> Personally I'd prefer to get this right/agreed-upon the first time around -- even if that means breaking ABI and pushing this adapter out to 18.05.

I agree and that makes sense too.