Re: [dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback and stats structure

Re: [dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback and stats structure

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Nikhil Rao <nikhil.rao@intel.com>
> Sent: Friday, June 14, 2019 4:58 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: nikhil.rao@intel.com; dev@dpdk.org
> Subject: [EXT] [PATCH v2] eventdev: change Rx adapter callback and stats
> structure
> 
> ----------------------------------------------------------------------
> Replace the mbuf pointer array in the event eth Rx adapter callback with an
> event array. Using an event array allows the application to change attributes
> of the events enqueued by the SW adapter.
> 
> The callback can drop packets and populate a callback argument with the
> number of dropped packets. Add a Rx adapter stats field to keep track of the
> total number of dropped packets.
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
>  lib/librte_eventdev/rte_event_eth_rx_adapter.h | 82 +++++++++++++-----
> --------  lib/librte_eventdev/rte_event_eth_rx_adapter.c | 39 +++++++-----
>  MAINTAINERS                                    |  2 +-
>  doc/guides/rel_notes/release_19_08.rst         | 13 +++-
>  lib/librte_eventdev/Makefile                   |  2 +-

1) Update version =  7 in lib/librte_eventdev/meson.build
2) remove the following
lib/librte_eventdev/Makefile:CFLAGS += -DALLOW_EXPERIMENTAL_API
lib/librte_eventdev/meson.build:allow_experimental_apis = true

3)  Update the "git comment and release notes"  to express the fact that experimental
Status has been removed.
See following change set for reference.

commit 60df5711972e97d81cbfb811c9246f87cc69409a
Author: Harry van Haaren <harry.van.haaren@intel.com>
Date:   Thu Apr 5 14:15:46 2018 +0100

    service: remove experimental tags


>  lib/librte_eventdev/rte_eventdev_version.map   |  4 +-
>  6 files changed, 80 insertions(+), 62 deletions(-)
> 
> v1:
> * add implementation to RFC
> 
> v2:
> * Bump library version
> * Combine patch 1 & 2 into a single patch (single library version bump)
> * Mention API change in release notes
> * Remove __rte_experimental tag
> * Remove EXPERIMENTAL status for eventdev
> 
> @@ -819,6 +806,26 @@ static uint16_t rxa_gcd_u16(uint16_t a, uint16_t b)
>  		ev++;
>  	}
> 
> +	if (dev_info->cb_fn) {
> +
> +		dropped = 0;
> +		nb_cb = dev_info->cb_fn(eth_dev_id,
> +					rx_queue_id,
> +					ETH_EVENT_BUFFER_SIZE,
> +					buf->count,
> +					ev,
> +					num,
> +					dev_info->cb_arg,
> +					&dropped);

I think, the above variables can squash to 80 characters limit to reduce the lines.

Other than, Above nits, The patch looks good to me.

Re: [dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback and stats structure

[Rao, Nikhil]

Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> Sent: Monday, June 17, 2019 10:03 AM
> To: Rao, Nikhil <nikhil.rao@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] eventdev: change Rx adapter callback and stats
> structure
> 
<snip>
> 2) remove the following
> lib/librte_eventdev/Makefile:CFLAGS += -DALLOW_EXPERIMENTAL_API

ALLOW_EXPERIMENTAL_API is needed to allow for a call to rte_service_may_be_active() from the Tx adapter.

Nikhil

Re: [dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback and stats structure

[Rao, Nikhil]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rao, Nikhil
> Sent: Wednesday, June 19, 2019 1:40 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback
> and stats structure
> 
> Hi Jerin,
> 
> > -----Original Message-----
> > From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> > Sent: Monday, June 17, 2019 10:03 AM
> > To: Rao, Nikhil <nikhil.rao@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH v2] eventdev: change Rx adapter callback and stats
> > structure
> >
> <snip>
> > 2) remove the following
> > lib/librte_eventdev/Makefile:CFLAGS += -DALLOW_EXPERIMENTAL_API
> 
> ALLOW_EXPERIMENTAL_API is needed to allow for a call to
> rte_service_may_be_active() from the Tx adapter.
> 
> Nikhil

+ Gage, Harry.

