diff mbox series

[RFC] net/ena: Add Windows support.

Message ID 20210814033609.58553-1-u9012063@gmail.com (mailing list archive)
State New
Delegated to: Ferruh Yigit
Headers show
Series [RFC] net/ena: Add Windows support. | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/Intel-compilation fail Compilation issues
ci/checkpatch warning coding style issues

Commit Message

William Tu Aug. 14, 2021, 3:36 a.m. UTC
I don't have a physical Windows testbed so I want to see if I can
get virtual nic working, in this case ENA driver on AWS.
The patch passes build on Windows, but I haven't tested loading
the ena driver. I want to know if this is the right direction,
or whether I also need to change other places in ENA driver?

I copy some of the pthread code from
https://nachtimwald.com/2019/04/05/cross-platform-thread-wrapper/
https://stackoverflow.com/questions/10905892/equivalent-of-gettimeday-for-windows
Thanks.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 drivers/net/ena/base/ena_com.c        |   4 +-
 drivers/net/ena/base/ena_plat.h       |   2 +-
 drivers/net/ena/base/ena_plat_dpdk.h  |  14 +++-
 drivers/net/ena/meson.build           |   5 --
 lib/eal/version.map                   |   2 +-
 lib/eal/windows/include/pthread.h     | 105 ++++++++++++++++++++++++++
 lib/eal/windows/include/rte_windows.h |   1 +
 7 files changed, 120 insertions(+), 13 deletions(-)

Comments

Dmitry Kozlyuk Aug. 14, 2021, 11:31 a.m. UTC | #1
Hi William,

2021-08-14 03:36 (UTC+0000), William Tu:
> I don't have a physical Windows testbed so I want to see if I can
> get virtual nic working, in this case ENA driver on AWS.
> The patch passes build on Windows, but I haven't tested loading
> the ena driver.
> I want to know if this is the right direction,
> or whether I also need to change other places in ENA driver?

Cc'ing maintainers.

> I copy some of the pthread code from
> https://nachtimwald.com/2019/04/05/cross-platform-thread-wrapper/

We don't want to add more pthread shims, pthread.h in DPDK will go away.
Condition variable support should be added to the new threading API:

	http://inbox.dpdk.org/dev/1628017291-3756-1-git-send-email-navasile@linux.microsoft.com

I suppose it can be done independently of the new threading API series.
When copying code to DPDK from elsewhere, please adapt its style to DPDK
conventions (e.g. `RTE_UNUSED(foo)` instead if `(void)foo`) and mind the
license if big pieces are copied verbatim (this is not the case here).

> https://stackoverflow.com/questions/10905892/equivalent-of-gettimeday-for-windows

POSIX gettimeofday() should be replaced with standard C timespec_get().

> Thanks.
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  drivers/net/ena/base/ena_com.c        |   4 +-
>  drivers/net/ena/base/ena_plat.h       |   2 +-
>  drivers/net/ena/base/ena_plat_dpdk.h  |  14 +++-
>  drivers/net/ena/meson.build           |   5 --
>  lib/eal/version.map                   |   2 +-
>  lib/eal/windows/include/pthread.h     | 105 ++++++++++++++++++++++++++
>  lib/eal/windows/include/rte_windows.h |   1 +
>  7 files changed, 120 insertions(+), 13 deletions(-)

This should be a series of two patches:
1) adding condition variable wrappers to EAL;
2) supporting net/ena for Windows.
William Tu Aug. 14, 2021, 3:37 p.m. UTC | #2
On Sat, Aug 14, 2021 at 4:31 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Hi William,
>
> 2021-08-14 03:36 (UTC+0000), William Tu:
> > I don't have a physical Windows testbed so I want to see if I can
> > get virtual nic working, in this case ENA driver on AWS.
> > The patch passes build on Windows, but I haven't tested loading
> > the ena driver.
> > I want to know if this is the right direction,
> > or whether I also need to change other places in ENA driver?
>
> Cc'ing maintainers.
>
> > I copy some of the pthread code from
> > https://nachtimwald.com/2019/04/05/cross-platform-thread-wrapper/
>
> We don't want to add more pthread shims, pthread.h in DPDK will go away.
> Condition variable support should be added to the new threading API:
>
>         http://inbox.dpdk.org/dev/1628017291-3756-1-git-send-email-navasile@linux.microsoft.com
>
> I suppose it can be done independently of the new threading API series.

