All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
@ 2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
  2017-09-23 11:35     ` Rao, Nikhil
  2017-09-22  6:08   ` santosh
  2017-09-22  9:10   ` Jerin Jacob
  2 siblings, 1 reply; 30+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2017-09-21 15:43 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar

Hi Nikhil,

Few comments Inline

On Fri, Sep 22, 2017 at 02:47:13AM +0530, Nikhil Rao wrote:
> Add common APIs for configuring packet transfer from ethernet Rx
> queues to event devices across HW & SW packet transfer mechanisms.
> A detailed description of the adapter is contained in the header's
> comments.
>
> The adapter implementation uses eventdev PMDs to configure the packet
> transfer if HW support is available and if not, it uses an EAL service
> function that reads packets from ethernet Rx queues and injects these
> as events into the event device.
>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
>  lib/Makefile                                   |    2 +-
>  lib/librte_eventdev/Makefile                   |    2 +
>  lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
>  5 files changed, 1635 insertions(+), 2 deletions(-)
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
>
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> new file mode 100644
> index 000000000..c3849ec31
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -0,0 +1,384 @@
> +/*
> +

<snip>

> +/**
> + * Create a new ethernet Rx event adapter with the specified identifier.
> + *
> + * @param id
> + *  The identifier of the ethernet Rx event adapter.
> + *
> + * @dev_id
> + *  The identifier of the device to configure.
> + *
> + * @eth_port_id
> + *  The identifier of the ethernet device.
> + *
Invalid param
> + * @param conf_cb
> + *  Callback function that fills in members of a
> + *  struct rte_event_eth_rx_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_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> +					rx_adapter_conf_cb conf_cb,
> +					void *conf_arg);
> +
> +/**
> + * Create a new ethernet Rx event 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_eth_rx_adapter_create_ext() version.
> + *
> + * @param id
> + *  The identifier of the ethernet Rx event adapter.
> + *
> + * @dev_id
> + *  The identifier of the device to configure.
> + *
> + * @eth_port_id
> + *  The identifier of the ethernet device.
> + *
> + * @param conf_cb
> + *  Callback function that fills in members of a
> + *  struct rte_event_eth_rx_adapter_conf struct passed into
> + *  it.
> + *
> + * @param conf_arg
> + *  Argument of type *rte_event_port_conf* that is passed to the conf_cb
> + *  function.
> + *
Invalid param
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure
> + */
> +int rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
> +				struct rte_event_port_conf *port_config);
> +
> +/**
> + * Free an event adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure, If the adapter still has Rx queues
> + *      added to it, the function returns -EBUSY.
> + */
> +int rte_event_eth_rx_adapter_free(uint8_t id);
> +
<snip>
> +/**
> + * Reset statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_stats_reset(uint8_t id);
> +
> +/**
> + * 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 missing

> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.

Invalid description.

> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> + * function, this function returns -ESRCH.
> + */
> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..d5b655dae
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> @@ -0,0 +1,1238 @@
<snip>
> +
> +static int
> +rx_adapter_ctrl(uint8_t id, int start)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	struct rte_eventdev *dev;
> +	struct eth_device_info *dev_info;
> +	uint32_t i;
> +	int use_service = 0;
> +	int stop = !start;
> +
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +	rx_adapter = id_to_rx_adapter(id);
> +	if (!rx_adapter)
> +		return -EINVAL;
> +
> +	dev = &rte_eventdevs[rx_adapter->eventdev_id];
> +
> +	for (i = 0; i < rte_eth_dev_count(); i++) {
> +		dev_info = &rx_adapter->eth_devices[i];
> +		/* if start  check for num dev queues */
> +		if (start && !dev_info->nb_dev_queues)
> +			continue;
> +		/* if stop check if dev has been started */
> +		if (stop && !dev_info->dev_rx_started)
> +			continue;
> +		use_service |= !dev_info->internal_event_port;
> +		dev_info->dev_rx_started = start;
> +		if (!dev_info->internal_event_port)
> +			continue;
> +		start ? (*dev->dev_ops->eth_rx_adapter_start)(dev,
> +						&rte_eth_devices[i]) :
> +			(*dev->dev_ops->eth_rx_adapter_stop)(dev,
> +						&rte_eth_devices[i]);
> +	}
> +
> +	if (use_service)

Here setting the service run state is not sufficient we need to enable the
service on a service core calling rte_service_start_with_defaults() should be
sufficient.

> +		rte_service_runstate_set(rx_adapter->service_id, start);
> +
> +	return 0;
> +}
> +
<snip>

Regards,
Pavan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
  2017-09-21 21:17 ` [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
@ 2017-09-21 15:46   ` Jerin Jacob
  2017-09-24 12:14     ` Rao, Nikhil
  0 siblings, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-09-21 15:46 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Fri, 22 Sep 2017 02:47:11 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>  harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, erik.g.carrillo@intel.com,
>  abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> Subject: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for
>  rte_event_eth_rx_adapter
> X-Mailer: git-send-email 2.7.4
> 
> The caps API allows application to get information
> needed to configure the ethernet receive adapter for the eventdev and
> ethdev pair.
> 
> The PMD callbacks are used by the rte_event_eth_rx_xxx() APIs to
> configure and control the ethernet receive adapter if packet transfers
> from the ethdev to eventdev is implemented in hardware.

IMO, it better to split the cap PMD API and cap normative API into one patch and
remaining PMD API to another patch.

> 
> For e.g., the ethdev, eventdev pairing maybe such that all of the
> Eth Rx queues can only be connected to a single event queue, in
> which case the application is required to pass in -1 as the queue id
> when adding a receive queue to the adapter.
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
>  lib/librte_eventdev/rte_eventdev.h           |  33 +++++
>  lib/librte_eventdev/rte_eventdev_pmd.h       | 173 +++++++++++++++++++++++++++
>  lib/librte_eventdev/rte_eventdev.c           |  24 ++++
>  lib/librte_eventdev/rte_eventdev_version.map |   8 ++
>  4 files changed, 238 insertions(+)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 128bc5221..a8bebac01 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -990,6 +990,39 @@ struct rte_event {
>  	};
>  };
>  
> +/* Ethdev Rx adapter capability bitmap flags */
> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT	0x1
> +/**< Eventdev can send packets to ethdev using internal event port */
> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ	0x2
> +/**< Ethdev Rx queues can be connected to single event queue */

I think, Its is more of limitation. Since we are expressing it as
capability. How about changing it as RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
(same as exiting !RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ and SW driver has this capability)
i.e Ethdev Rx queues of single ethdev port can be connected to multiple
event queue.

> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID		0x4
> +/**< Ethdev Rx adapter can set flow ID for event queue, if this flag
> + * is unset, the application needs to provide a flow id when adding

Looking at implementation, If I understand it correctly, it not application
"needs" to provide instead, it is application can provide. If so, I think,
making it as RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID or
RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID makes more sense.

something like,

#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID		0x4
/**< If this flag is set then the application can override the default
 * hash based flow id generated by the Rx adapter when the event
 * enqueues to the event queue.
 *
 * @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
 * @see rte_event_eth_rx_adapter_queue_conf::ev::flow_id
 */

>  
> +int
> +rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
> +				uint32_t *caps)
> +{
> +	struct rte_eventdev *dev;
> +
> +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_port_id, -EINVAL);
> +
> +	dev = &rte_eventdevs[dev_id];
> +
> +	if (caps == NULL)
> +		return -EINVAL;
> +	*caps = 0;
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_caps_get,
> +				-ENOTSUP);

How about not returning the -ENOTSUP. Instead just call
eth_rx_adapter_caps_get if it not NULL. By this way if a PMD does not
have any cap, it does not need to implement this hook.

