DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] eal/windows: implement alarms
@ 2020-09-11  0:22 Dmitry Kozlyuk
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-11  0:22 UTC (permalink / raw)
  To: dev; +Cc: Dmitry Kozlyuk

This patchset provides EAL alarm support for Windows. Basic interrupt
thread code is added to monitor alarm events. It doesn't include
callback management, because Windows alarms, unlike Unix EALs, rely on
the OS for callback execution scheduling.

Dmitry Kozlyuk (2):
  eal/windows: add interrupt thread skeleton
  eal/windows: implement alarm API

 lib/librte_eal/include/rte_eal_interrupts.h |  14 +-
 lib/librte_eal/rte_eal_exports.def          |   3 +
 lib/librte_eal/windows/eal.c                |   5 +
 lib/librte_eal/windows/eal_alarm.c          | 219 ++++++++++++++++++++
 lib/librte_eal/windows/eal_interrupts.c     |  99 +++++++++
 lib/librte_eal/windows/eal_windows.h        |  12 ++
 lib/librte_eal/windows/include/pthread.h    |   7 +
 lib/librte_eal/windows/meson.build          |   2 +
 8 files changed, 358 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_eal/windows/eal_alarm.c
 create mode 100644 lib/librte_eal/windows/eal_interrupts.c

-- 
2.25.4


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

