-----Original Message-----
> Date: Mon, 15 Jan 2018 16:23:50 +0530
> From: Abhinandan Gujjar <abhinandan.guj...@intel.com>
> To: jerin.ja...@caviumnetworks.com
> CC: dev@dpdk.org, narender.vang...@intel.com, Abhinandan Gujjar
> <abhinandan.guj...@intel.com>, Nikhil Rao <nikhil....@intel.com>, Gage
> Eads <gage.e...@intel.com>
> Subject: [RFC v2, 2/2] eventdev: add crypto adapter API header
> X-Mailer: git-send-email 1.9.1
>
> Add crypto event adapter APIs to support packet transfer
> mechanism between cryptodev and event device.
>
> Signed-off-by: Abhinandan Gujjar <abhinandan.guj...@intel.com>
> Signed-off-by: Nikhil Rao <nikhil....@intel.com>
> Signed-off-by: Gage Eads <gage.e...@intel.com>
> ---
Overall it looks good to me. Though I have some confusion over
ENQ-DEQ scheme. May be it will be get cleared along with implementation.
> Notes:
> V2:
> 1. Updated type as ENQ-DEQ in rte_event_crypto_adapter_type
> 2. Removed enum rte_event_crypto_conf_type
> 3. Updated struct rte_event_crypto_metadata
> 4. Removed struct rte_event_crypto_queue_pair_conf
> 5. Updated rte_event_crypto_adapter_queue_pair_add() API
>
> lib/librte_eventdev/Makefile | 1 +
> lib/librte_eventdev/rte_event_crypto_adapter.h | 452
> +++++++++++++++++++++++++
1) Please update MAINTAINERS file
2) Add meson build support
3) Update doc/api/doxy-api-index.md file and check the doxygen output
using "make doc-api-html"
4) Please add the programmers guide in doc/guides/prog_guide/
>
> +++ b/lib/librte_eventdev/rte_event_crypto_adapter.h
> @@ -0,0 +1,452 @@
> +/*
> + * Copyright(c) 2018 Intel Corporation. All rights reserved.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
Use SPDX tag scheme
> +#ifndef _RTE_EVENT_CRYPTO_ADAPTER_
> +#define _RTE_EVENT_CRYPTO_ADAPTER_
> +
> +/**
> + * This adapter adds support to enqueue crypto completions to event device.
> + * The packet flow from cryptodev to the event device can be accomplished
> + * using both SW and HW based transfer mechanisms.
> + * The adapter uses a EAL service core function for SW based packet transfer
> + * and uses the eventdev PMD functions to configure HW based packet transfer
> + * between the cryptodev and the event device.
> + *
> + * In the case of SW based transfers, application can choose to submit a
I think, we can remove "In the case of SW based transfers" as it should
be applicable for HW case too
> + * crypto operation directly to cryptodev or send it to the cryptodev
> + * adapter via eventdev, the cryptodev adapter then submits the crypto
> + * operation to the crypto device. The first mode is known as the
The first mode (DEQ) is very clear. In the second mode(ENQ_DEQ),
- How does "worker" submits the crypto work through crypto-adapter?
If I understand it correctly, "workers" always deals with only
cryptodev's rte_cryptodev_enqueue_burst() API and "service" function in crypto
adapter
would be responsible for dequeue() from cryptodev and enqueue to eventdev?
I understand the need for OP_NEW vs OP_FWD mode difference in both
modes. Other than that, What makes ENQ_DEQ different? Could you share the
flow for ENQ_DEQ mode with APIs.
> + * dequeue only (DEQ) mode and the second as the enqueue - dequeue
extra space between "mode" and "and"
> + * (ENQ_DEQ) mode. The choice of mode can be specified when creating
> + * the adapter.
> + * In the latter choice, the cryptodev adapter is able to use
> + * RTE_OP_FORWARD as the event dev enqueue type, this has a performance
> + * advantage in "closed system" eventdevs like the eventdev SW PMD and
s/eventdevs/eventdev ??
> + * also, the event cannot be dropped.
> + *
> + * In the ENQ_DEQ mode the application needs to specify the cryptodev ID
> + * and queue pair ID (request information) in addition to the event
> + * information (response information) needed to enqueue an event after
> + * the crypto operation has completed. The request and response information
> + * are specified in the rte_crypto_op private_data. In the DEQ mode the
> + * the application is required to provide only the response information.
Why ENQ_DEQ modes needs cryptodev ID and queue pair ID? Which is part of
rte_cryptodev_enqueue_burst(). May be I am missing something.
If ENQ_DEQ modes help in SW case then we can add it as capability.
I am just trying to see any scope of convergence.
> + *
> + * In the ENQ_DEQ mode, application sends crypto operations as events to
> + * the adapter which dequeues events and programs cryptodev operations.
> + * The adapter then dequeues crypto completions from cryptodev and
> + * enqueue events to the event device.
> + *
> + * In the case of HW based transfers, the cryptodev PMD callback invoked
> + * from rte_cryptodev_enqueue_burst() uses the response information to
> + * setup the event for the cryptodev completion.
> + *
> + * The event crypto adapter provides common APIs to configure the packet flow
> + * from the cryptodev to event devices across both SW and HW based transfers.
> + * The crypto event adapter's functions are:
> + * - rte_event_crypto_adapter_create_ext()
> + * - rte_event_crypto_adapter_create()
> + * - rte_event_crypto_adapter_free()
> + * - rte_event_crypto_adapter_queue_pair_add()
> + * - rte_event_crypto_adapter_queue_pair_del()
> + * - rte_event_crypto_adapter_start()
> + * - rte_event_crypto_adapter_stop()
> + * - rte_event_crypto_adapter_stats_get()
> + * - rte_event_crypto_adapter_stats_reset()
> +
> + * The applicaton creates an instance using rte_event_crypto_adapter_create()
s/applicaton/application
> + * or rte_event_crypto_adapter_create_ext().
> + *
> + * Cryptodev queue pair addition/deletion is done
> + * using the rte_event_crypto_adapter_queue_pair_xxx() APIs.
> + *
> + * The SW adapter or HW PMD uses rte_crypto_op::private_data_type to decide
> + * whether request/response data is located in the crypto session/crypto
> + * security session or at an offset in the rte_crypto_op.
> + * rte_crypto_op::private_data_offset is used to locate the request/response
> + * in the rte_crypto_op. If the rte_crypto_op::private_data_type
> + * indicates that the data is in the crypto session/crypto security session
> + * then the rte_crypto_op::sess_type is used to decide whether the private
> + * data is in the session or security session.
> + *
> + * For session-less it is mandatory to place the request/response data with
> + * the rte_crypto_op where as with crypto session/security session it can be
> + * placed with the rte_crypto_op or in the session/security session.
Please update(if needed) above two paragraphs based on the conclusion
from cryptodev change
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this enum may change without prior notice
Change to new EXPERIMENTAL tag scheme.
> + *
> + * Crypto event adapter type
> + */
> +enum rte_event_crypto_adapter_type {
> + RTE_EVENT_CRYPTO_ADAPTER_DEQ_ONLY = 1,
> + /**< Start only dequeue part of crypto adapter.
> + * Packets dequeued from cryptodev are enqueued to eventdev
> + * as new events and events will be treated as RTE_EVENT_OP_NEW. */
> + RTE_EVENT_CRYPTO_ADAPTER_ENQ_DEQ,
> + /**< Start both enqueue & dequeue part of crypto adapter.
> + * Packet's event context will be retained and
> + * event will be treated as RTE_EVENT_OP_FORWARD. */
This description makes sense.
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event request structure will be fill by application to
> + * provide event request information to the adapter.
> + */
> +struct rte_event_crypto_request {
> + uint8_t resv[8];
> + /**< Overlaps with first 8 bytes of struct rte_event
> + * that encode the response event information
> + */
> + uint16_t cdev_id;
> + /**< cryptodev ID to be used */
> + uint16_t queue_pair_id;
> + /**< cryptodev queue pair ID to be used */
Shouldn't we say it is need only with ENQ_DEQ mode as described earlier.
adding "@see" section would help
> + uint32_t resv1;
> + /**< Reserved bits */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Crypto event metadata structure will be filled by application
> + * to provide crypto request and event response information.
> + *
> + * If crypto events are enqueued using a HW mechanism, the cryptodev
> + * PMD will use the event response information to set up the event
> + * that is enqueued back to eventdev after completion of the crypto
> + * operation. If the transfer is done by SW, it will be used by the
> + * adapter.
> + */
> +union rte_event_crypto_metadata {
> + struct rte_event_crypto_request request_info;
> + struct rte_event response_info;
Perfect.
Other than above comments, everything else looks OK to me.