yes, I think so.

> When copying code to DPDK from elsewhere, please adapt its style to DPDK
> conventions (e.g. `RTE_UNUSED(foo)` instead if `(void)foo`) and mind the
> license if big pieces are copied verbatim (this is not the case here).
>
> > https://stackoverflow.com/questions/10905892/equivalent-of-gettimeday-for-windows
>
> POSIX gettimeofday() should be replaced with standard C timespec_get().
>
> > Thanks.
> >
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> >  drivers/net/ena/base/ena_com.c        |   4 +-
> >  drivers/net/ena/base/ena_plat.h       |   2 +-
> >  drivers/net/ena/base/ena_plat_dpdk.h  |  14 +++-
> >  drivers/net/ena/meson.build           |   5 --
> >  lib/eal/version.map                   |   2 +-
> >  lib/eal/windows/include/pthread.h     | 105 ++++++++++++++++++++++++++
> >  lib/eal/windows/include/rte_windows.h |   1 +
> >  7 files changed, 120 insertions(+), 13 deletions(-)
>
> This should be a series of two patches:
> 1) adding condition variable wrappers to EAL;
> 2) supporting net/ena for Windows.
>
Thanks Dmitry! Will work on v2.
William

William
William Tu Aug. 19, 2021, 2:19 a.m. UTC | #3
On Sat, Aug 14, 2021 at 4:31 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Hi William,
>
> 2021-08-14 03:36 (UTC+0000), William Tu:
> > I don't have a physical Windows testbed so I want to see if I can
> > get virtual nic working, in this case ENA driver on AWS.
> > The patch passes build on Windows, but I haven't tested loading
> > the ena driver.
> > I want to know if this is the right direction,
> > or whether I also need to change other places in ENA driver?
>
> Cc'ing maintainers.
>
> > I copy some of the pthread code from
> > https://nachtimwald.com/2019/04/05/cross-platform-thread-wrapper/
>
> We don't want to add more pthread shims, pthread.h in DPDK will go away.
> Condition variable support should be added to the new threading API:
>
>         http://inbox.dpdk.org/dev/1628017291-3756-1-git-send-email-navasile@linux.microsoft.com
>
> I suppose it can be done independently of the new threading API series.
> When copying code to DPDK from elsewhere, please adapt its style to DPDK
> conventions (e.g. `RTE_UNUSED(foo)` instead if `(void)foo`) and mind the
> license if big pieces are copied verbatim (this is not the case here).
>
> > https://stackoverflow.com/questions/10905892/equivalent-of-gettimeday-for-windows
>
> POSIX gettimeofday() should be replaced with standard C timespec_get().
>
> > Thanks.
> >
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> >  drivers/net/ena/base/ena_com.c        |   4 +-
> >  drivers/net/ena/base/ena_plat.h       |   2 +-
> >  drivers/net/ena/base/ena_plat_dpdk.h  |  14 +++-
> >  drivers/net/ena/meson.build           |   5 --
> >  lib/eal/version.map                   |   2 +-
> >  lib/eal/windows/include/pthread.h     | 105 ++++++++++++++++++++++++++
> >  lib/eal/windows/include/rte_windows.h |   1 +
> >  7 files changed, 120 insertions(+), 13 deletions(-)
>
> This should be a series of two patches:
> 1) adding condition variable wrappers to EAL;
> 2) supporting net/ena for Windows.
>
So I finally set up everything on AWS and tested this patch.
Virt2phys and netuio (I added ena conf) load OK.
Finally, starting dpdk-testpmd, I got some errors below:
---
PS C:\dpdk-kmods> cd c:\dpdk
PS C:\dpdk> .\build\app\dpdk-testpmd.exe
EAL: Detected 4 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process support is requested, but not available.
EAL: WARNING: TSC frequency estimated roughly - clock timings may be
less accurate.
EAL: Requested device 0000:00:05.0 cannot be used
EAL: Probe PCI driver: net_ena (1d0f:ec20) device: 0000:00:06.0 (socket 0)
EAL: eth_ena_pci_probe[ENA_COM:
ena_com_wait_and_process_admin_cq_interrupts]Invalid wait event.
pthread ret: 1
[ENA_COM: ena_com_wait_and_process_admin_cq_interrupts]The ena device
sent a completion but the driver didn
't receive a MSI-X interrupt (cmd 9), autopolling mode is OFF
[ENA_COM: ena_com_set_dev_mtu]Failed to set mtu 1500. error: -137
ena_mtu_set(): Could not set MTU: 1500
Failed to set MTU to 1500 for port 0
testpmd: create a new mbuf pool <mb_pool_0>: n=171456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc

Warning! port-topology=paired and odd forward ports number, the last
port will pair with itself.

Configuring Port 0 (socket 0)
[ENA_COM: ena_com_create_io_cq]Failed to create IO CQ. error: -19
ena_create_io_queue(): Failed to create IO queue[0] (qid:1), rc: -19
ena_queue_start(): Failed to create IO queue
ena_queue_start_all(): Failed to start queue[0] of type(1)
Fail to start port 0: No such device
Please stop the ports first
Done
Error during enabling promiscuous mode for port 0: Unknown error - ignore
No commandline core given, start packet forwarding
Not all ports were started
---
I will see if I can fix these issues and I will submit v2 patch.
Thanks
William
Michal Krawczyk Aug. 30, 2021, 7:11 a.m. UTC | #4
czw., 19 sie 2021 o 04:19 William Tu <u9012063@gmail.com> napisał(a):
>
> So I finally set up everything on AWS and tested this patch.
> Virt2phys and netuio (I added ena conf) load OK.
> Finally, starting dpdk-testpmd, I got some errors below:
> ---
> PS C:\dpdk-kmods> cd c:\dpdk
> PS C:\dpdk> .\build\app\dpdk-testpmd.exe
> EAL: Detected 4 lcore(s)
> EAL: Detected 1 NUMA nodes
> EAL: Multi-process support is requested, but not available.
> EAL: WARNING: TSC frequency estimated roughly - clock timings may be
> less accurate.
> EAL: Requested device 0000:00:05.0 cannot be used
> EAL: Probe PCI driver: net_ena (1d0f:ec20) device: 0000:00:06.0 (socket 0)
> EAL: eth_ena_pci_probe[ENA_COM:
> ena_com_wait_and_process_admin_cq_interrupts]Invalid wait event.
> pthread ret: 1
> [ENA_COM: ena_com_wait_and_process_admin_cq_interrupts]The ena device
> sent a completion but the driver didn
> 't receive a MSI-X interrupt (cmd 9), autopolling mode is OFF
> [ENA_COM: ena_com_set_dev_mtu]Failed to set mtu 1500. error: -137
> ena_mtu_set(): Could not set MTU: 1500
> Failed to set MTU to 1500 for port 0
> testpmd: create a new mbuf pool <mb_pool_0>: n=171456, size=2176, socket=0
> testpmd: preferred mempool ops selected: ring_mp_mc
>
> Warning! port-topology=paired and odd forward ports number, the last
> port will pair with itself.
>
> Configuring Port 0 (socket 0)
> [ENA_COM: ena_com_create_io_cq]Failed to create IO CQ. error: -19
> ena_create_io_queue(): Failed to create IO queue[0] (qid:1), rc: -19
> ena_queue_start(): Failed to create IO queue
> ena_queue_start_all(): Failed to start queue[0] of type(1)
> Fail to start port 0: No such device
> Please stop the ports first
> Done
> Error during enabling promiscuous mode for port 0: Unknown error - ignore
> No commandline core given, start packet forwarding
> Not all ports were started
> ---
> I will see if I can fix these issues and I will submit v2 patch.
> Thanks

Hi William,

It's great to hear that you're working on ENA support for Windows!

ENA PMD uses admin interrupt for processing all the commands like
creating the IO queues, setting up the MTU, etc., and also for the
AENQ events. With the current driver design it's critical to have this
admin interrupt working.

It looks like the admin interrupt is not functional and from what I've
seen in the email regarding the v21.11 roadmap for the Windows
support, the netuio interrupt support is going to be added in the
future. That might be the reason for you seeing those errors.

Thanks,
Michal

> William
William Tu Aug. 30, 2021, 2:05 p.m. UTC | #5
On Mon, Aug 30, 2021 at 12:12 AM Michał Krawczyk <mk@semihalf.com> wrote:
[...]
> Hi William,
>
> It's great to hear that you're working on ENA support for Windows!
>
> ENA PMD uses admin interrupt for processing all the commands like
> creating the IO queues, setting up the MTU, etc., and also for the
> AENQ events. With the current driver design it's critical to have this
> admin interrupt working.
>
> It looks like the admin interrupt is not functional and from what I've
> seen in the email regarding the v21.11 roadmap for the Windows
> support, the netuio interrupt support is going to be added in the
> future. That might be the reason for you seeing those errors.