Can we remove the experimental tag from  rte_service_may_be_active() ?

Thanks,
Nikhil

[dpdk-dev] [PATCH v2] eventdev: change Rx adapter callback and stats structure

[Nikhil Rao]

Replace the mbuf pointer array in the event eth Rx adapter
callback with an event array. Using an event array allows
the application to change attributes of the events enqueued
by the SW adapter.

The callback can drop packets and populate a callback
argument with the number of dropped packets. Add a Rx adapter
stats field to keep track of the total number of dropped packets.

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 lib/librte_eventdev/rte_event_eth_rx_adapter.h | 82 +++++++++++++-------------
 lib/librte_eventdev/rte_event_eth_rx_adapter.c | 39 +++++++-----
 MAINTAINERS                                    |  2 +-
 doc/guides/rel_notes/release_19_08.rst         | 13 +++-
 lib/librte_eventdev/Makefile                   |  2 +-
 lib/librte_eventdev/rte_eventdev_version.map   |  4 +-
 6 files changed, 80 insertions(+), 62 deletions(-)

v1:
* add implementation to RFC

v2:
* Bump library version
* Combine patch 1 & 2 into a single patch (single library version bump)
* Mention API change in release notes
* Remove __rte_experimental tag
* Remove EXPERIMENTAL status for eventdev

diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
index beab870..99b55a8 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
@@ -66,16 +66,17 @@
  * For SW based packet transfers, i.e., when the
  * RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT is not set in the adapter's
  * capabilities flags for a particular ethernet device, the service function
- * temporarily enqueues mbufs to an event buffer before batch enqueuing these
+ * temporarily enqueues events to an event buffer before batch enqueuing these
  * to the event device. If the buffer fills up, the service function stops
  * dequeuing packets from the ethernet device. The application may want to
  * monitor the buffer fill level and instruct the service function to
- * selectively buffer packets. The application may also use some other
+ * selectively buffer events. The application may also use some other
  * criteria to decide which packets should enter the event device even when
- * the event buffer fill level is low. The
- * rte_event_eth_rx_adapter_cb_register() function allows the
- * application to register a callback that selects which packets to enqueue
- * to the event device.
+ * the event buffer fill level is low or may want to enqueue packets to an
+ * internal event port. The rte_event_eth_rx_adapter_cb_register() function
+ * allows the application to register a callback that selects which packets are
+ * enqueued to the event device by the SW adapter. The callback interface is
+ * event based so the callback can also modify the event data if it needs to.
  */
 
 #ifdef __cplusplus
