DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress
@ 2018-09-12 16:16 Harry van Haaren
  2018-09-12 16:16 ` [dpdk-dev] [PATCH 2/3] event/sw: implement unlinks in progress function Harry van Haaren
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-12 16:16 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit introduces a new function in the eventdev API,
which allows applications to read the number of unlink requests
in progress on a particular port of an eventdev instance.

This information allows applications to verify when no more packets
from a particular queue (or any queue) will arrive at a port.
The application could decide to stop polling, or put the core into
a sleep state if it wishes, as it is ensured that no new packets
will arrive at a particular port anymore if all queues are unlinked.

Suggested-by: Matias Elo <matias.elo@nokia.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Cc: jerin.jacob@caviumnetworks.com

Hey, I've added this API as __rte_experimental, so we should be OK to
include in 18.11, and then possibly remove the experimental tag in
a later release, assuming it serves its purpose correctly.

For context, see thread here:
http://mails.dpdk.org/archives/dev/2018-September/111499.html

@Matias, is that workable for you?
@Jerin, is this acceptable as maintainer?

Cheers, -Harry
---
 lib/librte_eventdev/rte_eventdev.c           | 22 +++++++++++++++
 lib/librte_eventdev/rte_eventdev.h           | 28 ++++++++++++++++++--
 lib/librte_eventdev/rte_eventdev_pmd.h       | 19 +++++++++++++
 lib/librte_eventdev/rte_eventdev_version.map |  1 +
 4 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index 801810edd..0a8572b7b 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -980,6 +980,28 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 	return diag;
 }
 
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id)
+{
+	struct rte_eventdev *dev;
+
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	dev = &rte_eventdevs[dev_id];
+	if (!is_valid_port(dev, port_id)) {
+		RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
+		return -EINVAL;
+	}
+
+	/* Return 0 if the PMD does not implement unlinks in progress.
+	 * This allows PMDs which handle unlink synchronously to not implement
+	 * this function at all.
+	 */
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_unlinks_in_progress, 0);
+
+	return (*dev->dev_ops->port_unlinks_in_progress)(dev,
+			dev->data->ports[port_id]);
+}
+
 int
 rte_event_port_links_get(uint8_t dev_id, uint8_t port_id,
 			 uint8_t queues[], uint8_t priorities[])
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index b6fd6ee7f..d07e297bc 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1656,8 +1656,9 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
  * event port designated by its *port_id* on the event device designated
  * by its *dev_id*.
  *
- * The unlink establishment shall disable the event port *port_id* from
- * receiving events from the specified event queue *queue_id*
+ * The unlink call issues an async request to disable the event port *port_id*
+ * from receiving events from the specified event queue *queue_id*. See
+ * *rte_event_port_unlinks_in_progress* to poll for completed unlinks.
  *
  * Event queue(s) to event port unlink establishment can be changed at runtime
  * without re-configuring the device.
@@ -1694,6 +1695,29 @@ int
 rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 		      uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * Returns the number of unlinks in progress.
+ *
+ * This function provides the application with a method to detect when an
+ * unlink has been completed by the implementation. See *rte_event_port_unlink*
+ * on how to issue unlink requests.
+ *
+ * @param dev_id
+ *   The indentifier of the device.
+ *
+ * @param port_id
+ *   Event port identifier to select port to check for unlinks in progress.
+ *
+ * @return
+ * The number of unlinks that are in progress. A return of zero indicates that
+ * there are no outstanding unlink requests. A positive return value indicates
+ * the number of unlinks that are in progress, but are not yet complete.
+ * A negative return value indicates an error, -EINVAL indicates an invalid
+ * parameter passed for *dev_id* or *port_id*.
+ */
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id);
+
 /**
  * Retrieve the list of source event queues and its associated service priority
  * linked to the destination event port designated by its *port_id*
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 3fbb4d2b2..65645730a 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -332,6 +332,23 @@ typedef int (*eventdev_port_link_t)(struct rte_eventdev *dev, void *port,
 typedef int (*eventdev_port_unlink_t)(struct rte_eventdev *dev, void *port,
 		uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * Unlinks in progress. Returns number of unlinks that the PMD is currently
+ * performing, but have not yet been completed.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param port
+ *   Event port pointer
+ *
+ * @return
+ *   Returns the number of in-progress unlinks. Zero is returned if none are
+ *   in progress.
+ */
+typedef int (*eventdev_port_unlinks_in_progress_t)(struct rte_eventdev *dev,
+		void *port);
+
 /**
  * Converts nanoseconds to *timeout_ticks* value for rte_event_dequeue()
  *
@@ -815,6 +832,8 @@ struct rte_eventdev_ops {
 	/**< Link event queues to an event port. */
 	eventdev_port_unlink_t port_unlink;
 	/**< Unlink event queues from an event port. */
+	eventdev_port_unlinks_in_progress_t port_unlinks_in_progress;
+	/**< Unlinks in progress on an event port. */
 	eventdev_dequeue_timeout_ticks_t timeout_ticks;
 	/**< Converts ns to *timeout_ticks* value for rte_event_dequeue() */
 	eventdev_dump_t dump;
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 12835e9f2..24e7a45c0 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -96,6 +96,7 @@ EXPERIMENTAL {
 	rte_event_crypto_adapter_stats_reset;
 	rte_event_crypto_adapter_stop;
 	rte_event_eth_rx_adapter_cb_register;
+	rte_event_port_unlinks_in_progress;
 	rte_event_timer_adapter_caps_get;
 	rte_event_timer_adapter_create;
 	rte_event_timer_adapter_create_ext;
-- 
2.17.1

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

* [dpdk-dev] [PATCH 2/3] event/sw: implement unlinks in progress function
  2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
@ 2018-09-12 16:16 ` Harry van Haaren
  2018-09-12 16:16 ` [dpdk-dev] [PATCH 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-12 16:16 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit adds a counter to each port, which counts the
number of unlinks that have been performed. When the scheduler
thread starts its scheduling routine, it "acks" all unlinks that
have been requested, and the application is gauranteed that no
more events will be scheduled to the port from the unlinked queue.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/event/sw/sw_evdev.c           | 12 ++++++++++++
 drivers/event/sw/sw_evdev.h           |  8 ++++++++
 drivers/event/sw/sw_evdev_scheduler.c |  7 ++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index a6bb91388..9e1412537 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -113,9 +113,20 @@ sw_port_unlink(struct rte_eventdev *dev, void *port, uint8_t queues[],
 			}
 		}
 	}
+
+	p->unlinks_in_progress += unlinked;
+	rte_smp_mb();
+
 	return unlinked;
 }
 
+static int
+sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)
+{
+	struct sw_port *p = port;
+	return p->unlinks_in_progress;
+}
+
 static int
 sw_port_setup(struct rte_eventdev *dev, uint8_t port_id,
 		const struct rte_event_port_conf *conf)
@@ -925,6 +936,7 @@ sw_probe(struct rte_vdev_device *vdev)
 			.port_release = sw_port_release,
 			.port_link = sw_port_link,
 			.port_unlink = sw_port_unlink,
+			.port_unlinks_in_progress = sw_port_unlinks_in_progress,
 
 			.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
 
diff --git a/drivers/event/sw/sw_evdev.h b/drivers/event/sw/sw_evdev.h
index d90b96d4b..7c77b2495 100644
--- a/drivers/event/sw/sw_evdev.h
+++ b/drivers/event/sw/sw_evdev.h
@@ -148,6 +148,14 @@ struct sw_port {
 	/* A numeric ID for the port */
 	uint8_t id;
 
+	/* An atomic counter for when the port has been unlinked, and the
+	 * scheduler has not yet acked this unlink - hence there may still be
+	 * events in the buffers going to the port. When the unlinks in
+	 * progress is read by the scheduler, no more events will be pushed to
+	 * the port - hence the scheduler core can just assign zero.
+	 */
+	uint8_t unlinks_in_progress;
+
 	int16_t is_directed; /** Takes from a single directed QID */
 	/**
 	 * For loadbalanced we can optimise pulling packets from
diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e02f..9b54d5ce7 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -517,13 +517,18 @@ sw_event_schedule(struct rte_eventdev *dev)
 		/* Pull from rx_ring for ports */
 		do {
 			in_pkts = 0;
-			for (i = 0; i < sw->port_count; i++)
+			for (i = 0; i < sw->port_count; i++) {
+				/* ack the unlinks in progress as done */
+				if (sw->ports[i].unlinks_in_progress)
+					sw->ports[i].unlinks_in_progress = 0;
+
 				if (sw->ports[i].is_directed)
 					in_pkts += sw_schedule_pull_port_dir(sw, i);
 				else if (sw->ports[i].num_ordered_qids > 0)
 					in_pkts += sw_schedule_pull_port_lb(sw, i);
 				else
 					in_pkts += sw_schedule_pull_port_no_reorder(sw, i);
+			}
 
 			/* QID scan for re-ordered */
 			in_pkts += sw_schedule_reorder(sw, 0,
-- 
2.17.1

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

* [dpdk-dev] [PATCH 3/3] event/sw: add unit test for unlinks in progress
  2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
  2018-09-12 16:16 ` [dpdk-dev] [PATCH 2/3] event/sw: implement unlinks in progress function Harry van Haaren