Hi Michal,
Thank you! Then I will wait for netuio support.
William
diff mbox series

Patch

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 5ca36ab6d9..587e7b6ed1 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1375,11 +1375,11 @@  int ena_com_execute_admin_command(struct ena_com_admin_queue *admin_queue,
 	if (IS_ERR(comp_ctx)) {
 		if (comp_ctx == ERR_PTR(ENA_COM_NO_DEVICE))
 			ena_trc_dbg(admin_queue->ena_dev,
-				    "Failed to submit command [%ld]\n",
+				    "Failed to submit command [%" PRIu64 "]\n",
 				    PTR_ERR(comp_ctx));
 		else
 			ena_trc_err(admin_queue->ena_dev,
-				    "Failed to submit command [%ld]\n",
+				    "Failed to submit command [%" PRIu64 "]\n",
 				    PTR_ERR(comp_ctx));
 
 		return (int)PTR_ERR(comp_ctx);
diff --git a/drivers/net/ena/base/ena_plat.h b/drivers/net/ena/base/ena_plat.h
index 2583823080..555cf23e5e 100644
--- a/drivers/net/ena/base/ena_plat.h
+++ b/drivers/net/ena/base/ena_plat.h
@@ -21,7 +21,7 @@ 
 #include <ena_plat_dpdk.h>
 #endif
 #elif defined(_WIN32)
-#include <ena_plat_windows.h>
+#include <ena_plat_dpdk.h>
 #else
 #error "Invalid platform"
 #endif
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 4e7f52881a..bba61d7180 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -13,6 +13,7 @@ 
 #include <inttypes.h>
 #include <string.h>
 #include <errno.h>
+#include <pthread.h>
 
 #include <rte_atomic.h>
 #include <rte_branch_prediction.h>
@@ -24,7 +25,12 @@ 
 #include <rte_prefetch.h>
 #include <rte_spinlock.h>
 
+#if defined(_WIN32)
+#include <rte_time.h>
+#include <rte_thread.h>
+#else
 #include <sys/time.h>
+#endif
 #include <rte_memcpy.h>
 
 typedef uint64_t u64;
@@ -293,9 +299,9 @@  extern rte_atomic64_t ena_alloc_cnt;
 #define dma_rmb() rmb()
 
 #define MAX_ERRNO       4095
-#define IS_ERR(x) (((unsigned long)x) >= (unsigned long)-MAX_ERRNO)
-#define ERR_PTR(error) ((void *)(long)error)
-#define PTR_ERR(error) ((long)(void *)error)
+#define IS_ERR(x) (((uintptr_t)x) >= (uintptr_t)-MAX_ERRNO)
+#define ERR_PTR(error) ((void *)(uintptr_t)error)
+#define PTR_ERR(error) ((uintptr_t)(void *)error)
 #define might_sleep()
 
 #define prefetch(x) rte_prefetch0(x)
@@ -322,7 +328,7 @@  extern rte_atomic64_t ena_alloc_cnt;
 
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
-#define ENA_FFS(x) ffs(x)
+#define ENA_FFS(x) __builtin_ffs(x)
 
 void ena_rss_key_fill(void *key, size_t size);
 
diff --git a/drivers/net/ena/meson.build b/drivers/net/ena/meson.build
index d02ed3f64f..57f8050ccb 100644
--- a/drivers/net/ena/meson.build
+++ b/drivers/net/ena/meson.build
@@ -1,11 +1,6 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
-if is_windows
-    build = false
-    reason = 'not supported on Windows'
-    subdir_done()
-endif
 
 sources = files(
         'ena_ethdev.c',
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 887012d02a..3de59910cd 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -198,7 +198,7 @@  DPDK_21 {
 	rte_uuid_is_null; # WINDOWS_NO_EXPORT
 	rte_uuid_parse; # WINDOWS_NO_EXPORT
 	rte_uuid_unparse; # WINDOWS_NO_EXPORT
-	rte_version; # WINDOWS_NO_EXPORT
+	rte_version;
 	rte_vfio_clear_group; # WINDOWS_NO_EXPORT
 	rte_vfio_container_create; # WINDOWS_NO_EXPORT
 	rte_vfio_container_destroy; # WINDOWS_NO_EXPORT
diff --git a/lib/eal/windows/include/pthread.h b/lib/eal/windows/include/pthread.h
index 27fd2cca52..4dc39b9a60 100644
--- a/lib/eal/windows/include/pthread.h
+++ b/lib/eal/windows/include/pthread.h
@@ -7,6 +7,7 @@ 
 
 #include <stdint.h>
 #include <sched.h>
+#include <time.h>
 
 /**
  * This file is required to support the common code in eal_common_proc.c,
@@ -34,6 +35,10 @@  typedef CRITICAL_SECTION pthread_mutex_t;
 
 typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
 
+typedef CONDITION_VARIABLE pthread_cond_t;
+
+typedef void pthread_condattr_t;
+
 #define pthread_barrier_init(barrier, attr, count) \
 	!InitializeSynchronizationBarrier(barrier, count, -1)
 #define pthread_barrier_wait(barrier) EnterSynchronizationBarrier(barrier, \
@@ -185,6 +190,106 @@  pthread_mutex_destroy(pthread_mutex_t *mutex)
 	return 0;
 }
 
+static inline DWORD
+timespec_to_ms(const struct timespec *abstime)
+{
+	DWORD t;
+
+	if (abstime == NULL)
+		return INFINITE;
+
+	t = ((abstime->tv_sec - time(NULL)) * 1000) +
+		(abstime->tv_nsec / 1000000);
+	if (t < 0)
+		t = 1;
+	return t;
+}
+
+static inline int
+pthread_cond_init(pthread_cond_t *cond, pthread_condattr_t *attr)
+{
+	(void)attr;
+	if (cond == NULL)
+		return 1;
+	InitializeConditionVariable(cond);
+	return 0;
+}
+
+static inline int
+pthread_cond_destroy(pthread_cond_t *cond)
+{
+	/* Windows does not have a destroy for conditionals */
+	(void)cond;
+	return 0;
+}
+
+static inline int
+pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
+		const struct timespec *abstime)
+{
+	if (cond == NULL || mutex == NULL)
+		return 1;
+	if (!SleepConditionVariableCS(cond, mutex, timespec_to_ms(abstime)))
+		return 1;
+	return 0;
+}
+
+static inline int
+pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
+{
+	if (cond == NULL || mutex == NULL)
+		return 1;
+	return pthread_cond_timedwait(cond, mutex, NULL);
+}
+
+static inline int
+pthread_cond_signal(pthread_cond_t *cond)
+{
+	if (cond == NULL)
+		return 1;
+	WakeConditionVariable(cond);
+	return 0;
+}
+
+static inline int
+pthread_cond_broadcast(pthread_cond_t *cond)
+{
+	if (cond == NULL)
+		return 1;
+	WakeAllConditionVariable(cond);
+	return 0;
+}
+
+struct timezone {
+	int tz_minuteswest;
+	int tz_dsttime;
+};
+
+static inline int
+gettimeofday(struct timeval *tv, struct timezone *tz)
+{
+	if (tv) {
+		FILETIME filetime;
+		ULARGE_INTEGER x;
+		ULONGLONG usec;
+		static const ULONGLONG epoch_ofs_us = 11644473600000000ULL;
+
+		GetSystemTimePreciseAsFileTime(&filetime);
+		x.LowPart = filetime.dwLowDateTime;
+		x.HighPart = filetime.dwHighDateTime;
+		usec = x.QuadPart / 10 - epoch_ofs_us;
+		tv->tv_sec = (time_t)(usec / 1000000ULL);
+		tv->tv_usec = (long)(usec % 1000000ULL);
+	}
+	if (tz) {
+		TIME_ZONE_INFORMATION timezone;
+		GetTimeZoneInformation(&timezone);
+		tz->tz_minuteswest = timezone.Bias;
+		tz->tz_dsttime = 0;
+	}
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/windows/include/rte_windows.h b/lib/eal/windows/include/rte_windows.h
index 0063b5d78c..3240970ab7 100644
--- a/lib/eal/windows/include/rte_windows.h
+++ b/lib/eal/windows/include/rte_windows.h
@@ -31,6 +31,7 @@ 
 #include <psapi.h>
 #include <setupapi.h>
 #include <winioctl.h>
+#include <winsock2.h>
 
 /* Have GUIDs defined. */
 #ifndef INITGUID