@@ -173,9 +174,6 @@ struct rte_event_eth_rx_adapter_queue_conf {
 };
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * A structure used to retrieve statistics for an eth rx adapter instance.
  */
 struct rte_event_eth_rx_adapter_stats {
@@ -187,6 +185,8 @@ struct rte_event_eth_rx_adapter_stats {
 	/**< Eventdev enqueue count */
 	uint64_t rx_enq_retry;
 	/**< Eventdev enqueue retry count */
+	uint64_t rx_dropped;
+	/**< Received packet dropped count */
 	uint64_t rx_enq_start_ts;
 	/**< Rx enqueue start timestamp */
 	uint64_t rx_enq_block_cycles;
@@ -204,16 +204,25 @@ struct rte_event_eth_rx_adapter_stats {
 };
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
  *
  * Callback function invoked by the SW adapter before it continues
- * to process packets. The callback is passed the size of the enqueue
+ * to process events. The callback is passed the size of the enqueue
  * buffer in the SW adapter and the occupancy of the buffer. The
- * callback can use these values to decide which mbufs should be
- * enqueued to the event device. If the return value of the callback
- * is less than nb_mbuf then the SW adapter uses the return value to
- * enqueue enq_mbuf[] to the event device.
+ * callback can use these values to decide which events are
+ * enqueued to the event device by the SW adapter. The callback may
+ * also enqueue events internally using its own event port. The SW
+ * adapter populates the event information based on the Rx queue
+ * configuration in the adapter. The callback can modify the this event
+ * information for the events to be enqueued by the SW adapter.
+ *
+ * The callback return value is the number of events from the
+ * beginning of the event array that are to be enqueued by
+ * the SW adapter. It is the callback's responsibility to arrange
+ * these events at the beginning of the array, if these events are
+ * not contiguous in the original array. The *nb_dropped* parameter is
+ * a pointer to the number of events dropped by the callback, this
+ * number is used by the adapter to indicate the number of dropped packets
+ * as part of its statistics.
  *
  * @param eth_dev_id
  *  Port identifier of the Ethernet device.
@@ -222,27 +231,26 @@ struct rte_event_eth_rx_adapter_stats {
  * @param enqueue_buf_size
  *  Total enqueue buffer size.
  * @param enqueue_buf_count
- *  mbuf count in enqueue buffer.
- * @param mbuf
- *  mbuf array.
- * @param nb_mbuf
- *  mbuf count.
+ *  Event count in enqueue buffer.
+ * @param[in, out] ev
+ *  Event array.
+ * @param nb_event
+ *  Event array length.
  * @param cb_arg
  *  Callback argument.
- * @param[out] enq_mbuf
- *  The adapter enqueues enq_mbuf[] if the return value of the
- *  callback is less than nb_mbuf
+ * @param[out] nb_dropped
+ *  Packets dropped by callback.
  * @return
- *  Returns the number of mbufs should be enqueued to eventdev
+ *  - The number of events to be enqueued by the SW adapter.
  */
 typedef uint16_t (*rte_event_eth_rx_adapter_cb_fn)(uint16_t eth_dev_id,
 						uint16_t queue_id,
 						uint32_t enqueue_buf_size,
 						uint32_t enqueue_buf_count,
-						struct rte_mbuf **mbuf,
-						uint16_t nb_mbuf,
+						struct rte_event *ev,
+						uint16_t nb_event,
 						void *cb_arg,
-						struct rte_mbuf **enq_buf);
+						uint16_t *nb_dropped);
 
 /**
  * Create a new ethernet Rx event adapter with the specified identifier.
@@ -398,9 +406,6 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t eth_dev_id,
 int rte_event_eth_rx_adapter_stop(uint8_t id);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Retrieve statistics for an adapter
  *
  * @param id
@@ -413,8 +418,7 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t eth_dev_id,
  *  - 0: Success, retrieved successfully.
  *  - <0: Error code on failure.
  */
-int __rte_experimental
-rte_event_eth_rx_adapter_stats_get(uint8_t id,
+int rte_event_eth_rx_adapter_stats_get(uint8_t id,
 				struct rte_event_eth_rx_adapter_stats *stats);
 
 /**
@@ -447,9 +451,6 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t eth_dev_id,
 int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Register callback to process Rx packets, this is supported for
  * SW based packet transfers.
  * @see rte_event_eth_rx_cb_fn
@@ -466,11 +467,10 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t eth_dev_id,
  *  - 0: Success
  *  - <0: Error code on failure.
  */
-int __rte_experimental
-rte_event_eth_rx_adapter_cb_register(uint8_t id,
-				uint16_t eth_dev_id,
-				rte_event_eth_rx_adapter_cb_fn cb_fn,
-				void *cb_arg);
+int rte_event_eth_rx_adapter_cb_register(uint8_t id,
+					uint16_t eth_dev_id,
+					rte_event_eth_rx_adapter_cb_fn cb_fn,
+					void *cb_arg);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index eaf341c..718fbbf 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -768,8 +768,8 @@ static uint16_t rxa_gcd_u16(uint16_t a, uint16_t b)
 	uint32_t rss;
 	int do_rss;
 	uint64_t ts;
-	struct rte_mbuf *cb_mbufs[BATCH_SIZE];
 	uint16_t nb_cb;
+	uint16_t dropped;
 
 	/* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
 	rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
@@ -785,19 +785,6 @@ static uint16_t rxa_gcd_u16(uint16_t a, uint16_t b)
 		}
 	}
 
-
-	nb_cb = dev_info->cb_fn ? dev_info->cb_fn(eth_dev_id, rx_queue_id,
-						ETH_EVENT_BUFFER_SIZE,
-						buf->count, mbufs,
-						num,
-						dev_info->cb_arg,
-						cb_mbufs) :
-						num;
-	if (nb_cb < num) {
-		mbufs = cb_mbufs;
-		num = nb_cb;
-	}
-
 	for (i = 0; i < num; i++) {
 		m = mbufs[i];
 
@@ -819,6 +806,26 @@ static uint16_t rxa_gcd_u16(uint16_t a, uint16_t b)
 		ev++;
 	}
 
+	if (dev_info->cb_fn) {
+
+		dropped = 0;
+		nb_cb = dev_info->cb_fn(eth_dev_id,
+					rx_queue_id,
+					ETH_EVENT_BUFFER_SIZE,
+					buf->count,
+					ev,
+					num,
+					dev_info->cb_arg,
+					&dropped);
+		if (unlikely(nb_cb > num))
+			RTE_EDEV_LOG_ERR("Rx CB returned %d (> %d) events",
+				nb_cb, num);
+		else
+			num = nb_cb;
+		if (dropped)
+			rx_adapter->stats.rx_dropped += dropped;
+	}
+
 	buf->count += num;
 }
 
@@ -2285,7 +2292,7 @@ static int rxa_sw_add(struct rte_event_eth_rx_adapter *rx_adapter,
 	return rxa_ctrl(id, 0);
 }
 
-int __rte_experimental
+int
 rte_event_eth_rx_adapter_stats_get(uint8_t id,
 			       struct rte_event_eth_rx_adapter_stats *stats)
 {
@@ -2372,7 +2379,7 @@ static int rxa_sw_add(struct rte_event_eth_rx_adapter *rx_adapter,
 	return rx_adapter->service_inited ? 0 : -ESRCH;
 }
 
-int __rte_experimental
+int
 rte_event_eth_rx_adapter_cb_register(uint8_t id,
 					uint16_t eth_dev_id,
 					rte_event_eth_rx_adapter_cb_fn cb_fn,
diff --git a/MAINTAINERS b/MAINTAINERS
index d0bf259..d899d28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -399,7 +399,7 @@ F: lib/librte_eventdev/
 F: drivers/event/skeleton/
 F: app/test/test_eventdev.c
 
-Eventdev Ethdev Rx Adapter API - EXPERIMENTAL
+Eventdev Ethdev Rx Adapter API
 M: Nikhil Rao <nikhil.rao@intel.com>
 T: git://dpdk.org/next/dpdk-next-eventdev
 F: lib/librte_eventdev/*eth_rx_adapter*
diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
index c199270..bb773cd 100644
--- a/doc/guides/rel_notes/release_19_08.rst
+++ b/doc/guides/rel_notes/release_19_08.rst
@@ -106,6 +106,17 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* eventdev: Event based Rx adapter callback
+
+  The mbuf pointer array in the event eth Rx adapter callbacl
+  has been replaced with an event array. Using
+  an event array allows the application to change attributes
+  of the events enqueued by the SW adapter.
+
+  The callback can drop packets and populate
+  a callback argument with the number of dropped packets.
+  Add a Rx adapter stats field to keep track of the total
+  number of dropped packets.
 
 Shared Library Versions
 -----------------------
@@ -142,7 +153,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_eal.so.10
      librte_efd.so.1
      librte_ethdev.so.12
-     librte_eventdev.so.6
+   + librte_eventdev.so.7
      librte_flow_classify.so.1
      librte_gro.so.1
      librte_gso.so.1
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 53079f4..cd3ff80 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -8,7 +8,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 LIB = librte_eventdev.a
 
 # library version
-LIBABIVER := 6
+LIBABIVER := 7
 
 # build flags
 CFLAGS += -DALLOW_EXPERIMENTAL_API
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 95fd089..76b3021 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -123,9 +123,9 @@ DPDK_19.05 {
 	rte_event_timer_cancel_burst;
 } DPDK_18.05;
 
-EXPERIMENTAL {
+DPDK_19.08 {
 	global:
 
 	rte_event_eth_rx_adapter_cb_register;
 	rte_event_eth_rx_adapter_stats_get;
-};
+} DPDK_19.05;
-- 
1.8.3.1