> +	return (*dev->dev_ops->eth_rx_adapter_caps_get)
> +						(dev,
> +						&rte_eth_devices[eth_port_id],
> +						caps);
> +}
> +
>  static inline int
>  rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t nb_queues)
>  {
> diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
> index 4c48e5f0a..996b361a5 100644
> --- a/lib/librte_eventdev/rte_eventdev_version.map
> +++ b/lib/librte_eventdev/rte_eventdev_version.map
> @@ -51,3 +51,11 @@ DPDK_17.08 {
>  	rte_event_ring_init;
>  	rte_event_ring_lookup;
>  } DPDK_17.05;
> +
> +
Additional line feed.

> +DPDK_17.11 {
> +	global:
> +
> +	rte_event_eth_rx_adapter_caps_get;
> +
> +} DPDK_17.08;
> -- 
> 2.14.1.145.gb3622a4
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter
@ 2017-09-21 21:17 Nikhil Rao
  2017-09-21 21:17 ` [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Nikhil Rao @ 2017-09-21 21:17 UTC (permalink / raw)
  To: jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

Eventdev-based networking applications require a component to dequeue
packets from NIC Rx queues and inject them into eventdev queues[1]. While
some platforms (e.g. Cavium Octeontx) do this operation in hardware, other
platforms use software.

This patchset introduces an ethernet Rx event adapter that dequeues packets
from ethernet devices and enqueues them to event devices. This patch is based on
a previous RFC[2] and supercedes [3], the main difference being that
this version implements a common abstraction for HW and SW based packet transfers.

The adapter is designed to work with the EAL service core[4] for SW based
packet transfers. An eventdev PMD callback is used to determine that SW
based packet transfer service is required. The application can discover
and configure the service with a core mask using rte_service APIs.

The adapter can service multiple ethernet devices and queues. For SW based
packet transfers each queue is  configured with a servicing weight to
control the relative frequency with which the adapter polls the queue,
and the event fields to use when constructing packet events. The adapter
has two modes for programming an event's flow ID: use a static per-queue
user-specified value or use the RSS hash.

A detailed description of the adapter is contained in the header's
comments.

[1] http://dpdk.org/ml/archives/dev/2017-May/065341.html
[2] http://dpdk.org/ml/archives/dev/2017-May/065539.html
[3] http://dpdk.org/ml/archives/dev/2017-July/070452.html
[4] http://dpdk.org/ml/archives/dev/2017-July/069782.html

Note: The service API version used this patch is the posted
at http://dpdk.org/ml/archives/dev/2017-August/073102.html

v3:
- This patch extends the V2 implementation with some changes to
provide a common API across HW & SW packet transfer mechanisms from 
the eth devices to the event devices.
- Introduces a caps API that is used by the apps and the Rx
adapter to configure the packet transfer path.
- Adds an API to retrieve the service ID of the service function.
(if one is used by the adapter)

v4:
- Unit test fixes (Santosh Shukla)
- Pass ethernet device pointer to eventdev callback functions
instead of ethernet port ID. (Nipun Gupta)
- Check for existence of stats callback before invocation (Nipun Gupta)
- Various code cleanups (Harry Van Haaren)

Nikhil Rao (4):
  eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
  eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD
  eventdev: Add eventdev ethernet Rx adapter
  eventdev: Add tests for event eth Rx adapter APIs

 lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
 lib/librte_eventdev/rte_eventdev.h             |   33 +
 lib/librte_eventdev/rte_eventdev_pmd.h         |  179 ++++
 drivers/event/sw/sw_evdev.c                    |   15 +
 lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev.c             |   24 +
 test/test/test_event_eth_rx_adapter.c          |  399 ++++++++
 lib/Makefile                                   |    2 +-
 lib/librte_eventdev/Makefile                   |    2 +
 lib/librte_eventdev/rte_eventdev_version.map   |   17 +
 test/test/Makefile                             |    1 +
 11 files changed, 2293 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
 create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
 create mode 100644 test/test/test_event_eth_rx_adapter.c

-- 
2.14.1.145.gb3622a4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
  2017-09-21 21:17 [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
@ 2017-09-21 21:17 ` Nikhil Rao
  2017-09-21 15:46   ` Jerin Jacob
  2017-09-21 21:17 ` [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 30+ messages in thread
From: Nikhil Rao @ 2017-09-21 21:17 UTC (permalink / raw)
  To: jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

The caps API allows application to get information
needed to configure the ethernet receive adapter for the eventdev and
ethdev pair.

The PMD callbacks are used by the rte_event_eth_rx_xxx() APIs to
configure and control the ethernet receive adapter if packet transfers
from the ethdev to eventdev is implemented in hardware.

For e.g., the ethdev, eventdev pairing maybe such that all of the
Eth Rx queues can only be connected to a single event queue, in
which case the application is required to pass in -1 as the queue id
when adding a receive queue to the adapter.

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 lib/librte_eventdev/rte_eventdev.h           |  33 +++++
 lib/librte_eventdev/rte_eventdev_pmd.h       | 173 +++++++++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev.c           |  24 ++++
 lib/librte_eventdev/rte_eventdev_version.map |   8 ++
 4 files changed, 238 insertions(+)

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index 128bc5221..a8bebac01 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -990,6 +990,39 @@ struct rte_event {
 	};
 };
 
+/* Ethdev Rx adapter capability bitmap flags */
+#define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT	0x1
+/**< Eventdev can send packets to ethdev using internal event port */
+#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ	0x2
+/**< Ethdev Rx queues can be connected to single event queue */
+#define RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID		0x4
+/**< Ethdev Rx adapter can set flow ID for event queue, if this flag
+ * is unset, the application needs to provide a flow id when adding
+ * the Rx queue to the adapter.
+ */
+
+/**
+ * Retrieve the event device's ethdev Rx adapter capabilities for the
+ * specified ethernet port
+ *
+ * @param dev_id
+ *   The identifier of the device.
+ *
+ * @param eth_port_id
+ *   The identifier of the ethernet device.
+ *
+ * @param[out] caps
+ *   A pointer to memory filled with Rx event adapter capabilities.
+ *
+ * @return
+ *   - 0: Success, driver provides Rx event adapter capabilities for the
+ *	ethernet device.
+ *   - <0: Error code returned by the driver function.
+ *
+ */
+int
+rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
+				uint32_t *caps);
 
 struct rte_eventdev_driver;
 struct rte_eventdev_ops;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 3d72acf3a..89990d1c4 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -86,6 +86,8 @@ extern "C" {
 #define RTE_EVENTDEV_DETACHED  (0)
 #define RTE_EVENTDEV_ATTACHED  (1)
 
+struct rte_eth_dev;
+
 /** Global structure used for maintaining state of allocated event devices */
 struct rte_eventdev_global {
 	uint8_t nb_devs;	/**< Number of devices found */
@@ -429,6 +431,162 @@ typedef int (*eventdev_xstats_get_names_t)(const struct rte_eventdev *dev,
 typedef uint64_t (*eventdev_xstats_get_by_name)(const struct rte_eventdev *dev,
 		const char *name, unsigned int *id);
 
+
+/**
+ * Retrieve the event device's ethdev Rx adapter capabilities for the
+ * specified ethernet port
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @param[out] caps
+ *   A pointer to memory filled with Rx event adapter capabilities.
+ *
+ * @return
+ *   - 0: Success, driver provides Rx event adapter capabilities for the
+ *	ethernet device.
+ *   - <0: Error code returned by the driver function.
+ *
+ */
+typedef int (*eventdev_eth_rx_adapter_caps_get_t)
+					(const struct rte_eventdev *dev,
+					const struct rte_eth_dev *eth_dev,
+					uint32_t *caps);
+
+struct rte_event_eth_rx_adapter_queue_conf *queue_conf;
+
+/**
+ * Add ethernet Rx queues to event device. This callback is invoked if
+ * the caps returned from rte_eventdev_eth_rx_adapter_caps_get(, eth_port_id)
+ * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT set.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @param rx_queue_id
+ *   Ethernet device receive queue index
+ *
+ * @param queue_conf
+ *  Additonal configuration structure
+
+ * @return
+ *   - 0: Success, ethernet receive queue added successfully.
+ *   - <0: Error code returned by the driver function.
+ *
+ */
+typedef int (*eventdev_eth_rx_adapter_queue_add_t)(
+		const struct rte_eventdev *dev,
+		const struct rte_eth_dev *eth_dev,
+		int32_t rx_queue_id,
+		const struct rte_event_eth_rx_adapter_queue_conf *queue_conf);
+
+/**
+ * Delete ethernet Rx queues from event device. This callback is invoked if
+ * the caps returned from eventdev_eth_rx_adapter_caps_get(, eth_port_id)
+ * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT set.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @param rx_queue_id
+ *   Ethernet device receive queue index
+ *
+ * @return
+ *   - 0: Success, ethernet receive queue deleted successfully.
+ *   - <0: Error code returned by the driver function.
+ *
+ */
+typedef int (*eventdev_eth_rx_adapter_queue_del_t)
+					(const struct rte_eventdev *dev,
+					const struct rte_eth_dev *eth_dev,
+					int32_t rx_queue_id);
+
+/**
+ * Start ethernet Rx adapter. This callback is invoked if
+ * the caps returned from eventdev_eth_rx_adapter_caps_get(.., eth_port_id)
+ * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT set and Rx queues
+ * from eth_port_id have been added to the event device.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @return
+ *   - 0: Success, ethernet Rx adapter started successfully.
+ *   - <0: Error code returned by the driver function.
+ */
+typedef int (*eventdev_eth_rx_adapter_start_t)
+					(const struct rte_eventdev *dev,
+					const struct rte_eth_dev *eth_dev);
+
+/**
+ * Stop ethernet Rx adapter. This callback is invoked if
+ * the caps returned from eventdev_eth_rx_adapter_caps_get(..,eth_port_id)
+ * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT set and Rx queues
+ * from eth_port_id have been added to the event device.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @return
+ *   - 0: Success, ethernet Rx adapter stopped successfully.
+ *   - <0: Error code returned by the driver function.
+ */
+typedef int (*eventdev_eth_rx_adapter_stop_t)
+					(const struct rte_eventdev *dev,
+					const struct rte_eth_dev *eth_dev);
+
+struct rte_event_eth_rx_adapter_stats *stats;
+
+/**
+ * Retrieve ethernet Rx adapter statistics.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @param[out] stats
+ *   Pointer to stats structure
+ * @return
+ *   Return 0 on success.
+ */
+
+typedef int (*eventdev_eth_rx_adapter_stats_get)
+			(const struct rte_eventdev *dev,
+			const struct rte_eth_dev *eth_dev,
+			struct rte_event_eth_rx_adapter_stats *stats);
+/**
+ * Reset ethernet Rx adapter statistics.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param eth_dev
+ *   Ethernet device pointer
+ *
+ * @return
+ *   Return 0 on success.
+ */
+typedef int (*eventdev_eth_rx_adapter_stats_reset)
+			(const struct rte_eventdev *dev,
+			const struct rte_eth_dev *eth_dev);
+
 /** Event device operations function pointer table */
 struct rte_eventdev_ops {
 	eventdev_info_get_t dev_infos_get;	/**< Get device info. */
@@ -468,6 +626,21 @@ struct rte_eventdev_ops {
 	/**< Get one value by name. */
 	eventdev_xstats_reset_t xstats_reset;
 	/**< Reset the statistics values in xstats. */
+
+	eventdev_eth_rx_adapter_caps_get_t eth_rx_adapter_caps_get;
+	/**< Get ethernet Rx adapter capabilities */
+	eventdev_eth_rx_adapter_queue_add_t eth_rx_adapter_queue_add;
+	/**< Add Rx queues to ethernet Rx adapter */
+	eventdev_eth_rx_adapter_queue_del_t eth_rx_adapter_queue_del;
+	/**< Delete Rx queues from ethernet Rx adapter */
+	eventdev_eth_rx_adapter_start_t eth_rx_adapter_start;
+	/**< Start ethernet Rx adapter */
+	eventdev_eth_rx_adapter_stop_t eth_rx_adapter_stop;
+	/**< Stop ethernet Rx adapter */
+	eventdev_eth_rx_adapter_stats_get eth_rx_adapter_stats_get;
+	/**< Get ethernet Rx stats */
+	eventdev_eth_rx_adapter_stats_reset eth_rx_adapter_stats_reset;
+	/**< Reset ethernet Rx stats */
 };
 
 /**
diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index bbb380502..234cd8258 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -56,6 +56,7 @@
 #include <rte_common.h>
 #include <rte_malloc.h>
 #include <rte_errno.h>
+#include <rte_ethdev.h>
 
 #include "rte_eventdev.h"
 #include "rte_eventdev_pmd.h"
@@ -128,6 +129,29 @@ rte_event_dev_info_get(uint8_t dev_id, struct rte_event_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
+				uint32_t *caps)
+{
+	struct rte_eventdev *dev;
+
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_port_id, -EINVAL);
+
+	dev = &rte_eventdevs[dev_id];
+
+	if (caps == NULL)
+		return -EINVAL;
+	*caps = 0;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_caps_get,
+				-ENOTSUP);
+	return (*dev->dev_ops->eth_rx_adapter_caps_get)
+						(dev,
+						&rte_eth_devices[eth_port_id],
+						caps);
+}
+
 static inline int
 rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t nb_queues)
 {
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 4c48e5f0a..996b361a5 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -51,3 +51,11 @@ DPDK_17.08 {
 	rte_event_ring_init;
 	rte_event_ring_lookup;
 } DPDK_17.05;
+
+
+DPDK_17.11 {
+	global:
+
+	rte_event_eth_rx_adapter_caps_get;
+
+} DPDK_17.08;
-- 
2.14.1.145.gb3622a4

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD
  2017-09-21 21:17 [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
  2017-09-21 21:17 ` [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
@ 2017-09-21 21:17 ` Nikhil Rao
  2017-09-22  2:49   ` Jerin Jacob
  2017-09-22  5:27   ` santosh
  2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
  2017-09-21 21:17 ` [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
  3 siblings, 2 replies; 30+ messages in thread
From: Nikhil Rao @ 2017-09-21 21:17 UTC (permalink / raw)
  To: jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 lib/librte_eventdev/rte_eventdev_pmd.h |  6 ++++++
 drivers/event/sw/sw_evdev.c            | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 89990d1c4..de411fdf2 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -83,6 +83,12 @@ extern "C" {
 	} \
 } while (0)
 
+#define RTE_EVENT_ETH_RX_ADAPTER_SW_CAP \
+		RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID
+/**< Ethernet Rx adapter cap to return If the packet transfers from
+ * the ethdev to eventdev use a SW service function
+ */
+
 #define RTE_EVENTDEV_DETACHED  (0)
 #define RTE_EVENTDEV_ATTACHED  (1)
 
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index da6ac30f4..aed8b728f 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -437,6 +437,19 @@ sw_dev_configure(const struct rte_eventdev *dev)
 	return 0;
 }
 
+struct rte_eth_dev;
+
+static int
+sw_eth_rx_adapter_caps_get(const struct rte_eventdev *dev,
+			const struct rte_eth_dev *eth_dev,
+			uint32_t *caps)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(eth_dev);
+	*caps = RTE_EVENT_ETH_RX_ADAPTER_SW_CAP;
+	return 0;
+}
+
 static void
 sw_info_get(struct rte_eventdev *dev, struct rte_event_dev_info *info)
 {
@@ -751,6 +764,8 @@ sw_probe(struct rte_vdev_device *vdev)
 			.port_link = sw_port_link,
 			.port_unlink = sw_port_unlink,
 
+			.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
+
 			.xstats_get = sw_xstats_get,
 			.xstats_get_names = sw_xstats_get_names,
 			.xstats_get_by_name = sw_xstats_get_by_name,
-- 
2.14.1.145.gb3622a4

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-21 21:17 [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
  2017-09-21 21:17 ` [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
  2017-09-21 21:17 ` [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
@ 2017-09-21 21:17 ` Nikhil Rao
  2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
                     ` (2 more replies)
  2017-09-21 21:17 ` [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
  3 siblings, 3 replies; 30+ messages in thread
From: Nikhil Rao @ 2017-09-21 21:17 UTC (permalink / raw)
  To: jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

Add common APIs for configuring packet transfer from ethernet Rx
queues to event devices across HW & SW packet transfer mechanisms.
A detailed description of the adapter is contained in the header's
comments.

The adapter implementation uses eventdev PMDs to configure the packet
transfer if HW support is available and if not, it uses an EAL service
function that reads packets from ethernet Rx queues and injects these
as events into the event device.

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
Signed-off-by: Gage Eads <gage.eads@intel.com>
Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
---
 lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
 lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
 lib/Makefile                                   |    2 +-
 lib/librte_eventdev/Makefile                   |    2 +
 lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
 5 files changed, 1635 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
 create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c

diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
new file mode 100644
index 000000000..c3849ec31
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
@@ -0,0 +1,384 @@
+/*
+ *   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_ETH_RX_ADAPTER_
+#define _RTE_EVENT_ETH_RX_ADAPTER_
+
+/**
+ * @file
+ *
+ * RTE Event Ethernet Rx Adapter
+ *
+ * An eventdev-based packet processing application enqueues/dequeues mbufs
+ * to/from the event device. The application uses the adapter APIs to configure
+ * the packet flow between the ethernet devices and event devices. Depending on
+ * on the capabilties of the eventdev PMD, the adapter may use a EAL service
+ * core function for packet transfer or use internal PMD functions to configure
+ * the packet transfer between the ethernet device and the event device.
+ *
+ * The ethernet Rx event adapter's functions are:
+ *  - rte_event_eth_rx_adapter_create_ext()
+ *  - rte_event_eth_rx_adapter_create()/free()
+ *  - rte_event_eth_rx_adapter_queue_add()/del()
+ *  - rte_event_eth_rx_adapter_start()/stop()
+ *  - rte_event_eth_rx_adapter_stats_get()/reset()
+ *
+ * The applicaton creates an event to ethernet adapter using
+ * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
+ * functions.
+ * The adapter needs to know which ethernet rx queues to poll for mbufs as well
+ * as event device parameters such as the event queue identifier, event
+ * priority and scheduling type that the adapter should use when constructing
+ * events. The rte_event_eth_rx_adapter_queue_add() function is provided for
+ * this purpose.
+ * The servicing weight parameter in the rte_event_eth_rx_adapter_queue_conf
+ * is applicable when the Rx adapter uses a service core function and is
+ * intended to provide application control of the polling frequency of ethernet
+ * device receive queues, for example, the application may want to poll higher
+ * priority queues with a higher frequency but at the same time not starve
+ * lower priority queues completely. If this parameter is zero and the receive
+ * interrupt is enabled when configuring the device, the receive queue is
+ * interrupt driven; else, the queue is assigned a servicing weight of one.
+ *
+ * If the adapter uses a rte_service function, then the application is also
+ * required to assign a core to the service function and control the service
+ * core using the rte_service APIs. The rte_event_eth_rx_adapter_service_id_get
+ * function can be used to retrieve the service function ID of the adapter in
+ * this case.
+ *
+ * Note: Interrupt driven receive queues are currentely unimplemented.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <rte_service.h>
+
+#include "rte_eventdev.h"
+
+#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32
+
+/* struct rte_event_eth_rx_adapter_queue_conf flags definitions */
+#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID	0x1
+/**< This flag indicates the flow identifier is valid
+ * @see rte_event_eth_rx_adapter_queue_conf
+ */
+
+struct rte_event_eth_rx_adapter_conf {
+	uint8_t event_port_id;
+	/**< Event port identifier, the adapter enqueues mbuf events to this
+	 * port
+	 */
+	uint32_t max_nb_rx;
+	/**< The adapter can return early if it has processed at least
+	 * max_nb_rx mbufs. This isn't treated as a requirement; batching may
+	 * cause the adapter to process more than max_nb_rx mbufs
+	 */
+};
+
+/**
+ * Function type used for adapter configuration callback. The callback is
+ * used to fill in members of the struct rte_event_eth_rx_adapter_conf, this
+ * callback is invoked when creating a SW service for packet transfer from
+ * ethdev queues to the event device. The SW service is created within the
+ * rte_event_eth_rx_adapter_queue_add() function if packet required.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param dev_id
+ *  Event device identifier.
+ *
+ * @conf
+ *  Structure that needs to be populated by this callback.
+ *
+ * @arg
+ *  Argument to the callback. This is the same as the conf_arg passed to the
+ *  rte_event_eth_rx_adapter_create_ext()
+ */
+typedef int (*rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
+			struct rte_event_eth_rx_adapter_conf *conf,
+			void *arg);
+
+/** Rx queue configuration structure */
+struct rte_event_eth_rx_adapter_queue_conf {
+	uint32_t rx_queue_flags;
+	 /**< Flags for handling received packets
+	  * @see RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID
+	  */
+	uint16_t servicing_weight;
+	/**< Relative polling frequency of ethernet receive queue, if this
+	 * is set to zero, the Rx queue is interrupt driven (unless rx queue
+	 * interrupts are not enabled for the ethernet device)
+	 */
+	struct rte_event ev;
+	/**<
+	 *  The values from the following event fields will be used when
+	 *  enqueuing mbuf events:
+	 *   - event_queue_id: Targeted event queue ID for received packets.
+	 *   - event_priority: Event priority of packets from this Rx queue in
+	 *                     the event queue relative to other events.
+	 *   - sched_type: Scheduling type for packets from this Rx queue.
+	 *   - flow_id: If the RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit
+	 *		is set in rx_queue_flags, this flow_id is used for all
+	 *		packets received from this queue. Otherwise the flow ID
+	 *		is set to the RSS hash of the src and dst IPv4/6
+	 *		address.
+	 *
+	 * The event adapter sets ev.event_type to RTE_EVENT_TYPE_ETHDEV in the
+	 * enqueued event
+	 */
+};
+
+struct rte_event_eth_rx_adapter_stats {
+	uint64_t rx_poll_count;
+	/**< Receive queue poll count */
+	uint64_t rx_packets;
+	/**< Received packet count */
+	uint64_t rx_enq_count;
+	/**< Eventdev enqueue count */
+	uint64_t rx_enq_retry;
+	/**< Eventdev enqueue retry count */
+	uint64_t rx_enq_start_ts;
+	/**< Rx enqueue start timestamp */
+	uint64_t rx_enq_block_cycles;
+	/**< Cycles for which the service is blocked by the event device,
+	 * i.e, the service fails to enqueue to the event device.
+	 */
+	uint64_t rx_enq_end_ts;
+	/**< Latest timestamp at which the service is unblocked
+	 * by the event device. The start, end timestamps and
+	 * block cycles can be used to compute the percentage of
+	 * cycles the service is blocked by the event device.
+	 */
+};
+
+/**
+ * Create a new ethernet Rx event adapter with the specified identifier.
+ *
+ * @param id
+ *  The identifier of the ethernet Rx event adapter.
+ *
+ * @dev_id
+ *  The identifier of the device to configure.
+ *
+ * @eth_port_id
+ *  The identifier of the ethernet device.
+ *
+ * @param conf_cb
+ *  Callback function that fills in members of a
+ *  struct rte_event_eth_rx_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_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
+					rx_adapter_conf_cb conf_cb,
+					void *conf_arg);
+
+/**
+ * Create a new ethernet Rx event 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_eth_rx_adapter_create_ext() version.
+ *
+ * @param id
+ *  The identifier of the ethernet Rx event adapter.
+ *
+ * @dev_id
+ *  The identifier of the device to configure.
+ *
+ * @eth_port_id
+ *  The identifier of the ethernet device.
+ *
+ * @param conf_cb
+ *  Callback function that fills in members of a
+ *  struct rte_event_eth_rx_adapter_conf struct passed into
+ *  it.
+ *
+ * @param conf_arg
+ *  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_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
+				struct rte_event_port_conf *port_config);
+
+/**
+ * Free an event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure, If the adapter still has Rx queues
+ *      added to it, the function returns -EBUSY.
+ */
+int rte_event_eth_rx_adapter_free(uint8_t id);
+
+/**
+ * Add receive queue to an event adapter. After a queue has been
+ * added to the event adapter, the result of the application calling
+ * rte_eth_rx_burst(eth_dev_id, rx_queue_id, ..) is undefined.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param eth_dev_id
+ *  Port identifier of Ethernet device.
+ *
+ * @param rx_queue_id
+ *  Ethernet device receive queue index.
+ *  If rx_queue_id is -1, then all Rx queues configured for
+ *  the device are added. If the ethdev Rx queues can only be
+ *  connected to a single event queue then rx_queue_id is
+ *  required to be -1.
+ *
+ * @param conf
+ *  Additonal configuration structure of type *rte_event_eth_rx_adapte_conf*
+ *
+ * @see
+ * @return
+ *  - 0: Success, Receive queue added correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_rx_adapter_queue_add(uint8_t id,
+			uint8_t eth_dev_id,
+			int32_t rx_queue_id,
+			const struct rte_event_eth_rx_adapter_queue_conf *conf);
+
+/**
+ * Delete receive queue from an event adapter.
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param eth_dev_id
+ *  Port identifier of Ethernet device.
+ *
+ * @param rx_queue_id
+ *  Ethernet device receive queue index.
+ *  If rx_queue_id is -1, then all Rx queues configured for
+ *  the device are deleted. If the ethdev Rx queues can only be
+ *  connected to a single event queue then rx_queue_id is
+ *  required to be -1.
+ *
+ * @return
+ *  - 0: Success, Receive queue deleted correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
+				       int32_t rx_queue_id);
+
+/**
+ * Start  ethernet Rx event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_rx_adapter_start(uint8_t id);
+
+/**
+ * Stop  ethernet Rx event adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *  - 0: Success, Adapter started correctly.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_rx_adapter_stop(uint8_t id);
+
+/**
+ * Retrieve statistics for an adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @param 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_eth_rx_adapter_stats_get(uint8_t id,
+				struct rte_event_eth_rx_adapter_stats *stats);
+
+/**
+ * Reset statistics for an adapter
+ *
+ * @param id
+ *  Adapter identifier.
+ *
+ * @return
+ *  - 0: Success, statistics reset successfully.
+ *  - <0: Error code on failure.
+ */
+int rte_event_eth_rx_adapter_stats_reset(uint8_t id);
+
+/**
+ * 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.
+ *
+ * @return
+ *  - 0: Success, statistics reset successfully.
+ *  - <0: Error code on failure, if the adapter doesn't use a rte_service
+ * function, this function returns -ESRCH.
+ */
+int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
+
+#ifdef __cplusplus
+}
+#endif
+#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
new file mode 100644
index 000000000..d5b655dae
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -0,0 +1,1238 @@
+#include <rte_cycles.h>
+#include <rte_common.h>
+#include <rte_dev.h>
+#include <rte_errno.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_malloc.h>
+#include <rte_service_component.h>
+#include <rte_thash.h>
+
+#include "rte_eventdev.h"
+#include "rte_eventdev_pmd.h"
+#include "rte_event_eth_rx_adapter.h"
+
+#define BATCH_SIZE		32
+#define BLOCK_CNT_THRESHOLD	10
+#define ETH_EVENT_BUFFER_SIZE	(4*BATCH_SIZE)
+
+#define ETH_RX_ADAPTER_SERVICE_NAME_LEN	32
+#define ETH_RX_ADAPTER_MEM_NAME_LEN	32
+
+/*
+ * There is an instance of this struct per polled Rx queue added to the
+ * adapter
+ */
+struct eth_rx_poll_entry {
+	/* eth port to poll */
+	uint8_t eth_dev_id;
+	/* eth rx queue to poll */
+	uint16_t eth_rx_qid;
+};
+
+/* Instance per adapter */
+struct rte_eth_event_enqueue_buffer {
+	/* Count of events in this buffer */
+	uint16_t count;
+	/* Array of events in this buffer */
+	struct rte_event events[ETH_EVENT_BUFFER_SIZE];
+};
+
+struct rte_event_eth_rx_adapter {
+	/* event device identifier */
+	uint8_t eventdev_id;
+	/* per ethernet device structure */
+	struct eth_device_info *eth_devices;
+	/* malloc name */
+	char mem_name[ETH_RX_ADAPTER_MEM_NAME_LEN];
+	/* socket identifier cached from eventdev */
+	int socket_id;
+
+	/* elements below are used by SW service */
+
+	/* event port identifier */
+	uint8_t event_port_id;
+	/* per adapter EAL service */
+	uint32_t service_id;
+	/* lock to serialize config updates with service function */
+	rte_spinlock_t rx_lock;
+	/* max mbufs processed in any service function invocation */
+	uint32_t max_nb_rx;
+	/* Receive queues that need to be polled */
+	struct eth_rx_poll_entry *eth_rx_poll;
+	/* size of the eth_rx_poll array */
+	uint16_t num_rx_polled;
+	/* Weighted round robin schedule */
+	uint32_t *wrr_sched;
+	/* wrr_sched[] size */
+	uint32_t wrr_len;
+	/* Next entry in wrr[] to begin polling */
+	uint32_t wrr_pos;
+	/* Event burst buffer */
+	struct rte_eth_event_enqueue_buffer event_enqueue_buffer;
+	/* per adapter stats */
+	struct rte_event_eth_rx_adapter_stats stats;
+	/* Block count, counts upto BLOCK_CNT_THRESHOLD */
+	uint16_t enq_block_count;
+	/* Block start ts */
+	uint64_t rx_enq_block_start_ts;
+	/* Configuration callback for rte_service configuration */
+	rx_adapter_conf_cb conf_cb;
+	/* Configuration callback argument */
+	void *conf_arg;
+	/* Service initialization state */
+	uint8_t service_inited;
+	/* Total count of Rx queues in adapter */
+	uint32_t nb_queues;
+} __rte_cache_aligned;
+
+/* Per eth device */
+struct eth_device_info {
+	struct rte_eth_dev *dev;
+	struct eth_rx_queue_info *rx_queue;
+	/* Set if ethdev->eventdev packet transfer uses a
+	 * hardware mechanism
+	 */
+	uint8_t internal_event_port;
+	/* set if the adapter is processing rx queues for
+	 * this eth device and packet processing has been
+	 * started, allows for the code to know if the PMD
+	 * rx_adapter_stop callback needs to be invoked
+	 */
+	uint8_t dev_rx_started;
+	/* if nb_dev_queues > 0, the start callback will
+	 * be invoked if not already invoked
+	 */
+	uint16_t nb_dev_queues;
+};
+
+/* Per Rx queue */
+struct eth_rx_queue_info {
+	int queue_enabled;	/* true if added */
+	uint16_t wt;		/* polling weight */
+	uint8_t event_queue_id;	/* Event queue to enqueue packets to */
+	uint8_t sched_type;	/* sched type for events */
+	uint8_t priority;	/* event priority */
+	uint32_t flow_id;	/* app provided flow identifier */
+	uint32_t flow_id_mask;	/* Set to ~0 if app provides flow id else 0 */
+};
+
+static struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter;
+static struct rte_event_port_conf
+		create_port_conf[RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE];
+
+static uint8_t default_rss_key[] = {
+	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
+	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
+	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
+	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
+	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
+};
+static uint8_t *rss_key_be;
+
+static inline int
+valid_id(uint8_t id)
+{
+	return id < RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
+}
+
+#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, retval) do { \
+	if (!valid_id(id)) { \
+		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
+		return retval; \
+	} \
+} while (0)
+
+static inline int
+sw_rx_adapter_queue_count(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	return rx_adapter->num_rx_polled;
+}
+
+/* Greatest common divisor */
+static uint16_t gcd_u16(uint16_t a, uint16_t b)
+{
+	uint16_t r = a % b;
+
+	return r ? gcd_u16(b, r) : b;
+}
+
+/* Returns the next queue in the polling sequence
+ *
+ * http://kb.linuxvirtualserver.org/wiki/Weighted_Round-Robin_Scheduling
+ */
+static int
+wrr_next(struct rte_event_eth_rx_adapter *rx_adapter,
+	 unsigned int n, int *cw,
+	 struct eth_rx_poll_entry *eth_rx_poll, uint16_t max_wt,
+	 uint16_t gcd, int prev)
+{
+	int i = prev;
+	uint16_t w;
+
+	while (1) {
+		uint16_t q;
+		uint8_t d;
+
+		i = (i + 1) % n;
+		if (i == 0) {
+			*cw = *cw - gcd;
+			if (*cw <= 0)
+				*cw = max_wt;
+		}
+
+		q = eth_rx_poll[i].eth_rx_qid;
+		d = eth_rx_poll[i].eth_dev_id;
+		w = rx_adapter->eth_devices[d].rx_queue[q].wt;
+
+		if ((int)w >= *cw)
+			return i;
+	}
+}
+
+/* Precalculate WRR polling sequence for all queues in rx_adapter */
+static int
+eth_poll_wrr_calc(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	uint8_t d;
+	uint16_t q;
+	unsigned int i;
+
+	/* Initialize variables for calculaton of wrr schedule */
+	uint16_t max_wrr_pos = 0;
+	unsigned int poll_q = 0;
+	uint16_t max_wt = 0;
+	uint16_t gcd = 0;
+
+	struct eth_rx_poll_entry *rx_poll = NULL;
+	uint32_t *rx_wrr = NULL;
+
+	if (rx_adapter->num_rx_polled) {
+		size_t len = RTE_ALIGN(rx_adapter->num_rx_polled *
+				sizeof(*rx_adapter->eth_rx_poll),
+				RTE_CACHE_LINE_SIZE);
+		rx_poll = rte_zmalloc_socket(rx_adapter->mem_name,
+					     len,
+					     RTE_CACHE_LINE_SIZE,
+					     rx_adapter->socket_id);
+		if (!rx_poll)
+			return -ENOMEM;
+
+		/* Generate array of all queues to poll, the size of this
+		 * array is poll_q
+		 */
+		for (d = 0; d < rte_eth_dev_count(); d++) {
+			uint16_t nb_rx_queues;
+			struct eth_device_info *dev_info =
+					&rx_adapter->eth_devices[d];
+			nb_rx_queues = dev_info->dev->data->nb_rx_queues;
+			if (!dev_info->rx_queue)
+				continue;
+			for (q = 0; q < nb_rx_queues; q++) {
+				struct eth_rx_queue_info *queue_info =
+					&dev_info->rx_queue[q];
+				if (!queue_info->queue_enabled)
+					continue;
+
+				uint16_t wt = queue_info->wt;
+				rx_poll[poll_q].eth_dev_id = d;
+				rx_poll[poll_q].eth_rx_qid = q;
+				max_wrr_pos += wt;
+				max_wt = RTE_MAX(max_wt, wt);
+				gcd = (gcd) ? gcd_u16(gcd, wt) : wt;
+				poll_q++;
+			}
+		}
+
+		len = RTE_ALIGN(max_wrr_pos * sizeof(*rx_wrr),
+				RTE_CACHE_LINE_SIZE);
+		rx_wrr = rte_zmalloc_socket(rx_adapter->mem_name,
+					    len,
+					    RTE_CACHE_LINE_SIZE,
+					    rx_adapter->socket_id);
+		if (!rx_wrr) {
+			rte_free(rx_poll);
+			return -ENOMEM;
+		}
+
+		/* Generate polling sequence based on weights */
+		int prev = -1;
+		int cw = -1;
+		for (i = 0; i < max_wrr_pos; i++) {
+			rx_wrr[i] = wrr_next(rx_adapter, poll_q, &cw,
+					     rx_poll, max_wt, gcd, prev);
+			prev = rx_wrr[i];
+		}
+	}
+
+	rte_free(rx_adapter->eth_rx_poll);
+	rte_free(rx_adapter->wrr_sched);
+
+	rx_adapter->eth_rx_poll = rx_poll;
+	rx_adapter->wrr_sched = rx_wrr;
+	rx_adapter->wrr_len = max_wrr_pos;
+
+	return 0;
+}
+
+static inline void
+mtoip(struct rte_mbuf *m, struct ipv4_hdr **ipv4_hdr,
+	struct ipv6_hdr **ipv6_hdr)
+{
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
+	struct vlan_hdr *vlan_hdr;
+
+	*ipv4_hdr = NULL;
+	*ipv6_hdr = NULL;
+
+	switch (eth_hdr->ether_type) {
+	case RTE_BE16(ETHER_TYPE_IPv4):
+		*ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
+		break;
+
+	case RTE_BE16(ETHER_TYPE_IPv6):
+		*ipv6_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
+		break;
+
+	case RTE_BE16(ETHER_TYPE_VLAN):
+		vlan_hdr = (struct vlan_hdr *)(eth_hdr + 1);
+		switch (vlan_hdr->eth_proto) {
+		case RTE_BE16(ETHER_TYPE_IPv4):
+			*ipv4_hdr = (struct ipv4_hdr *)(vlan_hdr + 1);
+			break;
+		case RTE_BE16(ETHER_TYPE_IPv6):
+			*ipv6_hdr = (struct ipv6_hdr *)(vlan_hdr + 1);
+			break;
+		default:
+			break;
+		}
+		break;
+
+	default:
+		break;
+	}
+}
+
+/* Calculate RSS hash for IPv4/6 */
+static inline uint32_t
+do_softrss(struct rte_mbuf *m)
+{
+	uint32_t input_len;
+	void *tuple;
+	struct rte_ipv4_tuple ipv4_tuple;
+	struct rte_ipv6_tuple ipv6_tuple;
+	struct ipv4_hdr *ipv4_hdr;
+	struct ipv6_hdr *ipv6_hdr;
+
+	mtoip(m, &ipv4_hdr, &ipv6_hdr);
+
+	if (ipv4_hdr) {
+		ipv4_tuple.src_addr = rte_be_to_cpu_32(ipv4_hdr->src_addr);
+		ipv4_tuple.dst_addr = rte_be_to_cpu_32(ipv4_hdr->dst_addr);
+		tuple = &ipv4_tuple;
+		input_len = RTE_THASH_V4_L3_LEN;
+	} else if (ipv6_hdr) {
+		rte_thash_load_v6_addrs(ipv6_hdr,
+					(union rte_thash_tuple *)&ipv6_tuple);
+		tuple = &ipv6_tuple;
+		input_len = RTE_THASH_V6_L3_LEN;
+	} else
+		return 0;
+
+	return rte_softrss_be(tuple, input_len, rss_key_be);
+}
+
+static inline int
+rx_enq_blocked(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	return !!rx_adapter->enq_block_count;
+}
+
+static inline void
+rx_enq_block_start_ts(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	if (rx_adapter->rx_enq_block_start_ts)
+		return;
+
+	rx_adapter->enq_block_count++;
+	if (rx_adapter->enq_block_count < BLOCK_CNT_THRESHOLD)
+		return;
+
+	rx_adapter->rx_enq_block_start_ts = rte_get_tsc_cycles();
+}
+
+static inline void
+rx_enq_block_end_ts(struct rte_event_eth_rx_adapter *rx_adapter,
+		    struct rte_event_eth_rx_adapter_stats *stats)
+{
+	if (unlikely(!stats->rx_enq_start_ts))
+		stats->rx_enq_start_ts = rte_get_tsc_cycles();
+
+	if (likely(!rx_enq_blocked(rx_adapter)))
+		return;
+
+	rx_adapter->enq_block_count = 0;
+	if (rx_adapter->rx_enq_block_start_ts) {
+		stats->rx_enq_end_ts = rte_get_tsc_cycles();
+		stats->rx_enq_block_cycles += stats->rx_enq_end_ts -
+		    rx_adapter->rx_enq_block_start_ts;
+		rx_adapter->rx_enq_block_start_ts = 0;
+	}
+}
+
+/* Add event to buffer, free space check is done prior to calling
+ * this function
+ */
+static inline void
+buf_event_enqueue(struct rte_event_eth_rx_adapter *rx_adapter,
+		  struct rte_event *ev)
+{
+	struct rte_eth_event_enqueue_buffer *buf =
+	    &rx_adapter->event_enqueue_buffer;
+	rte_memcpy(&buf->events[buf->count++], ev, sizeof(struct rte_event));
+}
+
+/* Enqueue buffered events to event device */
+static inline uint16_t
+flush_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	struct rte_eth_event_enqueue_buffer *buf =
+	    &rx_adapter->event_enqueue_buffer;
+	struct rte_event_eth_rx_adapter_stats *stats = &rx_adapter->stats;
+
+	uint16_t n = rte_event_enqueue_burst(rx_adapter->eventdev_id,
+					rx_adapter->event_port_id,
+					buf->events,
+					buf->count);
+	if (n != buf->count) {
+		memmove(buf->events,
+			&buf->events[n],
+			(buf->count - n) * sizeof(struct rte_event));
+		stats->rx_enq_retry++;
+	}
+
+	n ? rx_enq_block_end_ts(rx_adapter, stats) :
+		rx_enq_block_start_ts(rx_adapter);
+
+	buf->count -= n;
+	stats->rx_enq_count += n;
+
+	return n;
+}
+
+static inline void
+fill_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter,
+	uint8_t dev_id,
+	uint16_t rx_queue_id,
+	struct rte_mbuf **mbufs,
+	uint16_t num)
+{
+	uint32_t i;
+	struct eth_device_info *eth_device_info =
+					&rx_adapter->eth_devices[dev_id];
+	struct eth_rx_queue_info *eth_rx_queue_info =
+					&eth_device_info->rx_queue[rx_queue_id];
+
+	int32_t qid = eth_rx_queue_info->event_queue_id;
+	uint8_t sched_type = eth_rx_queue_info->sched_type;
+	uint8_t priority = eth_rx_queue_info->priority;
+	uint32_t flow_id;
+	struct rte_event events[BATCH_SIZE];
+	struct rte_mbuf *m = mbufs[0];
+	uint32_t rss_mask;
+	uint32_t rss;
+	int do_rss;
+
+	/* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
+	rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
+	do_rss = !rss_mask && !eth_rx_queue_info->flow_id_mask;
+
+	for (i = 0; i < num; i++) {
+		m = mbufs[i];
+		struct rte_event *ev = &events[i];
+
+		rss = do_rss ? do_softrss(m) : m->hash.rss;
+		flow_id =
+		    eth_rx_queue_info->flow_id &
+				eth_rx_queue_info->flow_id_mask;
+		flow_id |= rss & ~eth_rx_queue_info->flow_id_mask;
+
+		ev->flow_id = flow_id;
+		ev->op = RTE_EVENT_OP_NEW;
+		ev->sched_type = sched_type;
+		ev->queue_id = qid;
+		ev->event_type = RTE_EVENT_TYPE_ETHDEV;
+		ev->sub_event_type = 0;
+		ev->priority = priority;
+		ev->mbuf = m;
+
+		buf_event_enqueue(rx_adapter, ev);
+	}
+}
+
+/*
+ * Polls receive queues added to the event adapter and enqueues received
+ * packets to the event device.
+ *
+ * The receive code enqueues initially to a temporary buffer, the
+ * temporary buffer is drained anytime it holds >= BATCH_SIZE packets
+ *
+ * If there isn't space available in the temporary buffer, packets from the
+ * Rx queue arent dequeued from the eth device, this backpressures the
+ * eth device, in virtual device enviroments this backpressure is relayed to the
+ * hypervisor's switching layer where adjustments can be made to deal with
+ * it.
+ */
+static inline uint32_t
+eth_rx_poll(struct rte_event_eth_rx_adapter *rx_adapter)
+{
+	uint32_t num_queue;
+	uint16_t n;
+	uint32_t nb_rx = 0;
+	struct rte_mbuf *mbufs[BATCH_SIZE];
+	struct rte_eth_event_enqueue_buffer *buf;
+	uint32_t wrr_pos;
+	uint32_t max_nb_rx;
+
+	wrr_pos = rx_adapter->wrr_pos;
+	max_nb_rx = rx_adapter->max_nb_rx;
+	buf = &rx_adapter->event_enqueue_buffer;
+	struct rte_event_eth_rx_adapter_stats *stats = &rx_adapter->stats;
+
+	/* Iterate through a WRR sequence */
+	for (num_queue = 0; num_queue < rx_adapter->wrr_len; num_queue++) {
+		unsigned int poll_idx = rx_adapter->wrr_sched[wrr_pos];
+		uint16_t qid = rx_adapter->eth_rx_poll[poll_idx].eth_rx_qid;
+		uint8_t d = rx_adapter->eth_rx_poll[poll_idx].eth_dev_id;
+
+		/* Don't do a batch dequeue from the rx queue if there isn't
+		 * enough space in the enqueue buffer.
+		 */
+		if (buf->count >= BATCH_SIZE)
+			flush_event_buffer(rx_adapter);
+		if (BATCH_SIZE > (ETH_EVENT_BUFFER_SIZE - buf->count))
+			break;
+
+		stats->rx_poll_count++;
+		n = rte_eth_rx_burst(d, qid, mbufs, BATCH_SIZE);
+
+		if (n) {
+			stats->rx_packets += n;
+			/* The check before rte_eth_rx_burst() ensures that
+			 * all n mbufs can be buffered
+			 */
+			fill_event_buffer(rx_adapter, d, qid, mbufs, n);
+			nb_rx += n;
+			if (nb_rx > max_nb_rx) {
+				rx_adapter->wrr_pos =
+				    (wrr_pos + 1) % rx_adapter->wrr_len;
+				return nb_rx;
+			}
+		}
+
+		if (++wrr_pos == rx_adapter->wrr_len)
+			wrr_pos = 0;
+	}
+
+	return nb_rx;
+}
+
+static int
+event_eth_rx_adapter_service_func(void *args)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter = args;
+	struct rte_eth_event_enqueue_buffer *buf;
+
+	buf = &rx_adapter->event_enqueue_buffer;
+	if (!rte_spinlock_trylock(&rx_adapter->rx_lock))
+		return 0;
+	if (eth_rx_poll(rx_adapter) == 0 && buf->count)
+		flush_event_buffer(rx_adapter);
+	rte_spinlock_unlock(&rx_adapter->rx_lock);
+	return 0;
+}
+
+static int
+rte_event_eth_rx_adapter_init(void)
+{
+	const char *name = "rte_event_eth_rx_adapter_array";
+	const struct rte_memzone *mz;
+	unsigned int sz;
+	unsigned int rss_key_off;
+
+	sz = sizeof(*rte_event_eth_rx_adapter) *
+	    RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
+	sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE);
+	rss_key_off = sz;
+	sz = RTE_ALIGN(sz + sizeof(default_rss_key), RTE_CACHE_LINE_SIZE);
+
+	mz = rte_memzone_lookup(name);
+	if (!mz) {
+		mz = rte_memzone_reserve_aligned(name, sz, rte_socket_id(), 0,
+						 RTE_CACHE_LINE_SIZE);
+		if (mz) {
+			rte_convert_rss_key((uint32_t *)default_rss_key,
+			    (uint32_t *)(uintptr_t)(mz->addr_64 + rss_key_off),
+			    RTE_DIM(default_rss_key));
+		} else {
+			RTE_EDEV_LOG_ERR("failed to reserve memzone err = %"
+					PRId32, rte_errno);
+			return -rte_errno;
+		}
+	}
+
+	rte_event_eth_rx_adapter = mz->addr;
+	rss_key_be = (uint8_t *)(mz->addr_64 + rss_key_off);
+	return 0;
+}
+
+static inline struct rte_event_eth_rx_adapter *
+id_to_rx_adapter(uint8_t id)
+{
+	return rte_event_eth_rx_adapter ?
+		rte_event_eth_rx_adapter[id] : NULL;
+}
+
+static int
+default_conf_cb(uint8_t id, uint8_t dev_id,
+		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
+{
+	int ret;
+	struct rte_eventdev *dev;
+	struct rte_event_dev_config dev_conf;
+	int started;
+	uint8_t port_id;
+	struct rte_event_port_conf *port_conf = arg;
+	struct rte_event_eth_rx_adapter *rx_adapter = id_to_rx_adapter(id);
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+	dev_conf = dev->data->dev_conf;
+
+	started = dev->data->dev_started;
+	if (started)
+		rte_event_dev_stop(dev_id);
+	port_id = dev_conf.nb_event_ports;
+	dev_conf.nb_event_ports += 1;
+	ret = rte_event_dev_configure(dev_id, &dev_conf);
+	if (ret) {
+		RTE_EDEV_LOG_ERR("failed to configure event dev %u\n",
+						dev_id);
+		/* Conf. failed, OK to start ? */
+		if (started)
+			rte_event_dev_start(dev_id);
+		return ret;
+	}
+
+	ret = rte_event_port_setup(dev_id, port_id, port_conf);
+	if (ret) {
+		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
+					port_id);
+	} else {
+		conf->event_port_id = port_id;
+		conf->max_nb_rx = 128;
+	}
+
+	if (started)
+		rte_event_dev_start(dev_id);
+	return ret;
+}
+
+static int
+init_service(struct rte_event_eth_rx_adapter *rx_adapter, uint8_t id)
+{
+	int ret;
+	struct rte_service_spec service;
+	struct rte_event_eth_rx_adapter_conf rx_adapter_conf;
+
+	if (rx_adapter->service_inited)
+		return 0;
+
+	memset(&service, 0, sizeof(service));
+	snprintf(service.name, ETH_RX_ADAPTER_SERVICE_NAME_LEN,
+		"rte_event_eth_rx_adapter_%d", id);
+	service.socket_id = rx_adapter->socket_id;
+	service.callback = event_eth_rx_adapter_service_func;
+	service.callback_userdata = rx_adapter;
+	/* Service function handles locking for queue add/del updates */
+	service.capabilities = RTE_SERVICE_CAP_MT_SAFE;
+	ret = rte_service_component_register(&service, &rx_adapter->service_id);
+	if (ret) {
+		RTE_EDEV_LOG_ERR("failed to register service %s err = %" PRId32,
+			service.name, ret);
+		return ret;
+	}
+
+	ret = rx_adapter->conf_cb(id, rx_adapter->eventdev_id,
+		&rx_adapter_conf, rx_adapter->conf_arg);
+	if (ret) {
+		RTE_EDEV_LOG_ERR("confguration callback failed err = %" PRId32,
+			ret);
+		goto err_done;
+	}
+	rx_adapter->event_port_id = rx_adapter_conf.event_port_id;
+	rx_adapter->max_nb_rx = rx_adapter_conf.max_nb_rx;
+	rx_adapter->service_inited = 1;
+	return 0;
+
+err_done:
+	rte_service_component_unregister(rx_adapter->service_id);
+	return ret;
+}
+
+static void
+update_queue_info(struct rte_event_eth_rx_adapter *rx_adapter,
+		struct eth_device_info *dev_info,
+		int32_t rx_queue_id,
+		uint8_t add)
+{
+	struct eth_rx_queue_info *queue_info;
+	int enabled;
+	uint16_t i;
+
+	if (!dev_info->rx_queue)
+		return;
+
+	if (rx_queue_id == -1) {
+		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++) {
+			queue_info = &dev_info->rx_queue[i];
+			enabled = queue_info->queue_enabled;
+			if (add) {
+				rx_adapter->nb_queues += !enabled;
+				dev_info->nb_dev_queues += !enabled;
+			} else {
+				rx_adapter->nb_queues -= enabled;
+				dev_info->nb_dev_queues -= enabled;
+			}
+			queue_info->queue_enabled = !!add;
+		}
+	} else {
+		queue_info = &dev_info->rx_queue[rx_queue_id];
+		enabled = queue_info->queue_enabled;
+		if (add) {
+			rx_adapter->nb_queues += !enabled;
+			dev_info->nb_dev_queues += !enabled;
+		} else {
+			rx_adapter->nb_queues -= enabled;
+			dev_info->nb_dev_queues -= enabled;
+		}
+		queue_info->queue_enabled = !!add;
+	}
+}
+
+static int
+event_eth_rx_adapter_queue_del(struct rte_event_eth_rx_adapter *rx_adapter,
+			    struct eth_device_info *dev_info,
+			    uint16_t rx_queue_id)
+{
+	struct eth_rx_queue_info *queue_info;
+
+	if (!rx_adapter->nb_queues)
+		return 0;
+
+	queue_info = &dev_info->rx_queue[rx_queue_id];
+	rx_adapter->num_rx_polled -= queue_info->queue_enabled;
+	update_queue_info(rx_adapter, dev_info, rx_queue_id, 0);
+	return 0;
+}
+
+static void
+event_eth_rx_adapter_queue_add(struct rte_event_eth_rx_adapter *rx_adapter,
+		struct eth_device_info *dev_info,
+		uint16_t rx_queue_id,
+		const struct rte_event_eth_rx_adapter_queue_conf *conf)
+
+{
+	struct eth_rx_queue_info *queue_info;
+	const struct rte_event *ev = &conf->ev;
+
+	queue_info = &dev_info->rx_queue[rx_queue_id];
+	queue_info->event_queue_id = ev->queue_id;
+	queue_info->sched_type = ev->sched_type;
+	queue_info->priority = ev->priority;
+	queue_info->wt = conf->servicing_weight;
+
+	if (conf->rx_queue_flags &
+			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID) {
+		queue_info->flow_id = ev->flow_id;
+		queue_info->flow_id_mask = ~0;
+	}
+
+	/* The same queue can be added more than once */
+	rx_adapter->num_rx_polled += !queue_info->queue_enabled;
+	update_queue_info(rx_adapter, dev_info, rx_queue_id, 1);
+}
+
+static int add_rx_queue(struct rte_event_eth_rx_adapter *rx_adapter,
+		uint8_t eth_dev_id,
+		int rx_queue_id,
+		const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
+{
+	struct eth_device_info *dev_info = &rx_adapter->eth_devices[eth_dev_id];
+	uint32_t i;
+	int ret;
+
+	if (queue_conf->servicing_weight == 0) {
+		struct rte_event_eth_rx_adapter_queue_conf temp_conf;
+
+		struct rte_eth_dev_data *data = dev_info->dev->data;
+		if (data->dev_conf.intr_conf.rxq) {
+			RTE_EDEV_LOG_ERR("Interrupt driven queues"
+					" not supported");
+			return -ENOTSUP;
+		}
+		temp_conf = *queue_conf;
+		temp_conf.servicing_weight = 1;
+		/* If Rx interrupts are disabled set wt = 1 */
+		queue_conf = &temp_conf;
+	}
+
+	if (!dev_info->rx_queue) {
+		dev_info->rx_queue =
+		    rte_zmalloc_socket(rx_adapter->mem_name,
+				       dev_info->dev->data->nb_rx_queues *
+				       sizeof(struct eth_rx_queue_info), 0,
+				       rx_adapter->socket_id);
+		if (!dev_info->rx_queue)
+			return -ENOMEM;
+	}
+
+	if (rx_queue_id == -1) {
+		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++)
+			event_eth_rx_adapter_queue_add(rx_adapter,
+						dev_info, i,
+						queue_conf);
+	} else {
+		event_eth_rx_adapter_queue_add(rx_adapter, dev_info,
+					  (uint16_t)rx_queue_id,
+					  queue_conf);
+	}
+
+	ret = eth_poll_wrr_calc(rx_adapter);
+	if (ret) {
+		event_eth_rx_adapter_queue_del(rx_adapter,
+					dev_info, rx_queue_id);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int
+rx_adapter_ctrl(uint8_t id, int start)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct rte_eventdev *dev;
+	struct eth_device_info *dev_info;
+	uint32_t i;
+	int use_service = 0;
+	int stop = !start;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter)
+		return -EINVAL;
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+
+	for (i = 0; i < rte_eth_dev_count(); i++) {
+		dev_info = &rx_adapter->eth_devices[i];
+		/* if start  check for num dev queues */
+		if (start && !dev_info->nb_dev_queues)
+			continue;
+		/* if stop check if dev has been started */
+		if (stop && !dev_info->dev_rx_started)
+			continue;
+		use_service |= !dev_info->internal_event_port;
+		dev_info->dev_rx_started = start;
+		if (!dev_info->internal_event_port)
+			continue;
+		start ? (*dev->dev_ops->eth_rx_adapter_start)(dev,
+						&rte_eth_devices[i]) :
+			(*dev->dev_ops->eth_rx_adapter_stop)(dev,
+						&rte_eth_devices[i]);
+	}
+
+	if (use_service)
+		rte_service_runstate_set(rx_adapter->service_id, start);
+
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
+				rx_adapter_conf_cb conf_cb, void *conf_arg)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	int ret;
+	int socket_id;
+	uint8_t i;
+	char mem_name[ETH_RX_ADAPTER_SERVICE_NAME_LEN];
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	if (!conf_cb)
+		return -EINVAL;
+
+	if (rte_event_eth_rx_adapter == NULL) {
+		ret = rte_event_eth_rx_adapter_init();
+		if (ret)
+			return ret;
+	}
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (rx_adapter != NULL) {
+		RTE_EDEV_LOG_ERR("Eth Rx adapter exists id = %" PRIu8, id);
+		return -EEXIST;
+	}
+
+	socket_id = rte_event_dev_socket_id(dev_id);
+	snprintf(mem_name, ETH_RX_ADAPTER_MEM_NAME_LEN,
+		"rte_event_eth_rx_adapter_%d",
+		id);
+
+	rx_adapter = rte_zmalloc_socket(mem_name, sizeof(*rx_adapter),
+			RTE_CACHE_LINE_SIZE, socket_id);
+	if (rx_adapter == NULL) {
+		RTE_EDEV_LOG_ERR("failed to get mem for rx adapter");
+		return -ENOMEM;
+	}
+
+	rx_adapter->eventdev_id = dev_id;
+	rx_adapter->socket_id = socket_id;
+	rx_adapter->conf_cb = conf_cb;
+	rx_adapter->conf_arg = conf_arg;
+	strcpy(rx_adapter->mem_name, mem_name);
+	rx_adapter->eth_devices = rte_zmalloc_socket(rx_adapter->mem_name,
+					rte_eth_dev_count() *
+					sizeof(struct eth_device_info), 0,
+					socket_id);
+	if (rx_adapter->eth_devices == NULL) {
+		RTE_EDEV_LOG_ERR("failed to get mem for eth devices\n");
+		rte_free(rx_adapter);
+		return -ENOMEM;
+	}
+	rte_spinlock_init(&rx_adapter->rx_lock);
+	for (i = 0; i < rte_eth_dev_count(); i++)
+		rx_adapter->eth_devices[i].dev = &rte_eth_devices[i];
+
+	rte_event_eth_rx_adapter[id] = rx_adapter;
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
+		struct rte_event_port_conf *port_config)
+{
+	if (!port_config)
+		return -EINVAL;
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+	create_port_conf[id] = *port_config;
+	return rte_event_eth_rx_adapter_create_ext(id, dev_id,
+					default_conf_cb,
+					&create_port_conf[id]);
+}
+
+int
+rte_event_eth_rx_adapter_free(uint8_t id)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter)
+		return -EINVAL;
+
+	if (rx_adapter->nb_queues) {
+		RTE_EDEV_LOG_ERR("%" PRIu16 " Rx queues not deleted",
+				rx_adapter->nb_queues);
+		return -EBUSY;
+	}
+
+	rte_free(rx_adapter->eth_devices);
+	rte_free(rx_adapter);
+	rte_event_eth_rx_adapter[id] = NULL;
+
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_queue_add(uint8_t id,
+		uint8_t eth_dev_id,
+		int32_t rx_queue_id,
+		const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
+{
+	int ret;
+	uint32_t rx_adapter_cap;
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct rte_eventdev *dev;
+	struct eth_device_info *dev_info;
+	int start_service = 0;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter || !queue_conf)
+		return -EINVAL;
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+	ret = (*dev->dev_ops->eth_rx_adapter_caps_get)(dev,
+						&rte_eth_devices[eth_dev_id],
+						&rx_adapter_cap);
+	if (ret) {
+		RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
+			"eth port %" PRIu8, id, eth_dev_id);
+		return ret;
+	}
+
+	if (!(rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID) &&
+		!(queue_conf->rx_queue_flags &
+			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID)) {
+		RTE_EDEV_LOG_ERR("Flow ID required for configuration,"
+				" eth port: %" PRIu8 " adapter id: %" PRIu8,
+				eth_dev_id, id);
+		return -EINVAL;
+	}
+
+	if ((rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ) &&
+		(rx_queue_id != -1)) {
+		RTE_EDEV_LOG_ERR("Rx queues can only be connected to single "
+			"event queue id %u eth port %u", id, eth_dev_id);
+		return -EINVAL;
+	}
+
+	if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
+			rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
+			 (uint16_t)rx_queue_id);
+		return -EINVAL;
+	}
+
+	start_service = 0;
+	dev_info = &rx_adapter->eth_devices[eth_dev_id];
+
+	if (rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT) {
+		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_queue_add,
+					-ENOTSUP);
+		if (!dev_info->rx_queue) {
+			dev_info->rx_queue =
+			    rte_zmalloc_socket(rx_adapter->mem_name,
+					dev_info->dev->data->nb_rx_queues *
+					sizeof(struct eth_rx_queue_info), 0,
+					rx_adapter->socket_id);
+			if (!dev_info->rx_queue)
+				return -ENOMEM;
+		}
+
+		ret = (*dev->dev_ops->eth_rx_adapter_queue_add)(dev,
+				&rte_eth_devices[eth_dev_id],
+				rx_queue_id, queue_conf);
+		if (!ret) {
+			update_queue_info(rx_adapter,
+					&rx_adapter->eth_devices[eth_dev_id],
+					rx_queue_id,
+					1);
+		}
+	} else {
+		rte_spinlock_lock(&rx_adapter->rx_lock);
+		ret = init_service(rx_adapter, id);
+		if (!ret)
+			ret = add_rx_queue(rx_adapter, eth_dev_id, rx_queue_id,
+					queue_conf);
+		rte_spinlock_unlock(&rx_adapter->rx_lock);
+		if (!ret)
+			start_service = !!sw_rx_adapter_queue_count(rx_adapter);
+	}
+
+	if (ret)
+		return ret;
+
+	if (start_service)
+		rte_service_component_runstate_set(rx_adapter->service_id, 1);
+
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
+				int32_t rx_queue_id)
+{
+	int ret = 0;
+	struct rte_eventdev *dev;
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct eth_device_info *dev_info;
+	uint32_t rx_adapter_cap;
+	uint16_t i;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter)
+		return -EINVAL;
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+	ret = dev->dev_ops->eth_rx_adapter_caps_get(dev,
+						&rte_eth_devices[eth_dev_id],
+						&rx_adapter_cap);
+	if (ret)
+		return ret;
+
+	if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
+		rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
+			 (uint16_t)rx_queue_id);
+		return -EINVAL;
+	}
+
+	dev_info = &rx_adapter->eth_devices[eth_dev_id];
+
+	if (rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT) {
+		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_queue_del,
+				 -ENOTSUP);
+		ret = (*dev->dev_ops->eth_rx_adapter_queue_del)(dev,
+						&rte_eth_devices[eth_dev_id],
+						rx_queue_id);
+		if (!ret) {
+			update_queue_info(rx_adapter,
+					&rx_adapter->eth_devices[eth_dev_id],
+					rx_queue_id,
+					0);
+			if (!dev_info->nb_dev_queues) {
+				rte_free(dev_info->rx_queue);
+				dev_info->rx_queue = NULL;
+			}
+		}
+	} else {
+		int rc;
+		rte_spinlock_lock(&rx_adapter->rx_lock);
+		if (rx_queue_id == -1) {
+			for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++)
+				event_eth_rx_adapter_queue_del(rx_adapter,
+							dev_info,
+							i);
+		} else {
+			event_eth_rx_adapter_queue_del(rx_adapter,
+						dev_info,
+						(uint16_t)rx_queue_id);
+		}
+
+		rc = eth_poll_wrr_calc(rx_adapter);
+		if (rc)
+			RTE_EDEV_LOG_ERR("WRR recalculation failed %" PRId32,
+					rc);
+
+		if (!dev_info->nb_dev_queues) {
+			rte_free(dev_info->rx_queue);
+			dev_info->rx_queue = NULL;
+		}
+
+		rte_spinlock_unlock(&rx_adapter->rx_lock);
+		rte_service_component_runstate_set(rx_adapter->service_id,
+				sw_rx_adapter_queue_count(rx_adapter));
+	}
+
+	return ret;
+}
+
+
+int
+rte_event_eth_rx_adapter_start(uint8_t id)
+{
+	return rx_adapter_ctrl(id, 1);
+}
+
+int
+rte_event_eth_rx_adapter_stop(uint8_t id)
+{
+	return rx_adapter_ctrl(id, 0);
+}
+
+int
+rte_event_eth_rx_adapter_stats_get(uint8_t id,
+			       struct rte_event_eth_rx_adapter_stats *stats)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct rte_event_eth_rx_adapter_stats dev_stats_sum = { 0 };
+	struct rte_event_eth_rx_adapter_stats dev_stats;
+	struct rte_eventdev *dev;
+	struct eth_device_info *dev_info;
+	uint32_t i;
+	int ret;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter || !stats)
+		return -EINVAL;
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+	memset(stats, 0, sizeof(*stats));
+	for (i = 0; i < rte_eth_dev_count(); i++) {
+		dev_info = &rx_adapter->eth_devices[i];
+		if (!dev_info->internal_event_port ||
+			!dev->dev_ops->eth_rx_adapter_stats_get)
+			continue;
+		ret = (*dev->dev_ops->eth_rx_adapter_stats_get)(dev,
+						&rte_eth_devices[i],
+						&dev_stats);
+		if (ret)
+			continue;
+		dev_stats_sum.rx_packets += dev_stats.rx_packets;
+		dev_stats_sum.rx_enq_count += dev_stats.rx_enq_count;
+	}
+
+	if (rx_adapter->service_inited)
+		*stats = rx_adapter->stats;
+
+	stats->rx_packets += dev_stats_sum.rx_packets;
+	stats->rx_enq_count += dev_stats_sum.rx_enq_count;
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_stats_reset(uint8_t id)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct rte_eventdev *dev;
+	struct eth_device_info *dev_info;
+	uint32_t i;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter)
+		return -EINVAL;
+
+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
+	for (i = 0; i < rte_eth_dev_count(); i++) {
+		dev_info = &rx_adapter->eth_devices[i];
+		if (!dev_info->internal_event_port ||
+			!dev->dev_ops->eth_rx_adapter_stats_reset)
+			continue;
+		(*dev->dev_ops->eth_rx_adapter_stats_reset)(dev,
+							&rte_eth_devices[i]);
+	}
+
+	memset(&rx_adapter->stats, 0, sizeof(rx_adapter->stats));
+	return 0;
+}
+
+int
+rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id)
+{
+	struct rte_event_eth_rx_adapter *rx_adapter;
+
+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+	rx_adapter = id_to_rx_adapter(id);
+	if (!rx_adapter || !service_id)
+		return -EINVAL;
+
+	if (rx_adapter->service_inited)
+		*service_id = rx_adapter->service_id;
+
+	return rx_adapter->service_inited ? 0 : -ESRCH;
+}
diff --git a/lib/Makefile b/lib/Makefile
index 86caba17b..dbe9b3dfa 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -52,7 +52,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev
 DEPDIRS-librte_cryptodev := librte_eal librte_mempool librte_ring librte_mbuf
 DEPDIRS-librte_cryptodev += librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += librte_eventdev
-DEPDIRS-librte_eventdev := librte_eal librte_ring
+DEPDIRS-librte_eventdev := librte_eal librte_ring librte_hash librte_ether
 DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += librte_vhost
 DEPDIRS-librte_vhost := librte_eal librte_mempool librte_mbuf librte_ether
 DIRS-$(CONFIG_RTE_LIBRTE_HASH) += librte_hash
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 410578a14..c404d673f 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -43,6 +43,7 @@ CFLAGS += $(WERROR_FLAGS)
 # library source files
 SRCS-y += rte_eventdev.c
 SRCS-y += rte_event_ring.c
+SRCS-y += rte_event_eth_rx_adapter.c
 
 # export include files
 SYMLINK-y-include += rte_eventdev.h
@@ -50,6 +51,7 @@ SYMLINK-y-include += rte_eventdev_pmd.h
 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
 
 # versioning export map
 EXPORT_MAP := rte_eventdev_version.map
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 996b361a5..e10546f73 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -56,6 +56,15 @@ DPDK_17.08 {
 DPDK_17.11 {
 	global:
 
+	rte_event_eth_rx_adapter_create_ext;
+	rte_event_eth_rx_adapter_create;
+	rte_event_eth_rx_adapter_free;
+	rte_event_eth_rx_adapter_queue_add;
+	rte_event_eth_rx_adapter_queue_del;
+	rte_event_eth_rx_adapter_start;
+	rte_event_eth_rx_adapter_stop;
+	rte_event_eth_rx_adapter_stats_get;
+	rte_event_eth_rx_adapter_stats_reset;
 	rte_event_eth_rx_adapter_caps_get;
-
+	rte_event_eth_rx_adapter_service_id_get;
 } DPDK_17.08;
-- 
2.14.1.145.gb3622a4

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-09-21 21:17 [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
                   ` (2 preceding siblings ...)
  2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
@ 2017-09-21 21:17 ` Nikhil Rao
  2017-09-22 12:12   ` Jerin Jacob
  2017-10-03 11:36   ` Pavan Nikhilesh Bhagavatula
  3 siblings, 2 replies; 30+ messages in thread
From: Nikhil Rao @ 2017-09-21 21:17 UTC (permalink / raw)
  To: jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

Add unit tests for rte_event_eth_rx_adapter_xxx() APIs

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 test/test/test_event_eth_rx_adapter.c | 399 ++++++++++++++++++++++++++++++++++
 test/test/Makefile                    |   1 +
 2 files changed, 400 insertions(+)
 create mode 100644 test/test/test_event_eth_rx_adapter.c

diff --git a/test/test/test_event_eth_rx_adapter.c b/test/test/test_event_eth_rx_adapter.c
new file mode 100644
index 000000000..5d448dc27
--- /dev/null
+++ b/test/test/test_event_eth_rx_adapter.c
@@ -0,0 +1,399 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 Intel Corporation. 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.
+ */
+#include <string.h>
+#include <rte_common.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_ethdev.h>
+#include <rte_eventdev.h>
+
+#include <rte_event_eth_rx_adapter.h>
+
+#include "test.h"
+
+/* i40e limits max to 64 */
+#define MAX_NUM_RX_QUEUE	64
+#define NB_MBUFS		(8192 * num_ports * MAX_NUM_RX_QUEUE)
+#define MBUF_CACHE_SIZE		512
+#define MBUF_PRIV_SIZE		0
+
+struct event_eth_rx_adapter_test_params {
+	struct rte_mempool *mp;
+	uint16_t rx_rings, tx_rings;
+	uint32_t caps;
+};
+
+static struct event_eth_rx_adapter_test_params default_params;
+
+static inline int
+port_init(uint8_t port, struct rte_mempool *mp)
+{
+	static const struct rte_eth_conf port_conf_default = {
+		.rxmode = {
+			.mq_mode = ETH_MQ_RX_RSS,
+			.max_rx_pkt_len = ETHER_MAX_LEN
+		},
+		.rx_adv_conf = {
+			.rss_conf = {
+				.rss_hf = ETH_RSS_IP |
+					  ETH_RSS_TCP |
+					  ETH_RSS_UDP,
+			}
+		}
+	};
+	const uint16_t rx_ring_size = 512, tx_ring_size = 512;
+	struct rte_eth_conf port_conf = port_conf_default;
+	int retval;
+	uint16_t q;
+	struct rte_eth_dev_info dev_info;
+
+	if (port >= rte_eth_dev_count())
+		return -1;
+
+	rte_eth_dev_info_get(port, &dev_info);
+	default_params.rx_rings = RTE_MIN(MAX_NUM_RX_QUEUE,
+					dev_info.max_rx_queues);
+	default_params.tx_rings = 1;
+
+	/* Configure the Ethernet device. */
+	retval = rte_eth_dev_configure(port, default_params.rx_rings,
+				default_params.tx_rings, &port_conf);
+	if (retval != 0)
+		return retval;
+
+	for (q = 0; q < default_params.rx_rings; q++) {
+		retval = rte_eth_rx_queue_setup(port, q, rx_ring_size,
+				rte_eth_dev_socket_id(port), NULL, mp);
+		if (retval < 0)
+			return retval;
+	}
+
+	/* Allocate and set up 1 TX queue per Ethernet port. */
+	for (q = 0; q < default_params.tx_rings; q++) {
+		retval = rte_eth_tx_queue_setup(port, q, tx_ring_size,
+				rte_eth_dev_socket_id(port), NULL);
+		if (retval < 0)
+			return retval;
+	}
+
+	/* Start the Ethernet port. */
+	retval = rte_eth_dev_start(port);
+	if (retval < 0)
+		return retval;
+
+	/* Display the port MAC address. */
+	struct ether_addr addr;
+	rte_eth_macaddr_get(port, &addr);
+	printf("Port %u MAC: %02" PRIx8 " %02" PRIx8 " %02" PRIx8
+			   " %02" PRIx8 " %02" PRIx8 " %02" PRIx8 "\n",
+			(unsigned int)port,
+			addr.addr_bytes[0], addr.addr_bytes[1],
+			addr.addr_bytes[2], addr.addr_bytes[3],
+			addr.addr_bytes[4], addr.addr_bytes[5]);
+
+	/* Enable RX in promiscuous mode for the Ethernet device. */
+	rte_eth_promiscuous_enable(port);
+
+	return 0;
+}
+
+static int
+init_ports(int num_ports)
+{
+	uint8_t portid;
+	int retval;
+
+	default_params.mp = rte_pktmbuf_pool_create("packet_pool",
+						NB_MBUFS,
+						MBUF_CACHE_SIZE,
+						MBUF_PRIV_SIZE,
+						RTE_MBUF_DEFAULT_BUF_SIZE,
+						rte_socket_id());
+	if (!default_params.mp)
+		return -ENOMEM;
+
+	for (portid = 0; portid < num_ports; portid++) {
+		retval = port_init(portid, default_params.mp);
+		if (retval)
+			return retval;
+	}
+
+	return 0;
+}
+
+static int
+testsuite_setup(void)
+{
+	int err;
+	err = init_ports(rte_eth_dev_count());
+	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
+
+	struct rte_event_dev_config config = {
+			.nb_event_queues = 1,
+			.nb_event_ports = 1,
+			.nb_events_limit  = 4096,
+			.nb_event_queue_flows = 1024,
+			.nb_event_port_dequeue_depth = 16,
+			.nb_event_port_enqueue_depth = 16
+	};
+
+	err = rte_event_dev_configure(0, &config);
+	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
+			err);
+
+	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
+	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",
+			err);
+
+	return err;
+}
+
+static void
+testsuite_teardown(void)
+{
+	uint32_t i;
+	rte_mempool_free(default_params.mp);
+	for (i = 0; i < rte_eth_dev_count(); i++)
+		rte_eth_dev_stop(i);
+}
+
+static int adapter_create(void)
+{
+	struct rte_event_port_conf rx_p_conf = {
+			.dequeue_depth = 8,
+			.enqueue_depth = 8,
+			.new_event_threshold = 1200,
+	};
+
+	int err = rte_event_eth_rx_adapter_create(0, 0, &rx_p_conf);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	return err;
+}
+
+static void adapter_free(void)
+{
+	rte_event_eth_rx_adapter_free(0);
+}
+
+static int
+adapter_create_free(void)
+{
+	int err;
+
+	struct rte_event_port_conf rx_p_conf = {
+			.dequeue_depth = 8,
+			.enqueue_depth = 8,
+			.new_event_threshold = 1200,
+	};
+
+	err = rte_event_eth_rx_adapter_create(0, 0, NULL);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	err = rte_event_eth_rx_adapter_create(0, 0, &rx_p_conf);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_create(0, 0, &rx_p_conf);
+	TEST_ASSERT(err == -EEXIST, "Expected -EEXIST %d got %d", -EEXIST, err);
+
+	err = rte_event_eth_rx_adapter_free(0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_free(0);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL %d got %d", -EINVAL, err);
+
+	err = rte_event_eth_rx_adapter_free(1);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL %d got %d", -EINVAL, err);
+
+	return TEST_SUCCESS;
+}
+
+static int
+adapter_queue_add_del(void)
+{
+	int err;
+	struct rte_event ev;
+	uint32_t cap;
+
+	struct rte_event_eth_rx_adapter_queue_conf queue_config;
+
+	err = rte_event_eth_rx_adapter_caps_get(0, 0, &cap);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	ev.queue_id = 0;
+	ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
+	ev.priority = 0;
+
+	queue_config.rx_queue_flags = 0;
+	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)) {
+		ev.flow_id = 1;
+		queue_config.rx_queue_flags =
+			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
+	}
+	queue_config.ev = ev;
+	queue_config.servicing_weight = 1;
+
+	err = rte_event_eth_rx_adapter_queue_add(0, rte_eth_dev_count(),
+					-1, &queue_config);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ)) {
+		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
+							&queue_config);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_del(0, 0, 0);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_add(0, 0, -1,
+							&queue_config);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+	} else {
+		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
+							&queue_config);
+		TEST_ASSERT(err == -EINVAL, "Expected EINVAL got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_add(0, 0, -1,
+							&queue_config);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_del(0, 0, 0);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+	}
+
+	err = rte_event_eth_rx_adapter_queue_add(1, 0, -1, &queue_config);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	err = rte_event_eth_rx_adapter_queue_del(1, 0, -1);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	return TEST_SUCCESS;
+}
+
+static int
+adapter_start_stop(void)
+{
+	int err;
+	struct rte_event ev;
+
+	ev.queue_id = 0;
+	ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
+	ev.priority = 0;
+
+	struct rte_event_eth_rx_adapter_queue_conf queue_config;
+
+	queue_config.rx_queue_flags = 0;
+	if (!(default_params.caps & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)) {
+		ev.flow_id = 1;
+		queue_config.rx_queue_flags =
+			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
+	}
+
+	queue_config.ev = ev;
+	queue_config.servicing_weight = 1;
+
+	err = rte_event_eth_rx_adapter_queue_add(0, 0, -1, &queue_config);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_start(0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_stop(0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_start(0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_stop(0);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_start(1);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	err = rte_event_eth_rx_adapter_stop(1);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	return TEST_SUCCESS;
+}
+
+static int
+adapter_stats(void)
+{
+	int err;
+	struct rte_event_eth_rx_adapter_stats stats;
+
+	err = rte_event_eth_rx_adapter_stats_get(0, NULL);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	err = rte_event_eth_rx_adapter_stats_get(0, &stats);
+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+	err = rte_event_eth_rx_adapter_stats_get(1, &stats);
+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
+
+	return TEST_SUCCESS;
+}
+
+static struct unit_test_suite service_tests  = {
+	.suite_name = "rx event eth adapter test suite",
+	.setup = testsuite_setup,
+	.teardown = testsuite_teardown,
+	.unit_test_cases = {
+		TEST_CASE_ST(NULL, NULL, adapter_create_free),
+		TEST_CASE_ST(adapter_create, adapter_free,
+					adapter_queue_add_del),
+		TEST_CASE_ST(adapter_create, adapter_free, adapter_start_stop),
+		TEST_CASE_ST(adapter_create, adapter_free, adapter_stats),
+		TEST_CASES_END() /**< NULL terminate unit test array */
+	}
+};
+
+static int
+test_event_eth_rx_adapter_common(void)
+{
+	return unit_test_suite_runner(&service_tests);
+}
+
+REGISTER_TEST_COMMAND(event_eth_rx_adapter_autotest,
+		test_event_eth_rx_adapter_common);
diff --git a/test/test/Makefile b/test/test/Makefile
index 42d9a49e2..011288219 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -204,6 +204,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev.c
 ifeq ($(CONFIG_RTE_LIBRTE_EVENTDEV),y)
 SRCS-y += test_eventdev.c
 SRCS-y += test_event_ring.c
+SRCS-y += test_event_eth_rx_adapter.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += test_eventdev_sw.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += test_eventdev_octeontx.c
 endif
-- 
2.14.1.145.gb3622a4

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD
  2017-09-21 21:17 ` [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
@ 2017-09-22  2:49   ` Jerin Jacob
  2017-09-22  5:27   ` santosh
  1 sibling, 0 replies; 30+ messages in thread
From: Jerin Jacob @ 2017-09-22  2:49 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Fri, 22 Sep 2017 02:47:12 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>  harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, erik.g.carrillo@intel.com,
>  abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> Subject: [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to
>  eventdev SW PMD
> X-Mailer: git-send-email 2.7.4
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD
  2017-09-21 21:17 ` [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
  2017-09-22  2:49   ` Jerin Jacob
@ 2017-09-22  5:27   ` santosh
  1 sibling, 0 replies; 30+ messages in thread
From: santosh @ 2017-09-22  5:27 UTC (permalink / raw)
  To: Nikhil Rao, jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar


On Friday 22 September 2017 02:47 AM, Nikhil Rao wrote:
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---

Reviewed-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
  2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
@ 2017-09-22  6:08   ` santosh
  2017-10-02 10:20     ` Rao, Nikhil
  2017-09-22  9:10   ` Jerin Jacob
  2 siblings, 1 reply; 30+ messages in thread
From: santosh @ 2017-09-22  6:08 UTC (permalink / raw)
  To: Nikhil Rao, jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar


On Friday 22 September 2017 02:47 AM, Nikhil Rao wrote:
> Add common APIs for configuring packet transfer from ethernet Rx
> queues to event devices across HW & SW packet transfer mechanisms.
> A detailed description of the adapter is contained in the header's
> comments.
>
> The adapter implementation uses eventdev PMDs to configure the packet
> transfer if HW support is available and if not, it uses an EAL service
> function that reads packets from ethernet Rx queues and injects these
> as events into the event device.
>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
>  lib/Makefile                                   |    2 +-
>  lib/librte_eventdev/Makefile                   |    2 +
>  lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
>  5 files changed, 1635 insertions(+), 2 deletions(-)
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
>
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> new file mode 100644
> index 000000000..c3849ec31
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -0,0 +1,384 @@
> +/*
> + *   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_ETH_RX_ADAPTER_
> +#define _RTE_EVENT_ETH_RX_ADAPTER_
> +
> +/**
> + * @file
> + *
> + * RTE Event Ethernet Rx Adapter
> + *
> + * An eventdev-based packet processing application enqueues/dequeues mbufs
> + * to/from the event device. The application uses the adapter APIs to configure
> + * the packet flow between the ethernet devices and event devices. Depending on
> + * on the capabilties of the eventdev PMD, the adapter may use a EAL service
> + * core function for packet transfer or use internal PMD functions to configure
> + * the packet transfer between the ethernet device and the event device.
> + *
> + * The ethernet Rx event adapter's functions are:
> + *  - rte_event_eth_rx_adapter_create_ext()
> + *  - rte_event_eth_rx_adapter_create()/free()
> + *  - rte_event_eth_rx_adapter_queue_add()/del()
> + *  - rte_event_eth_rx_adapter_start()/stop()
> + *  - rte_event_eth_rx_adapter_stats_get()/reset()
> + *
> + * The applicaton creates an event to ethernet adapter using
> + * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
> + * functions.
> + * The adapter needs to know which ethernet rx queues to poll for mbufs as well
> + * as event device parameters such as the event queue identifier, event
> + * priority and scheduling type that the adapter should use when constructing
> + * events. The rte_event_eth_rx_adapter_queue_add() function is provided for
> + * this purpose.
> + * The servicing weight parameter in the rte_event_eth_rx_adapter_queue_conf
> + * is applicable when the Rx adapter uses a service core function and is
> + * intended to provide application control of the polling frequency of ethernet
> + * device receive queues, for example, the application may want to poll higher
> + * priority queues with a higher frequency but at the same time not starve
> + * lower priority queues completely. If this parameter is zero and the receive
> + * interrupt is enabled when configuring the device, the receive queue is
> + * interrupt driven; else, the queue is assigned a servicing weight of one.
> + *
> + * If the adapter uses a rte_service function, then the application is also
> + * required to assign a core to the service function and control the service
> + * core using the rte_service APIs. The rte_event_eth_rx_adapter_service_id_get
> + * function can be used to retrieve the service function ID of the adapter in
> + * this case.
> + *
> + * Note: Interrupt driven receive queues are currentely unimplemented.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32
> +
> +/* struct rte_event_eth_rx_adapter_queue_conf flags definitions */
> +#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID	0x1
> +/**< This flag indicates the flow identifier is valid
> + * @see rte_event_eth_rx_adapter_queue_conf
> + */
> +
> +struct rte_event_eth_rx_adapter_conf {
> +	uint8_t event_port_id;
> +	/**< Event port identifier, the adapter enqueues mbuf events to this
> +	 * port
> +	 */
> +	uint32_t max_nb_rx;
> +	/**< The adapter can return early if it has processed at least
> +	 * max_nb_rx mbufs. This isn't treated as a requirement; batching may
> +	 * cause the adapter to process more than max_nb_rx mbufs
> +	 */
> +};
> +
> +/**
> + * Function type used for adapter configuration callback. The callback is
> + * used to fill in members of the struct rte_event_eth_rx_adapter_conf, this
> + * callback is invoked when creating a SW service for packet transfer from
> + * ethdev queues to the event device. The SW service is created within the
> + * rte_event_eth_rx_adapter_queue_add() function if packet required.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param dev_id
> + *  Event device identifier.
> + *
> + * @conf
> + *  Structure that needs to be populated by this callback.
> + *
> + * @arg
> + *  Argument to the callback. This is the same as the conf_arg passed to the
> + *  rte_event_eth_rx_adapter_create_ext()
> + */
> +typedef int (*rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
> +			struct rte_event_eth_rx_adapter_conf *conf,
> +			void *arg);
> +
> +/** Rx queue configuration structure */
> +struct rte_event_eth_rx_adapter_queue_conf {
> +	uint32_t rx_queue_flags;
> +	 /**< Flags for handling received packets
> +	  * @see RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID
> +	  */
> +	uint16_t servicing_weight;
> +	/**< Relative polling frequency of ethernet receive queue, if this
> +	 * is set to zero, the Rx queue is interrupt driven (unless rx queue
> +	 * interrupts are not enabled for the ethernet device)
> +	 */
> +	struct rte_event ev;
> +	/**<
> +	 *  The values from the following event fields will be used when
> +	 *  enqueuing mbuf events:
> +	 *   - event_queue_id: Targeted event queue ID for received packets.
> +	 *   - event_priority: Event priority of packets from this Rx queue in
> +	 *                     the event queue relative to other events.
> +	 *   - sched_type: Scheduling type for packets from this Rx queue.
> +	 *   - flow_id: If the RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit
> +	 *		is set in rx_queue_flags, this flow_id is used for all
> +	 *		packets received from this queue. Otherwise the flow ID
> +	 *		is set to the RSS hash of the src and dst IPv4/6
> +	 *		address.
> +	 *
> +	 * The event adapter sets ev.event_type to RTE_EVENT_TYPE_ETHDEV in the
> +	 * enqueued event
> +	 */
> +};
> +
> +struct rte_event_eth_rx_adapter_stats {
> +	uint64_t rx_poll_count;
> +	/**< Receive queue poll count */
> +	uint64_t rx_packets;
> +	/**< Received packet count */
> +	uint64_t rx_enq_count;
> +	/**< Eventdev enqueue count */
> +	uint64_t rx_enq_retry;
> +	/**< Eventdev enqueue retry count */
> +	uint64_t rx_enq_start_ts;
> +	/**< Rx enqueue start timestamp */
> +	uint64_t rx_enq_block_cycles;
> +	/**< Cycles for which the service is blocked by the event device,
> +	 * i.e, the service fails to enqueue to the event device.
> +	 */
> +	uint64_t rx_enq_end_ts;
> +	/**< Latest timestamp at which the service is unblocked
> +	 * by the event device. The start, end timestamps and
> +	 * block cycles can be used to compute the percentage of
> +	 * cycles the service is blocked by the event device.
> +	 */
> +};
> +
> +/**
> + * Create a new ethernet Rx event adapter with the specified identifier.
> + *
> + * @param id
> + *  The identifier of the ethernet Rx event adapter.
> + *
> + * @dev_id
> + *  The identifier of the device to configure.
> + *
> + * @eth_port_id
> + *  The identifier of the ethernet device.
> + *
> + * @param conf_cb
> + *  Callback function that fills in members of a
> + *  struct rte_event_eth_rx_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_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> +					rx_adapter_conf_cb conf_cb,
> +					void *conf_arg);
> +
> +/**
> + * Create a new ethernet Rx event 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_eth_rx_adapter_create_ext() version.
> + *
> + * @param id
> + *  The identifier of the ethernet Rx event adapter.
> + *
> + * @dev_id
> + *  The identifier of the device to configure.
> + *
> + * @eth_port_id
> + *  The identifier of the ethernet device.
> + *
> + * @param conf_cb
> + *  Callback function that fills in members of a
> + *  struct rte_event_eth_rx_adapter_conf struct passed into
> + *  it.
> + *
> + * @param conf_arg
> + *  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_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
> +				struct rte_event_port_conf *port_config);
> +
> +/**
> + * Free an event adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure, If the adapter still has Rx queues
> + *      added to it, the function returns -EBUSY.
> + */
> +int rte_event_eth_rx_adapter_free(uint8_t id);
> +
> +/**
> + * Add receive queue to an event adapter. After a queue has been
> + * added to the event adapter, the result of the application calling
> + * rte_eth_rx_burst(eth_dev_id, rx_queue_id, ..) is undefined.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are added. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.
> + *
> + * @param conf
> + *  Additonal configuration structure of type *rte_event_eth_rx_adapte_conf*
> + *
> + * @see
> + * @return
> + *  - 0: Success, Receive queue added correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_queue_add(uint8_t id,
> +			uint8_t eth_dev_id,
> +			int32_t rx_queue_id,
> +			const struct rte_event_eth_rx_adapter_queue_conf *conf);
> +
> +/**
> + * Delete receive queue from an event adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are deleted. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.
> + *
> + * @return
> + *  - 0: Success, Receive queue deleted correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
> +				       int32_t rx_queue_id);
> +
> +/**
> + * Start  ethernet Rx event adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, Adapter started correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_start(uint8_t id);
> +
> +/**
> + * Stop  ethernet Rx event adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, Adapter started correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_stop(uint8_t id);
> +
> +/**
> + * Retrieve statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param 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_eth_rx_adapter_stats_get(uint8_t id,
> +				struct rte_event_eth_rx_adapter_stats *stats);
> +
> +/**
> + * Reset statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_stats_reset(uint8_t id);
> +
> +/**
> + * 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.
> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> + * function, this function returns -ESRCH.
> + */
> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> +

In general api comment: Fix missing param definition like *service_id* above
and pl. remove other unnecessary params description from api above.

> +#ifdef __cplusplus
> +}
> +#endif
> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..d5b655dae
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> @@ -0,0 +1,1238 @@
> +#include <rte_cycles.h>
> +#include <rte_common.h>
> +#include <rte_dev.h>
> +#include <rte_errno.h>
> +#include <rte_ethdev.h>
> +#include <rte_log.h>
> +#include <rte_malloc.h>
> +#include <rte_service_component.h>
> +#include <rte_thash.h>
> +
> +#include "rte_eventdev.h"
> +#include "rte_eventdev_pmd.h"
> +#include "rte_event_eth_rx_adapter.h"
> +
> +#define BATCH_SIZE		32
> +#define BLOCK_CNT_THRESHOLD	10
> +#define ETH_EVENT_BUFFER_SIZE	(4*BATCH_SIZE)
> +
> +#define ETH_RX_ADAPTER_SERVICE_NAME_LEN	32
> +#define ETH_RX_ADAPTER_MEM_NAME_LEN	32
> +
> +/*
> + * There is an instance of this struct per polled Rx queue added to the
> + * adapter
> + */
> +struct eth_rx_poll_entry {
> +	/* eth port to poll */
> +	uint8_t eth_dev_id;
> +	/* eth rx queue to poll */
> +	uint16_t eth_rx_qid;
> +};
> +
> +/* Instance per adapter */
> +struct rte_eth_event_enqueue_buffer {
> +	/* Count of events in this buffer */
> +	uint16_t count;
> +	/* Array of events in this buffer */
> +	struct rte_event events[ETH_EVENT_BUFFER_SIZE];
> +};
> +
> +struct rte_event_eth_rx_adapter {
> +	/* event device identifier */
> +	uint8_t eventdev_id;
> +	/* per ethernet device structure */
> +	struct eth_device_info *eth_devices;
> +	/* malloc name */
> +	char mem_name[ETH_RX_ADAPTER_MEM_NAME_LEN];
> +	/* socket identifier cached from eventdev */
> +	int socket_id;
> +
> +	/* elements below are used by SW service */
> +
> +	/* event port identifier */
> +	uint8_t event_port_id;
> +	/* per adapter EAL service */
> +	uint32_t service_id;
> +	/* lock to serialize config updates with service function */
> +	rte_spinlock_t rx_lock;
> +	/* max mbufs processed in any service function invocation */
> +	uint32_t max_nb_rx;
> +	/* Receive queues that need to be polled */
> +	struct eth_rx_poll_entry *eth_rx_poll;
> +	/* size of the eth_rx_poll array */
> +	uint16_t num_rx_polled;
> +	/* Weighted round robin schedule */
> +	uint32_t *wrr_sched;
> +	/* wrr_sched[] size */
> +	uint32_t wrr_len;
> +	/* Next entry in wrr[] to begin polling */
> +	uint32_t wrr_pos;
> +	/* Event burst buffer */
> +	struct rte_eth_event_enqueue_buffer event_enqueue_buffer;
> +	/* per adapter stats */
> +	struct rte_event_eth_rx_adapter_stats stats;
> +	/* Block count, counts upto BLOCK_CNT_THRESHOLD */
> +	uint16_t enq_block_count;
> +	/* Block start ts */
> +	uint64_t rx_enq_block_start_ts;
> +	/* Configuration callback for rte_service configuration */
> +	rx_adapter_conf_cb conf_cb;
> +	/* Configuration callback argument */
> +	void *conf_arg;
> +	/* Service initialization state */
> +	uint8_t service_inited;
> +	/* Total count of Rx queues in adapter */
> +	uint32_t nb_queues;
> +} __rte_cache_aligned;
> +
> +/* Per eth device */
> +struct eth_device_info {
> +	struct rte_eth_dev *dev;
> +	struct eth_rx_queue_info *rx_queue;
> +	/* Set if ethdev->eventdev packet transfer uses a
> +	 * hardware mechanism
> +	 */
> +	uint8_t internal_event_port;
> +	/* set if the adapter is processing rx queues for
> +	 * this eth device and packet processing has been
> +	 * started, allows for the code to know if the PMD
> +	 * rx_adapter_stop callback needs to be invoked
> +	 */
> +	uint8_t dev_rx_started;
> +	/* if nb_dev_queues > 0, the start callback will
> +	 * be invoked if not already invoked
> +	 */
> +	uint16_t nb_dev_queues;
> +};
> +
> +/* Per Rx queue */
> +struct eth_rx_queue_info {
> +	int queue_enabled;	/* true if added */
> +	uint16_t wt;		/* polling weight */
> +	uint8_t event_queue_id;	/* Event queue to enqueue packets to */
> +	uint8_t sched_type;	/* sched type for events */
> +	uint8_t priority;	/* event priority */
> +	uint32_t flow_id;	/* app provided flow identifier */
> +	uint32_t flow_id_mask;	/* Set to ~0 if app provides flow id else 0 */
> +};
> +
> +static struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter;
> +static struct rte_event_port_conf
> +		create_port_conf[RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE];
> +
> +static uint8_t default_rss_key[] = {
> +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> +};
> +static uint8_t *rss_key_be;
> +
> +static inline int
> +valid_id(uint8_t id)
> +{
> +	return id < RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
> +}
> +
> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, retval) do { \
> +	if (!valid_id(id)) { \
> +		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> +		return retval; \
> +	} \
> +} while (0)
> +

Worth, moving this macro to rte_eventdev_pmd.h
Or How about reusing existing one ie.. RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET?

> +static inline int
> +sw_rx_adapter_queue_count(struct rte_event_eth_rx_adapter *rx_adapter)
> +{
> +	return rx_adapter->num_rx_polled;
> +}
> +
> +/* Greatest common divisor */
> +static uint16_t gcd_u16(uint16_t a, uint16_t b)
> +{
> +	uint16_t r = a % b;
> +
> +	return r ? gcd_u16(b, r) : b;
> +}
> +
> +/* Returns the next queue in the polling sequence
> + *
> + * http://kb.linuxvirtualserver.org/wiki/Weighted_Round-Robin_Scheduling
> + */
> +static int
> +wrr_next(struct rte_event_eth_rx_adapter *rx_adapter,
> +	 unsigned int n, int *cw,
> +	 struct eth_rx_poll_entry *eth_rx_poll, uint16_t max_wt,
> +	 uint16_t gcd, int prev)
> +{
> +	int i = prev;
> +	uint16_t w;
> +
> +	while (1) {
> +		uint16_t q;
> +		uint8_t d;
> +
> +		i = (i + 1) % n;
> +		if (i == 0) {
> +			*cw = *cw - gcd;
> +			if (*cw <= 0)
> +				*cw = max_wt;
> +		}
> +
> +		q = eth_rx_poll[i].eth_rx_qid;
> +		d = eth_rx_poll[i].eth_dev_id;
> +		w = rx_adapter->eth_devices[d].rx_queue[q].wt;
> +
> +		if ((int)w >= *cw)
> +			return i;
> +	}
> +}
> +
> +/* Precalculate WRR polling sequence for all queues in rx_adapter */
> +static int
> +eth_poll_wrr_calc(struct rte_event_eth_rx_adapter *rx_adapter)
> +{
> +	uint8_t d;
> +	uint16_t q;
> +	unsigned int i;
> +
> +	/* Initialize variables for calculaton of wrr schedule */
> +	uint16_t max_wrr_pos = 0;
> +	unsigned int poll_q = 0;
> +	uint16_t max_wt = 0;
> +	uint16_t gcd = 0;
> +
> +	struct eth_rx_poll_entry *rx_poll = NULL;
> +	uint32_t *rx_wrr = NULL;
> +
> +	if (rx_adapter->num_rx_polled) {
> +		size_t len = RTE_ALIGN(rx_adapter->num_rx_polled *
> +				sizeof(*rx_adapter->eth_rx_poll),
> +				RTE_CACHE_LINE_SIZE);
> +		rx_poll = rte_zmalloc_socket(rx_adapter->mem_name,
> +					     len,
> +					     RTE_CACHE_LINE_SIZE,
> +					     rx_adapter->socket_id);
> +		if (!rx_poll)
> +			return -ENOMEM;
> +
> +		/* Generate array of all queues to poll, the size of this
> +		 * array is poll_q
> +		 */
> +		for (d = 0; d < rte_eth_dev_count(); d++) {
> +			uint16_t nb_rx_queues;
> +			struct eth_device_info *dev_info =
> +					&rx_adapter->eth_devices[d];
> +			nb_rx_queues = dev_info->dev->data->nb_rx_queues;
> +			if (!dev_info->rx_queue)
> +				continue;
> +			for (q = 0; q < nb_rx_queues; q++) {
> +				struct eth_rx_queue_info *queue_info =
> +					&dev_info->rx_queue[q];
> +				if (!queue_info->queue_enabled)
> +					continue;
> +
> +				uint16_t wt = queue_info->wt;
> +				rx_poll[poll_q].eth_dev_id = d;
> +				rx_poll[poll_q].eth_rx_qid = q;
> +				max_wrr_pos += wt;
> +				max_wt = RTE_MAX(max_wt, wt);
> +				gcd = (gcd) ? gcd_u16(gcd, wt) : wt;
> +				poll_q++;
> +			}
> +		}
> +
> +		len = RTE_ALIGN(max_wrr_pos * sizeof(*rx_wrr),
> +				RTE_CACHE_LINE_SIZE);
> +		rx_wrr = rte_zmalloc_socket(rx_adapter->mem_name,
> +					    len,
> +					    RTE_CACHE_LINE_SIZE,
> +					    rx_adapter->socket_id);
> +		if (!rx_wrr) {
> +			rte_free(rx_poll);
> +			return -ENOMEM;
> +		}
> +
> +		/* Generate polling sequence based on weights */
> +		int prev = -1;
> +		int cw = -1;
> +		for (i = 0; i < max_wrr_pos; i++) {
> +			rx_wrr[i] = wrr_next(rx_adapter, poll_q, &cw,
> +					     rx_poll, max_wt, gcd, prev);
> +			prev = rx_wrr[i];
> +		}
> +	}
> +
> +	rte_free(rx_adapter->eth_rx_poll);
> +	rte_free(rx_adapter->wrr_sched);
> +
> +	rx_adapter->eth_rx_poll = rx_poll;
> +	rx_adapter->wrr_sched = rx_wrr;
> +	rx_adapter->wrr_len = max_wrr_pos;
> +
> +	return 0;
> +}
> +
> +static inline void
> +mtoip(struct rte_mbuf *m, struct ipv4_hdr **ipv4_hdr,
> +	struct ipv6_hdr **ipv6_hdr)
> +{

mtoip(), imo is more of global api, likely other modules may use in future..
perhaps move to rte_io.h Or more correct place.. thought?

Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
  2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
  2017-09-22  6:08   ` santosh
@ 2017-09-22  9:10   ` Jerin Jacob
  2017-09-24 18:16     ` Rao, Nikhil
  2 siblings, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-09-22  9:10 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Fri, 22 Sep 2017 02:47:13 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>  harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, erik.g.carrillo@intel.com,
>  abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> Subject: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
> X-Mailer: git-send-email 2.7.4
> 
> Add common APIs for configuring packet transfer from ethernet Rx
> queues to event devices across HW & SW packet transfer mechanisms.
> A detailed description of the adapter is contained in the header's
> comments.
> 
> The adapter implementation uses eventdev PMDs to configure the packet
> transfer if HW support is available and if not, it uses an EAL service
> function that reads packets from ethernet Rx queues and injects these
> as events into the event device.
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>

Overall it looks good. A few top-level comments to start with,

1) Please split this patch to minimum two
a) Specification header
b) and the implementation

2) Please add a new section in MAINTAINERS files and update the new files
responsibility.

3) The doxygen file is not hooked into the documentation build.
Check the doc/api/doxy-api-index.md file. You can use "make doc-api-html"
to verify the doxygen html output.

4) Since the APIs looks good and if there is no other objection,
Can you add a programmer guide for Rx adapter.
If you are busy it is fine not have in next version. Post RC1 or RC2 is
fine. What do you think?


> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
>  lib/Makefile                                   |    2 +-
>  lib/librte_eventdev/Makefile                   |    2 +
>  lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
>  5 files changed, 1635 insertions(+), 2 deletions(-)
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>  create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
> 
> +#ifndef _RTE_EVENT_ETH_RX_ADAPTER_
> +#define _RTE_EVENT_ETH_RX_ADAPTER_
> +
> +/**
> + * @file
> + *
> + * RTE Event Ethernet Rx Adapter
> + *
> + * An eventdev-based packet processing application enqueues/dequeues mbufs
> + * to/from the event device. The application uses the adapter APIs to configure
> + * the packet flow between the ethernet devices and event devices. Depending on
> + * on the capabilties of the eventdev PMD, the adapter may use a EAL service

s/capabilties/capabilities

> + * core function for packet transfer or use internal PMD functions to configure
> + * the packet transfer between the ethernet device and the event device.
> + *
> + * The ethernet Rx event adapter's functions are:
> + *  - rte_event_eth_rx_adapter_create_ext()
> + *  - rte_event_eth_rx_adapter_create()/free()
> + *  - rte_event_eth_rx_adapter_queue_add()/del()
> + *  - rte_event_eth_rx_adapter_start()/stop()
> + *  - rte_event_eth_rx_adapter_stats_get()/reset()
> + *
> + * The applicaton creates an event to ethernet adapter using

How about,
The application creates an ethernet device to event device adapter using

> + * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
> + * functions.
> + * The adapter needs to know which ethernet rx queues to poll for mbufs as well
> + * as event device parameters such as the event queue identifier, event
> + * priority and scheduling type that the adapter should use when constructing
> + * events. The rte_event_eth_rx_adapter_queue_add() function is provided for
> + * this purpose.
> + * The servicing weight parameter in the rte_event_eth_rx_adapter_queue_conf
> + * is applicable when the Rx adapter uses a service core function and is
> + * intended to provide application control of the polling frequency of ethernet
> + * device receive queues, for example, the application may want to poll higher
> + * priority queues with a higher frequency but at the same time not starve
> + * lower priority queues completely. If this parameter is zero and the receive
> + * interrupt is enabled when configuring the device, the receive queue is
> + * interrupt driven; else, the queue is assigned a servicing weight of one.
> + *
> + * If the adapter uses a rte_service function, then the application is also
> + * required to assign a core to the service function and control the service
> + * core using the rte_service APIs. The rte_event_eth_rx_adapter_service_id_get
> + * function can be used to retrieve the service function ID of the adapter in
> + * this case.
> + *
> + * Note: Interrupt driven receive queues are currentely unimplemented.

s/currentely/currently

> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>

A Linefeed is norm here.

> +#include <rte_service.h>
> +
> +#include "rte_eventdev.h"
> +
> +#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32

Considering the name space, How about to change to
RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE? and fix the missing the doxygen comments

> +
> +/* struct rte_event_eth_rx_adapter_queue_conf flags definitions */
> +#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID	0x1
> +/**< This flag indicates the flow identifier is valid
> + * @see rte_event_eth_rx_adapter_queue_conf

@see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
@see rte_event_eth_rx_adapter_queue_conf::ev::flow_id


> + */
> +
> +/**
> + * Function type used for adapter configuration callback. The callback is
> + * used to fill in members of the struct rte_event_eth_rx_adapter_conf, this
> + * callback is invoked when creating a SW service for packet transfer from
> + * ethdev queues to the event device. The SW service is created within the
> + * rte_event_eth_rx_adapter_queue_add() function if packet required.

"if packet is required", does not seem to be correct usage.
I guess, you mean, if packet required to transfer from ethdev queues to
the event device or something like that?

> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param dev_id
> + *  Event device identifier.
> + *
> + * @conf
> + *  Structure that needs to be populated by this callback.
> + *
> + * @arg
> + *  Argument to the callback. This is the same as the conf_arg passed to the
> + *  rte_event_eth_rx_adapter_create_ext()
> + */
> +typedef int (*rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
> +			struct rte_event_eth_rx_adapter_conf *conf,
> +			void *arg);

Public symbols should start with rte_

> +
> +/** Rx queue configuration structure */
> +struct rte_event_eth_rx_adapter_queue_conf {
> +	uint32_t rx_queue_flags;
> +	 /**< Flags for handling received packets
> +	  * @see RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID
> +	  */
> +	uint16_t servicing_weight;
> +	/**< Relative polling frequency of ethernet receive queue, if this
> +	 * is set to zero, the Rx queue is interrupt driven (unless rx queue
> +	 * interrupts are not enabled for the ethernet device)

IMO, You can mentione it as hint and applicable only when using with
SW based Rx adapter or something on similar lines.

> +	 */
> +	struct rte_event ev;
> +	/**<
> +	 *  The values from the following event fields will be used when
> +	 *  enqueuing mbuf events:
> +	 *   - event_queue_id: Targeted event queue ID for received packets.
> +	 *   - event_priority: Event priority of packets from this Rx queue in
> +	 *                     the event queue relative to other events.
> +	 *   - sched_type: Scheduling type for packets from this Rx queue.
> +	 *   - flow_id: If the RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit
> +	 *		is set in rx_queue_flags, this flow_id is used for all
> +	 *		packets received from this queue. Otherwise the flow ID
> +	 *		is set to the RSS hash of the src and dst IPv4/6
> +	 *		address.
> +	 *
> +	 * The event adapter sets ev.event_type to RTE_EVENT_TYPE_ETHDEV in the
> +	 * enqueued event

When we worked on a prototype, we figured out that we need a separate event type
for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
The Reason is:
- In the HW based Rx adapter case, the packet are coming directly to eventdev once it is configured.
- So on a HW implementation of the event dequeue(), CPU needs to convert HW specific
metadata to mbuf
- The event dequeue() is used in two cases
a) octeontx eventdev driver used with any external NIC
b) octeontx eventdev driver used with integrated NIC(without service
core to inject the packet)
We need some identifier to understand case (a) and (b).So, in dequeue(), if the
packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific metadata" to mbuf
conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result in no mbuf
conversion.

Application can check if it is an Ethernet type event by
ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER

Thoughts?


> +/**
> + * Add receive queue to an event adapter. After a queue has been
> + * added to the event adapter, the result of the application calling
> + * rte_eth_rx_burst(eth_dev_id, rx_queue_id, ..) is undefined.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are added. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.
> + *
> + * @param conf
> + *  Additonal configuration structure of type *rte_event_eth_rx_adapte_conf*
> + *
> + * @see

You can add @ see to denote the multi event queue enqueue capability

> + * @return
> + *  - 0: Success, Receive queue added correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_queue_add(uint8_t id,
> +			uint8_t eth_dev_id,
> +			int32_t rx_queue_id,
> +			const struct rte_event_eth_rx_adapter_queue_conf *conf);
> +
> +/**
> + * Delete receive queue from an event adapter.
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device.
> + *
> + * @param rx_queue_id
> + *  Ethernet device receive queue index.
> + *  If rx_queue_id is -1, then all Rx queues configured for
> + *  the device are deleted. If the ethdev Rx queues can only be
> + *  connected to a single event queue then rx_queue_id is
> + *  required to be -1.

You can add @ see to denote the multi event queue enqueue capability

> + *
> + * @return
> + *  - 0: Success, Receive queue deleted correctly.
> + *  - <0: Error code on failure.
> + */
> +int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
> +				       int32_t rx_queue_id);
> +
> +
> +/**
> + * Retrieve statistics for an adapter
> + *
> + * @param id
> + *  Adapter identifier.
> + *
> + * @param stats

@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_eth_rx_adapter_stats_get(uint8_t id,
> +				struct rte_event_eth_rx_adapter_stats *stats);
> +
> +
> +/**
> + * 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

> + *
> + * @return
> + *  - 0: Success, statistics reset successfully.
> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> + * function, this function returns -ESRCH.
> + */
> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..d5b655dae
> --- /dev/null
> +
> +struct rte_event_eth_rx_adapter {
> +	/* event device identifier */

Every elements you can start with capital letter.

> +	uint8_t eventdev_id;
> +	/* per ethernet device structure */
> +	struct eth_device_info *eth_devices;
> +	/* malloc name */
> +	char mem_name[ETH_RX_ADAPTER_MEM_NAME_LEN];
> +	/* socket identifier cached from eventdev */
> +	int socket_id;
> +
> +	/* elements below are used by SW service */
> +
> +	/* event port identifier */
> +	uint8_t event_port_id;
> +	/* per adapter EAL service */
> +	uint32_t service_id;
> +	/* lock to serialize config updates with service function */
> +	rte_spinlock_t rx_lock;
> +	/* max mbufs processed in any service function invocation */
> +	uint32_t max_nb_rx;
> +	/* Receive queues that need to be polled */
> +	struct eth_rx_poll_entry *eth_rx_poll;
> +	/* size of the eth_rx_poll array */
> +	uint16_t num_rx_polled;
> +	/* Weighted round robin schedule */
> +	uint32_t *wrr_sched;
> +	/* wrr_sched[] size */
> +	uint32_t wrr_len;
> +	/* Next entry in wrr[] to begin polling */
> +	uint32_t wrr_pos;
> +	/* Event burst buffer */
> +	struct rte_eth_event_enqueue_buffer event_enqueue_buffer;
> +	/* per adapter stats */
> +	struct rte_event_eth_rx_adapter_stats stats;
> +	/* Block count, counts upto BLOCK_CNT_THRESHOLD */
> +	uint16_t enq_block_count;
> +	/* Block start ts */
> +	uint64_t rx_enq_block_start_ts;
> +	/* Configuration callback for rte_service configuration */
> +	rx_adapter_conf_cb conf_cb;
> +	/* Configuration callback argument */
> +	void *conf_arg;
> +	/* Service initialization state */
> +	uint8_t service_inited;
> +	/* Total count of Rx queues in adapter */
> +	uint32_t nb_queues;
> +} __rte_cache_aligned;
> +
> +/* Per eth device */
> +struct eth_device_info {
> +	struct rte_eth_dev *dev;
> +	struct eth_rx_queue_info *rx_queue;
> +	/* Set if ethdev->eventdev packet transfer uses a
> +	 * hardware mechanism
> +	 */
> +	uint8_t internal_event_port;
> +	/* set if the adapter is processing rx queues for

s/set/Set

> +	 * this eth device and packet processing has been
> +	 * started, allows for the code to know if the PMD
> +	 * rx_adapter_stop callback needs to be invoked
> +	 */
> +	uint8_t dev_rx_started;
> +	/* if nb_dev_queues > 0, the start callback will
> +	 * be invoked if not already invoked
> +	 */
> +	uint16_t nb_dev_queues;
> +};
> +
> +static struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter;

Avoid using rte for Internal object(**rte_event_eth_rx_adapter)_

> +static struct rte_event_port_conf
> +		create_port_conf[RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE];

IMO, this memory can be stored in adapter memory to avoid global variable.

> +
> +static uint8_t default_rss_key[] = {
> +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> +};

Looks like the scope of this array is only for rte_event_eth_rx_adapter_init,
if so please move it to stack.


> +static uint8_t *rss_key_be;

Can we remove this global variable add it in in adapter memory?

> +}
> +
> +static inline void
> +fill_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter,
> +	uint8_t dev_id,
> +	uint16_t rx_queue_id,
> +	struct rte_mbuf **mbufs,
> +	uint16_t num)
> +{
> +	uint32_t i;
> +	struct eth_device_info *eth_device_info =
> +					&rx_adapter->eth_devices[dev_id];
> +	struct eth_rx_queue_info *eth_rx_queue_info =
> +					&eth_device_info->rx_queue[rx_queue_id];
> +
> +	int32_t qid = eth_rx_queue_info->event_queue_id;
> +	uint8_t sched_type = eth_rx_queue_info->sched_type;
> +	uint8_t priority = eth_rx_queue_info->priority;
> +	uint32_t flow_id;
> +	struct rte_event events[BATCH_SIZE];
> +	struct rte_mbuf *m = mbufs[0];
> +	uint32_t rss_mask;
> +	uint32_t rss;
> +	int do_rss;
> +
> +	/* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
> +	rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
> +	do_rss = !rss_mask && !eth_rx_queue_info->flow_id_mask;
> +
> +	for (i = 0; i < num; i++) {
> +		m = mbufs[i];
> +		struct rte_event *ev = &events[i];
> +
> +		rss = do_rss ? do_softrss(m) : m->hash.rss;
> +		flow_id =
> +		    eth_rx_queue_info->flow_id &
> +				eth_rx_queue_info->flow_id_mask;
> +		flow_id |= rss & ~eth_rx_queue_info->flow_id_mask;
> +
> +		ev->flow_id = flow_id;
> +		ev->op = RTE_EVENT_OP_NEW;
> +		ev->sched_type = sched_type;
> +		ev->queue_id = qid;
> +		ev->event_type = RTE_EVENT_TYPE_ETHDEV;

Thoughts on changing to RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER as a solution for the
problem described earlier.


> +		ev->sub_event_type = 0;
> +		ev->priority = priority;
> +		ev->mbuf = m;
> +
> +		buf_event_enqueue(rx_adapter, ev);
> +	}
> +}
> +
> +/*
> + * Polls receive queues added to the event adapter and enqueues received
> + * packets to the event device.
> + *
> + * The receive code enqueues initially to a temporary buffer, the
> + * temporary buffer is drained anytime it holds >= BATCH_SIZE packets
> + *
> + * If there isn't space available in the temporary buffer, packets from the
> + * Rx queue arent dequeued from the eth device, this backpressures the

s/arent/aren't


> + * eth device, in virtual device enviroments this backpressure is relayed to the

s/enviroments/environments

> + * hypervisor's switching layer where adjustments can be made to deal with
> + * it.
> + */
> +static inline uint32_t
> +eth_rx_poll(struct rte_event_eth_rx_adapter *rx_adapter)
> +static int
> +event_eth_rx_adapter_service_func(void *args)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter = args;
> +	struct rte_eth_event_enqueue_buffer *buf;
> +
> +	buf = &rx_adapter->event_enqueue_buffer;
> +	if (!rte_spinlock_trylock(&rx_adapter->rx_lock))
> +		return 0;
> +	if (eth_rx_poll(rx_adapter) == 0 && buf->count)
> +		flush_event_buffer(rx_adapter);
> +	rte_spinlock_unlock(&rx_adapter->rx_lock);
> +	return 0;
> +}
> +
> +static int
> +rte_event_eth_rx_adapter_init(void)
> +{
> +	const char *name = "rte_event_eth_rx_adapter_array";
> +	const struct rte_memzone *mz;
> +	unsigned int sz;
> +	unsigned int rss_key_off;
> +
> +	sz = sizeof(*rte_event_eth_rx_adapter) *
> +	    RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;

I think, you need to use size of struct rte_event_eth_rx_adapter here. if so,
we need **rte_event_eth_rx_adapter here. Right?

test code
struct abc {

        uint64_t a[64];
};

struct abc **k;

int main()
{
	printf("%d %d %d\n", sizeof(k), sizeof(*k), sizeof(**k));

	return 0;
}

$./a.out
8 8 512



> +	sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE);
> +	rss_key_off = sz;
> +	sz = RTE_ALIGN(sz + sizeof(default_rss_key), RTE_CACHE_LINE_SIZE);
> +
> +	mz = rte_memzone_lookup(name);
> +	if (!mz) {
> +		mz = rte_memzone_reserve_aligned(name, sz, rte_socket_id(), 0,
> +						 RTE_CACHE_LINE_SIZE);

How about passing the socket id from the rte_event_dev_socket_id()?
if eventdev is in node !=0  then it may not correct thing?

> +		if (mz) {
> +			rte_convert_rss_key((uint32_t *)default_rss_key,
> +			    (uint32_t *)(uintptr_t)(mz->addr_64 + rss_key_off),
> +			    RTE_DIM(default_rss_key));
> +		} else {
> +			RTE_EDEV_LOG_ERR("failed to reserve memzone err = %"
> +					PRId32, rte_errno);
> +			return -rte_errno;
> +		}
> +	}
> +
> +	rte_event_eth_rx_adapter = mz->addr;
> +	rss_key_be = (uint8_t *)(mz->addr_64 + rss_key_off);
> +	return 0;
> +}
> +
> +static int
> +default_conf_cb(uint8_t id, uint8_t dev_id,
> +		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
> +{
> +
> +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
> +					port_id);

return or add goto to exit from here to avoid calling rte_event_dev_start below

> +	} else {
> +		conf->event_port_id = port_id;
> +		conf->max_nb_rx = 128;
> +	}
> +
> +	if (started)
> +		rte_event_dev_start(dev_id);
> +	return ret;
> +}
> +
> +static int
> +init_service(struct rte_event_eth_rx_adapter *rx_adapter, uint8_t id)
> +{
> +		&rx_adapter_conf, rx_adapter->conf_arg);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("confguration callback failed err = %" PRId32,

s/configuration/configuration

> +			ret);
> +		goto err_done;
> +	}
> +	rx_adapter->event_port_id = rx_adapter_conf.event_port_id;
> +	rx_adapter->max_nb_rx = rx_adapter_conf.max_nb_rx;
> +	rx_adapter->service_inited = 1;
> +	return 0;
> +
> +err_done:
> +	rte_service_component_unregister(rx_adapter->service_id);
> +	return ret;
> +}
> +
> +static void
> +update_queue_info(struct rte_event_eth_rx_adapter *rx_adapter,
> +		struct eth_device_info *dev_info,
> +		int32_t rx_queue_id,
> +		uint8_t add)
> +{
> +	struct eth_rx_queue_info *queue_info;
> +	int enabled;
> +	uint16_t i;
> +
> +	if (!dev_info->rx_queue)
> +		return;
> +
> +	if (rx_queue_id == -1) {
> +		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++) {
> +			queue_info = &dev_info->rx_queue[i];
> +			enabled = queue_info->queue_enabled;
> +			if (add) {
> +				rx_adapter->nb_queues += !enabled;
> +				dev_info->nb_dev_queues += !enabled;
> +			} else {
> +				rx_adapter->nb_queues -= enabled;
> +				dev_info->nb_dev_queues -= enabled;
> +			}
> +			queue_info->queue_enabled = !!add;

See next comment.

> +		}
> +	} else {
> +		queue_info = &dev_info->rx_queue[rx_queue_id];
> +		enabled = queue_info->queue_enabled;
> +		if (add) {
> +			rx_adapter->nb_queues += !enabled;
> +			dev_info->nb_dev_queues += !enabled;
> +		} else {
> +			rx_adapter->nb_queues -= enabled;
> +			dev_info->nb_dev_queues -= enabled;
> +		}
> +		queue_info->queue_enabled = !!add;

Duplicate code. Worth to make it static inline to avoid duplicate code

> +	}
> +}
> +
> +int
> +rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> +				rx_adapter_conf_cb conf_cb, void *conf_arg)
> +{
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	int ret;
> +	int socket_id;
> +	uint8_t i;
> +	char mem_name[ETH_RX_ADAPTER_SERVICE_NAME_LEN];
> +
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
> +	if (!conf_cb)

Remove negative logic and change to conf_cb == NULL. Its DPDK coding
standard. There is a lot similar instance in this file.Please fix those

> +		return -EINVAL;
> +
> +	if (rte_event_eth_rx_adapter == NULL) {
> +		ret = rte_event_eth_rx_adapter_init();
> +		if (ret)
> +			return ret;
> +	}
> +
> +	rx_adapter = id_to_rx_adapter(id);
> +	if (rx_adapter != NULL) {
> +int
> +rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
> +		struct rte_event_port_conf *port_config)
> +{
> +	if (!port_config)

port_config == NULL

> +		return -EINVAL;
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +
> +	create_port_conf[id] = *port_config;
> +	return rte_event_eth_rx_adapter_create_ext(id, dev_id,
> +					default_conf_cb,
> +					&create_port_conf[id]);
> +}
> +
> +
> +int
> +rte_event_eth_rx_adapter_queue_add(uint8_t id,
> +		uint8_t eth_dev_id,
> +		int32_t rx_queue_id,
> +		const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
> +{
> +	int ret;
> +	uint32_t rx_adapter_cap;
> +	struct rte_event_eth_rx_adapter *rx_adapter;
> +	struct rte_eventdev *dev;
> +	struct eth_device_info *dev_info;
> +	int start_service = 0;

Duplicate store to zero.

> +
> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
> +
> +	rx_adapter = id_to_rx_adapter(id);
> +	if (!rx_adapter || !queue_conf)
> +		return -EINVAL;
> +
> +	dev = &rte_eventdevs[rx_adapter->eventdev_id];
> +	ret = (*dev->dev_ops->eth_rx_adapter_caps_get)(dev,
> +						&rte_eth_devices[eth_dev_id],
> +						&rx_adapter_cap);
> +	if (ret) {
> +		RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
> +			"eth port %" PRIu8, id, eth_dev_id);
> +		return ret;
> +	}
> +
> +	if (!(rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID) &&
> +		!(queue_conf->rx_queue_flags &
> +			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID)) {
> +		RTE_EDEV_LOG_ERR("Flow ID required for configuration,"
> +				" eth port: %" PRIu8 " adapter id: %" PRIu8,
> +				eth_dev_id, id);
> +		return -EINVAL;
> +	}
> +
> +	if ((rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ) &&
> +		(rx_queue_id != -1)) {
> +		RTE_EDEV_LOG_ERR("Rx queues can only be connected to single "
> +			"event queue id %u eth port %u", id, eth_dev_id);
> +		return -EINVAL;
> +	}
> +
> +	if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
> +			rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> +		RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
> +			 (uint16_t)rx_queue_id);
> +		return -EINVAL;
> +	}
> +
> +	start_service = 0;

See above comment.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-09-21 21:17 ` [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
@ 2017-09-22 12:12   ` Jerin Jacob
  2017-09-24 18:24     ` Rao, Nikhil
  2017-10-03 11:36   ` Pavan Nikhilesh Bhagavatula
  1 sibling, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-09-22 12:12 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Fri, 22 Sep 2017 02:47:14 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>  harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, erik.g.carrillo@intel.com,
>  abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> Subject: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
> X-Mailer: git-send-email 2.7.4
> 
> Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
>  test/test/test_event_eth_rx_adapter.c | 399 ++++++++++++++++++++++++++++++++++
>  test/test/Makefile                    |   1 +
>  2 files changed, 400 insertions(+)
>  create mode 100644 test/test/test_event_eth_rx_adapter.c
> 
> diff --git a/test/test/test_event_eth_rx_adapter.c b/test/test/test_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..5d448dc27
> --- /dev/null
> +++ b/test/test/test_event_eth_rx_adapter.c
> @@ -0,0 +1,399 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2017 Intel Corporation. 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.
> + */
> +#include <string.h>
> +#include <rte_common.h>
> +#include <rte_mempool.h>
> +#include <rte_mbuf.h>
> +#include <rte_ethdev.h>
> +#include <rte_eventdev.h>
> +
> +#include <rte_event_eth_rx_adapter.h>
> +
> +#include "test.h"
> +
> +/* i40e limits max to 64 */

This comment could be removed.

> +#define MAX_NUM_RX_QUEUE	64
> +#define NB_MBUFS		(8192 * num_ports * MAX_NUM_RX_QUEUE)
> +#define MBUF_CACHE_SIZE		512
> +#define MBUF_PRIV_SIZE		0
> +
> +struct event_eth_rx_adapter_test_params {
> +	struct rte_mempool *mp;
> +	uint16_t rx_rings, tx_rings;
> +	uint32_t caps;
> +};
> +
> +static struct event_eth_rx_adapter_test_params default_params;
> +
> +static int
> +testsuite_setup(void)
> +{
> +	int err;
> +	err = init_ports(rte_eth_dev_count());
> +	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);

I guess, We check rte_event_dev_count() >= 1 before proceeding.

> +
> +	struct rte_event_dev_config config = {
> +			.nb_event_queues = 1,
> +			.nb_event_ports = 1,
> +			.nb_events_limit  = 4096,
> +			.nb_event_queue_flows = 1024,
> +			.nb_event_port_dequeue_depth = 16,
> +			.nb_event_port_enqueue_depth = 16
> +	};
> +
> +	err = rte_event_dev_configure(0, &config);
> +	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
> +			err);
> +
> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
> +	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",
> +			err);
> +
> +	return err;
> +}
> +

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
@ 2017-09-23 11:35     ` Rao, Nikhil
  2017-10-03  9:09       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-09-23 11:35 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar

On 9/21/2017 9:13 PM, Pavan Nikhilesh Bhagavatula wrote:
> Hi Nikhil,
> 
> Few comments Inline
>
<snip>

>  + *  - 0: Success, statistics reset successfully.
> 
> Invalid description.

Thanks Pavan, for catching these, will fix.

> 
>> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
>> + * function, this function returns -ESRCH.
>> + */
>> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
>> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
>> new file mode 100644
>> index 000000000..d5b655dae
>> --- /dev/null
>> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
>> @@ -0,0 +1,1238 @@
> <snip>
>> +
>> +static int
>> +rx_adapter_ctrl(uint8_t id, int start)
>> +{
>> +	struct rte_event_eth_rx_adapter *rx_adapter;
>> +	struct rte_eventdev *dev;
>> +	struct eth_device_info *dev_info;
>> +	uint32_t i;
>> +	int use_service = 0;
>> +	int stop = !start;
>> +
>> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
>> +	rx_adapter = id_to_rx_adapter(id);
>> +	if (!rx_adapter)
>> +		return -EINVAL;
>> +
>> +	dev = &rte_eventdevs[rx_adapter->eventdev_id];
>> +
>> +	for (i = 0; i < rte_eth_dev_count(); i++) {
>> +		dev_info = &rx_adapter->eth_devices[i];
>> +		/* if start  check for num dev queues */
>> +		if (start && !dev_info->nb_dev_queues)
>> +			continue;
>> +		/* if stop check if dev has been started */
>> +		if (stop && !dev_info->dev_rx_started)
>> +			continue;:1
>> +		use_service |= !dev_info->internal_event_port;
>> +		dev_info->dev_rx_started = start;
>> +		if (!dev_info->internal_event_port)
>> +			continue;
>> +		start ? (*dev->dev_ops->eth_rx_adapter_start)(dev,
>> +						&rte_eth_devices[i]) :
>> +			(*dev->dev_ops->eth_rx_adapter_stop)(dev,
>> +						&rte_eth_devices[i]);
>> +	}
>> +
>> +	if (use_service)
> 
> Here setting the service run state is not sufficient we need to enable the
> service on a service core calling rte_service_start_with_defaults() should be
> sufficient.
> 

Yes it is necessary but insufficient.

IMO,
If the application is controlling core masks, the application flow at 
startup looks like:

rte_event_eth_rx_adapter_create(id,..)
...
rte_event_eth_rx_adapter_start(id)
if (!rte_event_eth_rx_adapter_service_id_get(id, &service_id)) {
	rte_service_lcore_add(rx_lcore_id);
         rte_service_map_lcore_set(service_id, rx_lcore_id, 1);
	rte_service_lcore_start(rx_lcore_id)
}

Since rte_service_start_with_defaults() is invoked before the adapter is 
created, how would it get assigned a core etc ?


Nikhil
>> +		rte_service_runstate_set(rx_adapter->service_id, start);
>> +
>> +	return 0;
>> +}
>> +
> <snip>
> 
> Regards,
> Pavan
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
  2017-09-21 15:46   ` Jerin Jacob
@ 2017-09-24 12:14     ` Rao, Nikhil
  2017-10-02  8:48       ` Jerin Jacob
  0 siblings, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-09-24 12:14 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 9/21/2017 9:16 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Fri, 22 Sep 2017 02:47:11 +0530
>> From: Nikhil Rao <nikhil.rao@intel.com>
>> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
>> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>>   harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>>   narender.vangati@intel.com, erik.g.carrillo@intel.com,
>>   abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
>> Subject: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for
>>   rte_event_eth_rx_adapter
>> X-Mailer: git-send-email 2.7.4
>>
>> The caps API allows application to get information
>> needed to configure the ethernet receive adapter for the eventdev and
>> ethdev pair.
>>
>> The PMD callbacks are used by the rte_event_eth_rx_xxx() APIs to
>> configure and control the ethernet receive adapter if packet transfers
>> from the ethdev to eventdev is implemented in hardware.
> 
> IMO, it better to split the cap PMD API and cap normative API into one patch and
> remaining PMD API to another patch.

OK.

> 
>>
>> For e.g., the ethdev, eventdev pairing maybe such that all of the
>> Eth Rx queues can only be connected to a single event queue, in
>> which case the application is required to pass in -1 as the queue id
>> when adding a receive queue to the adapter.
>>
>> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
>> ---
>>   lib/librte_eventdev/rte_eventdev.h           |  33 +++++
>>   lib/librte_eventdev/rte_eventdev_pmd.h       | 173 +++++++++++++++++++++++++++
>>   lib/librte_eventdev/rte_eventdev.c           |  24 ++++
>>   lib/librte_eventdev/rte_eventdev_version.map |   8 ++
>>   4 files changed, 238 insertions(+)
>>
>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>> index 128bc5221..a8bebac01 100644
>> --- a/lib/librte_eventdev/rte_eventdev.h
>> +++ b/lib/librte_eventdev/rte_eventdev.h
>> @@ -990,6 +990,39 @@ struct rte_event {
>>   	};
>>   };
>>   
>> +/* Ethdev Rx adapter capability bitmap flags */
>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT	0x1
>> +/**< Eventdev can send packets to ethdev using internal event port */
>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ	0x2
>> +/**< Ethdev Rx queues can be connected to single event queue */
> 
> I think, Its is more of limitation. Since we are expressing it as
> capability. How about changing it as RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
> (same as exiting !RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ and SW driver has this capability)
> i.e Ethdev Rx queues of single ethdev port can be connected to multiple
> event queue.
>
OK. I agree that the MULTI_EVENTQ is better suited to be expressed as a 
capability.

>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID		0x4
>> +/**< Ethdev Rx adapter can set flow ID for event queue, if this flag
>> + * is unset, the application needs to provide a flow id when adding
> 
> Looking at implementation, If I understand it correctly, it not application
> "needs" to provide instead, it is application can provide. If so, I think,
> making it as RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID or
> RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID makes more sense.
> 
If the FLOW_ID cap is not set, it is required for the application to 
provide it, else the application optionally can provide it but the 
feature of the application being able to provide (override) the flag 
should be a separate flag.

If it's only the override behavior that is required, we can rename the 
flag to OVERRIDE_FLOW_ID.

> something like,
> 
> #define RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID		0x4
> /**< If this flag is set then the application can override the default
>   * hash based flow id generated by the Rx adapter when the event
>   * enqueues to the event queue.
>   *
>   * @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
>   * @see rte_event_eth_rx_adapter_queue_conf::ev::flow_id
>   */
> 
>>   
>> +int
>> +rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
>> +				uint32_t *caps)
>> +{
>> +	struct rte_eventdev *dev;
>> +
>> +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_port_id, -EINVAL);
>> +
>> +	dev = &rte_eventdevs[dev_id];
>> +
>> +	if (caps == NULL)
>> +		return -EINVAL;
>> +	*caps = 0;
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_caps_get,
>> +				-ENOTSUP);
> 
> How about not returning the -ENOTSUP. Instead just call
> eth_rx_adapter_caps_get if it not NULL. By this way if a PMD does not
> have any cap, it does not need to implement this hook.

OK.

> 
>> +	return (*dev->dev_ops->eth_rx_adapter_caps_get)
>> +						(dev,
>> +						&rte_eth_devices[eth_port_id],
>> +						caps);
>> +}
>> +
>>   static inline int
>>   rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t nb_queues)
>>   {
>> diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
>> index 4c48e5f0a..996b361a5 100644
>> --- a/lib/librte_eventdev/rte_eventdev_version.map
>> +++ b/lib/librte_eventdev/rte_eventdev_version.map
>> @@ -51,3 +51,11 @@ DPDK_17.08 {
>>   	rte_event_ring_init;
>>   	rte_event_ring_lookup;
>>   } DPDK_17.05;
>> +
>> +
> Additional line feed.
> 
>> +DPDK_17.11 {
>> +	global:
>> +
>> +	rte_event_eth_rx_adapter_caps_get;
>> +
>> +} DPDK_17.08;
>> -- 
>> 2.14.1.145.gb3622a4
>>
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-22  9:10   ` Jerin Jacob
@ 2017-09-24 18:16     ` Rao, Nikhil
  2017-09-25  2:59       ` Rao, Nikhil
  2017-10-03 13:52       ` Jerin Jacob
  0 siblings, 2 replies; 30+ messages in thread
From: Rao, Nikhil @ 2017-09-24 18:16 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 9/22/2017 2:40 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Fri, 22 Sep 2017 02:47:13 +0530
>> From: Nikhil Rao <nikhil.rao@intel.com>
>> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
>> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>>   harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>>   narender.vangati@intel.com, erik.g.carrillo@intel.com,
>>   abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
>> Subject: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
>> X-Mailer: git-send-email 2.7.4
>>
>> Add common APIs for configuring packet transfer from ethernet Rx
>> queues to event devices across HW & SW packet transfer mechanisms.
>> A detailed description of the adapter is contained in the header's
>> comments.
>>
>> The adapter implementation uses eventdev PMDs to configure the packet
>> transfer if HW support is available and if not, it uses an EAL service
>> function that reads packets from ethernet Rx queues and injects these
>> as events into the event device.
>>
>> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
>> Signed-off-by: Gage Eads <gage.eads@intel.com>
>> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> 
> Overall it looks good. A few top-level comments to start with,
> 
> 1) Please split this patch to minimum two
> a) Specification header
> b) and the implementation
> 
> 2) Please add a new section in MAINTAINERS files and update the new files
> responsibility.
> 
> 3) The doxygen file is not hooked into the documentation build.
> Check the doc/api/doxy-api-index.md file. You can use "make doc-api-html"
> to verify the doxygen html output.
> 
> 4) Since the APIs looks good and if there is no other objection,
> Can you add a programmer guide for Rx adapter.
> If you are busy it is fine not have in next version. Post RC1 or RC2 is
> fine. What do you think?

OK, Thanks for the detailed review. Will add the programmer guide to RC1.

> 
> 
>> ---
>>   lib/librte_eventdev/rte_event_eth_rx_adapter.h |  384 ++++++++
>>   lib/librte_eventdev/rte_event_eth_rx_adapter.c | 1238 ++++++++++++++++++++++++
>>   lib/Makefile                                   |    2 +-
>>   lib/librte_eventdev/Makefile                   |    2 +
>>   lib/librte_eventdev/rte_eventdev_version.map   |   11 +-
>>   5 files changed, 1635 insertions(+), 2 deletions(-)
>>   create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.h
>>   create mode 100644 lib/librte_eventdev/rte_event_eth_rx_adapter.c
>>
>> +#ifndef _RTE_EVENT_ETH_RX_ADAPTER_
>> +#define _RTE_EVENT_ETH_RX_ADAPTER_
>> +
>> +/**
>> + * @file
>> + *
>> + * RTE Event Ethernet Rx Adapter
>> + *
>> + * An eventdev-based packet processing application enqueues/dequeues mbufs
>> + * to/from the event device. The application uses the adapter APIs to configure
>> + * the packet flow between the ethernet devices and event devices. Depending on
>> + * on the capabilties of the eventdev PMD, the adapter may use a EAL service
> 
> s/capabilties/capabilities
> 
>> + * core function for packet transfer or use internal PMD functions to configure
>> + * the packet transfer between the ethernet device and the event device.
>> + *
>> + * The ethernet Rx event adapter's functions are:
>> + *  - rte_event_eth_rx_adapter_create_ext()
>> + *  - rte_event_eth_rx_adapter_create()/free()
>> + *  - rte_event_eth_rx_adapter_queue_add()/del()
>> + *  - rte_event_eth_rx_adapter_start()/stop()
>> + *  - rte_event_eth_rx_adapter_stats_get()/reset()
>> + *
>> + * The applicaton creates an event to ethernet adapter using
> 
> How about,
> The application creates an ethernet device to event device adapter using
> 
>> + * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
>> + * functions.
>> + * The adapter needs to know which ethernet rx queues to poll for mbufs as well
>> + * as event device parameters such as the event queue identifier, event
>> + * priority and scheduling type that the adapter should use when constructing
>> + * events. The rte_event_eth_rx_adapter_queue_add() function is provided for
>> + * this purpose.
>> + * The servicing weight parameter in the rte_event_eth_rx_adapter_queue_conf
>> + * is applicable when the Rx adapter uses a service core function and is
>> + * intended to provide application control of the polling frequency of ethernet
>> + * device receive queues, for example, the application may want to poll higher
>> + * priority queues with a higher frequency but at the same time not starve
>> + * lower priority queues completely. If this parameter is zero and the receive
>> + * interrupt is enabled when configuring the device, the receive queue is
>> + * interrupt driven; else, the queue is assigned a servicing weight of one.
>> + *
>> + * If the adapter uses a rte_service function, then the application is also
>> + * required to assign a core to the service function and control the service
>> + * core using the rte_service APIs. The rte_event_eth_rx_adapter_service_id_get
>> + * function can be used to retrieve the service function ID of the adapter in
>> + * this case.
>> + *
>> + * Note: Interrupt driven receive queues are currentely unimplemented.
> 
> s/currentely/currently
> 
>> + */
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include <stdint.h>
> 
> A Linefeed is norm here.
> 
>> +#include <rte_service.h>
>> +
>> +#include "rte_eventdev.h"
>> +
>> +#define RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE 32
> 
> Considering the name space, How about to change to
> RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE? and fix the missing the doxygen comments
> 
>> +
>> +/* struct rte_event_eth_rx_adapter_queue_conf flags definitions */
>> +#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID	0x1
>> +/**< This flag indicates the flow identifier is valid
>> + * @see rte_event_eth_rx_adapter_queue_conf
> 
> @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
> @see rte_event_eth_rx_adapter_queue_conf::ev::flow_id
> 
> 

OK.
>> + */
>> +
>> +/**
>> + * Function type used for adapter configuration callback. The callback is
>> + * used to fill in members of the struct rte_event_eth_rx_adapter_conf, this
>> + * callback is invoked when creating a SW service for packet transfer from
>> + * ethdev queues to the event device. The SW service is created within the
>> + * rte_event_eth_rx_adapter_queue_add() function if packet required.
> 
> "if packet is required", does not seem to be correct usage.
> I guess, you mean, if packet required to transfer from ethdev queues to
> the event device or something like that?
> 

Yes, the text should have read "if SW based packet transfers from ethdev 
queues to the event device are required".

>> + *
>> + * @param id
>> + *  Adapter identifier.
>> + *
>> + * @param dev_id
>> + *  Event device identifier.
>> + *
>> + * @conf
>> + *  Structure that needs to be populated by this callback.
>> + *
>> + * @arg
>> + *  Argument to the callback. This is the same as the conf_arg passed to the
>> + *  rte_event_eth_rx_adapter_create_ext()
>> + */
>> +typedef int (*rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
>> +			struct rte_event_eth_rx_adapter_conf *conf,
>> +			void *arg);
> 
> Public symbols should start with rte_

OK.
> 
>> +
>> +/** Rx queue configuration structure */
>> +struct rte_event_eth_rx_adapter_queue_conf {
>> +	uint32_t rx_queue_flags;
>> +	 /**< Flags for handling received packets
>> +	  * @see RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID
>> +	  */
>> +	uint16_t servicing_weight;
>> +	/**< Relative polling frequency of ethernet receive queue, if this
>> +	 * is set to zero, the Rx queue is interrupt driven (unless rx queue
>> +	 * interrupts are not enabled for the ethernet device)
> 
> IMO, You can mentione it as hint and applicable only when using with
> SW based Rx adapter or something on similar lines.
> 

OK.
>> +	 */
>> +	struct rte_event ev;
>> +	/**<
>> +	 *  The values from the following event fields will be used when
>> +	 *  enqueuing mbuf events:
>> +	 *   - event_queue_id: Targeted event queue ID for received packets.
>> +	 *   - event_priority: Event priority of packets from this Rx queue in
>> +	 *                     the event queue relative to other events.
>> +	 *   - sched_type: Scheduling type for packets from this Rx queue.
>> +	 *   - flow_id: If the RTE_ETH_RX_EVENT_ADAPTER_QUEUE_FLOW_ID_VALID bit
>> +	 *		is set in rx_queue_flags, this flow_id is used for all
>> +	 *		packets received from this queue. Otherwise the flow ID
>> +	 *		is set to the RSS hash of the src and dst IPv4/6
>> +	 *		address.
>> +	 *
>> +	 * The event adapter sets ev.event_type to RTE_EVENT_TYPE_ETHDEV in the
>> +	 * enqueued event
> 
> When we worked on a prototype, we figured out that we need a separate event type
> for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
> The Reason is:
> - In the HW based Rx adapter case, the packet are coming directly to eventdev once it is configured.
> - So on a HW implementation of the event dequeue(), CPU needs to convert HW specific
> metadata to mbuf
> - The event dequeue() is used in two cases
> a) octeontx eventdev driver used with any external NIC
> b) octeontx eventdev driver used with integrated NIC(without service
> core to inject the packet)
> We need some identifier to understand case (a) and (b).So, in dequeue(), if the
> packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific metadata" to mbuf
> conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result in no mbuf
> conversion.
> 
> Application can check if it is an Ethernet type event by
> ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
> RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER
> 

As per my understanding, the case (a) uses an in built port
Is it possible for the eventdev PMD to do the conversion based off the 
eventdev port ?

> Thoughts?
> 
> 
>> +/**
>> + * Add receive queue to an event adapter. After a queue has been
>> + * added to the event adapter, the result of the application calling
>> + * rte_eth_rx_burst(eth_dev_id, rx_queue_id, ..) is undefined.
>> + *
>> + * @param id
>> + *  Adapter identifier.
>> + *
>> + * @param eth_dev_id
>> + *  Port identifier of Ethernet device.
>> + *
>> + * @param rx_queue_id
>> + *  Ethernet device receive queue index.
>> + *  If rx_queue_id is -1, then all Rx queues configured for
>> + *  the device are added. If the ethdev Rx queues can only be
>> + *  connected to a single event queue then rx_queue_id is
>> + *  required to be -1.
>> + *
>> + * @param conf
>> + *  Additonal configuration structure of type *rte_event_eth_rx_adapte_conf*
>> + *
>> + * @see
> 
> You can add @ see to denote the multi event queue enqueue capability
> 
>> + * @return
>> + *  - 0: Success, Receive queue added correctly.
>> + *  - <0: Error code on failure.
>> + */
>> +int rte_event_eth_rx_adapter_queue_add(uint8_t id,
>> +			uint8_t eth_dev_id,
>> +			int32_t rx_queue_id,
>> +			const struct rte_event_eth_rx_adapter_queue_conf *conf);
>> +
>> +/**
>> + * Delete receive queue from an event adapter.
>> + *
>> + * @param id
>> + *  Adapter identifier.
>> + *
>> + * @param eth_dev_id
>> + *  Port identifier of Ethernet device.
>> + *
>> + * @param rx_queue_id
>> + *  Ethernet device receive queue index.
>> + *  If rx_queue_id is -1, then all Rx queues configured for
>> + *  the device are deleted. If the ethdev Rx queues can only be
>> + *  connected to a single event queue then rx_queue_id is
>> + *  required to be -1.
> 
> You can add @ see to denote the multi event queue enqueue capability
> 
>> + *
>> + * @return
>> + *  - 0: Success, Receive queue deleted correctly.
>> + *  - <0: Error code on failure.
>> + */
>> +int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
>> +				       int32_t rx_queue_id);
>> +
>> +
>> +/**
>> + * Retrieve statistics for an adapter
>> + *
>> + * @param id
>> + *  Adapter identifier.
>> + *
>> + * @param stats
> 
> @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_eth_rx_adapter_stats_get(uint8_t id,
>> +				struct rte_event_eth_rx_adapter_stats *stats);
>> +
>> +
>> +/**
>> + * 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
> 

OK will fix.

>> + *
>> + * @return
>> + *  - 0: Success, statistics reset successfully.
>> + *  - <0: Error code on failure, if the adapter doesn't use a rte_service
>> + * function, this function returns -ESRCH.
>> + */
>> +int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
>> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
>> new file mode 100644
>> index 000000000..d5b655dae
>> --- /dev/null
>> +
>> +struct rte_event_eth_rx_adapter {
>> +	/* event device identifier */
> 
> Every elements you can start with capital letter.
> 
>> +	uint8_t eventdev_id;
>> +	/* per ethernet device structure */
>> +	struct eth_device_info *eth_devices;
>> +	/* malloc name */
>> +	char mem_name[ETH_RX_ADAPTER_MEM_NAME_LEN];
>> +	/* socket identifier cached from eventdev */
>> +	int socket_id;
>> +
>> +	/* elements below are used by SW service */
>> +
>> +	/* event port identifier */
>> +	uint8_t event_port_id;
>> +	/* per adapter EAL service */
>> +	uint32_t service_id;
>> +	/* lock to serialize config updates with service function */
>> +	rte_spinlock_t rx_lock;
>> +	/* max mbufs processed in any service function invocation */
>> +	uint32_t max_nb_rx;
>> +	/* Receive queues that need to be polled */
>> +	struct eth_rx_poll_entry *eth_rx_poll;
>> +	/* size of the eth_rx_poll array */
>> +	uint16_t num_rx_polled;
>> +	/* Weighted round robin schedule */
>> +	uint32_t *wrr_sched;
>> +	/* wrr_sched[] size */
>> +	uint32_t wrr_len;
>> +	/* Next entry in wrr[] to begin polling */
>> +	uint32_t wrr_pos;
>> +	/* Event burst buffer */
>> +	struct rte_eth_event_enqueue_buffer event_enqueue_buffer;
>> +	/* per adapter stats */
>> +	struct rte_event_eth_rx_adapter_stats stats;
>> +	/* Block count, counts upto BLOCK_CNT_THRESHOLD */
>> +	uint16_t enq_block_count;
>> +	/* Block start ts */
>> +	uint64_t rx_enq_block_start_ts;
>> +	/* Configuration callback for rte_service configuration */
>> +	rx_adapter_conf_cb conf_cb;
>> +	/* Configuration callback argument */
>> +	void *conf_arg;
>> +	/* Service initialization state */
>> +	uint8_t service_inited;
>> +	/* Total count of Rx queues in adapter */
>> +	uint32_t nb_queues;
>> +} __rte_cache_aligned;
>> +
>> +/* Per eth device */
>> +struct eth_device_info {
>> +	struct rte_eth_dev *dev;
>> +	struct eth_rx_queue_info *rx_queue;
>> +	/* Set if ethdev->eventdev packet transfer uses a
>> +	 * hardware mechanism
>> +	 */
>> +	uint8_t internal_event_port;
>> +	/* set if the adapter is processing rx queues for
> 
> s/set/Set
> 
>> +	 * this eth device and packet processing has been
>> +	 * started, allows for the code to know if the PMD
>> +	 * rx_adapter_stop callback needs to be invoked
>> +	 */
>> +	uint8_t dev_rx_started;
>> +	/* if nb_dev_queues > 0, the start callback will
>> +	 * be invoked if not already invoked
>> +	 */
>> +	uint16_t nb_dev_queues;
>> +};
>> +
>> +static struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter;
> 
> Avoid using rte for Internal object(**rte_event_eth_rx_adapter)_
>OK.

>> +static struct rte_event_port_conf
>> +		create_port_conf[RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE];
> 
> IMO, this memory can be stored in adapter memory to avoid global variable.
> 
Yes, if create() and queue_add() are called from different processes, it 
wouldn't work.

>> +
>> +static uint8_t default_rss_key[] = {
>> +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
>> +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
>> +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
>> +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
>> +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
>> +};
> 
> Looks like the scope of this array is only for rte_event_eth_rx_adapter_init,
> if so please move it to stack.
> 
OK.

> 
>> +static uint8_t *rss_key_be;
> 
> Can we remove this global variable add it in in adapter memory?
> 

There is currently struct rte_event_eth_rx_adapter 
**rte_event_eth_rx_adapter that is an array of pointers to the adapters. 
rss_key_be points to memory after this array.

are you thinking of something like:

struct {
struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter
uint8_t *rss_key_be;
} global;


>> +}
>> +
>> +static inline void
>> +fill_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter,
>> +	uint8_t dev_id,
>> +	uint16_t rx_queue_id,
>> +	struct rte_mbuf **mbufs,
>> +	uint16_t num)
>> +{
>> +	uint32_t i;
>> +	struct eth_device_info *eth_device_info =
>> +					&rx_adapter->eth_devices[dev_id];
>> +	struct eth_rx_queue_info *eth_rx_queue_info =
>> +					&eth_device_info->rx_queue[rx_queue_id];
>> +
>> +	int32_t qid = eth_rx_queue_info->event_queue_id;
>> +	uint8_t sched_type = eth_rx_queue_info->sched_type;
>> +	uint8_t priority = eth_rx_queue_info->priority;
>> +	uint32_t flow_id;
>> +	struct rte_event events[BATCH_SIZE];
>> +	struct rte_mbuf *m = mbufs[0];
>> +	uint32_t rss_mask;
>> +	uint32_t rss;
>> +	int do_rss;
>> +
>> +	/* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
>> +	rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
>> +	do_rss = !rss_mask && !eth_rx_queue_info->flow_id_mask;
>> +
>> +	for (i = 0; i < num; i++) {
>> +		m = mbufs[i];
>> +		struct rte_event *ev = &events[i];
>> +
>> +		rss = do_rss ? do_softrss(m) : m->hash.rss;
>> +		flow_id =
>> +		    eth_rx_queue_info->flow_id &
>> +				eth_rx_queue_info->flow_id_mask;
>> +		flow_id |= rss & ~eth_rx_queue_info->flow_id_mask;
>> +
>> +		ev->flow_id = flow_id;
>> +		ev->op = RTE_EVENT_OP_NEW;
>> +		ev->sched_type = sched_type;
>> +		ev->queue_id = qid;
>> +		ev->event_type = RTE_EVENT_TYPE_ETHDEV;
> 
> Thoughts on changing to RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER as a solution for the
> problem described earlier.
> 
> 
>> +		ev->sub_event_type = 0;
>> +		ev->priority = priority;
>> +		ev->mbuf = m;
>> +
>> +		buf_event_enqueue(rx_adapter, ev);
>> +	}
>> +}
>> +
>> +/*
>> + * Polls receive queues added to the event adapter and enqueues received
>> + * packets to the event device.
>> + *
>> + * The receive code enqueues initially to a temporary buffer, the
>> + * temporary buffer is drained anytime it holds >= BATCH_SIZE packets
>> + *
>> + * If there isn't space available in the temporary buffer, packets from the
>> + * Rx queue arent dequeued from the eth device, this backpressures the
> 
> s/arent/aren't
> 
> 
>> + * eth device, in virtual device enviroments this backpressure is relayed to the
> 
> s/enviroments/environments
> 
>> + * hypervisor's switching layer where adjustments can be made to deal with
>> + * it.
>> + */
>> +static inline uint32_t
>> +eth_rx_poll(struct rte_event_eth_rx_adapter *rx_adapter)
>> +static int
>> +event_eth_rx_adapter_service_func(void *args)
>> +{
>> +	struct rte_event_eth_rx_adapter *rx_adapter = args;
>> +	struct rte_eth_event_enqueue_buffer *buf;
>> +
>> +	buf = &rx_adapter->event_enqueue_buffer;
>> +	if (!rte_spinlock_trylock(&rx_adapter->rx_lock))
>> +		return 0;
>> +	if (eth_rx_poll(rx_adapter) == 0 && buf->count)
>> +		flush_event_buffer(rx_adapter);
>> +	rte_spinlock_unlock(&rx_adapter->rx_lock);
>> +	return 0;
>> +}
>> +
>> +static int
>> +rte_event_eth_rx_adapter_init(void)
>> +{
>> +	const char *name = "rte_event_eth_rx_adapter_array";
>> +	const struct rte_memzone *mz;
>> +	unsigned int sz;
>> +	unsigned int rss_key_off;
>> +
>> +	sz = sizeof(*rte_event_eth_rx_adapter) *
>> +	    RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
> 
> I think, you need to use size of struct rte_event_eth_rx_adapter here. if so,
> we need **rte_event_eth_rx_adapter here. Right?
> 
> test code
> struct abc {
> 
>          uint64_t a[64];
> };
> 
> struct abc **k;
> 
> int main()
> {
> 	printf("%d %d %d\n", sizeof(k), sizeof(*k), sizeof(**k));
> 
> 	return 0;
> }
> 
> $./a.out
> 8 8 512
> 

The struct rte_event_eth_rx_adapter gets allocated in 
rte_event_eth_rx_adapter_create_ext()

> 
>> +	sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE);
>> +	rss_key_off = sz;
>> +	sz = RTE_ALIGN(sz + sizeof(default_rss_key), RTE_CACHE_LINE_SIZE);
>> +
>> +	mz = rte_memzone_lookup(name);
>> +	if (!mz) {
>> +		mz = rte_memzone_reserve_aligned(name, sz, rte_socket_id(), 0,
>> +						 RTE_CACHE_LINE_SIZE);
> 
> How about passing the socket id from the rte_event_dev_socket_id()?
> if eventdev is in node !=0  then it may not correct thing?

rte_event_eth_rx_adapter[] is a global across all adapters, event dev ID 
is unknown at this point. The struct rte_event_eth_rx_adapter that 
rte_event_eth_rx_adapter[] gets allocated using rte_event_dev_socket_id()

> 
>> +		if (mz) {
>> +			rte_convert_rss_key((uint32_t *)default_rss_key,
>> +			    (uint32_t *)(uintptr_t)(mz->addr_64 + rss_key_off),
>> +			    RTE_DIM(default_rss_key));
>> +		} else {
>> +			RTE_EDEV_LOG_ERR("failed to reserve memzone err = %"
>> +					PRId32, rte_errno);
>> +			return -rte_errno;
>> +		}
>> +	}
>> +
>> +	rte_event_eth_rx_adapter = mz->addr;
>> +	rss_key_be = (uint8_t *)(mz->addr_64 + rss_key_off);
>> +	return 0;
>> +}
>> +
>> +static int
>> +default_conf_cb(uint8_t id, uint8_t dev_id,
>> +		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
>> +{
>> +
>> +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
>> +	if (ret) {
>> +		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
>> +					port_id);
> 
> return or add goto to exit from here to avoid calling rte_event_dev_start below
> 
Could do the return but I wanted to leave the device in the same state 
as it was at entry into this function. Thoughts ?

>> +	} else {
>> +		conf->event_port_id = port_id;
>> +		conf->max_nb_rx = 128;
>> +	}
>> +
>> +	if (started)
>> +		rte_event_dev_start(dev_id);
>> +	return ret;
>> +}
>> +
>> +static int
>> +init_service(struct rte_event_eth_rx_adapter *rx_adapter, uint8_t id)
>> +{
>> +		&rx_adapter_conf, rx_adapter->conf_arg);
>> +	if (ret) {
>> +		RTE_EDEV_LOG_ERR("confguration callback failed err = %" PRId32,
> 
> s/configuration/configuration
> 
>> +			ret);
>> +		goto err_done;
>> +	}
>> +	rx_adapter->event_port_id = rx_adapter_conf.event_port_id;
>> +	rx_adapter->max_nb_rx = rx_adapter_conf.max_nb_rx;
>> +	rx_adapter->service_inited = 1;
>> +	return 0;
>> +
>> +err_done:
>> +	rte_service_component_unregister(rx_adapter->service_id);
>> +	return ret;
>> +}
>> +
>> +static void
>> +update_queue_info(struct rte_event_eth_rx_adapter *rx_adapter,
>> +		struct eth_device_info *dev_info,
>> +		int32_t rx_queue_id,
>> +		uint8_t add)
>> +{
>> +	struct eth_rx_queue_info *queue_info;
>> +	int enabled;
>> +	uint16_t i;
>> +
>> +	if (!dev_info->rx_queue)
>> +		return;
>> +
>> +	if (rx_queue_id == -1) {
>> +		for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++) {
>> +			queue_info = &dev_info->rx_queue[i];
>> +			enabled = queue_info->queue_enabled;
>> +			if (add) {
>> +				rx_adapter->nb_queues += !enabled;
>> +				dev_info->nb_dev_queues += !enabled;
>> +			} else {
>> +				rx_adapter->nb_queues -= enabled;
>> +				dev_info->nb_dev_queues -= enabled;
>> +			}
>> +			queue_info->queue_enabled = !!add;
> 
> See next comment.
> 
>> +		}
>> +	} else {
>> +		queue_info = &dev_info->rx_queue[rx_queue_id];
>> +		enabled = queue_info->queue_enabled;
>> +		if (add) {
>> +			rx_adapter->nb_queues += !enabled;
>> +			dev_info->nb_dev_queues += !enabled;
>> +		} else {
>> +			rx_adapter->nb_queues -= enabled;
>> +			dev_info->nb_dev_queues -= enabled;
>> +		}
>> +		queue_info->queue_enabled = !!add;
> 
> Duplicate code. Worth to make it static inline to avoid duplicate code
> 

OK.
>> +	}
>> +}
>> +
>> +int
>> +rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
>> +				rx_adapter_conf_cb conf_cb, void *conf_arg)
>> +{
>> +	struct rte_event_eth_rx_adapter *rx_adapter;
>> +	int ret;
>> +	int socket_id;
>> +	uint8_t i;
>> +	char mem_name[ETH_RX_ADAPTER_SERVICE_NAME_LEN];
>> +
>> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
>> +	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
>> +	if (!conf_cb)
> 
> Remove negative logic and change to conf_cb == NULL. Its DPDK coding
> standard. There is a lot similar instance in this file.Please fix those
>

OK.
>> +		return -EINVAL;
>> +
>> +	if (rte_event_eth_rx_adapter == NULL) {
>> +		ret = rte_event_eth_rx_adapter_init();
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	rx_adapter = id_to_rx_adapter(id);
>> +	if (rx_adapter != NULL) {
>> +int
>> +rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
>> +		struct rte_event_port_conf *port_config)
>> +{
>> +	if (!port_config)
> 
> port_config == NULL
> 
>> +		return -EINVAL;
>> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
>> +
>> +	create_port_conf[id] = *port_config;
>> +	return rte_event_eth_rx_adapter_create_ext(id, dev_id,
>> +					default_conf_cb,
>> +					&create_port_conf[id]);
>> +}
>> +
>> +
>> +int
>> +rte_event_eth_rx_adapter_queue_add(uint8_t id,
>> +		uint8_t eth_dev_id,
>> +		int32_t rx_queue_id,
>> +		const struct rte_event_eth_rx_adapter_queue_conf *queue_conf)
>> +{
>> +	int ret;
>> +	uint32_t rx_adapter_cap;
>> +	struct rte_event_eth_rx_adapter *rx_adapter;
>> +	struct rte_eventdev *dev;
>> +	struct eth_device_info *dev_info;
>> +	int start_service = 0;
> 
> Duplicate store to zero.
> 
>> +
>> +	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
>> +
>> +	rx_adapter = id_to_rx_adapter(id);
>> +	if (!rx_adapter || !queue_conf)
>> +		return -EINVAL;
>> +
>> +	dev = &rte_eventdevs[rx_adapter->eventdev_id];
>> +	ret = (*dev->dev_ops->eth_rx_adapter_caps_get)(dev,
>> +						&rte_eth_devices[eth_dev_id],
>> +						&rx_adapter_cap);
>> +	if (ret) {
>> +		RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
>> +			"eth port %" PRIu8, id, eth_dev_id);
>> +		return ret;
>> +	}
>> +
>> +	if (!(rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID) &&
>> +		!(queue_conf->rx_queue_flags &
>> +			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID)) {
>> +		RTE_EDEV_LOG_ERR("Flow ID required for configuration,"
>> +				" eth port: %" PRIu8 " adapter id: %" PRIu8,
>> +				eth_dev_id, id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if ((rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ) &&
>> +		(rx_queue_id != -1)) {
>> +		RTE_EDEV_LOG_ERR("Rx queues can only be connected to single "
>> +			"event queue id %u eth port %u", id, eth_dev_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
>> +			rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
>> +		RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
>> +			 (uint16_t)rx_queue_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	start_service = 0;
> 
> See above comment.
> 
OK will fix.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-09-22 12:12   ` Jerin Jacob
@ 2017-09-24 18:24     ` Rao, Nikhil
  2017-10-02 10:31       ` Jerin Jacob
  0 siblings, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-09-24 18:24 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 9/22/2017 5:42 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Fri, 22 Sep 2017 02:47:14 +0530
>> From: Nikhil Rao <nikhil.rao@intel.com>
>> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
>> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>>   harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>>   narender.vangati@intel.com, erik.g.carrillo@intel.com,
>>   abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
>> Subject: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
>> X-Mailer: git-send-email 2.7.4
>>
>> Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
>
<snip>

>> +#include <string.h>
>> +#include <rte_common.h>
>> +#include <rte_mempool.h>
>> +#include <rte_mbuf.h>
>> +#include <rte_ethdev.h>
>> +#include <rte_eventdev.h>
>> +
>> +#include <rte_event_eth_rx_adapter.h>
>> +
>> +#include "test.h"
>> +
>> +/* i40e limits max to 64 */
> 
> This comment could be removed.
> 
OK, I am documenting why the code doesn't just use 
dev_info.max_rx_queues, won't the comment be useful to retain ?

>> +#define MAX_NUM_RX_QUEUE	64
>> +#define NB_MBUFS		(8192 * num_ports * MAX_NUM_RX_QUEUE)
>> +#define MBUF_CACHE_SIZE		512
>> +#define MBUF_PRIV_SIZE		0
>> +
>> +struct event_eth_rx_adapter_test_params {
>> +	struct rte_mempool *mp;
>> +	uint16_t rx_rings, tx_rings;
>> +	uint32_t caps;
>> +};
>> +
>> +static struct event_eth_rx_adapter_test_params default_params;
>> +
>> +static int
>> +testsuite_setup(void)
>> +{
>> +	int err;
>> +	err = init_ports(rte_eth_dev_count());
>> +	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
> 
> I guess, We check rte_event_dev_count() >= 1 before proceeding.

Yes, I can add the same logic as in test_evendev.
> 
>> +
>> +	struct rte_event_dev_config config = {
>> +			.nb_event_queues = 1,
>> +			.nb_event_ports = 1,
>> +			.nb_events_limit  = 4096,
>> +			.nb_event_queue_flows = 1024,
>> +			.nb_event_port_dequeue_depth = 16,
>> +			.nb_event_port_enqueue_depth = 16
>> +	};
>> +
>> +	err = rte_event_dev_configure(0, &config);
>> +	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
>> +			err);
>> +
>> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
>> +	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",
>> +			err);
>> +
>> +	return err;
>> +}
>> +
> 
Thanks for the review,
Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-24 18:16     ` Rao, Nikhil
@ 2017-09-25  2:59       ` Rao, Nikhil
  2017-10-02 10:28         ` Rao, Nikhil
  2017-10-03 13:52       ` Jerin Jacob
  1 sibling, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-09-25  2:59 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 9/24/2017 11:46 PM, Rao, Nikhil wrote:
> On 9/22/2017 2:40 PM, Jerin Jacob wrote:
>
>> When we worked on a prototype, we figured out that we need a separate 
>> event type
>> for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
>> The Reason is:
>> - In the HW based Rx adapter case, the packet are coming directly to 
>> eventdev once it is configured.
>> - So on a HW implementation of the event dequeue(), CPU needs to 
>> convert HW specific
>> metadata to mbuf
>> - The event dequeue() is used in two cases
>> a) octeontx eventdev driver used with any external NIC
>> b) octeontx eventdev driver used with integrated NIC(without service
>> core to inject the packet)
>> We need some identifier to understand case (a) and (b).So, in 
>> dequeue(), if the
>> packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific 
>> metadata" to mbuf
>> conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result in no mbuf
>> conversion.
>>
>> Application can check if it is an Ethernet type event by
>> ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
>> RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER
>>
> 
> As per my understanding, the case (a) uses an in built port
> Is it possible for the eventdev PMD to do the conversion based off the 
> eventdev port ?
> 

I realized the dequeue wouldn't have knowledge of the port the event was 
injected from, the application shouldn't have to see the difference 
between case (a) & (b).

Would it be possible to use the impl_opaque field within struct rte_event ?

Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
  2017-09-24 12:14     ` Rao, Nikhil
@ 2017-10-02  8:48       ` Jerin Jacob
  0 siblings, 0 replies; 30+ messages in thread
From: Jerin Jacob @ 2017-10-02  8:48 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Sun, 24 Sep 2017 17:44:06 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>  thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>  nipun.gupta@nxp.com, narender.vangati@intel.com,
>  erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for
>  rte_event_eth_rx_adapter
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.3.0
> 
> On 9/21/2017 9:16 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Fri, 22 Sep 2017 02:47:11 +0530
> > > From: Nikhil Rao <nikhil.rao@intel.com>
> > > To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> > > CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
> > >   harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
> > >   narender.vangati@intel.com, erik.g.carrillo@intel.com,
> > >   abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> > > Subject: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for
> > >   rte_event_eth_rx_adapter
> > > X-Mailer: git-send-email 2.7.4
> > > +/* Ethdev Rx adapter capability bitmap flags */
> > > +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT	0x1
> > > +/**< Eventdev can send packets to ethdev using internal event port */
> > > +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ	0x2
> > > +/**< Ethdev Rx queues can be connected to single event queue */
> > 
> > I think, Its is more of limitation. Since we are expressing it as
> > capability. How about changing it as RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
> > (same as exiting !RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ and SW driver has this capability)
> > i.e Ethdev Rx queues of single ethdev port can be connected to multiple
> > event queue.
> > 
> OK. I agree that the MULTI_EVENTQ is better suited to be expressed as a
> capability.
> 
> > > +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID		0x4
> > > +/**< Ethdev Rx adapter can set flow ID for event queue, if this flag
> > > + * is unset, the application needs to provide a flow id when adding
> > 
> > Looking at implementation, If I understand it correctly, it not application
> > "needs" to provide instead, it is application can provide. If so, I think,
> > making it as RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID or
> > RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID makes more sense.
> > 
> If the FLOW_ID cap is not set, it is required for the application to provide
> it, else the application optionally can provide it but the feature of the
> application being able to provide (override) the flag should be a separate
> flag.
> 
> If it's only the override behavior that is required, we can rename the flag
> to OVERRIDE_FLOW_ID.

Yes. OVERRIDE_FLOW_ID behavior makes sense to me. Please update the
doxygen comments as well.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-22  6:08   ` santosh
@ 2017-10-02 10:20     ` Rao, Nikhil
  0 siblings, 0 replies; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-02 10:20 UTC (permalink / raw)
  To: santosh, jerin.jacob, bruce.richardson
  Cc: gage.eads, dev, thomas, harry.van.haaren, hemant.agrawal,
	nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar

On 9/22/2017 11:38 AM, santosh wrote:
> 
>
>
> 
> In general api comment: Fix missing param definition like *service_id* above
> and pl. remove other unnecessary params description from api above.

OK.
> 
>> +static inline int
>> +valid_id(uint8_t id)
>> +{
>> +	return id < RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
>> +}
>> +
>> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, retval) do { \
>> +	if (!valid_id(id)) { \
>> +		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
>> +		return retval; \
>> +	} \
>> +} while (0)
>> +
> 
> Worth, moving this macro to rte_eventdev_pmd.h
> Or How about reusing existing one ie.. RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET? >
I didn't see a reason for this macro to be used elsewhere apart from 
rte_event_eth_rx_adapter.c.
Also, the check is different from the one in 
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET.

>> +
>> +static inline void
>> +mtoip(struct rte_mbuf *m, struct ipv4_hdr **ipv4_hdr,
>> +	struct ipv6_hdr **ipv6_hdr)
>> +{
> 
> mtoip(), imo is more of global api, likely other modules may use in future..
> perhaps move to rte_io.h Or more correct place.. thought?
> 

Good suggestion, Will post a separate patch for this in the future.

Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-25  2:59       ` Rao, Nikhil
@ 2017-10-02 10:28         ` Rao, Nikhil
  2017-10-02 10:39           ` Jerin Jacob
  0 siblings, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-02 10:28 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 9/25/2017 8:29 AM, Rao, Nikhil wrote:
> On 9/24/2017 11:46 PM, Rao, Nikhil wrote:
>> On 9/22/2017 2:40 PM, Jerin Jacob wrote:
>>
>>> When we worked on a prototype, we figured out that we need a separate 
>>> event type
>>> for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
>>> The Reason is:
>>> - In the HW based Rx adapter case, the packet are coming directly to 
>>> eventdev once it is configured.
>>> - So on a HW implementation of the event dequeue(), CPU needs to 
>>> convert HW specific
>>> metadata to mbuf
>>> - The event dequeue() is used in two cases
>>> a) octeontx eventdev driver used with any external NIC
>>> b) octeontx eventdev driver used with integrated NIC(without service
>>> core to inject the packet)
>>> We need some identifier to understand case (a) and (b).So, in 
>>> dequeue(), if the
>>> packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific 
>>> metadata" to mbuf
>>> conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result in no 
>>> mbuf
>>> conversion.
>>>
>>> Application can check if it is an Ethernet type event by
>>> ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
>>> RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER
>>>
>>
>> As per my understanding, the case (a) uses an in built port
>> Is it possible for the eventdev PMD to do the conversion based off the 
>> eventdev port ?
>>
> 
> I realized the dequeue wouldn't have knowledge of the port the event was 
> injected from, the application shouldn't have to see the difference 
> between case (a) & (b).
> 
> Would it be possible to use the impl_opaque field within struct rte_event ?
> 
> Nikhil

Hi Jerin,

Any further thoughts on this ?

Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-09-24 18:24     ` Rao, Nikhil
@ 2017-10-02 10:31       ` Jerin Jacob
  2017-10-04 11:28         ` Rao, Nikhil
  0 siblings, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-10-02 10:31 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Sun, 24 Sep 2017 23:54:38 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>  thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>  nipun.gupta@nxp.com, narender.vangati@intel.com,
>  erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter
>  APIs
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.3.0
> 
> On 9/22/2017 5:42 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Fri, 22 Sep 2017 02:47:14 +0530
> > > From: Nikhil Rao <nikhil.rao@intel.com>
> > > To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
> > > CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
> > >   harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
> > >   narender.vangati@intel.com, erik.g.carrillo@intel.com,
> > >   abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
> > > Subject: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
> > > X-Mailer: git-send-email 2.7.4
> > > 
> > > Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
> > 
> <snip>
> 
> > > +#include <string.h>
> > > +#include <rte_common.h>
> > > +#include <rte_mempool.h>
> > > +#include <rte_mbuf.h>
> > > +#include <rte_ethdev.h>
> > > +#include <rte_eventdev.h>
> > > +
> > > +#include <rte_event_eth_rx_adapter.h>
> > > +
> > > +#include "test.h"
> > > +
> > > +/* i40e limits max to 64 */
> > 
> > This comment could be removed.
> > 
> OK, I am documenting why the code doesn't just use dev_info.max_rx_queues,
> won't the comment be useful to retain ?

OK. If dev_info.max_rx_queues for i40e is not 64 as expected then we
could the fix the i40e driver as well.


> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-10-02 10:28         ` Rao, Nikhil
@ 2017-10-02 10:39           ` Jerin Jacob
  2017-10-05  8:54             ` Rao, Nikhil
  0 siblings, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-10-02 10:39 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Mon, 2 Oct 2017 15:58:56 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>  thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>  nipun.gupta@nxp.com, narender.vangati@intel.com,
>  erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.3.0
> 
> On 9/25/2017 8:29 AM, Rao, Nikhil wrote:
> > On 9/24/2017 11:46 PM, Rao, Nikhil wrote:
> > > On 9/22/2017 2:40 PM, Jerin Jacob wrote:
> > > 
> > > > When we worked on a prototype, we figured out that we need a
> > > > separate event type
> > > > for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
> > > > The Reason is:
> > > > - In the HW based Rx adapter case, the packet are coming
> > > > directly to eventdev once it is configured.
> > > > - So on a HW implementation of the event dequeue(), CPU needs to
> > > > convert HW specific
> > > > metadata to mbuf
> > > > - The event dequeue() is used in two cases
> > > > a) octeontx eventdev driver used with any external NIC
> > > > b) octeontx eventdev driver used with integrated NIC(without service
> > > > core to inject the packet)
> > > > We need some identifier to understand case (a) and (b).So, in
> > > > dequeue(), if the
> > > > packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific
> > > > metadata" to mbuf
> > > > conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result
> > > > in no mbuf
> > > > conversion.
> > > > 
> > > > Application can check if it is an Ethernet type event by
> > > > ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
> > > > RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER
> > > > 
> > > 
> > > As per my understanding, the case (a) uses an in built port
> > > Is it possible for the eventdev PMD to do the conversion based off
> > > the eventdev port ?
> > > 
> > 
> > I realized the dequeue wouldn't have knowledge of the port the event was
> > injected from, the application shouldn't have to see the difference
> > between case (a) & (b).
> > 
> > Would it be possible to use the impl_opaque field within struct rte_event ?
> > 
> > Nikhil
> 
> Hi Jerin,
> 
> Any further thoughts on this ?

impl_opaque field could be one option. But I think, NXP driver is using
it for internal operation. So overriding it from Rx adapter will cause
issue. How about adding new event type? So it gets a new name space so no
collision.

➜ [master][dpdk-next-eventdev] $ git diff
diff --git a/lib/librte_eventdev/rte_eventdev.h
b/lib/librte_eventdev/rte_eventdev.h
index ec7aabd9a..b33423c7e 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -878,6 +878,8 @@ rte_event_dev_close(uint8_t dev_id);
 /**< The event generated from cpu for pipelining.
  * Application may use *sub_event_type* to further classify the event
  */
+#define RTE_EVENT_TYPE_ETHDEV_ADAPTER   0x4
+/**< The event generated from ethdev Rx adapter */
 #define RTE_EVENT_TYPE_MAX              0x10
 /**< Maximum number of event types */

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-23 11:35     ` Rao, Nikhil
@ 2017-10-03  9:09       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 30+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2017-10-03  9:09 UTC (permalink / raw)
  To: Rao, Nikhil; +Cc: dev

On Sat, Sep 23, 2017 at 05:05:21PM +0530, Rao, Nikhil wrote:
Hi Nikhil,
> On 9/21/2017 9:13 PM, Pavan Nikhilesh Bhagavatula wrote:
> >Hi Nikhil,
> >
> >Few comments Inline
> >
> <snip>
>
> > + *  - 0: Success, statistics reset successfully.
> >
> >Invalid description.
>
> Thanks Pavan, for catching these, will fix.
>
> >
> >>+ *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> >>+ * function, this function returns -ESRCH.
> >>+ */
> >>+int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> >>+
> >>+#ifdef __cplusplus
> >>+}
> >>+#endif
> >>+#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> >>diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> >>new file mode 100644
> >>index 000000000..d5b655dae
> >>--- /dev/null
> >>+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> >>@@ -0,0 +1,1238 @@
> ><snip>
> >>+
> >>+static int
> >>+rx_adapter_ctrl(uint8_t id, int start)
> >>+{
> >>+	struct rte_event_eth_rx_adapter *rx_adapter;
> >>+	struct rte_eventdev *dev;
> >>+	struct eth_device_info *dev_info;
> >>+	uint32_t i;
> >>+	int use_service = 0;
> >>+	int stop = !start;
> >>+
> >>+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> >>+	rx_adapter = id_to_rx_adapter(id);
> >>+	if (!rx_adapter)
> >>+		return -EINVAL;
> >>+
> >>+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
> >>+
> >>+	for (i = 0; i < rte_eth_dev_count(); i++) {
> >>+		dev_info = &rx_adapter->eth_devices[i];
> >>+		/* if start  check for num dev queues */
> >>+		if (start && !dev_info->nb_dev_queues)
> >>+			continue;
> >>+		/* if stop check if dev has been started */
> >>+		if (stop && !dev_info->dev_rx_started)
> >>+			continue;:1
> >>+		use_service |= !dev_info->internal_event_port;
> >>+		dev_info->dev_rx_started = start;
> >>+		if (!dev_info->internal_event_port)
> >>+			continue;
> >>+		start ? (*dev->dev_ops->eth_rx_adapter_start)(dev,
> >>+						&rte_eth_devices[i]) :
> >>+			(*dev->dev_ops->eth_rx_adapter_stop)(dev,
> >>+						&rte_eth_devices[i]);
> >>+	}
> >>+
> >>+	if (use_service)
> >
> >Here setting the service run state is not sufficient we need to enable the
> >service on a service core calling rte_service_start_with_defaults() should be
> >sufficient.
> >
>
> Yes it is necessary but insufficient.
>
> IMO,
> If the application is controlling core masks, the application flow at
> startup looks like:
>
> rte_event_eth_rx_adapter_create(id,..)
> ...
> rte_event_eth_rx_adapter_start(id)
> if (!rte_event_eth_rx_adapter_service_id_get(id, &service_id)) {
> 	rte_service_lcore_add(rx_lcore_id);
>         rte_service_map_lcore_set(service_id, rx_lcore_id, 1);
> 	rte_service_lcore_start(rx_lcore_id)
> }
>
> Since rte_service_start_with_defaults() is invoked before the adapter is
> created, how would it get assigned a core etc ?
>
I might have caused a bit of confusion, I meant to say that If we call
rte_service_start_with_defaults() when "use_service" is set while starting the
RX adapter it will assign the polling function to a service core (default 1:1
mapping).
But this might destroy the user configured service core mappings.
I think adding the above mentioned code snippet in the programmers guide is
self sufficient.

>
> Nikhil

Thanks,
Pavan
> >>+		rte_service_runstate_set(rx_adapter->service_id, start);
> >>+
> >>+	return 0;
> >>+}
> >>+
> ><snip>
> >
> >Regards,
> >Pavan
> >
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-09-21 21:17 ` [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
  2017-09-22 12:12   ` Jerin Jacob
@ 2017-10-03 11:36   ` Pavan Nikhilesh Bhagavatula
  2017-10-05  5:57     ` Rao, Nikhil
  1 sibling, 1 reply; 30+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2017-10-03 11:36 UTC (permalink / raw)
  To: Nikhil Rao; +Cc: dev

On Fri, Sep 22, 2017 at 02:47:14AM +0530, Nikhil Rao wrote:

Hi Nikhil,


> Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
>  test/test/test_event_eth_rx_adapter.c | 399 ++++++++++++++++++++++++++++++++++
>  test/test/Makefile                    |   1 +
>  2 files changed, 400 insertions(+)
>  create mode 100644 test/test/test_event_eth_rx_adapter.c
>
> diff --git a/test/test/test_event_eth_rx_adapter.c b/test/test/test_event_eth_rx_adapter.c
> new file mode 100644
> index 000000000..5d448dc27
<snip>
> +
> +static int
> +testsuite_setup(void)
> +{
> +	int err;
> +	err = init_ports(rte_eth_dev_count());
> +	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
> +
> +	struct rte_event_dev_config config = {
> +			.nb_event_queues = 1,
> +			.nb_event_ports = 1,
> +			.nb_events_limit  = 4096,
> +			.nb_event_queue_flows = 1024,
> +			.nb_event_port_dequeue_depth = 16,
> +			.nb_event_port_enqueue_depth = 16
> +	};
> +

Some eth devices like octeontx[1] use event device to receive packets, So in
this special case it would require to stop the event device before configuring
the event device as it is already started in port_init.

Calling rte_event_dev_stop(0) here would satisfy such use case.

[1] http://dpdk.org/ml/archives/dev/2017-August/073982.html

> +	err = rte_event_dev_configure(0, &config);
> +	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
> +			err);
> +
> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
> +	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",

<snip>

> +
> +static int
> +adapter_queue_add_del(void)
> +{
> +	int err;
> +	struct rte_event ev;
> +	uint32_t cap;
> +
> +	struct rte_event_eth_rx_adapter_queue_conf queue_config;
> +
> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &cap);
> +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
> +	ev.queue_id = 0;
> +	ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
> +	ev.priority = 0;
> +
> +	queue_config.rx_queue_flags = 0;
> +	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)) {
> +		ev.flow_id = 1;
> +		queue_config.rx_queue_flags =
> +			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
> +	}
> +	queue_config.ev = ev;
> +	queue_config.servicing_weight = 1;
> +