@ 2018-09-12 16:16 ` Harry van Haaren
  2018-09-13 14:45 ` [dpdk-dev] [PATCH 1/3] event: add function for reading unlink " Jerin Jacob
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-12 16:16 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit adds a unit test that checks the behaviour
of the unlinks_in_progress() function, ensuring that the
returned values are the number of unlinks requested,
until the scheduler runs and "acks" the requests, after
which the count should be zero again.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/event/sw/sw_evdev_selftest.c | 76 ++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c
index c40912db5..374abef7c 100644
--- a/drivers/event/sw/sw_evdev_selftest.c
+++ b/drivers/event/sw/sw_evdev_selftest.c
@@ -1903,6 +1903,77 @@ qid_priorities(struct test *t)
 	return 0;
 }
 
+static int
+unlink_in_progress(struct test *t)
+{
+	/* Test unlinking API, in particular that when an unlink request has
+	 * not yet been seen by the scheduler thread, that the
+	 * unlink_in_progress() function returns the number of unlinks.
+	 */
+	unsigned int i;
+	/* Create instance with 1 ports, and 3 qids */
+	if (init(t, 3, 1) < 0 ||
+			create_ports(t, 1) < 0) {
+		printf("%d: Error initializing device\n", __LINE__);
+		return -1;
+	}
+
+	for (i = 0; i < 3; i++) {
+		/* Create QID */
+		const struct rte_event_queue_conf conf = {
+			.schedule_type = RTE_SCHED_TYPE_ATOMIC,
+			/* increase priority (0 == highest), as we go */
+			.priority = RTE_EVENT_DEV_PRIORITY_NORMAL - i,
+			.nb_atomic_flows = 1024,
+			.nb_atomic_order_sequences = 1024,
+		};
+
+		if (rte_event_queue_setup(evdev, i, &conf) < 0) {
+			printf("%d: error creating qid %d\n", __LINE__, i);
+			return -1;
+		}
+		t->qid[i] = i;
+	}
+	t->nb_qids = i;
+	/* map all QIDs to port */
+	rte_event_port_link(evdev, t->port[0], NULL, NULL, 0);
+
+	if (rte_event_dev_start(evdev) < 0) {
+		printf("%d: Error with start call\n", __LINE__);
+		return -1;
+	}
+
+	/* unlink all ports to have outstanding unlink requests */
+	int ret = rte_event_port_unlink(evdev, t->port[0], NULL, 0);
+	if (ret < 0) {
+		printf("%d: Failed to unlink queues\n", __LINE__);
+		return -1;
+	}
+
+	/* get active unlinks here, expect 3 */
+	int unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 3) {
+		printf("%d: Expected num unlinks in progress == 3, got %d\n",
+				__LINE__, unlinks_in_progress);
+		return -1;
+	}
+
+	/* run scheduler service on this thread to ack the unlinks */
+	rte_service_run_iter_on_app_lcore(t->service_id, 1);
+
+	/* active unlinks expected as 0 as scheduler thread has acked */
+	unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 0) {
+		printf("%d: Expected num unlinks in progress == 0, got %d\n",
+				__LINE__, unlinks_in_progress);
+	}
+
+	cleanup(t);
+	return 0;
+}
+
 static int
 load_balancing(struct test *t)
 {
@@ -3260,6 +3331,11 @@ test_sw_eventdev(void)
 		printf("ERROR - QID Priority test FAILED.\n");
 		goto test_fail;
 	}
