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

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

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Nikhil Rao <nikhil.rao@intel.com>
> Sent: Tuesday, June 25, 2019 4:46 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: nikhil.rao@intel.com; dev@dpdk.org
> Subject: [EXT] [PATCH v3] eventdev: change Rx adapter callback and stats
> structure
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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.
> 
> This commit removes the experimental tags from the callback and stats APIs,
> the experimental tag from eventdev is also removed and eventdev functions
> become part of the main DPDK API/ABI.
> 
> 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 | 34 ++++++-----
>  MAINTAINERS                                    |  2 +-
>  doc/guides/rel_notes/release_19_08.rst         | 20 ++++++-
>  lib/librte_eventdev/Makefile                   |  2 +-
>  lib/librte_eventdev/meson.build                |  2 +-
>  lib/librte_eventdev/rte_eventdev_version.map   |  4 +-
>  7 files changed, 83 insertions(+), 63 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
> 
> v3:
> * Update version = 7 in lib/librte_eventdev/meson.build
> * Update the "git comment and release notes" to mention the removal of
>   experimental status
> * Reduce rte_event_eth_rx_adapter.c line count

Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied to dpdk-next-eventdev/master. Thanks.

[dpdk-dev] [PATCH v3] 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.

This commit removes the experimental tags from
the callback and stats APIs, the experimental tag from eventdev
is also removed and eventdev functions become part of the
main DPDK API/ABI.

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 | 34 ++++++-----
 MAINTAINERS                                    |  2 +-
 doc/guides/rel_notes/release_19_08.rst         | 20 ++++++-
 lib/librte_eventdev/Makefile                   |  2 +-
 lib/librte_eventdev/meson.build                |  2 +-
 lib/librte_eventdev/rte_eventdev_version.map   |  4 +-
 7 files changed, 83 insertions(+), 63 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

v3:
* Update version = 7 in lib/librte_eventdev/meson.build
* Update the "git comment and release notes" to mention the removal of
  experimental status
* Reduce rte_event_eth_rx_adapter.c line count

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 161799e..95dd478 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -763,8 +763,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);
@@ -780,19 +780,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];
 
@@ -806,6 +793,21 @@ 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;
 }
 
@@ -2276,7 +2278,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)
 {
@@ -2363,7 +2365,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 0212fe6..09f0a98 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 8c3932d..0689ac1 100644
--- a/doc/guides/rel_notes/release_19_08.rst
+++ b/doc/guides/rel_notes/release_19_08.rst
@@ -124,6 +124,13 @@ API Changes
 * The network structures, definitions and functions have
   been prefixed by ``rte_`` to resolve conflicts with libc headers.
 
+* eventdev: No longer marked as experimental.
+
+  The eventdev functions are no longer marked as experimental, and have
+  become part of the normal DPDK API and ABI. Any future ABI changes will be
+  announced at least one release before the ABI change is made. There are no
+  ABI breaking changes planned.
+
 
 ABI Changes
 -----------
@@ -140,6 +147,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 callback
+  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
 -----------------------
@@ -176,7 +194,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/meson.build b/lib/librte_eventdev/meson.build
index 6cfe60e..19541f2 100644
--- a/lib/librte_eventdev/meson.build
+++ b/lib/librte_eventdev/meson.build
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-version = 6
+version = 7
 allow_experimental_apis = true
 
 if is_linux
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