As mentioned above[1] in case of HW accelerated coprocessors the eth_port has
to be stopped before reconfiguring the eth queue to event queue remapping.
Calling rte_eth_dev_stop(0) is required before trying to map the eth queue.

> +	err = rte_event_eth_rx_adapter_queue_add(0, rte_eth_dev_count(),
> +					-1, &queue_config);
> +	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> +
> +	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ)) {
> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
> +							&queue_config);
> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
> +		err = rte_event_eth_rx_adapter_queue_del(0, 0, 0);
> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, -1,
> +							&queue_config);
> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +
> +		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> +	} else {
> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
> +							&queue_config);
> +		TEST_ASSERT(err == -EINVAL, "Expected EINVAL got %d", err);
> +
>
<snip>

Thanks,
Pavan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-09-24 18:16     ` Rao, Nikhil
  2017-09-25  2:59       ` Rao, Nikhil
@ 2017-10-03 13:52       ` Jerin Jacob
  2017-10-05  8:12         ` Rao, Nikhil
  1 sibling, 1 reply; 30+ messages in thread
From: Jerin Jacob @ 2017-10-03 13:52 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

-----Original Message-----
> Date: Sun, 24 Sep 2017 23:46:51 +0530
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>  thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>  nipun.gupta@nxp.com, narender.vangati@intel.com,
>  erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>  santosh.shukla@caviumnetworks.com
> Subject: Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.3.0
> 
> 
> OK, Thanks for the detailed review. Will add the programmer guide to RC1.