+	ret = unlink_in_progress(t);
+	if (ret != 0) {
+		printf("ERROR - Unlink in progress test FAILED.\n");
+		goto test_fail;
+	}
 	printf("*** Running Ordered Reconfigure test...\n");
 	ret = ordered_reconfigure(t);
 	if (ret != 0) {
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress
  2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
  2018-09-12 16:16 ` [dpdk-dev] [PATCH 2/3] event/sw: implement unlinks in progress function Harry van Haaren
  2018-09-12 16:16 ` [dpdk-dev] [PATCH 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
@ 2018-09-13 14:45 ` Jerin Jacob
  2018-09-18 12:05 ` Elo, Matias (Nokia - FI/Espoo)
  2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
  4 siblings, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2018-09-13 14:45 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, matias.elo

-----Original Message-----
> Date: Wed, 12 Sep 2018 17:16:14 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, matias.elo@nokia.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH 1/3] event: add function for reading unlink in progress
> X-Mailer: git-send-email 2.17.1
> 
> 
> This commit introduces a new function in the eventdev API,
> which allows applications to read the number of unlink requests
> in progress on a particular port of an eventdev instance.
> 
> This information allows applications to verify when no more packets
> from a particular queue (or any queue) will arrive at a port.
> The application could decide to stop polling, or put the core into
> a sleep state if it wishes, as it is ensured that no new packets
> will arrive at a particular port anymore if all queues are unlinked.
> 
> Suggested-by: Matias Elo <matias.elo@nokia.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> Cc: jerin.jacob@caviumnetworks.com
> 
> Hey, I've added this API as __rte_experimental, so we should be OK to
> include in 18.11, and then possibly remove the experimental tag in
> a later release, assuming it serves its purpose correctly.
> 
> For context, see thread here:
> http://mails.dpdk.org/archives/dev/2018-September/111499.html
> 
> @Matias, is that workable for you?
> @Jerin, is this acceptable as maintainer?


Yes.

Overall patch looks good to me. Please find inline a minor comment.

# There is build error in this series(not in this patch), please check error log

/export/dpdk-next-eventdev/drivers/event/sw/sw_evdev.c: In function
‘sw_port_unlinks_in_progress’:
/export/dpdk-next-eventdev/drivers/event/sw/sw_evdev.c:124:50: error:
unused parameter ‘dev’ [-Werror=unused-parameter]
 sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)


With above changes:

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

> 
> Cheers, -Harry
> ---
>  lib/librte_eventdev/rte_eventdev.c           | 22 +++++++++++++++
>  lib/librte_eventdev/rte_eventdev.h           | 28 ++++++++++++++++++--
>  lib/librte_eventdev/rte_eventdev_pmd.h       | 19 +++++++++++++
>  lib/librte_eventdev/rte_eventdev_version.map |  1 +
>  4 files changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index 801810edd..0a8572b7b 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -980,6 +980,28 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
>         return diag;
>  }
> 
> +int __rte_experimental
> +rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id)
> +{
> +       struct rte_eventdev *dev;
> +
> +       RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
> +       dev = &rte_eventdevs[dev_id];
> +       if (!is_valid_port(dev, port_id)) {
> +               RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
> +               return -EINVAL;
> +       }
> +
> +       /* Return 0 if the PMD does not implement unlinks in progress.
> +        * This allows PMDs which handle unlink synchronously to not implement
> +        * this function at all.
> +        */
> +       RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_unlinks_in_progress, 0);
> +
> +       return (*dev->dev_ops->port_unlinks_in_progress)(dev,
> +                       dev->data->ports[port_id]);
> +}
> +
>  int
>  rte_event_port_links_get(uint8_t dev_id, uint8_t port_id,
>                          uint8_t queues[], uint8_t priorities[])
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index b6fd6ee7f..d07e297bc 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -1656,8 +1656,9 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
>   * event port designated by its *port_id* on the event device designated
>   * by its *dev_id*.
>   *
> - * The unlink establishment shall disable the event port *port_id* from
> - * receiving events from the specified event queue *queue_id*
> + * The unlink call issues an async request to disable the event port *port_id*
> + * from receiving events from the specified event queue *queue_id*. See
> + * *rte_event_port_unlinks_in_progress* to poll for completed unlinks.

use @see rte_event_port_unlinks_in_progress() like reset of file.


>   *
>   * Event queue(s) to event port unlink establishment can be changed at runtime
>   * without re-configuring the device.
> @@ -1694,6 +1695,29 @@ int
>  rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
>                       uint8_t queues[], uint16_t nb_unlinks);
> 
> +/**
> + * Returns the number of unlinks in progress.
> + *
> + * This function provides the application with a method to detect when an
> + * unlink has been completed by the implementation. See *rte_event_port_unlink*
> + * on how to issue unlink requests.

Same as above comment.

> + *
> + * @param dev_id
> + *   The indentifier of the device.
> + *
> + * @param port_id
> + *   Event port identifier to select port to check for unlinks in progress.
> + *
> + * @return
> + * The number of unlinks that are in progress. A return of zero indicates that
> + * there are no outstanding unlink requests. A positive return value indicates
> + * the number of unlinks that are in progress, but are not yet complete.
> + * A negative return value indicates an error, -EINVAL indicates an invalid
> + * parameter passed for *dev_id* or *port_id*.
> + */
> +int __rte_experimental
> +rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id);
> +
>  /**
>   * Retrieve the list of source event queues and its associated service priority
>   * linked to the destination event port designated by its *port_id*
> diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
> index 3fbb4d2b2..65645730a 100644
> --- a/lib/librte_eventdev/rte_eventdev_pmd.h
> +++ b/lib/librte_eventdev/rte_eventdev_pmd.h
> @@ -332,6 +332,23 @@ typedef int (*eventdev_port_link_t)(struct rte_eventdev *dev, void *port,
>  typedef int (*eventdev_port_unlink_t)(struct rte_eventdev *dev, void *port,
>                 uint8_t queues[], uint16_t nb_unlinks);
> 

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

* Re: [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress
  2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
                   ` (2 preceding siblings ...)
  2018-09-13 14:45 ` [dpdk-dev] [PATCH 1/3] event: add function for reading unlink " Jerin Jacob
@ 2018-09-18 12:05 ` Elo, Matias (Nokia - FI/Espoo)
  2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
  4 siblings, 0 replies; 13+ messages in thread
From: Elo, Matias (Nokia - FI/Espoo) @ 2018-09-18 12:05 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, jerin.jacob

> 
> +/**
> + * Returns the number of unlinks in progress.
> + *
> + * This function provides the application with a method to detect when an
> + * unlink has been completed by the implementation. See *rte_event_port_unlink*
> + * on how to issue unlink requests.
> + *
> + * @param dev_id
> + *   The indentifier of the device.
> + *
> + * @param port_id
> + *   Event port identifier to select port to check for unlinks in progress.
> + *
> + * @return
> + * The number of unlinks that are in progress. A return of zero indicates that
> + * there are no outstanding unlink requests. A positive return value indicates
> + * the number of unlinks that are in progress, but are not yet complete.
> + * A negative return value indicates an error, -EINVAL indicates an invalid
> + * parameter passed for *dev_id* or *port_id*.
> + */
> +int __rte_experimental
> +rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id);
> +