* [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton
  2020-09-11  0:22 [dpdk-dev] [PATCH 0/2] eal/windows: implement alarms Dmitry Kozlyuk
@ 2020-09-11  0:22 ` Dmitry Kozlyuk
  2020-09-25  1:19   ` Narcisa Ana Maria Vasile
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
  2020-09-25 23:32 ` [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms Dmitry Kozlyuk
  2 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-11  0:22 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, Harman Kalra, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam

Windows interrupt support is based on IO completion ports (IOCP).
Interrupt thread would send the devices requests to notify about
interrupts and then wait for any request completion. Add skeleton code
of this model without any hardware support.

Another way to wake up the interrupt thread is APC (asynchronous procedure
call), scheduled by any other thread via eal_intr_thread_schedule().
This internal API is intended for alarm implementation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/include/rte_eal_interrupts.h | 14 ++-
 lib/librte_eal/rte_eal_exports.def          |  1 +
 lib/librte_eal/windows/eal.c                |  5 ++
 lib/librte_eal/windows/eal_interrupts.c     | 99 +++++++++++++++++++++
 lib/librte_eal/windows/eal_windows.h        | 12 +++
 lib/librte_eal/windows/include/pthread.h    |  7 ++
 lib/librte_eal/windows/meson.build          |  1 +
 7 files changed, 136 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_eal/windows/eal_interrupts.c

diff --git a/lib/librte_eal/include/rte_eal_interrupts.h b/lib/librte_eal/include/rte_eal_interrupts.h
index b1e8a2934..b80edfc65 100644
--- a/lib/librte_eal/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/include/rte_eal_interrupts.h
@@ -69,10 +69,18 @@ struct rte_epoll_event {
 struct rte_intr_handle {
 	RTE_STD_C11
 	union {
-		int vfio_dev_fd;  /**< VFIO device file descriptor */
-		int uio_cfg_fd;  /**< UIO cfg file desc for uio_pci_generic */
+		struct {
+			RTE_STD_C11
+			union {
+				/** VFIO device file descriptor */
+				int vfio_dev_fd;
+				/** UIO cfg file desc for uio_pci_generic */
+				int uio_cfg_fd;
+			};
+			int fd;	/**< interrupt event file descriptor */
+		};
+		void *handle; /**< device driver handle (Windows) */
 	};
-	int fd;	 /**< interrupt event file descriptor */
 	enum rte_intr_handle_type type;  /**< handle type */
 	uint32_t max_intr;             /**< max interrupt requested */
 	uint32_t nb_efd;               /**< number of available efd(event fd) */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index f54ed74a5..9baca0110 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -72,6 +72,7 @@ EXPORTS
 	rte_vlog
 	rte_realloc
 	rte_strscpy
+	rte_thread_is_intr
 	rte_zmalloc
 	rte_zmalloc_socket
 
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index bc48f27ab..141f22adb 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -344,6 +344,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (rte_eal_intr_init() < 0) {
+		rte_eal_init_alert("Cannot init interrupt-handling thread");
+		return -1;
+	}
+
 	if (rte_eal_timer_init() < 0) {
 		rte_eal_init_alert("Cannot init TSC timer");
 		rte_errno = EFAULT;
diff --git a/lib/librte_eal/windows/eal_interrupts.c b/lib/librte_eal/windows/eal_interrupts.c
new file mode 100644
index 000000000..d9bc5afc7
--- /dev/null
+++ b/lib/librte_eal/windows/eal_interrupts.c
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <rte_interrupts.h>
+
+#include "eal_private.h"
+#include "eal_windows.h"
+
+static pthread_t intr_thread;
+
+static HANDLE intr_iocp;
+
+static void
+eal_intr_process(const OVERLAPPED_ENTRY *event)
+{
+	RTE_SET_USED(event);
+}
+
+static void *
+eal_intr_thread_main(LPVOID arg __rte_unused)
+{
+	while (1) {
+		OVERLAPPED_ENTRY events[16];
+		ULONG event_count, i;
+		BOOL result;
+
+		result = GetQueuedCompletionStatusEx(
+			intr_iocp, events, RTE_DIM(events), &event_count,
+			INFINITE, /* no timeout */
+			TRUE);    /* alertable wait for alarm APCs */
+
+		if (!result) {
+			DWORD error = GetLastError();
+			if (error != WAIT_IO_COMPLETION) {
+				RTE_LOG_WIN32_ERR("GetQueuedCompletionStatusEx()");
+				RTE_LOG(ERR, EAL, "Failed waiting for interrupts\n");
+				break;
+			}
+
+			/* No I/O events, all work is done in completed APCs. */
+			continue;
+		}
+
+		for (i = 0; i < event_count; i++)
+			eal_intr_process(&events[i]);
+	}
+
+	CloseHandle(intr_iocp);
+	intr_iocp = NULL;
+	return NULL;
+}
+
+int
+rte_eal_intr_init(void)
+{
+	int ret = 0;
+
+	intr_iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
+	if (intr_iocp == NULL) {
+		RTE_LOG_WIN32_ERR("CreateIoCompletionPort()");
+		RTE_LOG(ERR, EAL, "Cannot create interrupt IOCP\n");
+		return -1;
+	}
+
+	ret = rte_ctrl_thread_create(&intr_thread, "eal-intr-thread", NULL,
+			eal_intr_thread_main, NULL);
+	if (ret != 0) {
+		rte_errno = -ret;
+		RTE_LOG(ERR, EAL, "Cannot create interrupt thread\n");
+	}
+
+	return ret;
+}
+
+int
+rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
+
+int
+eal_intr_thread_schedule(void (*func)(void *arg), void *arg)
+{
+	HANDLE handle;
+
+	handle = OpenThread(THREAD_ALL_ACCESS, FALSE, intr_thread);
+	if (handle == NULL) {
+		RTE_LOG_WIN32_ERR("OpenThread(%llu)", intr_thread);
+		return -ENOENT;
+	}
+
+	if (!QueueUserAPC((PAPCFUNC)(ULONG_PTR)func, handle, (ULONG_PTR)arg)) {
+		RTE_LOG_WIN32_ERR("QueueUserAPC()");
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/lib/librte_eal/windows/eal_windows.h b/lib/librte_eal/windows/eal_windows.h
index d48ee0a12..478accc1b 100644
--- a/lib/librte_eal/windows/eal_windows.h
+++ b/lib/librte_eal/windows/eal_windows.h
@@ -55,6 +55,18 @@ int eal_thread_create(pthread_t *thread);
  */
 unsigned int eal_socket_numa_node(unsigned int socket_id);
 
+/**
+ * Schedule code for execution in the interrupt thread.
+ *
+ * @param func
+ *  Function to call.
+ * @param arg
+ *  Argument to the called function.
+ * @return
+ *  0 on success, netagive error code on failure.
+ */
+int eal_intr_thread_schedule(void (*func)(void *arg), void *arg);
+
 /**
  * Open virt2phys driver interface device.
  *
diff --git a/lib/librte_eal/windows/include/pthread.h b/lib/librte_eal/windows/include/pthread.h
index 99013dc94..a4ab4d094 100644
--- a/lib/librte_eal/windows/include/pthread.h
+++ b/lib/librte_eal/windows/include/pthread.h
@@ -42,6 +42,13 @@ typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
 #define pthread_self() \
 	((pthread_t)GetCurrentThreadId())
 
+
+static inline int
+pthread_equal(pthread_t t1, pthread_t t2)
+{
+	return t1 == t2;
+}
+
 static inline int
 pthread_setaffinity_np(pthread_t threadid, size_t cpuset_size,
 			rte_cpuset_t *cpuset)
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 08c888e01..b690bc6b0 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -8,6 +8,7 @@ sources += files(
 	'eal_debug.c',
 	'eal_file.c',
 	'eal_hugepages.c',
+	'eal_interrupts.c',
 	'eal_lcore.c',
 	'eal_log.c',
 	'eal_memalloc.c',
-- 
2.25.4


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

* [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API
  2020-09-11  0:22 [dpdk-dev] [PATCH 0/2] eal/windows: implement alarms Dmitry Kozlyuk
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
@ 2020-09-11  0:22 ` Dmitry Kozlyuk
  2020-09-21 19:08   ` [dpdk-dev] [EXTERNAL] " Khoa To
  2020-09-25 23:32 ` [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms Dmitry Kozlyuk
  2 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-11  0:22 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, Narcisa Ana Maria Vasile, Dmitry Malloy, Pallavi Kadam

Implementation is based on waitable timers Win32 API. When timer is set,
a callback and its argument are supplied to the OS, while timer handle
is stored in EAL alarm list. When timer expires, OS wakes up the
interrupt thread and runs the callback. Upon completion it removes the
alarm.

Waitable timers must be set from the thread their callback will run in,
eal_intr_thread_schedule() provides a way to schedule asyncronuous code
execution in the interrupt thread. Alarm module builds synchronous timer
setup on top of it.

Windows alarms are not a type of DPDK interrupt handle and do not
interact with interrupt module beyond executing in the same thread.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/rte_eal_exports.def |   2 +
 lib/librte_eal/windows/eal_alarm.c | 219 +++++++++++++++++++++++++++++
 lib/librte_eal/windows/meson.build |   1 +
 3 files changed, 222 insertions(+)
 create mode 100644 lib/librte_eal/windows/eal_alarm.c

diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 9baca0110..b6abb5ae1 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -14,6 +14,8 @@ EXPORTS
 	rte_devargs_insert
 	rte_devargs_next
 	rte_devargs_remove
+	rte_eal_alarm_set
+	rte_eal_alarm_cancel
 	rte_eal_get_configuration
 	rte_eal_has_hugepages
 	rte_eal_has_pci
diff --git a/lib/librte_eal/windows/eal_alarm.c b/lib/librte_eal/windows/eal_alarm.c
new file mode 100644
index 000000000..3b262793a
--- /dev/null
+++ b/lib/librte_eal/windows/eal_alarm.c
@@ -0,0 +1,219 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <stdatomic.h>
+#include <stdbool.h>
+
+#include <rte_alarm.h>
+#include <rte_spinlock.h>
+
+#include <rte_eal_trace.h>
+
+#include "eal_windows.h"
+
+enum alarm_state {
+	ALARM_ARMED,
+	ALARM_TRIGGERED,
+	ALARM_CANCELLED
+};
+
+struct alarm_entry {
+	LIST_ENTRY(alarm_entry) next;
+	rte_eal_alarm_callback cb_fn;
+	void *cb_arg;
+	HANDLE timer;
+	atomic_uint state;
+};
+
+static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER();
+
+static rte_spinlock_t alarm_lock = RTE_SPINLOCK_INITIALIZER;
+
+static int intr_thread_exec(void (*func)(void *arg), void *arg);
+
+static void
+alarm_remove_unsafe(struct alarm_entry *ap)
+{
+	LIST_REMOVE(ap, next);
+	CloseHandle(ap->timer);
+	free(ap);
+}
+
+static void
+alarm_callback(void *arg, DWORD low __rte_unused, DWORD high __rte_unused)
+{
+	struct alarm_entry *ap = arg;
+	unsigned int state = ALARM_ARMED;
+
+	if (!atomic_compare_exchange_strong(
+			&ap->state, &state, ALARM_TRIGGERED))
+		return;
+
+	ap->cb_fn(ap->cb_arg);
+
+	rte_spinlock_lock(&alarm_lock);
+	alarm_remove_unsafe(ap);
+	rte_spinlock_unlock(&alarm_lock);
+}
+
+struct alarm_task {
+	struct alarm_entry *entry;
+	LARGE_INTEGER deadline;
+	int ret;
+};
+
+static void
+alarm_set(void *arg)
+{
+	struct alarm_task *task = arg;
+
+	BOOL ret = SetWaitableTimer(
+		task->entry->timer, &task->deadline,
+		0, alarm_callback, task->entry, FALSE);
+	task->ret = ret ? 0 : (-1);
+}
+
+int
+rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	struct alarm_entry *ap;
+	HANDLE timer;
+	FILETIME ft;
+	struct alarm_task task;
+	int ret;
+
+	/* Calculate deadline ASAP, unit of measure = 100ns. */
+	GetSystemTimePreciseAsFileTime(&ft);
+	task.deadline.LowPart = ft.dwLowDateTime;
+	task.deadline.HighPart = ft.dwHighDateTime;
+	task.deadline.QuadPart += 10 * us;
+
+	ap = calloc(1, sizeof(*ap));
+	if (ap == NULL) {
+		RTE_LOG(ERR, EAL, "Cannot allocate alarm entry\n");
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	timer = CreateWaitableTimer(NULL, FALSE, NULL);
+	if (timer == NULL) {
+		RTE_LOG_WIN32_ERR("CreateWaitableTimer()");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ap->timer = timer;
+	ap->cb_fn = cb_fn;
+	ap->cb_arg = cb_arg;
+	task.entry = ap;
+
+	/* Waitable timer must be set in the same thread that will
+	 * do an alertable wait for the alarm to trigger, that is,
+	 * in the interrupt thread. Setting can fail, so do it synchronously.
+	 */
+	ret = intr_thread_exec(alarm_set, &task);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Cannot setup alarm in interrupt thread\n");
+		goto fail;
+	}
+
+	ret = task.ret;
+	if (ret < 0)
+		goto fail;
+
+	rte_spinlock_lock(&alarm_lock);
+	LIST_INSERT_HEAD(&alarm_list, ap, next);
+	rte_spinlock_unlock(&alarm_lock);
+
+	goto exit;
+
+fail:
+	if (timer != NULL)
+		CloseHandle(timer);
+	if (ap != NULL)
+		free(ap);
+
+exit:
+	rte_eal_trace_alarm_set(us, cb_fn, cb_arg, ret);
+	return ret;
+}
+
+static bool
+alarm_matches(const struct alarm_entry *ap,
+	rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	bool any_arg = cb_arg == (void *)(-1);
+	return (ap->cb_fn == cb_fn) && (any_arg || ap->cb_arg == cb_arg);
+}
+
+int
+rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	struct alarm_entry *ap;
+	unsigned int state;
+	int removed;
+	bool executing;
+
+	removed = 0;
+	do {
+		executing = false;
+
+		rte_spinlock_lock(&alarm_lock);
+
+		LIST_FOREACH(ap, &alarm_list, next) {
+			if (!alarm_matches(ap, cb_fn, cb_arg))
+				continue;
+
+			state = ALARM_ARMED;
+			if (atomic_compare_exchange_strong(
+					&ap->state, &state, ALARM_CANCELLED)) {
+				alarm_remove_unsafe(ap);
+				removed++;
+			} else if (state == ALARM_TRIGGERED)
+				executing = true;
+		}
+
+		rte_spinlock_unlock(&alarm_lock);
+	} while (executing);
+
+	rte_eal_trace_alarm_cancel(cb_fn, cb_arg, removed);
+	return removed;
+}
+
+struct intr_task {
+	void (*func)(void *arg);
+	void *arg;
+	rte_spinlock_t lock; /* unlocked at task completion */
+};
+
+static void
+intr_thread_entry(void *arg)
+{
+	struct intr_task *task = arg;
+	task->func(task->arg);
+	rte_spinlock_unlock(&task->lock);
+}
+
+static int
+intr_thread_exec(void (*func)(void *arg), void *arg)
+{
+	struct intr_task task;
+	int ret;
+
+	task.func = func;
+	task.arg = arg;
+	rte_spinlock_init(&task.lock);
+
+	/* Make timers more precise by synchronizing in userspace. */
+	rte_spinlock_lock(&task.lock);
+	ret = eal_intr_thread_schedule(intr_thread_entry, &task);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Cannot schedule task to interrupt thread\n");
+		return -EINVAL;
+	}
+
+	/* Wait for the task to complete. */
+	rte_spinlock_lock(&task.lock);
+	return 0;
+}
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index b690bc6b0..3b2faf29e 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -5,6 +5,7 @@ subdir('include')
 
 sources += files(
 	'eal.c',
+	'eal_alarm.c',
 	'eal_debug.c',
 	'eal_file.c',
 	'eal_hugepages.c',
-- 
2.25.4


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

* Re: [dpdk-dev] [EXTERNAL] [PATCH 2/2] eal/windows: implement alarm API
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
@ 2020-09-21 19:08   ` Khoa To
  2020-09-24 21:38     ` Dmitry Kozlyuk
  0 siblings, 1 reply; 14+ messages in thread
From: Khoa To @ 2020-09-21 19:08 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: Narcisa Ana Maria Vasile, Dmitry Malloy (MESHCHANINOV), Pallavi Kadam

Hi Dmitry,

Since all alarm callbacks are scheduled on the interrupt thread, looks like this implementation of rte_eal_alarm_set() would create a head-of-line blocking issue, as the callbacks are serialized one after the other.
Is that correct?  If so, while this does not violate the API contract, it seems undesirable.

I looked at the Linux implementation of alarm, and although I am not very familiar with Linux system, it seems to have similar implementation, with the same head-of-line blocking issue.

I'm wondering why in both Linux and this Windows implementation, we don't spawn a new thread to service each alarm independently.
And, regardless of the Linux implementation, should we consider another implementation that avoids the head-of-line blocking issue?

Khoa.

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dmitry Kozlyuk
> Sent: Thursday, September 10, 2020 5:22 PM
> To: dev@dpdk.org
> Cc: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>; Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Dmitry Malloy
> (MESHCHANINOV) <dmitrym@microsoft.com>; Pallavi Kadam <pallavi.kadam@intel.com>
> Subject: [EXTERNAL] [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API
> 
> Implementation is based on waitable timers Win32 API. When timer is set,
> a callback and its argument are supplied to the OS, while timer handle
> is stored in EAL alarm list. When timer expires, OS wakes up the
> interrupt thread and runs the callback. Upon completion it removes the
> alarm.
> 
> Waitable timers must be set from the thread their callback will run in,
> eal_intr_thread_schedule() provides a way to schedule asyncronuous code
> execution in the interrupt thread. Alarm module builds synchronous timer
> setup on top of it.
> 
> Windows alarms are not a type of DPDK interrupt handle and do not
> interact with interrupt module beyond executing in the same thread.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/librte_eal/rte_eal_exports.def |   2 +
>  lib/librte_eal/windows/eal_alarm.c | 219 +++++++++++++++++++++++++++++
>  lib/librte_eal/windows/meson.build |   1 +
>  3 files changed, 222 insertions(+)
>  create mode 100644 lib/librte_eal/windows/eal_alarm.c
> 
> diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
> index 9baca0110..b6abb5ae1 100644
> --- a/lib/librte_eal/rte_eal_exports.def
> +++ b/lib/librte_eal/rte_eal_exports.def
> @@ -14,6 +14,8 @@ EXPORTS
>  	rte_devargs_insert
>  	rte_devargs_next
>  	rte_devargs_remove
> +	rte_eal_alarm_set
> +	rte_eal_alarm_cancel
>  	rte_eal_get_configuration
>  	rte_eal_has_hugepages
>  	rte_eal_has_pci
> diff --git a/lib/librte_eal/windows/eal_alarm.c b/lib/librte_eal/windows/eal_alarm.c
> new file mode 100644
> index 000000000..3b262793a
> --- /dev/null
> +++ b/lib/librte_eal/windows/eal_alarm.c
> @@ -0,0 +1,219 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2020 Dmitry Kozlyuk
> + */
> +
> +#include <stdatomic.h>
> +#include <stdbool.h>
> +
> +#include <rte_alarm.h>
> +#include <rte_spinlock.h>
> +
> +#include <rte_eal_trace.h>
> +
> +#include "eal_windows.h"
> +
> +enum alarm_state {
> +	ALARM_ARMED,
> +	ALARM_TRIGGERED,
> +	ALARM_CANCELLED
> +};
> +
> +struct alarm_entry {
> +	LIST_ENTRY(alarm_entry) next;
> +	rte_eal_alarm_callback cb_fn;
> +	void *cb_arg;
> +	HANDLE timer;
> +	atomic_uint state;
> +};
> +
> +static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER();
> +
> +static rte_spinlock_t alarm_lock = RTE_SPINLOCK_INITIALIZER;
> +
> +static int intr_thread_exec(void (*func)(void *arg), void *arg);
> +
> +static void
> +alarm_remove_unsafe(struct alarm_entry *ap)
> +{
> +	LIST_REMOVE(ap, next);
> +	CloseHandle(ap->timer);
> +	free(ap);
> +}
> +
> +static void
> +alarm_callback(void *arg, DWORD low __rte_unused, DWORD high __rte_unused)
> +{
> +	struct alarm_entry *ap = arg;
> +	unsigned int state = ALARM_ARMED;
> +
> +	if (!atomic_compare_exchange_strong(
> +			&ap->state, &state, ALARM_TRIGGERED))
> +		return;
> +
> +	ap->cb_fn(ap->cb_arg);
> +
> +	rte_spinlock_lock(&alarm_lock);
> +	alarm_remove_unsafe(ap);
> +	rte_spinlock_unlock(&alarm_lock);
> +}
> +
> +struct alarm_task {
> +	struct alarm_entry *entry;
> +	LARGE_INTEGER deadline;
> +	int ret;
> +};
> +
> +static void
> +alarm_set(void *arg)
> +{
> +	struct alarm_task *task = arg;
> +
> +	BOOL ret = SetWaitableTimer(
> +		task->entry->timer, &task->deadline,
> +		0, alarm_callback, task->entry, FALSE);
> +	task->ret = ret ? 0 : (-1);
> +}
> +
> +int
> +rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
> +{
> +	struct alarm_entry *ap;
> +	HANDLE timer;
> +	FILETIME ft;
> +	struct alarm_task task;
> +	int ret;
> +
> +	/* Calculate deadline ASAP, unit of measure = 100ns. */
> +	GetSystemTimePreciseAsFileTime(&ft);
> +	task.deadline.LowPart = ft.dwLowDateTime;
> +	task.deadline.HighPart = ft.dwHighDateTime;
> +	task.deadline.QuadPart += 10 * us;
> +
> +	ap = calloc(1, sizeof(*ap));
> +	if (ap == NULL) {
> +		RTE_LOG(ERR, EAL, "Cannot allocate alarm entry\n");
> +		ret = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	timer = CreateWaitableTimer(NULL, FALSE, NULL);
> +	if (timer == NULL) {
> +		RTE_LOG_WIN32_ERR("CreateWaitableTimer()");
> +		ret = -EINVAL;
> +		goto fail;
> +	}
> +
> +	ap->timer = timer;
> +	ap->cb_fn = cb_fn;
> +	ap->cb_arg = cb_arg;
> +	task.entry = ap;
> +
> +	/* Waitable timer must be set in the same thread that will
> +	 * do an alertable wait for the alarm to trigger, that is,
> +	 * in the interrupt thread. Setting can fail, so do it synchronously.
> +	 */
> +	ret = intr_thread_exec(alarm_set, &task);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, EAL, "Cannot setup alarm in interrupt thread\n");
> +		goto fail;
> +	}
> +
> +	ret = task.ret;
> +	if (ret < 0)
> +		goto fail;
> +
> +	rte_spinlock_lock(&alarm_lock);
> +	LIST_INSERT_HEAD(&alarm_list, ap, next);
> +	rte_spinlock_unlock(&alarm_lock);
> +
> +	goto exit;
> +
> +fail:
> +	if (timer != NULL)
> +		CloseHandle(timer);
> +	if (ap != NULL)
> +		free(ap);
> +
> +exit:
> +	rte_eal_trace_alarm_set(us, cb_fn, cb_arg, ret);
> +	return ret;
> +}
> +
> +static bool
> +alarm_matches(const struct alarm_entry *ap,
> +	rte_eal_alarm_callback cb_fn, void *cb_arg)
> +{
> +	bool any_arg = cb_arg == (void *)(-1);
> +	return (ap->cb_fn == cb_fn) && (any_arg || ap->cb_arg == cb_arg);
> +}
> +
> +int
> +rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg)
> +{
> +	struct alarm_entry *ap;
> +	unsigned int state;
> +	int removed;
> +	bool executing;
> +
> +	removed = 0;
> +	do {
> +		executing = false;
> +
> +		rte_spinlock_lock(&alarm_lock);
> +
> +		LIST_FOREACH(ap, &alarm_list, next) {
> +			if (!alarm_matches(ap, cb_fn, cb_arg))
> +				continue;
> +
> +			state = ALARM_ARMED;
> +			if (atomic_compare_exchange_strong(
> +					&ap->state, &state, ALARM_CANCELLED)) {
> +				alarm_remove_unsafe(ap);
> +				removed++;
> +			} else if (state == ALARM_TRIGGERED)
> +				executing = true;
> +		}
> +
> +		rte_spinlock_unlock(&alarm_lock);
> +	} while (executing);
> +
> +	rte_eal_trace_alarm_cancel(cb_fn, cb_arg, removed);
> +	return removed;
> +}
> +
> +struct intr_task {
> +	void (*func)(void *arg);
> +	void *arg;
> +	rte_spinlock_t lock; /* unlocked at task completion */
> +};
> +
> +static void
> +intr_thread_entry(void *arg)
> +{
> +	struct intr_task *task = arg;
> +	task->func(task->arg);
> +	rte_spinlock_unlock(&task->lock);
> +}
> +
> +static int
> +intr_thread_exec(void (*func)(void *arg), void *arg)
> +{
> +	struct intr_task task;
> +	int ret;
> +
> +	task.func = func;
> +	task.arg = arg;
> +	rte_spinlock_init(&task.lock);
> +
> +	/* Make timers more precise by synchronizing in userspace. */
> +	rte_spinlock_lock(&task.lock);
> +	ret = eal_intr_thread_schedule(intr_thread_entry, &task);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, EAL, "Cannot schedule task to interrupt thread\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Wait for the task to complete. */
> +	rte_spinlock_lock(&task.lock);
> +	return 0;
> +}
> diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
> index b690bc6b0..3b2faf29e 100644
> --- a/lib/librte_eal/windows/meson.build
> +++ b/lib/librte_eal/windows/meson.build
> @@ -5,6 +5,7 @@ subdir('include')
> 
>  sources += files(
>  	'eal.c',
> +	'eal_alarm.c',
>  	'eal_debug.c',
>  	'eal_file.c',
>  	'eal_hugepages.c',
> --
> 2.25.4


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

* Re: [dpdk-dev] [EXTERNAL] [PATCH 2/2] eal/windows: implement alarm API
  2020-09-21 19:08   ` [dpdk-dev] [EXTERNAL] " Khoa To
@ 2020-09-24 21:38     ` Dmitry Kozlyuk
  2020-09-25  2:19       ` Khoa To
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-24 21:38 UTC (permalink / raw)
  To: Khoa To
  Cc: dev, Narcisa Ana Maria Vasile, Dmitry Malloy (MESHCHANINOV),
	Pallavi Kadam

Hi Khoa,

Disclaimer: my experience with interrupts and alarms is limited.

> Since all alarm callbacks are scheduled on the interrupt thread, looks like this implementation of rte_eal_alarm_set() would create a head-of-line blocking issue, as the callbacks are serialized one after the other.
> Is that correct?  If so, while this does not violate the API contract, it seems undesirable.

Yes, the issues is valid, however, probably irrelevant. In DPDK, alarms
are considered on par with interrupts. Much like kernel ISRs, DPDK interrupt
callbacks are supposed to be short. When they need to do more work, there are
facilities to schedule deferred work (again, somewhat like NT kernel DPC):
rte_ctrl_thread_create().

> I'm wondering why in both Linux and this Windows implementation, we don't spawn a new thread to service each alarm independently.

These threads would need cores to be scheduled on. If we set affinity to the
primary lcore, head-of-line issue is back. If we leave that to the OS, they
may land on data-plane cores with a context switch.

> And, regardless of the Linux implementation, should we consider another implementation that avoids the head-of-line blocking issue?

I don't think so. Looking at PMD code, timers are mostly used during
initialization or reconfiguration.


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

* Re: [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
@ 2020-09-25  1:19   ` Narcisa Ana Maria Vasile
  2020-09-25  6:28     ` Dmitry Kozlyuk
  0 siblings, 1 reply; 14+ messages in thread
From: Narcisa Ana Maria Vasile @ 2020-09-25  1:19 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: dev, Harman Kalra, Dmitry Malloy, Pallavi Kadam

On Fri, Sep 11, 2020 at 03:22:06AM +0300, Dmitry Kozlyuk wrote:
> Windows interrupt support is based on IO completion ports (IOCP).
> Interrupt thread would send the devices requests to notify about
> interrupts and then wait for any request completion. Add skeleton code
> of this model without any hardware support.
> 
> Another way to wake up the interrupt thread is APC (asynchronous procedure
> call), scheduled by any other thread via eal_intr_thread_schedule().
> This internal API is intended for alarm implementation.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/librte_eal/include/rte_eal_interrupts.h | 14 ++-
>  lib/librte_eal/rte_eal_exports.def          |  1 +
>  lib/librte_eal/windows/eal.c                |  5 ++
>  lib/librte_eal/windows/eal_interrupts.c     | 99 +++++++++++++++++++++
>  lib/librte_eal/windows/eal_windows.h        | 12 +++
>  lib/librte_eal/windows/include/pthread.h    |  7 ++
>  lib/librte_eal/windows/meson.build          |  1 +
>  7 files changed, 136 insertions(+), 3 deletions(-)
>  create mode 100644 lib/librte_eal/windows/eal_interrupts.c
> 
Reviewed-by: Narcisa Vasile <navasile@linux.microsoft.com>

Getting an "undefined reference to rte_intr_rx_ctl", guess we need to add a stub for this.
Otherwise, compiles successfully with both clang and mingw.

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

* Re: [dpdk-dev] [EXTERNAL] [PATCH 2/2] eal/windows: implement alarm API
  2020-09-24 21:38     ` Dmitry Kozlyuk
@ 2020-09-25  2:19       ` Khoa To
  0 siblings, 0 replies; 14+ messages in thread
From: Khoa To @ 2020-09-25  2:19 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Narcisa Ana Maria Vasile, Dmitry Malloy (MESHCHANINOV),
	Pallavi Kadam


> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Sent: Thursday, September 24, 2020 2:39 PM
> To: Khoa To <khot@microsoft.com>
> Cc: dev@dpdk.org; Narcisa Ana Maria Vasile
> <navasile@linux.microsoft.com>; Dmitry Malloy (MESHCHANINOV)
> <dmitrym@microsoft.com>; Pallavi Kadam <pallavi.kadam@intel.com>
> Subject: Re: [EXTERNAL] [dpdk-dev] [PATCH 2/2] eal/windows: implement
> alarm API
> 
> Hi Khoa,
> 
> Disclaimer: my experience with interrupts and alarms is limited.
> 
> > Since all alarm callbacks are scheduled on the interrupt thread, looks like
> this implementation of rte_eal_alarm_set() would create a head-of-line
> blocking issue, as the callbacks are serialized one after the other.
> > Is that correct?  If so, while this does not violate the API contract, it seems
> undesirable.
> 
> Yes, the issues is valid, however, probably irrelevant. In DPDK, alarms
> are considered on par with interrupts. Much like kernel ISRs, DPDK interrupt
> callbacks are supposed to be short. When they need to do more work, there
> are
> facilities to schedule deferred work (again, somewhat like NT kernel DPC):
> rte_ctrl_thread_create().

Thank you for clarification on the usage model.

> > I'm wondering why in both Linux and this Windows implementation, we
> don't spawn a new thread to service each alarm independently.
> 
> These threads would need cores to be scheduled on. If we set affinity to the
> primary lcore, head-of-line issue is back. If we leave that to the OS, they
> may land on data-plane cores with a context switch.
> 
> > And, regardless of the Linux implementation, should we consider another
> implementation that avoids the head-of-line blocking issue?
> 
> I don't think so. Looking at PMD code, timers are mostly used during
> initialization or reconfiguration.

Sounds good. The implementation looks good. 
I'll take another pass at the code for any additional feedbacks.

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

* Re: [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton
  2020-09-25  1:19   ` Narcisa Ana Maria Vasile
@ 2020-09-25  6:28     ` Dmitry Kozlyuk
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-25  6:28 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile; +Cc: dev, Harman Kalra, Dmitry Malloy, Pallavi Kadam

> Getting an "undefined reference to rte_intr_rx_ctl", guess we need to add a stub for this.
> Otherwise, compiles successfully with both clang and mingw.

This is because a patch got merged that also eal_interrupts.c and this stub.
I'll rebase and send v2.


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

* [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms
  2020-09-11  0:22 [dpdk-dev] [PATCH 0/2] eal/windows: implement alarms Dmitry Kozlyuk
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
  2020-09-11  0:22 ` [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
@ 2020-09-25 23:32 ` Dmitry Kozlyuk
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
  2 siblings, 2 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-25 23:32 UTC (permalink / raw)
  To: dev; +Cc: Khoa To, Dmitry Kozlyuk

This patchset provides EAL alarm support for Windows. Basic interrupt
thread code is added to monitor alarm events. It doesn't include
callback management, because Windows alarms, unlike Unix EALs, rely on
the OS for callback execution scheduling.

v2: rebase on ToT to resolve conflicts.

Dmitry Kozlyuk (2):
  eal/windows: add interrupt thread skeleton
  eal/windows: implement alarm API

 lib/librte_eal/include/rte_eal_interrupts.h |  14 +-
 lib/librte_eal/rte_eal_exports.def          |   3 +
 lib/librte_eal/windows/eal.c                |   5 +
 lib/librte_eal/windows/eal_alarm.c          | 219 ++++++++++++++++++++
 lib/librte_eal/windows/eal_interrupts.c     |  94 +++++++++
 lib/librte_eal/windows/eal_windows.h        |  12 ++
 lib/librte_eal/windows/include/pthread.h    |   7 +
 lib/librte_eal/windows/meson.build          |   1 +
 8 files changed, 352 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_eal/windows/eal_alarm.c

-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton
  2020-09-25 23:32 ` [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms Dmitry Kozlyuk
@ 2020-09-25 23:32   ` Dmitry Kozlyuk
  2020-09-25 23:40     ` Narcisa Ana Maria Vasile
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-25 23:32 UTC (permalink / raw)
  To: dev
  Cc: Khoa To, Dmitry Kozlyuk, Harman Kalra, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam

Windows interrupt support is based on IO completion ports (IOCP).
Interrupt thread would send the devices requests to notify about
interrupts and then wait for any request completion. Add skeleton code
of this model without any hardware support.

Another way to wake up the interrupt thread is APC (asynchronous procedure
call), scheduled by any other thread via eal_intr_thread_schedule().
This internal API is intended for alarm implementation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/include/rte_eal_interrupts.h | 14 ++-
 lib/librte_eal/rte_eal_exports.def          |  1 +
 lib/librte_eal/windows/eal.c                |  5 ++
 lib/librte_eal/windows/eal_interrupts.c     | 94 +++++++++++++++++++++
 lib/librte_eal/windows/eal_windows.h        | 12 +++
 lib/librte_eal/windows/include/pthread.h    |  7 ++
 6 files changed, 130 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/include/rte_eal_interrupts.h b/lib/librte_eal/include/rte_eal_interrupts.h
index b1e8a2934..b80edfc65 100644
--- a/lib/librte_eal/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/include/rte_eal_interrupts.h
@@ -69,10 +69,18 @@ struct rte_epoll_event {
 struct rte_intr_handle {
 	RTE_STD_C11
 	union {
-		int vfio_dev_fd;  /**< VFIO device file descriptor */
-		int uio_cfg_fd;  /**< UIO cfg file desc for uio_pci_generic */
+		struct {
+			RTE_STD_C11
+			union {
+				/** VFIO device file descriptor */
+				int vfio_dev_fd;
+				/** UIO cfg file desc for uio_pci_generic */
+				int uio_cfg_fd;
+			};
+			int fd;	/**< interrupt event file descriptor */
+		};
+		void *handle; /**< device driver handle (Windows) */
 	};
-	int fd;	 /**< interrupt event file descriptor */
 	enum rte_intr_handle_type type;  /**< handle type */
 	uint32_t max_intr;             /**< max interrupt requested */
 	uint32_t nb_efd;               /**< number of available efd(event fd) */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index cda3e0c79..dd8adfa23 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -70,6 +70,7 @@ EXPORTS
 	rte_vlog
 	rte_realloc
 	rte_strscpy
+	rte_thread_is_intr
 	rte_zmalloc
 	rte_zmalloc_socket
 
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index bc48f27ab..141f22adb 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -344,6 +344,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (rte_eal_intr_init() < 0) {
+		rte_eal_init_alert("Cannot init interrupt-handling thread");
+		return -1;
+	}
+
 	if (rte_eal_timer_init() < 0) {
 		rte_eal_init_alert("Cannot init TSC timer");
 		rte_errno = EFAULT;
diff --git a/lib/librte_eal/windows/eal_interrupts.c b/lib/librte_eal/windows/eal_interrupts.c
index d3ecdaccb..6c64a48f3 100644
--- a/lib/librte_eal/windows/eal_interrupts.c
+++ b/lib/librte_eal/windows/eal_interrupts.c
@@ -4,6 +4,81 @@
 
 #include <rte_interrupts.h>
 
+#include "eal_private.h"
+#include "eal_windows.h"
+
+static pthread_t intr_thread;
+
+static HANDLE intr_iocp;
+
+static void
+eal_intr_process(const OVERLAPPED_ENTRY *event)
+{
+	RTE_SET_USED(event);
+}
+
+static void *
+eal_intr_thread_main(LPVOID arg __rte_unused)
+{
+	while (1) {
+		OVERLAPPED_ENTRY events[16];
+		ULONG event_count, i;
+		BOOL result;
+
+		result = GetQueuedCompletionStatusEx(
+			intr_iocp, events, RTE_DIM(events), &event_count,
+			INFINITE, /* no timeout */
+			TRUE);    /* alertable wait for alarm APCs */
+
+		if (!result) {
+			DWORD error = GetLastError();
+			if (error != WAIT_IO_COMPLETION) {
+				RTE_LOG_WIN32_ERR("GetQueuedCompletionStatusEx()");
+				RTE_LOG(ERR, EAL, "Failed waiting for interrupts\n");
+				break;
+			}
+
+			/* No I/O events, all work is done in completed APCs. */
+			continue;
+		}
+
+		for (i = 0; i < event_count; i++)
+			eal_intr_process(&events[i]);
+	}
+
+	CloseHandle(intr_iocp);
+	intr_iocp = NULL;
+	return NULL;
+}
+
+int
+rte_eal_intr_init(void)
+{
+	int ret = 0;
+
+	intr_iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
+	if (intr_iocp == NULL) {
+		RTE_LOG_WIN32_ERR("CreateIoCompletionPort()");
+		RTE_LOG(ERR, EAL, "Cannot create interrupt IOCP\n");
+		return -1;
+	}
+
+	ret = rte_ctrl_thread_create(&intr_thread, "eal-intr-thread", NULL,
+			eal_intr_thread_main, NULL);
+	if (ret != 0) {
+		rte_errno = -ret;
+		RTE_LOG(ERR, EAL, "Cannot create interrupt thread\n");
+	}
+
+	return ret;
+}
+
+int
+rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
+
 int
 rte_intr_rx_ctl(__rte_unused struct rte_intr_handle *intr_handle,
 		__rte_unused int epfd, __rte_unused int op,
@@ -11,3 +86,22 @@ rte_intr_rx_ctl(__rte_unused struct rte_intr_handle *intr_handle,
 {
 	return -ENOTSUP;
 }
+
+int
+eal_intr_thread_schedule(void (*func)(void *arg), void *arg)
+{
+	HANDLE handle;
+
+	handle = OpenThread(THREAD_ALL_ACCESS, FALSE, intr_thread);
+	if (handle == NULL) {
+		RTE_LOG_WIN32_ERR("OpenThread(%llu)", intr_thread);
+		return -ENOENT;
+	}
+
+	if (!QueueUserAPC((PAPCFUNC)(ULONG_PTR)func, handle, (ULONG_PTR)arg)) {
+		RTE_LOG_WIN32_ERR("QueueUserAPC()");
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/lib/librte_eal/windows/eal_windows.h b/lib/librte_eal/windows/eal_windows.h
index d48ee0a12..478accc1b 100644
--- a/lib/librte_eal/windows/eal_windows.h
+++ b/lib/librte_eal/windows/eal_windows.h
@@ -55,6 +55,18 @@ int eal_thread_create(pthread_t *thread);
  */
 unsigned int eal_socket_numa_node(unsigned int socket_id);
 
+/**
+ * Schedule code for execution in the interrupt thread.
+ *
+ * @param func
+ *  Function to call.
+ * @param arg
+ *  Argument to the called function.
+ * @return
+ *  0 on success, netagive error code on failure.
+ */
+int eal_intr_thread_schedule(void (*func)(void *arg), void *arg);
+
 /**
  * Open virt2phys driver interface device.
  *
diff --git a/lib/librte_eal/windows/include/pthread.h b/lib/librte_eal/windows/include/pthread.h
index 99013dc94..a4ab4d094 100644
--- a/lib/librte_eal/windows/include/pthread.h
+++ b/lib/librte_eal/windows/include/pthread.h
@@ -42,6 +42,13 @@ typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
 #define pthread_self() \
 	((pthread_t)GetCurrentThreadId())
 
+
+static inline int
+pthread_equal(pthread_t t1, pthread_t t2)
+{
+	return t1 == t2;
+}
+
 static inline int
 pthread_setaffinity_np(pthread_t threadid, size_t cpuset_size,
 			rte_cpuset_t *cpuset)
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API
  2020-09-25 23:32 ` [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms Dmitry Kozlyuk
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
@ 2020-09-25 23:32   ` Dmitry Kozlyuk
  2020-09-25 23:41     ` Narcisa Ana Maria Vasile
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-25 23:32 UTC (permalink / raw)
  To: dev
  Cc: Khoa To, Dmitry Kozlyuk, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam

Implementation is based on waitable timers Win32 API. When timer is set,
a callback and its argument are supplied to the OS, while timer handle
is stored in EAL alarm list. When timer expires, OS wakes up the
interrupt thread and runs the callback. Upon completion it removes the
alarm.

Waitable timers must be set from the thread their callback will run in,
eal_intr_thread_schedule() provides a way to schedule asyncronuous code
execution in the interrupt thread. Alarm module builds synchronous timer
setup on top of it.

Windows alarms are not a type of DPDK interrupt handle and do not
interact with interrupt module beyond executing in the same thread.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/rte_eal_exports.def |   2 +
 lib/librte_eal/windows/eal_alarm.c | 219 +++++++++++++++++++++++++++++
 lib/librte_eal/windows/meson.build |   1 +
 3 files changed, 222 insertions(+)
 create mode 100644 lib/librte_eal/windows/eal_alarm.c

diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index dd8adfa23..8d8300645 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -11,6 +11,8 @@ EXPORTS
 	rte_devargs_next
 	rte_devargs_parse
 	rte_devargs_remove
+	rte_eal_alarm_set
+	rte_eal_alarm_cancel
 	rte_eal_has_hugepages
 	rte_eal_has_pci
 	rte_eal_init
diff --git a/lib/librte_eal/windows/eal_alarm.c b/lib/librte_eal/windows/eal_alarm.c
new file mode 100644
index 000000000..3b262793a
--- /dev/null
+++ b/lib/librte_eal/windows/eal_alarm.c
@@ -0,0 +1,219 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <stdatomic.h>
+#include <stdbool.h>
+
+#include <rte_alarm.h>
+#include <rte_spinlock.h>
+
+#include <rte_eal_trace.h>
+
+#include "eal_windows.h"
+
+enum alarm_state {
+	ALARM_ARMED,
+	ALARM_TRIGGERED,
+	ALARM_CANCELLED
+};
+
+struct alarm_entry {
+	LIST_ENTRY(alarm_entry) next;
+	rte_eal_alarm_callback cb_fn;
+	void *cb_arg;
+	HANDLE timer;
+	atomic_uint state;
+};
+
+static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER();
+
+static rte_spinlock_t alarm_lock = RTE_SPINLOCK_INITIALIZER;
+
+static int intr_thread_exec(void (*func)(void *arg), void *arg);
+
+static void
+alarm_remove_unsafe(struct alarm_entry *ap)
+{
+	LIST_REMOVE(ap, next);
+	CloseHandle(ap->timer);
+	free(ap);
+}
+
+static void
+alarm_callback(void *arg, DWORD low __rte_unused, DWORD high __rte_unused)
+{
+	struct alarm_entry *ap = arg;
+	unsigned int state = ALARM_ARMED;
+
+	if (!atomic_compare_exchange_strong(
+			&ap->state, &state, ALARM_TRIGGERED))
+		return;
+
+	ap->cb_fn(ap->cb_arg);
+
+	rte_spinlock_lock(&alarm_lock);
+	alarm_remove_unsafe(ap);
+	rte_spinlock_unlock(&alarm_lock);
+}
+
+struct alarm_task {
+	struct alarm_entry *entry;
+	LARGE_INTEGER deadline;
+	int ret;
+};
+
+static void
+alarm_set(void *arg)
+{
+	struct alarm_task *task = arg;
+
+	BOOL ret = SetWaitableTimer(
+		task->entry->timer, &task->deadline,
+		0, alarm_callback, task->entry, FALSE);
+	task->ret = ret ? 0 : (-1);
+}
+
+int
+rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	struct alarm_entry *ap;
+	HANDLE timer;
+	FILETIME ft;
+	struct alarm_task task;
+	int ret;
+
+	/* Calculate deadline ASAP, unit of measure = 100ns. */
+	GetSystemTimePreciseAsFileTime(&ft);
+	task.deadline.LowPart = ft.dwLowDateTime;
+	task.deadline.HighPart = ft.dwHighDateTime;
+	task.deadline.QuadPart += 10 * us;
+
+	ap = calloc(1, sizeof(*ap));
+	if (ap == NULL) {
+		RTE_LOG(ERR, EAL, "Cannot allocate alarm entry\n");
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	timer = CreateWaitableTimer(NULL, FALSE, NULL);
+	if (timer == NULL) {
+		RTE_LOG_WIN32_ERR("CreateWaitableTimer()");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ap->timer = timer;
+	ap->cb_fn = cb_fn;
+	ap->cb_arg = cb_arg;
+	task.entry = ap;
+
+	/* Waitable timer must be set in the same thread that will
+	 * do an alertable wait for the alarm to trigger, that is,
+	 * in the interrupt thread. Setting can fail, so do it synchronously.
+	 */
+	ret = intr_thread_exec(alarm_set, &task);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Cannot setup alarm in interrupt thread\n");
+		goto fail;
+	}
+
+	ret = task.ret;
+	if (ret < 0)
+		goto fail;
+
+	rte_spinlock_lock(&alarm_lock);
+	LIST_INSERT_HEAD(&alarm_list, ap, next);
+	rte_spinlock_unlock(&alarm_lock);
+
+	goto exit;
+
+fail:
+	if (timer != NULL)
+		CloseHandle(timer);
+	if (ap != NULL)
+		free(ap);
+
+exit:
+	rte_eal_trace_alarm_set(us, cb_fn, cb_arg, ret);
+	return ret;
+}
+
+static bool
+alarm_matches(const struct alarm_entry *ap,
+	rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	bool any_arg = cb_arg == (void *)(-1);
+	return (ap->cb_fn == cb_fn) && (any_arg || ap->cb_arg == cb_arg);
+}
+
+int
+rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg)
+{
+	struct alarm_entry *ap;
+	unsigned int state;
+	int removed;
+	bool executing;
+
+	removed = 0;
+	do {
+		executing = false;
+
+		rte_spinlock_lock(&alarm_lock);
+
+		LIST_FOREACH(ap, &alarm_list, next) {
+			if (!alarm_matches(ap, cb_fn, cb_arg))
+				continue;
+
+			state = ALARM_ARMED;
+			if (atomic_compare_exchange_strong(
+					&ap->state, &state, ALARM_CANCELLED)) {
+				alarm_remove_unsafe(ap);
+				removed++;
+			} else if (state == ALARM_TRIGGERED)
+				executing = true;
+		}
+
+		rte_spinlock_unlock(&alarm_lock);
+	} while (executing);
+
+	rte_eal_trace_alarm_cancel(cb_fn, cb_arg, removed);
+	return removed;
+}
+
+struct intr_task {
+	void (*func)(void *arg);
+	void *arg;
+	rte_spinlock_t lock; /* unlocked at task completion */
+};
+
+static void
+intr_thread_entry(void *arg)
+{
+	struct intr_task *task = arg;
+	task->func(task->arg);
+	rte_spinlock_unlock(&task->lock);
+}
+
+static int
+intr_thread_exec(void (*func)(void *arg), void *arg)
+{
+	struct intr_task task;
+	int ret;
+
+	task.func = func;
+	task.arg = arg;
+	rte_spinlock_init(&task.lock);
+
+	/* Make timers more precise by synchronizing in userspace. */
+	rte_spinlock_lock(&task.lock);
+	ret = eal_intr_thread_schedule(intr_thread_entry, &task);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Cannot schedule task to interrupt thread\n");
+		return -EINVAL;
+	}
+
+	/* Wait for the task to complete. */
+	rte_spinlock_lock(&task.lock);
+	return 0;
+}
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index b690bc6b0..3b2faf29e 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -5,6 +5,7 @@ subdir('include')
 
 sources += files(
 	'eal.c',
+	'eal_alarm.c',
 	'eal_debug.c',
 	'eal_file.c',
 	'eal_hugepages.c',
-- 
2.25.4


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

* Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
@ 2020-09-25 23:40     ` Narcisa Ana Maria Vasile
  0 siblings, 0 replies; 14+ messages in thread
From: Narcisa Ana Maria Vasile @ 2020-09-25 23:40 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: dev, Khoa To, Harman Kalra, Dmitry Malloy, Pallavi Kadam

On Sat, Sep 26, 2020 at 02:32:42AM +0300, Dmitry Kozlyuk wrote:
> Windows interrupt support is based on IO completion ports (IOCP).
> Interrupt thread would send the devices requests to notify about
> interrupts and then wait for any request completion. Add skeleton code
> of this model without any hardware support.
> 
> Another way to wake up the interrupt thread is APC (asynchronous procedure
> call), scheduled by any other thread via eal_intr_thread_schedule().
> This internal API is intended for alarm implementation.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/librte_eal/include/rte_eal_interrupts.h | 14 ++-
>  lib/librte_eal/rte_eal_exports.def          |  1 +
>  lib/librte_eal/windows/eal.c                |  5 ++
>  lib/librte_eal/windows/eal_interrupts.c     | 94 +++++++++++++++++++++
>  lib/librte_eal/windows/eal_windows.h        | 12 +++
>  lib/librte_eal/windows/include/pthread.h    |  7 ++
>  6 files changed, 130 insertions(+), 3 deletions(-)
> 

Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>

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

* Re: [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API
  2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
@ 2020-09-25 23:41     ` Narcisa Ana Maria Vasile
  2020-10-14 21:36       ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Narcisa Ana Maria Vasile @ 2020-09-25 23:41 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: dev, Khoa To, Dmitry Malloy, Pallavi Kadam

On Sat, Sep 26, 2020 at 02:32:43AM +0300, Dmitry Kozlyuk wrote:
> Implementation is based on waitable timers Win32 API. When timer is set,
> a callback and its argument are supplied to the OS, while timer handle
> is stored in EAL alarm list. When timer expires, OS wakes up the
> interrupt thread and runs the callback. Upon completion it removes the
> alarm.
> 
> Waitable timers must be set from the thread their callback will run in,
> eal_intr_thread_schedule() provides a way to schedule asyncronuous code
> execution in the interrupt thread. Alarm module builds synchronous timer
> setup on top of it.
> 
> Windows alarms are not a type of DPDK interrupt handle and do not
> interact with interrupt module beyond executing in the same thread.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/librte_eal/rte_eal_exports.def |   2 +
>  lib/librte_eal/windows/eal_alarm.c | 219 +++++++++++++++++++++++++++++
>  lib/librte_eal/windows/meson.build |   1 +
>  3 files changed, 222 insertions(+)
>  create mode 100644 lib/librte_eal/windows/eal_alarm.c
> 

Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>

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

* Re: [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API
  2020-09-25 23:41     ` Narcisa Ana Maria Vasile
@ 2020-10-14 21:36       ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2020-10-14 21:36 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Khoa To, Dmitry Malloy, Pallavi Kadam, Narcisa Ana Maria Vasile

26/09/2020 01:41, Narcisa Ana Maria Vasile:
> On Sat, Sep 26, 2020 at 02:32:43AM +0300, Dmitry Kozlyuk wrote:
> > Implementation is based on waitable timers Win32 API. When timer is set,
> > a callback and its argument are supplied to the OS, while timer handle
> > is stored in EAL alarm list. When timer expires, OS wakes up the
> > interrupt thread and runs the callback. Upon completion it removes the
> > alarm.
> > 
> > Waitable timers must be set from the thread their callback will run in,
> > eal_intr_thread_schedule() provides a way to schedule asyncronuous code
> > execution in the interrupt thread. Alarm module builds synchronous timer
> > setup on top of it.
> > 
> > Windows alarms are not a type of DPDK interrupt handle and do not
> > interact with interrupt module beyond executing in the same thread.
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>

Applied, thanks



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

end of thread, other threads:[~2020-10-14 21:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11  0:22 [dpdk-dev] [PATCH 0/2] eal/windows: implement alarms Dmitry Kozlyuk
2020-09-11  0:22 ` [dpdk-dev] [PATCH 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
2020-09-25  1:19   ` Narcisa Ana Maria Vasile
2020-09-25  6:28     ` Dmitry Kozlyuk
2020-09-11  0:22 ` [dpdk-dev] [PATCH 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
2020-09-21 19:08   ` [dpdk-dev] [EXTERNAL] " Khoa To
2020-09-24 21:38     ` Dmitry Kozlyuk
2020-09-25  2:19       ` Khoa To
2020-09-25 23:32 ` [dpdk-dev] [PATCH v2 0/2] eal/windows: implement alarms Dmitry Kozlyuk
2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 1/2] eal/windows: add interrupt thread skeleton Dmitry Kozlyuk
2020-09-25 23:40     ` Narcisa Ana Maria Vasile
2020-09-25 23:32   ` [dpdk-dev] [PATCH v2 2/2] eal/windows: implement alarm API Dmitry Kozlyuk
2020-09-25 23:41     ` Narcisa Ana Maria Vasile
2020-10-14 21:36       ` Thomas Monjalon

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).