OK. Thanks.

> 
> > 
> > 
> > 
> Yes, if create() and queue_add() are called from different processes, it
> wouldn't work.
> 
> > > +
> > > +static uint8_t default_rss_key[] = {
> > > +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> > > +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> > > +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> > > +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> > > +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> > > +};
> > 
> > Looks like the scope of this array is only for rte_event_eth_rx_adapter_init,
> > if so please move it to stack.
> > 
> OK.
> 
> > 
> > > +static uint8_t *rss_key_be;
> > 
> > Can we remove this global variable add it in in adapter memory?
> > 
> 
> There is currently struct rte_event_eth_rx_adapter
> **rte_event_eth_rx_adapter that is an array of pointers to the adapters.
> rss_key_be points to memory after this array.
> 
> are you thinking of something like:
> 
> struct {
> struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter
> uint8_t *rss_key_be;
> } global;

I was thinking, to hold 40B in struct rte_event_eth_rx_adapter for
rss_key_be and initialize per rx_adapter to avoid global variable 
as fill_event_buffer() has access to rte_event_eth_rx_adapter.

Something like below as rough idea.
➜ [dpdk-next-eventdev] $ git diff
lib/librte_eventdev/rte_event_eth_rx_adapter.c 
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index cd19e7c28..ba6148931 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -37,6 +37,7 @@ struct rte_eth_event_enqueue_buffer {
 };
 
 struct rte_event_eth_rx_adapter {
+       uint8_t rss_key[40];
        /* event device identifier */
        uint8_t eventdev_id;
        /* per ethernet device structure */




> > > +static inline uint32_t
> > > +eth_rx_poll(struct rte_event_eth_rx_adapter *rx_adapter)
> > > +static int
> > > +event_eth_rx_adapter_service_func(void *args)
> > > +{
> > > +	struct rte_event_eth_rx_adapter *rx_adapter = args;
> > > +	struct rte_eth_event_enqueue_buffer *buf;
> > > +
> > > +	buf = &rx_adapter->event_enqueue_buffer;
> > > +	if (!rte_spinlock_trylock(&rx_adapter->rx_lock))
> > > +		return 0;
> > > +	if (eth_rx_poll(rx_adapter) == 0 && buf->count)
> > > +		flush_event_buffer(rx_adapter);
> > > +	rte_spinlock_unlock(&rx_adapter->rx_lock);
> > > +	return 0;
> > > +}
> > > +
> > > +static int
> > > +rte_event_eth_rx_adapter_init(void)
> > > +{
> > > +	const char *name = "rte_event_eth_rx_adapter_array";
> > > +	const struct rte_memzone *mz;
> > > +	unsigned int sz;
> > > +	unsigned int rss_key_off;
> > > +
> > > +	sz = sizeof(*rte_event_eth_rx_adapter) *
> > > +	    RTE_MAX_EVENT_ETH_RX_ADAPTER_INSTANCE;
> > 
> > I think, you need to use size of struct rte_event_eth_rx_adapter here. if so,
> > we need **rte_event_eth_rx_adapter here. Right?
> > 
> > test code
> > struct abc {
> > 
> >          uint64_t a[64];
> > };
> > 
> > struct abc **k;
> > 
> > int main()
> > {
> > 	printf("%d %d %d\n", sizeof(k), sizeof(*k), sizeof(**k));
> > 
> > 	return 0;
> > }
> > 
> > $./a.out
> > 8 8 512
> > 
> 
> The struct rte_event_eth_rx_adapter gets allocated in
> rte_event_eth_rx_adapter_create_ext()

OK. I missed that.
> > 
> > > +		if (mz) {
> > > +			rte_convert_rss_key((uint32_t *)default_rss_key,
> > > +			    (uint32_t *)(uintptr_t)(mz->addr_64 + rss_key_off),
> > > +			    RTE_DIM(default_rss_key));
> > > +		} else {
> > > +			RTE_EDEV_LOG_ERR("failed to reserve memzone err = %"
> > > +					PRId32, rte_errno);
> > > +			return -rte_errno;
> > > +		}
> > > +	}
> > > +
> > > +	rte_event_eth_rx_adapter = mz->addr;
> > > +	rss_key_be = (uint8_t *)(mz->addr_64 + rss_key_off);
> > > +	return 0;
> > > +}
> > > +
> > > +static int
> > > +default_conf_cb(uint8_t id, uint8_t dev_id,
> > > +		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
> > > +{
> > > +
> > > +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
> > > +	if (ret) {
> > > +		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
> > > +					port_id);
> > 
> > return or add goto to exit from here to avoid calling rte_event_dev_start below
> > 
> Could do the return but I wanted to leave the device in the same state as it
> was at entry into this function. Thoughts ?

Will calling rte_event_dev_start() down(in case if wont return) change
the state? if not, it is fine.

No another comments. Looks good to me.

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-10-02 10:31       ` Jerin Jacob
@ 2017-10-04 11:28         ` Rao, Nikhil
  0 siblings, 0 replies; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-04 11:28 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 10/2/2017 4:01 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Sun, 24 Sep 2017 23:54:38 +0530
>> From: "Rao, Nikhil" <nikhil.rao@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>>   thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>>   nipun.gupta@nxp.com, narender.vangati@intel.com,
>>   erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>>   santosh.shukla@caviumnetworks.com
>> Subject: Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter
>>   APIs
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.3.0
>>
>> On 9/22/2017 5:42 PM, Jerin Jacob wrote:
>>> -----Original Message-----
>>>> Date: Fri, 22 Sep 2017 02:47:14 +0530
>>>> From: Nikhil Rao <nikhil.rao@intel.com>
>>>> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
>>>> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>>>>    harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>>>>    narender.vangati@intel.com, erik.g.carrillo@intel.com,
>>>>    abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
>>>> Subject: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
>>>> X-Mailer: git-send-email 2.7.4
>>>>
>>>> Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
>>>
>> <snip>
>>
>>>> +#include <string.h>
>>>> +#include <rte_common.h>
>>>> +#include <rte_mempool.h>
>>>> +#include <rte_mbuf.h>
>>>> +#include <rte_ethdev.h>
>>>> +#include <rte_eventdev.h>
>>>> +
>>>> +#include <rte_event_eth_rx_adapter.h>
>>>> +
>>>> +#include "test.h"
>>>> +
>>>> +/* i40e limits max to 64 */
>>>
>>> This comment could be removed.
>>>
>> OK, I am documenting why the code doesn't just use dev_info.max_rx_queues,
>> won't the comment be useful to retain ?
> 
> OK. If dev_info.max_rx_queues for i40e is not 64 as expected then we
> could the fix the i40e driver as well.

This change was added in c9eb97fb9212a38, will check with i40e PMD 
maintainer (this is a holiday week in PRC though), a quick check showed 
that it is possible to allocate dev_info.max_rx_queues - 
dev_info.vmdq_queue_num Rx queues

I will re post with the comment removed for now and post a patch later.

Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-10-03 11:36   ` Pavan Nikhilesh Bhagavatula
@ 2017-10-05  5:57     ` Rao, Nikhil
  2017-10-05  8:08       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-05  5:57 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula; +Cc: dev

On 10/3/2017 5:06 PM, Pavan Nikhilesh Bhagavatula wrote:
> On Fri, Sep 22, 2017 at 02:47:14AM +0530, Nikhil Rao wrote:
> 
> Hi Nikhil,
> 
> 
>> Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
>>
>> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
>> ---
>>   test/test/test_event_eth_rx_adapter.c | 399 ++++++++++++++++++++++++++++++++++
>>   test/test/Makefile                    |   1 +
>>   2 files changed, 400 insertions(+)
>>   create mode 100644 test/test/test_event_eth_rx_adapter.c
>>
>> diff --git a/test/test/test_event_eth_rx_adapter.c b/test/test/test_event_eth_rx_adapter.c
>> new file mode 100644
>> index 000000000..5d448dc27
> <snip>
>> +
>> +static int
>> +testsuite_setup(void)
>> +{
>> +	int err;
>> +	err = init_ports(rte_eth_dev_count());
>> +	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
>> +
>> +	struct rte_event_dev_config config = {
>> +			.nb_event_queues = 1,
>> +			.nb_event_ports = 1,
>> +			.nb_events_limit  = 4096,
>> +			.nb_event_queue_flows = 1024,
>> +			.nb_event_port_dequeue_depth = 16,
>> +			.nb_event_port_enqueue_depth = 16
>> +	};
>> +
> 
> Some eth devices like octeontx[1] use event device to receive packets, So in
> this special case it would require to stop the event device before configuring
> the event device as it is already started in port_init.
> 
> Calling rte_event_dev_stop(0) here would satisfy such use case.

Hi Pavan,

port_init is starting the eth device not the event device.

Moving init_ports to after rte_event_dev_configure should also work ?

> 
> [1] http://dpdk.org/ml/archives/dev/2017-August/073982.html
> 
>> +	err = rte_event_dev_configure(0, &config);
>> +	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
>> +			err);
>> +
>> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
>> +	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",
> 
> <snip>
> 
>> +
>> +static int
>> +adapter_queue_add_del(void)
>> +{
>> +	int err;
>> +	struct rte_event ev;
>> +	uint32_t cap;
>> +
>> +	struct rte_event_eth_rx_adapter_queue_conf queue_config;
>> +
>> +	err = rte_event_eth_rx_adapter_caps_get(0, 0, &cap);
>> +	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>> +
>> +	ev.queue_id = 0;
>> +	ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
>> +	ev.priority = 0;
>> +
>> +	queue_config.rx_queue_flags = 0;
>> +	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)) {
>> +		ev.flow_id = 1;
>> +		queue_config.rx_queue_flags =
>> +			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
>> +	}
>> +	queue_config.ev = ev;
>> +	queue_config.servicing_weight = 1;
>> +
> 
> As mentioned above[1] in case of HW accelerated coprocessors the eth_port has
> to be stopped before reconfiguring the eth queue to event queue remapping.
> Calling rte_eth_dev_stop(0) is required before trying to map the eth queue.
> 

Is it possible to do this internally within the queue_add call ?

If not, the application would call rte_eth_dev_stop() if 
RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT is set or do we need a 
separate capability for this ?

>> +	err = rte_event_eth_rx_adapter_queue_add(0, rte_eth_dev_count(),
>> +					-1, &queue_config);
>> +	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
>> +
>> +	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ)) {
>> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
>> +							&queue_config);
>> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>> +
>> +		err = rte_event_eth_rx_adapter_queue_del(0, 0, 0);
>> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>> +
>> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, -1,
>> +							&queue_config);
>> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>> +
>> +		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
>> +		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>> +	} else {
>> +		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
>> +							&queue_config);
>> +		TEST_ASSERT(err == -EINVAL, "Expected EINVAL got %d", err);
>> +
>>
> <snip>
> 
> Thanks,
> Pavan
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs
  2017-10-05  5:57     ` Rao, Nikhil
@ 2017-10-05  8:08       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 30+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2017-10-05  8:08 UTC (permalink / raw)
  To: Rao, Nikhil; +Cc: dev

On Thu, Oct 05, 2017 at 11:27:53AM +0530, Rao, Nikhil wrote:
> On 10/3/2017 5:06 PM, Pavan Nikhilesh Bhagavatula wrote:
> >On Fri, Sep 22, 2017 at 02:47:14AM +0530, Nikhil Rao wrote:
> >
> >Hi Nikhil,
> >
> >
> >>Add unit tests for rte_event_eth_rx_adapter_xxx() APIs
> >>
> >>Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> >>---
> >>  test/test/test_event_eth_rx_adapter.c | 399 ++++++++++++++++++++++++++++++++++
> >>  test/test/Makefile                    |   1 +
> >>  2 files changed, 400 insertions(+)
> >>  create mode 100644 test/test/test_event_eth_rx_adapter.c
> >>
> >>diff --git a/test/test/test_event_eth_rx_adapter.c b/test/test/test_event_eth_rx_adapter.c
> >>new file mode 100644
> >>index 000000000..5d448dc27
> ><snip>
> >>+
> >>+static int
> >>+testsuite_setup(void)
> >>+{
> >>+	int err;
> >>+	err = init_ports(rte_eth_dev_count());
> >>+	TEST_ASSERT(err == 0, "Port initialization failed err %d\n", err);
> >>+
> >>+	struct rte_event_dev_config config = {
> >>+			.nb_event_queues = 1,
> >>+			.nb_event_ports = 1,
> >>+			.nb_events_limit  = 4096,
> >>+			.nb_event_queue_flows = 1024,
> >>+			.nb_event_port_dequeue_depth = 16,
> >>+			.nb_event_port_enqueue_depth = 16
> >>+	};
> >>+
> >
> >Some eth devices like octeontx[1] use event device to receive packets, So in
> >this special case it would require to stop the event device before configuring
> >the event device as it is already started in port_init.
> >
> >Calling rte_event_dev_stop(0) here would satisfy such use case.
>
> Hi Pavan,
>
> port_init is starting the eth device not the event device.

If eth_octeontx is the eth device It uses event_octeontx to work. So, when
rte_eth_dev_start is called in port_init it invokes rte_event_dev_start
internally.

>
> Moving init_ports to after rte_event_dev_configure should also work ?

Yep, this works too.

>
> >
> >[1] http://dpdk.org/ml/archives/dev/2017-August/073982.html
> >
> >>+	err = rte_event_dev_configure(0, &config);
> >>+	TEST_ASSERT(err == 0, "Event device initialization failed err %d\n",
> >>+			err);
> >>+
> >>+	err = rte_event_eth_rx_adapter_caps_get(0, 0, &default_params.caps);
> >>+	TEST_ASSERT(err == 0, "Failed to get adapter cap err %d\n",
> >
> ><snip>
> >
> >>+
> >>+static int
> >>+adapter_queue_add_del(void)
> >>+{
> >>+	int err;
> >>+	struct rte_event ev;
> >>+	uint32_t cap;
> >>+
> >>+	struct rte_event_eth_rx_adapter_queue_conf queue_config;
> >>+
> >>+	err = rte_event_eth_rx_adapter_caps_get(0, 0, &cap);
> >>+	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >>+
> >>+	ev.queue_id = 0;
> >>+	ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
> >>+	ev.priority = 0;
> >>+
> >>+	queue_config.rx_queue_flags = 0;
> >>+	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)) {
> >>+		ev.flow_id = 1;
> >>+		queue_config.rx_queue_flags =
> >>+			RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
> >>+	}
> >>+	queue_config.ev = ev;
> >>+	queue_config.servicing_weight = 1;
> >>+
> >
> >As mentioned above[1] in case of HW accelerated coprocessors the eth_port has
> >to be stopped before reconfiguring the eth queue to event queue remapping.
> >Calling rte_eth_dev_stop(0) is required before trying to map the eth queue.
> >
>
> Is it possible to do this internally within the queue_add call ?

It is possible to handle this internally.
AFAIK it is a very specific case that exists when we are using eth_octeontx and
event_octeontx. So, I think this changes is not required.

>
> If not, the application would call rte_eth_dev_stop() if
> RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT is set or do we need a separate
> capability for this ?
>
> >>+	err = rte_event_eth_rx_adapter_queue_add(0, rte_eth_dev_count(),
> >>+					-1, &queue_config);
> >>+	TEST_ASSERT(err == -EINVAL, "Expected -EINVAL got %d", err);
> >>+
> >>+	if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ)) {
> >>+		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
> >>+							&queue_config);
> >>+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >>+
> >>+		err = rte_event_eth_rx_adapter_queue_del(0, 0, 0);
> >>+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >>+
> >>+		err = rte_event_eth_rx_adapter_queue_add(0, 0, -1,
> >>+							&queue_config);
> >>+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >>+
> >>+		err = rte_event_eth_rx_adapter_queue_del(0, 0, -1);
> >>+		TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >>+	} else {
> >>+		err = rte_event_eth_rx_adapter_queue_add(0, 0, 0,
> >>+							&queue_config);
> >>+		TEST_ASSERT(err == -EINVAL, "Expected EINVAL got %d", err);
> >>+
> >>
> ><snip>
> >
> >Thanks,
> >Pavan
> >
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-10-03 13:52       ` Jerin Jacob
@ 2017-10-05  8:12         ` Rao, Nikhil
  0 siblings, 0 replies; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-05  8:12 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 10/3/2017 7:22 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Sun, 24 Sep 2017 23:46:51 +0530
>> From: "Rao, Nikhil" <nikhil.rao@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>>   thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>>   nipun.gupta@nxp.com, narender.vangati@intel.com,
>>   erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>>   santosh.shukla@caviumnetworks.com
>> Subject: Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.3.0
>>
>>
>> OK, Thanks for the detailed review. Will add the programmer guide to RC1.
> 
> OK. Thanks.
> 
>>
>>>
>>>
>>>
>> Yes, if create() and queue_add() are called from different processes, it
>> wouldn't work.
>>
>>>> +
>>>> +static uint8_t default_rss_key[] = {
>>>> +	0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
>>>> +	0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
>>>> +	0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
>>>> +	0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
>>>> +	0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
>>>> +};
>>>
>>> Looks like the scope of this array is only for rte_event_eth_rx_adapter_init,
>>> if so please move it to stack.
>>>
>> OK.
>>
>>>
>>>> +static uint8_t *rss_key_be;
>>>
>>> Can we remove this global variable add it in in adapter memory?
>>>
>>
>> There is currently struct rte_event_eth_rx_adapter
>> **rte_event_eth_rx_adapter that is an array of pointers to the adapters.
>> rss_key_be points to memory after this array.
>>
>> are you thinking of something like:
>>
>> struct {
>> struct rte_event_eth_rx_adapter **rte_event_eth_rx_adapter
>> uint8_t *rss_key_be;
>> } global;
> 
> I was thinking, to hold 40B in struct rte_event_eth_rx_adapter for
> rss_key_be and initialize per rx_adapter to avoid global variable
> as fill_event_buffer() has access to rte_event_eth_rx_adapter.
> 
> Something like below as rough idea.
> ➜ [dpdk-next-eventdev] $ git diff
> lib/librte_eventdev/rte_event_eth_rx_adapter.c
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> index cd19e7c28..ba6148931 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> @@ -37,6 +37,7 @@ struct rte_eth_event_enqueue_buffer {
>   };
>   
>   struct rte_event_eth_rx_adapter {
> +       uint8_t rss_key[40];
>          /* event device identifier */
>          uint8_t eventdev_id;
>          /* per ethernet device structure */

OK.


>>>> +
>>>> +static int
>>>> +default_conf_cb(uint8_t id, uint8_t dev_id,
>>>> +		struct rte_event_eth_rx_adapter_conf *conf, void *arg)
>>>> +{
>>>> +
>>>> +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
>>>> +	if (ret) {
>>>> +		RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
>>>> +					port_id);
>>>
>>> return or add goto to exit from here to avoid calling rte_event_dev_start below
>>>
>> Could do the return but I wanted to leave the device in the same state as it
>> was at entry into this function. Thoughts ?
> 
> Will calling rte_event_dev_start() down(in case if wont return) change
> the state? if not, it is fine.
> 

OK, will put in the return. if the device were configured with an 
additional port and the setup for this port fails. The 
rte_event_dev_start() call will dereference a NULL ptr.

Nikhil



> No another comments. Looks good to me.
> 
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
  2017-10-02 10:39           ` Jerin Jacob