Sorry for slow response (been out of office for the past week). This looks good to me and should solve my original problem.

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

* [dpdk-dev] [PATCH v2 1/3] event: add function for reading unlink in progress
  2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
                   ` (3 preceding siblings ...)
  2018-09-18 12:05 ` Elo, Matias (Nokia - FI/Espoo)
@ 2018-09-20 11:22 ` Harry van Haaren
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function Harry van Haaren
                     ` (2 more replies)
  4 siblings, 3 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-20 11:22 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit introduces a new function in the eventdev API,
which allows applications to read the number of unlink requests
in progress on a particular port of an eventdev instance.

This information allows applications to verify when no more packets
from a particular queue (or any queue) will arrive at a port.
The application could decide to stop polling, or put the core into
a sleep state if it wishes, as it is ensured that no new packets
will arrive at a particular port anymore if all queues are unlinked.

Suggested-by: Matias Elo <matias.elo@nokia.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com

---

v2:
- Fix @see function_name() syntax (Jerin)
- Add @warning to indicate experimental API in header
- Update unlink() return docs to state async behaviour
- Added Ack as per ML

Cheers, -Harry
---
 lib/librte_eventdev/rte_eventdev.c           | 22 +++++++++++
 lib/librte_eventdev/rte_eventdev.h           | 39 +++++++++++++++++---
 lib/librte_eventdev/rte_eventdev_pmd.h       | 19 ++++++++++
 lib/librte_eventdev/rte_eventdev_version.map |  1 +
 4 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index 801810edd..0a8572b7b 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -980,6 +980,28 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 	return diag;
 }
 
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id)
+{
+	struct rte_eventdev *dev;
+
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	dev = &rte_eventdevs[dev_id];
+	if (!is_valid_port(dev, port_id)) {
+		RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
+		return -EINVAL;
+	}
+
+	/* Return 0 if the PMD does not implement unlinks in progress.
+	 * This allows PMDs which handle unlink synchronously to not implement
+	 * this function at all.
+	 */
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_unlinks_in_progress, 0);
+
+	return (*dev->dev_ops->port_unlinks_in_progress)(dev,
+			dev->data->ports[port_id]);
+}
+
 int
 rte_event_port_links_get(uint8_t dev_id, uint8_t port_id,
 			 uint8_t queues[], uint8_t priorities[])
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index b6fd6ee7f..a24213ea7 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1656,12 +1656,13 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
  * event port designated by its *port_id* on the event device designated
  * by its *dev_id*.
  *
- * The unlink establishment shall disable the event port *port_id* from
- * receiving events from the specified event queue *queue_id*
- *
+ * The unlink call issues an async request to disable the event port *port_id*
+ * from receiving events from the specified event queue *queue_id*.
  * Event queue(s) to event port unlink establishment can be changed at runtime
  * without re-configuring the device.
  *
+ * @see rte_event_port_unlinks_in_progress() to poll for completed unlinks.
+ *
  * @param dev_id
  *   The identifier of the device.
  *
@@ -1679,21 +1680,47 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
  *   NULL.
  *
  * @return
- * The number of unlinks actually established. The return value can be less
+ * The number of unlinks successfully requested. The return value can be less
  * than the value of the *nb_unlinks* parameter when the implementation has the
  * limitation on specific queue to port unlink establishment or
  * if invalid parameters are specified.
  * If the return value is less than *nb_unlinks*, the remaining queues at the
- * end of queues[] are not established, and the caller has to take care of them.
+ * end of queues[] are not unlinked, and the caller has to take care of them.
  * If return value is less than *nb_unlinks* then implementation shall update
  * the rte_errno accordingly, Possible rte_errno values are
  * (-EINVAL) Invalid parameter
- *
  */
 int
 rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 		      uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Returns the number of unlinks in progress.
+ *
+ * This function provides the application with a method to detect when an
+ * unlink has been completed by the implementation.
+ *
+ * @see rte_event_port_unlink() to issue unlink requests.
+ *
+ * @param dev_id
+ *   The indentifier of the device.
+ *
+ * @param port_id
+ *   Event port identifier to select port to check for unlinks in progress.
+ *
+ * @return
+ * The number of unlinks that are in progress. A return of zero indicates that
+ * there are no outstanding unlink requests. A positive return value indicates
+ * the number of unlinks that are in progress, but are not yet complete.
+ * A negative return value indicates an error, -EINVAL indicates an invalid
+ * parameter passed for *dev_id* or *port_id*.
+ */
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id);
+
 /**
  * Retrieve the list of source event queues and its associated service priority
  * linked to the destination event port designated by its *port_id*
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 3fbb4d2b2..65645730a 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -332,6 +332,23 @@ typedef int (*eventdev_port_link_t)(struct rte_eventdev *dev, void *port,
 typedef int (*eventdev_port_unlink_t)(struct rte_eventdev *dev, void *port,
 		uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * Unlinks in progress. Returns number of unlinks that the PMD is currently
+ * performing, but have not yet been completed.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param port
+ *   Event port pointer
+ *
+ * @return
+ *   Returns the number of in-progress unlinks. Zero is returned if none are
+ *   in progress.
+ */
+typedef int (*eventdev_port_unlinks_in_progress_t)(struct rte_eventdev *dev,
+		void *port);
+
 /**
  * Converts nanoseconds to *timeout_ticks* value for rte_event_dequeue()
  *
@@ -815,6 +832,8 @@ struct rte_eventdev_ops {
 	/**< Link event queues to an event port. */
 	eventdev_port_unlink_t port_unlink;
 	/**< Unlink event queues from an event port. */
+	eventdev_port_unlinks_in_progress_t port_unlinks_in_progress;
+	/**< Unlinks in progress on an event port. */
 	eventdev_dequeue_timeout_ticks_t timeout_ticks;
 	/**< Converts ns to *timeout_ticks* value for rte_event_dequeue() */
 	eventdev_dump_t dump;
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 12835e9f2..24e7a45c0 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -96,6 +96,7 @@ EXPERIMENTAL {
 	rte_event_crypto_adapter_stats_reset;
 	rte_event_crypto_adapter_stop;
 	rte_event_eth_rx_adapter_cb_register;
+	rte_event_port_unlinks_in_progress;
 	rte_event_timer_adapter_caps_get;
 	rte_event_timer_adapter_create;
 	rte_event_timer_adapter_create_ext;
-- 
2.17.1

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

* [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function
  2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
@ 2018-09-20 11:22   ` Harry van Haaren
  2018-09-23 11:08     ` Jerin Jacob
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
  2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
  2 siblings, 1 reply; 13+ messages in thread
From: Harry van Haaren @ 2018-09-20 11:22 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit adds a counter to each port, which counts the
number of unlinks that have been performed. When the scheduler
thread starts its scheduling routine, it "acks" all unlinks that
have been requested, and the application is gauranteed that no
more events will be scheduled to the port from the unlinked queue.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Fix unused "dev" variable (Jerin)
---
 drivers/event/sw/sw_evdev.c           | 12 ++++++++++++
 drivers/event/sw/sw_evdev.h           |  8 ++++++++
 drivers/event/sw/sw_evdev_scheduler.c |  7 ++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index a6bb91388..9e1412537 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -113,9 +113,20 @@ sw_port_unlink(struct rte_eventdev *dev, void *port, uint8_t queues[],
 			}
 		}
 	}
+
+	p->unlinks_in_progress += unlinked;
+	rte_smp_mb();
+
 	return unlinked;
 }
 
+static int
+sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)
+{
+	struct sw_port *p = port;
+	return p->unlinks_in_progress;
+}
+
 static int
 sw_port_setup(struct rte_eventdev *dev, uint8_t port_id,
 		const struct rte_event_port_conf *conf)
@@ -925,6 +936,7 @@ sw_probe(struct rte_vdev_device *vdev)
 			.port_release = sw_port_release,
 			.port_link = sw_port_link,
 			.port_unlink = sw_port_unlink,
+			.port_unlinks_in_progress = sw_port_unlinks_in_progress,
 
 			.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
 
diff --git a/drivers/event/sw/sw_evdev.h b/drivers/event/sw/sw_evdev.h
index d90b96d4b..7c77b2495 100644
--- a/drivers/event/sw/sw_evdev.h
+++ b/drivers/event/sw/sw_evdev.h
@@ -148,6 +148,14 @@ struct sw_port {
 	/* A numeric ID for the port */
 	uint8_t id;
 
+	/* An atomic counter for when the port has been unlinked, and the
+	 * scheduler has not yet acked this unlink - hence there may still be
+	 * events in the buffers going to the port. When the unlinks in
+	 * progress is read by the scheduler, no more events will be pushed to
+	 * the port - hence the scheduler core can just assign zero.
+	 */
+	uint8_t unlinks_in_progress;
+
 	int16_t is_directed; /** Takes from a single directed QID */
 	/**
 	 * For loadbalanced we can optimise pulling packets from
diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e02f..9b54d5ce7 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -517,13 +517,18 @@ sw_event_schedule(struct rte_eventdev *dev)
 		/* Pull from rx_ring for ports */
 		do {
 			in_pkts = 0;
-			for (i = 0; i < sw->port_count; i++)
+			for (i = 0; i < sw->port_count; i++) {
+				/* ack the unlinks in progress as done */
+				if (sw->ports[i].unlinks_in_progress)
+					sw->ports[i].unlinks_in_progress = 0;
+
 				if (sw->ports[i].is_directed)
 					in_pkts += sw_schedule_pull_port_dir(sw, i);
 				else if (sw->ports[i].num_ordered_qids > 0)
 					in_pkts += sw_schedule_pull_port_lb(sw, i);
 				else
 					in_pkts += sw_schedule_pull_port_no_reorder(sw, i);
+			}
 
 			/* QID scan for re-ordered */
 			in_pkts += sw_schedule_reorder(sw, 0,
-- 
2.17.1

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

* [dpdk-dev] [PATCH v2 3/3] event/sw: add unit test for unlinks in progress
  2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function Harry van Haaren
@ 2018-09-20 11:22   ` Harry van Haaren
  2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
  2 siblings, 0 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-20 11:22 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, matias.elo, Harry van Haaren

This commit adds a unit test that checks the behaviour
of the unlinks_in_progress() function, ensuring that the
returned values are the number of unlinks requested,
until the scheduler runs and "acks" the requests, after
which the count should be zero again.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Add print before running unlink test (Harry)
---
 drivers/event/sw/sw_evdev.c          |  1 +
 drivers/event/sw/sw_evdev_selftest.c | 77 ++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index 9e1412537..1175d6cdb 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -123,6 +123,7 @@ sw_port_unlink(struct rte_eventdev *dev, void *port, uint8_t queues[],
 static int
 sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)
 {
+	RTE_SET_USED(dev);
 	struct sw_port *p = port;
 	return p->unlinks_in_progress;
 }
diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c
index c40912db5..d00d5de61 100644
--- a/drivers/event/sw/sw_evdev_selftest.c
+++ b/drivers/event/sw/sw_evdev_selftest.c
@@ -1903,6 +1903,77 @@ qid_priorities(struct test *t)
 	return 0;
 }
 
+static int
+unlink_in_progress(struct test *t)
+{
+	/* Test unlinking API, in particular that when an unlink request has
+	 * not yet been seen by the scheduler thread, that the
+	 * unlink_in_progress() function returns the number of unlinks.
+	 */
+	unsigned int i;
+	/* Create instance with 1 ports, and 3 qids */
+	if (init(t, 3, 1) < 0 ||
+			create_ports(t, 1) < 0) {
+		printf("%d: Error initializing device\n", __LINE__);
+		return -1;
+	}
+
+	for (i = 0; i < 3; i++) {
+		/* Create QID */
+		const struct rte_event_queue_conf conf = {
+			.schedule_type = RTE_SCHED_TYPE_ATOMIC,
+			/* increase priority (0 == highest), as we go */
+			.priority = RTE_EVENT_DEV_PRIORITY_NORMAL - i,
+			.nb_atomic_flows = 1024,
+			.nb_atomic_order_sequences = 1024,
+		};
+
+		if (rte_event_queue_setup(evdev, i, &conf) < 0) {
+			printf("%d: error creating qid %d\n", __LINE__, i);
+			return -1;
+		}
+		t->qid[i] = i;
+	}
+	t->nb_qids = i;
+	/* map all QIDs to port */
+	rte_event_port_link(evdev, t->port[0], NULL, NULL, 0);
+
+	if (rte_event_dev_start(evdev) < 0) {
+		printf("%d: Error with start call\n", __LINE__);
+		return -1;
+	}
+
+	/* unlink all ports to have outstanding unlink requests */
+	int ret = rte_event_port_unlink(evdev, t->port[0], NULL, 0);
+	if (ret < 0) {
+		printf("%d: Failed to unlink queues\n", __LINE__);
+		return -1;
+	}
+
+	/* get active unlinks here, expect 3 */
+	int unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 3) {
+		printf("%d: Expected num unlinks in progress == 3, got %d\n",
+				__LINE__, unlinks_in_progress);
+		return -1;
+	}
+
+	/* run scheduler service on this thread to ack the unlinks */
+	rte_service_run_iter_on_app_lcore(t->service_id, 1);
+
+	/* active unlinks expected as 0 as scheduler thread has acked */
+	unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 0) {
+		printf("%d: Expected num unlinks in progress == 0, got %d\n",
+				__LINE__, unlinks_in_progress);
+	}
+
+	cleanup(t);
+	return 0;
+}
+
 static int
 load_balancing(struct test *t)
 {
@@ -3260,6 +3331,12 @@ test_sw_eventdev(void)
 		printf("ERROR - QID Priority test FAILED.\n");
 		goto test_fail;
 	}
+	printf("*** Running Unlink-in-progress test...\n");
+	ret = unlink_in_progress(t);
+	if (ret != 0) {
+		printf("ERROR - Unlink in progress test FAILED.\n");
+		goto test_fail;
+	}
 	printf("*** Running Ordered Reconfigure test...\n");
 	ret = ordered_reconfigure(t);
 	if (ret != 0) {
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function Harry van Haaren
@ 2018-09-23 11:08     ` Jerin Jacob
  0 siblings, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2018-09-23 11:08 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, matias.elo

-----Original Message-----
> Date: Thu, 20 Sep 2018 12:22:50 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, matias.elo@nokia.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v2 2/3] event/sw: implement unlinks in progress function
> X-Mailer: git-send-email 2.17.1
> 
> 
> This commit adds a counter to each port, which counts the
> number of unlinks that have been performed. When the scheduler
> thread starts its scheduling routine, it "acks" all unlinks that
> have been requested, and the application is gauranteed that no
> more events will be scheduled to the port from the unlinked queue.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> 
> +static int
> +sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)
> +{
> +       struct sw_port *p = port;
> +       return p->unlinks_in_progress;

Compilation error:

/export/dpdk-next-eventdev/drivers/event/sw/sw_evdev.c: In function
‘sw_port_unlinks_in_progress’:
/export/dpdk-next-eventdev/drivers/event/sw/sw_evdev.c:124:50: error:
unused parameter ‘dev’ [-Werror=unused-parameter]
 sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)



> +}
> +

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