@ 2017-10-05  8:54             ` Rao, Nikhil
  0 siblings, 0 replies; 30+ messages in thread
From: Rao, Nikhil @ 2017-10-05  8:54 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: bruce.richardson, gage.eads, dev, thomas, harry.van.haaren,
	hemant.agrawal, nipun.gupta, narender.vangati, erik.g.carrillo,
	abhinandan.gujjar, santosh.shukla

On 10/2/2017 4:09 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Mon, 2 Oct 2017 15:58:56 +0530
>> From: "Rao, Nikhil" <nikhil.rao@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
>>   thomas@monjalon.net, harry.van.haaren@intel.com, hemant.agrawal@nxp.com,
>>   nipun.gupta@nxp.com, narender.vangati@intel.com,
>>   erik.g.carrillo@intel.com, abhinandan.gujjar@intel.com,
>>   santosh.shukla@caviumnetworks.com
>> Subject: Re: [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.3.0
>>
>> On 9/25/2017 8:29 AM, Rao, Nikhil wrote:
>>> On 9/24/2017 11:46 PM, Rao, Nikhil wrote:
>>>> On 9/22/2017 2:40 PM, Jerin Jacob wrote:
>>>>
>>>>> When we worked on a prototype, we figured out that we need a
>>>>> separate event type
>>>>> for RX adapter. Probably RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER?
>>>>> The Reason is:
>>>>> - In the HW based Rx adapter case, the packet are coming
>>>>> directly to eventdev once it is configured.
>>>>> - So on a HW implementation of the event dequeue(), CPU needs to
>>>>> convert HW specific
>>>>> metadata to mbuf
>>>>> - The event dequeue() is used in two cases
>>>>> a) octeontx eventdev driver used with any external NIC
>>>>> b) octeontx eventdev driver used with integrated NIC(without service
>>>>> core to inject the packet)
>>>>> We need some identifier to understand case (a) and (b).So, in
>>>>> dequeue(), if the
>>>>> packet is from RTE_EVENT_TYPE_ETHDEV then we can do "HW specific
>>>>> metadata" to mbuf
>>>>> conversion and in another case (!RTE_EVENT_TYPE_ETHDEV) result
>>>>> in no mbuf
>>>>> conversion.
>>>>>
>>>>> Application can check if it is an Ethernet type event by
>>>>> ev.event_type == RTE_EVENT_TYPE_ETHDEV || ev.event_type ==
>>>>> RTE_EVENT_TYPE_ETHDEV_RX_ADAPTER
>>>>>
>>>>
>>>> As per my understanding, the case (a) uses an in built port
>>>> Is it possible for the eventdev PMD to do the conversion based off
>>>> the eventdev port ?
>>>>
>>>
>>> I realized the dequeue wouldn't have knowledge of the port the event was
>>> injected from, the application shouldn't have to see the difference
>>> between case (a) & (b).
>>>
>>> Would it be possible to use the impl_opaque field within struct rte_event ?
>>>
>>> Nikhil
>>
>> Hi Jerin,
>>
>> Any further thoughts on this ?
> 
> impl_opaque field could be one option. But I think, NXP driver is using
> it for internal operation. So overriding it from Rx adapter will cause
> issue. How about adding new event type? So it gets a new name space so no
> collision.
> 
> ➜ [master][dpdk-next-eventdev] $ git diff
> diff --git a/lib/librte_eventdev/rte_eventdev.h
> b/lib/librte_eventdev/rte_eventdev.h
> index ec7aabd9a..b33423c7e 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -878,6 +878,8 @@ rte_event_dev_close(uint8_t dev_id);
>   /**< The event generated from cpu for pipelining.
>    * Application may use *sub_event_type* to further classify the event
>    */
> +#define RTE_EVENT_TYPE_ETHDEV_ADAPTER   0x4
> +/**< The event generated from ethdev Rx adapter */
>   #define RTE_EVENT_TYPE_MAX              0x10
>   /**< Maximum number of event types */
> 
The event source is really RTE_EVENT_TYPE_CPU here, but is the 
assumption that the RTE_EVENT_TYPE_CPU name space is owned by the 
application (it's actions are driven by a combination of event type and 
event sub type) and extending the event source count is the only option 
here.

Nikhil

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2017-10-05  8:54 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 21:17 [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
2017-09-21 21:17 ` [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
2017-09-21 15:46   ` Jerin Jacob
2017-09-24 12:14     ` Rao, Nikhil
2017-10-02  8:48       ` Jerin Jacob
2017-09-21 21:17 ` [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
2017-09-22  2:49   ` Jerin Jacob
2017-09-22  5:27   ` santosh
2017-09-21 21:17 ` [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
2017-09-21 15:43   ` Pavan Nikhilesh Bhagavatula
2017-09-23 11:35     ` Rao, Nikhil
2017-10-03  9:09       ` Pavan Nikhilesh Bhagavatula
2017-09-22  6:08   ` santosh
2017-10-02 10:20     ` Rao, Nikhil
2017-09-22  9:10   ` Jerin Jacob
2017-09-24 18:16     ` Rao, Nikhil
2017-09-25  2:59       ` Rao, Nikhil
2017-10-02 10:28         ` Rao, Nikhil
2017-10-02 10:39           ` Jerin Jacob
2017-10-05  8:54             ` Rao, Nikhil
2017-10-03 13:52       ` Jerin Jacob
2017-10-05  8:12         ` Rao, Nikhil
2017-09-21 21:17 ` [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
2017-09-22 12:12   ` Jerin Jacob
2017-09-24 18:24     ` Rao, Nikhil
2017-10-02 10:31       ` Jerin Jacob
2017-10-04 11:28         ` Rao, Nikhil
2017-10-03 11:36   ` Pavan Nikhilesh Bhagavatula
2017-10-05  5:57     ` Rao, Nikhil
2017-10-05  8:08       ` Pavan Nikhilesh Bhagavatula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.