* [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink in progress
  2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function Harry van Haaren
  2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
@ 2018-09-24  8:23   ` Harry van Haaren
  2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 2/3] event/sw: implement unlinks in progress function Harry van Haaren
                       ` (2 more replies)
  2 siblings, 3 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-24  8:23 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, Harry van Haaren

This commit introduces a new function in the eventdev API,
which allows applications to read the number of unlink requests
in progress on a particular port of an eventdev instance.

This information allows applications to verify when no more packets
from a particular queue (or any queue) will arrive at a port.
The application could decide to stop polling, or put the core into
a sleep state if it wishes, as it is ensured that no new packets
will arrive at a particular port anymore if all queues are unlinked.

Suggested-by: Matias Elo <matias.elo@nokia.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

---

v3:
- Fix ack (was missing a > symbol) (Checkpatch email report)

v2:
- Fix @see function_name() syntax (Jerin)
- Add @warning to indicate experimental API in header
- Update unlink() return docs to state async behaviour
- Added Ack as per ML

Cheers, -Harry
---
 lib/librte_eventdev/rte_eventdev.c           | 22 +++++++++++
 lib/librte_eventdev/rte_eventdev.h           | 39 +++++++++++++++++---
 lib/librte_eventdev/rte_eventdev_pmd.h       | 19 ++++++++++
 lib/librte_eventdev/rte_eventdev_version.map |  1 +
 4 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index 801810edd..0a8572b7b 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -980,6 +980,28 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 	return diag;
 }
 
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id)
+{
+	struct rte_eventdev *dev;
+
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	dev = &rte_eventdevs[dev_id];
+	if (!is_valid_port(dev, port_id)) {
+		RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
+		return -EINVAL;
+	}
+
+	/* Return 0 if the PMD does not implement unlinks in progress.
+	 * This allows PMDs which handle unlink synchronously to not implement
+	 * this function at all.
+	 */
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_unlinks_in_progress, 0);
+
+	return (*dev->dev_ops->port_unlinks_in_progress)(dev,
+			dev->data->ports[port_id]);
+}
+
 int
 rte_event_port_links_get(uint8_t dev_id, uint8_t port_id,
 			 uint8_t queues[], uint8_t priorities[])
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index b6fd6ee7f..a24213ea7 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1656,12 +1656,13 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
  * event port designated by its *port_id* on the event device designated
  * by its *dev_id*.
  *
- * The unlink establishment shall disable the event port *port_id* from
- * receiving events from the specified event queue *queue_id*
- *
+ * The unlink call issues an async request to disable the event port *port_id*
+ * from receiving events from the specified event queue *queue_id*.
  * Event queue(s) to event port unlink establishment can be changed at runtime
  * without re-configuring the device.
  *
+ * @see rte_event_port_unlinks_in_progress() to poll for completed unlinks.
+ *
  * @param dev_id
  *   The identifier of the device.
  *
@@ -1679,21 +1680,47 @@ rte_event_port_link(uint8_t dev_id, uint8_t port_id,
  *   NULL.
  *
  * @return
- * The number of unlinks actually established. The return value can be less
+ * The number of unlinks successfully requested. The return value can be less
  * than the value of the *nb_unlinks* parameter when the implementation has the
  * limitation on specific queue to port unlink establishment or
  * if invalid parameters are specified.
  * If the return value is less than *nb_unlinks*, the remaining queues at the
- * end of queues[] are not established, and the caller has to take care of them.
+ * end of queues[] are not unlinked, and the caller has to take care of them.
  * If return value is less than *nb_unlinks* then implementation shall update
  * the rte_errno accordingly, Possible rte_errno values are
  * (-EINVAL) Invalid parameter
- *
  */
 int
 rte_event_port_unlink(uint8_t dev_id, uint8_t port_id,
 		      uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Returns the number of unlinks in progress.
+ *
+ * This function provides the application with a method to detect when an
+ * unlink has been completed by the implementation.
+ *
+ * @see rte_event_port_unlink() to issue unlink requests.
+ *
+ * @param dev_id
+ *   The indentifier of the device.
+ *
+ * @param port_id
+ *   Event port identifier to select port to check for unlinks in progress.
+ *
+ * @return
+ * The number of unlinks that are in progress. A return of zero indicates that
+ * there are no outstanding unlink requests. A positive return value indicates
+ * the number of unlinks that are in progress, but are not yet complete.
+ * A negative return value indicates an error, -EINVAL indicates an invalid
+ * parameter passed for *dev_id* or *port_id*.
+ */
+int __rte_experimental
+rte_event_port_unlinks_in_progress(uint8_t dev_id, uint8_t port_id);
+
 /**
  * Retrieve the list of source event queues and its associated service priority
  * linked to the destination event port designated by its *port_id*
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 3fbb4d2b2..65645730a 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -332,6 +332,23 @@ typedef int (*eventdev_port_link_t)(struct rte_eventdev *dev, void *port,
 typedef int (*eventdev_port_unlink_t)(struct rte_eventdev *dev, void *port,
 		uint8_t queues[], uint16_t nb_unlinks);
 
+/**
+ * Unlinks in progress. Returns number of unlinks that the PMD is currently
+ * performing, but have not yet been completed.
+ *
+ * @param dev
+ *   Event device pointer
+ *
+ * @param port
+ *   Event port pointer
+ *
+ * @return
+ *   Returns the number of in-progress unlinks. Zero is returned if none are
+ *   in progress.
+ */
+typedef int (*eventdev_port_unlinks_in_progress_t)(struct rte_eventdev *dev,
+		void *port);
+
 /**
  * Converts nanoseconds to *timeout_ticks* value for rte_event_dequeue()
  *
@@ -815,6 +832,8 @@ struct rte_eventdev_ops {
 	/**< Link event queues to an event port. */
 	eventdev_port_unlink_t port_unlink;
 	/**< Unlink event queues from an event port. */
+	eventdev_port_unlinks_in_progress_t port_unlinks_in_progress;
+	/**< Unlinks in progress on an event port. */
 	eventdev_dequeue_timeout_ticks_t timeout_ticks;
 	/**< Converts ns to *timeout_ticks* value for rte_event_dequeue() */
 	eventdev_dump_t dump;
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 12835e9f2..24e7a45c0 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -96,6 +96,7 @@ EXPERIMENTAL {
 	rte_event_crypto_adapter_stats_reset;
 	rte_event_crypto_adapter_stop;
 	rte_event_eth_rx_adapter_cb_register;
+	rte_event_port_unlinks_in_progress;
 	rte_event_timer_adapter_caps_get;
 	rte_event_timer_adapter_create;
 	rte_event_timer_adapter_create_ext;
-- 
2.17.1

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

* [dpdk-dev] [PATCH v3 2/3] event/sw: implement unlinks in progress function
  2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
@ 2018-09-24  8:23     ` Harry van Haaren
  2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
  2018-09-24 14:56     ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Jerin Jacob
  2 siblings, 0 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-24  8:23 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, Harry van Haaren

This commit adds a counter to each port, which counts the
number of unlinks that have been performed. When the scheduler
thread starts its scheduling routine, it "acks" all unlinks that
have been requested, and the application is gauranteed that no
more events will be scheduled to the port from the unlinked queue.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v3:
- Move RTE_SET_USED() to correct patch (Jerin)

v2:
- Fix unused "dev" variable (Jerin)
---
 drivers/event/sw/sw_evdev.c           | 13 +++++++++++++
 drivers/event/sw/sw_evdev.h           |  8 ++++++++
 drivers/event/sw/sw_evdev_scheduler.c |  7 ++++++-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index a6bb91388..1175d6cdb 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -113,9 +113,21 @@ sw_port_unlink(struct rte_eventdev *dev, void *port, uint8_t queues[],
 			}
 		}
 	}
+
+	p->unlinks_in_progress += unlinked;
+	rte_smp_mb();
+
 	return unlinked;
 }
 
+static int
+sw_port_unlinks_in_progress(struct rte_eventdev *dev, void *port)
+{
+	RTE_SET_USED(dev);
+	struct sw_port *p = port;
+	return p->unlinks_in_progress;
+}
+
 static int
 sw_port_setup(struct rte_eventdev *dev, uint8_t port_id,
 		const struct rte_event_port_conf *conf)
@@ -925,6 +937,7 @@ sw_probe(struct rte_vdev_device *vdev)
 			.port_release = sw_port_release,
 			.port_link = sw_port_link,
 			.port_unlink = sw_port_unlink,
+			.port_unlinks_in_progress = sw_port_unlinks_in_progress,
 
 			.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
 
diff --git a/drivers/event/sw/sw_evdev.h b/drivers/event/sw/sw_evdev.h
index d90b96d4b..7c77b2495 100644
--- a/drivers/event/sw/sw_evdev.h
+++ b/drivers/event/sw/sw_evdev.h
@@ -148,6 +148,14 @@ struct sw_port {
 	/* A numeric ID for the port */
 	uint8_t id;
 
+	/* An atomic counter for when the port has been unlinked, and the
+	 * scheduler has not yet acked this unlink - hence there may still be
+	 * events in the buffers going to the port. When the unlinks in
+	 * progress is read by the scheduler, no more events will be pushed to
+	 * the port - hence the scheduler core can just assign zero.
+	 */
+	uint8_t unlinks_in_progress;
+
 	int16_t is_directed; /** Takes from a single directed QID */
 	/**
 	 * For loadbalanced we can optimise pulling packets from
diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e02f..9b54d5ce7 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -517,13 +517,18 @@ sw_event_schedule(struct rte_eventdev *dev)
 		/* Pull from rx_ring for ports */
 		do {
 			in_pkts = 0;
-			for (i = 0; i < sw->port_count; i++)
+			for (i = 0; i < sw->port_count; i++) {
+				/* ack the unlinks in progress as done */
+				if (sw->ports[i].unlinks_in_progress)
+					sw->ports[i].unlinks_in_progress = 0;
+
 				if (sw->ports[i].is_directed)
 					in_pkts += sw_schedule_pull_port_dir(sw, i);
 				else if (sw->ports[i].num_ordered_qids > 0)
 					in_pkts += sw_schedule_pull_port_lb(sw, i);
 				else
 					in_pkts += sw_schedule_pull_port_no_reorder(sw, i);
+			}
 
 			/* QID scan for re-ordered */
 			in_pkts += sw_schedule_reorder(sw, 0,
-- 
2.17.1

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

* [dpdk-dev] [PATCH v3 3/3] event/sw: add unit test for unlinks in progress
  2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
  2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 2/3] event/sw: implement unlinks in progress function Harry van Haaren
@ 2018-09-24  8:23     ` Harry van Haaren
  2018-09-24 14:56     ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Jerin Jacob
  2 siblings, 0 replies; 13+ messages in thread
From: Harry van Haaren @ 2018-09-24  8:23 UTC (permalink / raw)
  To: dev; +Cc: jerin.jacob, Harry van Haaren

This commit adds a unit test that checks the behaviour
of the unlinks_in_progress() function, ensuring that the
returned values are the number of unlinks requested,
until the scheduler runs and "acks" the requests, after
which the count should be zero again.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v3:
- Move RTE_SET_USED() to correct patch (Jerin)

v2:
- Add print before running unlink test (Harry)
---
 drivers/event/sw/sw_evdev_selftest.c | 77 ++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c
index c40912db5..d00d5de61 100644
--- a/drivers/event/sw/sw_evdev_selftest.c
+++ b/drivers/event/sw/sw_evdev_selftest.c
@@ -1903,6 +1903,77 @@ qid_priorities(struct test *t)
 	return 0;
 }
 
+static int
+unlink_in_progress(struct test *t)
+{
+	/* Test unlinking API, in particular that when an unlink request has
+	 * not yet been seen by the scheduler thread, that the
+	 * unlink_in_progress() function returns the number of unlinks.
+	 */
+	unsigned int i;
+	/* Create instance with 1 ports, and 3 qids */
+	if (init(t, 3, 1) < 0 ||
+			create_ports(t, 1) < 0) {
+		printf("%d: Error initializing device\n", __LINE__);
+		return -1;
+	}
+
+	for (i = 0; i < 3; i++) {
+		/* Create QID */
+		const struct rte_event_queue_conf conf = {
+			.schedule_type = RTE_SCHED_TYPE_ATOMIC,
+			/* increase priority (0 == highest), as we go */
+			.priority = RTE_EVENT_DEV_PRIORITY_NORMAL - i,
+			.nb_atomic_flows = 1024,
+			.nb_atomic_order_sequences = 1024,
+		};
+
+		if (rte_event_queue_setup(evdev, i, &conf) < 0) {
+			printf("%d: error creating qid %d\n", __LINE__, i);
+			return -1;
+		}
+		t->qid[i] = i;
+	}
+	t->nb_qids = i;
+	/* map all QIDs to port */
+	rte_event_port_link(evdev, t->port[0], NULL, NULL, 0);
+
+	if (rte_event_dev_start(evdev) < 0) {
+		printf("%d: Error with start call\n", __LINE__);
+		return -1;
+	}
+
+	/* unlink all ports to have outstanding unlink requests */
+	int ret = rte_event_port_unlink(evdev, t->port[0], NULL, 0);
+	if (ret < 0) {
+		printf("%d: Failed to unlink queues\n", __LINE__);
+		return -1;
+	}
+
+	/* get active unlinks here, expect 3 */
+	int unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 3) {
+		printf("%d: Expected num unlinks in progress == 3, got %d\n",
+				__LINE__, unlinks_in_progress);
+		return -1;
+	}
+
+	/* run scheduler service on this thread to ack the unlinks */
+	rte_service_run_iter_on_app_lcore(t->service_id, 1);
+
+	/* active unlinks expected as 0 as scheduler thread has acked */
+	unlinks_in_progress =
+		rte_event_port_unlinks_in_progress(evdev, t->port[0]);
+	if (unlinks_in_progress != 0) {
+		printf("%d: Expected num unlinks in progress == 0, got %d\n",
+				__LINE__, unlinks_in_progress);
+	}
+
+	cleanup(t);
+	return 0;
+}
+
 static int
 load_balancing(struct test *t)
 {
@@ -3260,6 +3331,12 @@ test_sw_eventdev(void)
 		printf("ERROR - QID Priority test FAILED.\n");
 		goto test_fail;
 	}
+	printf("*** Running Unlink-in-progress test...\n");
+	ret = unlink_in_progress(t);
+	if (ret != 0) {
+		printf("ERROR - Unlink in progress test FAILED.\n");
+		goto test_fail;
+	}
 	printf("*** Running Ordered Reconfigure test...\n");
 	ret = ordered_reconfigure(t);
 	if (ret != 0) {
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink in progress
  2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
  2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 2/3] event/sw: implement unlinks in progress function Harry van Haaren
  2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
@ 2018-09-24 14:56     ` Jerin Jacob
  2 siblings, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2018-09-24 14:56 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev

-----Original Message-----
> Date: Mon, 24 Sep 2018 09:23:31 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v3 1/3] event: add function for reading unlink in progress
> X-Mailer: git-send-email 2.17.1
> 
> External Email
> 
> This commit introduces a new function in the eventdev API,
> which allows applications to read the number of unlink requests
> in progress on a particular port of an eventdev instance.
> 
> This information allows applications to verify when no more packets
> from a particular queue (or any queue) will arrive at a port.
> The application could decide to stop polling, or put the core into
> a sleep state if it wishes, as it is ensured that no new packets
> will arrive at a particular port anymore if all queues are unlinked.
> 
> Suggested-by: Matias Elo <matias.elo@nokia.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

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

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

end of thread, other threads:[~2018-09-24 14:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 16:16 [dpdk-dev] [PATCH 1/3] event: add function for reading unlink in progress Harry van Haaren
2018-09-12 16:16 ` [dpdk-dev] [PATCH 2/3] event/sw: implement unlinks in progress function Harry van Haaren
2018-09-12 16:16 ` [dpdk-dev] [PATCH 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
2018-09-13 14:45 ` [dpdk-dev] [PATCH 1/3] event: add function for reading unlink " Jerin Jacob
2018-09-18 12:05 ` Elo, Matias (Nokia - FI/Espoo)
2018-09-20 11:22 ` [dpdk-dev] [PATCH v2 " Harry van Haaren
2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 2/3] event/sw: implement unlinks in progress function Harry van Haaren
2018-09-23 11:08     ` Jerin Jacob
2018-09-20 11:22   ` [dpdk-dev] [PATCH v2 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
2018-09-24  8:23   ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Harry van Haaren
2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 2/3] event/sw: implement unlinks in progress function Harry van Haaren
2018-09-24  8:23     ` [dpdk-dev] [PATCH v3 3/3] event/sw: add unit test for unlinks in progress Harry van Haaren
2018-09-24 14:56     ` [dpdk-dev] [PATCH v3 1/3] event: add function for reading unlink " Jerin Jacob

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).