DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/7] cmdline: support Windows
@ 2020-06-20 21:05 Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
                   ` (9 more replies)
  0 siblings, 10 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk

This patchset enables librte_cmdline on Windows. To do that, it creates
a number of wrappers for OS-dependent terminal handling and I/O.
Considered alternative was to revive [1] and use libedit (Unix-only)
for terminal handling. However, testing revealed that WinEditLine [2]
is not a drop-in replacement for libedit, so this solution wouldn't be
universal.

Note: not all dependency series are up-to-date at the time of
submission, so this version may require hacking to test.

[1]: http://patchwork.dpdk.org/patch/38561
[2]: http://mingweditline.sourceforge.net

---
Depends-on: series-10512 ("Windows bus/pci support ")
Depends-on: series-10244 ("[v4] eal/windows: ring build on Windows")
Depends-on: series-10282 ("build mempool on Windows ")
Depends-on: series-10382 ("compile librte_net for windows")

Dmitry Kozlyuk (7):
  cmdline: make implementation opaque
  cmdline: add internal wrappers for terminal handling
  cmdline: add internal wrappers for character input
  cmdline: add internal wrapper for vdprintf
  eal/windows: improve compatibility networking headers
  cmdline: support Windows
  examples/cmdline: build on Windows

 app/test-cmdline/commands.c                 |   8 +-
 app/test/test_cmdline_lib.c                 |  42 ++--
 config/meson.build                          |   2 +
 examples/cmdline/commands.c                 |   1 -
 examples/cmdline/main.c                     |   1 -
 examples/meson.build                        |   6 +-
 lib/librte_cmdline/Makefile                 |   4 +
 lib/librte_cmdline/cmdline.c                |  29 +--
 lib/librte_cmdline/cmdline.h                |  15 +-
 lib/librte_cmdline/cmdline_os_unix.c        |  53 +++++
 lib/librte_cmdline/cmdline_os_windows.c     | 207 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_parse.c          |   4 +-
 lib/librte_cmdline/cmdline_private.h        |  61 ++++++
 lib/librte_cmdline/cmdline_socket.c         |  25 +--
 lib/librte_cmdline/cmdline_vt100.c          |   1 -
 lib/librte_cmdline/cmdline_vt100.h          |   4 +
 lib/librte_cmdline/meson.build              |   6 +
 lib/librte_cmdline/rte_cmdline_version.map  |   8 +
 lib/librte_eal/windows/include/arpa/inet.h  |  30 +++
 lib/librte_eal/windows/include/netinet/in.h |  11 ++
 lib/librte_eal/windows/include/pthread.h    |   2 +-
 lib/librte_eal/windows/include/sys/socket.h |  24 +++
 lib/meson.build                             |   1 +
 23 files changed, 470 insertions(+), 75 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c
 create mode 100644 lib/librte_cmdline/cmdline_private.h
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

-- 
2.25.4


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

* [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Olivier Matz, Ray Kinsella,
	Neil Horman

struct cmdline exposes platform-specific members it contains, most
notably struct termios that is only available on Unix. Make the
structure opaque.

Remove tests checking struct cmdline content as meaningless.

Add cmdline_get_rdline() to access history buffer.
The new function is currently used only in tests.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 app/test-cmdline/commands.c                |  8 +++--
 app/test/test_cmdline_lib.c                | 42 +++++++++-------------
 lib/librte_cmdline/cmdline.c               | 10 ++++--
 lib/librte_cmdline/cmdline.h               | 15 ++++----
 lib/librte_cmdline/cmdline_parse.c         |  4 +--
 lib/librte_cmdline/cmdline_private.h       | 22 ++++++++++++
 lib/librte_cmdline/cmdline_socket.c        |  6 ++--
 lib/librte_cmdline/rte_cmdline_version.map |  8 +++++
 8 files changed, 68 insertions(+), 47 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_private.h

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d67c0ca6a..ff7ac973e 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -294,8 +294,10 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(cl->rdl.history_buf));
+			sizeof(rdl->history_buf));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
@@ -326,7 +328,9 @@ cmd_clear_history_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
-	rdline_clear_history(&cl->rdl);
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
+	rdline_clear_history(rdl);
 }
 
 cmdline_parse_token_string_t cmd_clear_history_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index dec465da5..a7f5a7f7f 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
 static int
 test_cmdline_parse_fns(void)
 {
-	struct cmdline cl;
+	struct cmdline *cl;
+	cmdline_parse_ctx_t ctx;
 	int i = 0;
 	char dst[CMDLINE_TEST_BUFSIZE];
 
+	cl = cmdline_new(&ctx, "prompt", -1, -1);
+	if (cl == NULL) {
+		printf("Error: cannot create cmdline to test parse fns!\n");
+		return -1;
+	}
+
 	if (cmdline_parse(NULL, "buffer") >= 0)
 		goto error;
-	if (cmdline_parse(&cl, NULL) >= 0)
+	if (cmdline_parse(cl, NULL) >= 0)
 		goto error;
 
 	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
 		goto error;
 
 	return 0;
@@ -166,11 +173,11 @@ static int
 test_cmdline_fns(void)
 {
 	cmdline_parse_ctx_t ctx;
-	struct cmdline cl, *tmp;
+	struct cmdline *cl;
 
 	memset(&ctx, 0, sizeof(ctx));
-	tmp = cmdline_new(&ctx, "test", -1, -1);
-	if (tmp == NULL)
+	cl = cmdline_new(&ctx, "test", -1, -1);
+	if (cl == NULL)
 		goto error;
 
 	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
@@ -179,7 +186,7 @@ test_cmdline_fns(void)
 		goto error;
 	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
-	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
+	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
 	if (cmdline_write_char(NULL, 0) >= 0)
 		goto error;
@@ -188,31 +195,14 @@ test_cmdline_fns(void)
 	cmdline_set_prompt(NULL, "prompt");
 	cmdline_free(NULL);
 	cmdline_printf(NULL, "format");
-	/* this should fail as stream handles are invalid */
-	cmdline_printf(tmp, "format");
 	cmdline_interact(NULL);
 	cmdline_quit(NULL);
 
-	/* check if void calls change anything when they should fail */
-	cl = *tmp;
-
-	cmdline_printf(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_set_prompt(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-
-	cmdline_free(tmp);
-
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
 	return -1;
-mismatch:
-	printf("Error: data changed!\n");
-	return -1;
 }
 
 /* test library functions. the point of these tests is not so much to test
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index cfd703e5b..6f3fdd598 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,14 +13,12 @@
 #include <fcntl.h>
 #include <poll.h>
 #include <errno.h>
-#include <termios.h>
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
 
 static void
 cmdline_valid_buffer(struct rdline *rdl, const char *buf,
@@ -103,6 +101,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	return cl;
 }
 
+struct rdline*
+cmdline_get_rdline(struct cmdline *cl)
+{
+	return &cl->rdl;
+}
+
 void
 cmdline_free(struct cmdline *cl)
 {
diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 243f99d20..96674dfda 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -8,8 +8,8 @@
 #define _CMDLINE_H_
 
 #include <rte_common.h>
+#include <rte_compat.h>
 
-#include <termios.h>
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
@@ -23,14 +23,7 @@
 extern "C" {
 #endif
 
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
+struct cmdline;
 
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
@@ -40,6 +33,10 @@ void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 
+__rte_experimental
+struct rdline *
+cmdline_get_rdline(struct cmdline *cl);
+
 /**
  * This function is nonblocking equivalent of ``cmdline_interact()``. It polls
  * *cl* for one character and interpret it. If return value is *RDLINE_EXITED*
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index b57b30e8f..ea0979158 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -10,15 +10,13 @@
 #include <string.h>
 #include <inttypes.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_rdline.h"
-#include "cmdline_parse.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
 
 #ifdef RTE_LIBRTE_CMDLINE_DEBUG
 #define debug_printf printf
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
new file mode 100644
index 000000000..3b1c70e9f
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _CMDLINE_PRIVATE_H_
+#define _CMDLINE_PRIVATE_H_
+
+#include <termios.h>
+
+#include <cmdline_rdline.h>
+#include <cmdline_parse.h>
+
+struct cmdline {
+	int s_in;
+	int s_out;
+	cmdline_parse_ctx_t *ctx;
+	struct rdline rdl;
+	char prompt[RDLINE_PROMPT_SIZE];
+	struct termios oldterm;
+};
+
+#endif
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index ecb3d82b6..5e4b734d6 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -11,12 +11,10 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <termios.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
-#include "cmdline_socket.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
+#include "cmdline_socket.h"
 
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
diff --git a/lib/librte_cmdline/rte_cmdline_version.map b/lib/librte_cmdline/rte_cmdline_version.map
index 95fce812f..135ecc71f 100644
--- a/lib/librte_cmdline/rte_cmdline_version.map
+++ b/lib/librte_cmdline/rte_cmdline_version.map
@@ -69,3 +69,11 @@ DPDK_20.0 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	cmdline_get_rdline;
+
+	local: *;
+};
-- 
2.25.4


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

* [dpdk-dev] [PATCH 2/7] cmdline: add internal wrappers for terminal handling
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Olivier Matz

Extract struct terminal and associated functions that set up, save, and
restore terminal parameters. Use existing code as Unix implementation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/Makefile          |  4 ++++
 lib/librte_cmdline/cmdline_os_unix.c | 27 +++++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h | 12 +++++++++++-
 lib/librte_cmdline/cmdline_socket.c  | 15 ++++-----------
 lib/librte_cmdline/cmdline_vt100.c   |  1 -
 lib/librte_cmdline/meson.build       |  4 ++++
 6 files changed, 50 insertions(+), 13 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c

diff --git a/lib/librte_cmdline/Makefile b/lib/librte_cmdline/Makefile
index 619d9a242..3d8e84c07 100644
--- a/lib/librte_cmdline/Makefile
+++ b/lib/librte_cmdline/Makefile
@@ -23,6 +23,10 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_vt100.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_socket.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_parse_portlist.c
 
+ifneq ($(CONFIG_RTE_EXEC_ENV_WINDOWS),y)
+SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_os_unix.c
+endif
+
 LDLIBS += -lrte_net -lrte_eal
 
 # install includes
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
new file mode 100644
index 000000000..ca47bd19f
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <string.h>
+
+#include "cmdline_private.h"
+
+void
+terminal_adjust(struct terminal *oldterm)
+{
+	struct termios term;
+
+	tcgetattr(0, &oldterm->termios);
+
+	memcpy(&term, &oldterm->termios, sizeof(term));
+	term.c_lflag &= ~(ICANON | ECHO | ISIG);
+	tcsetattr(0, TCSANOW, &term);
+
+	setbuf(stdin, NULL);
+}
+
+void
+terminal_restore(const struct terminal *oldterm)
+{
+	tcsetattr(fileno(stdin), TCSANOW, &oldterm->termios);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 3b1c70e9f..adc552845 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -10,13 +10,23 @@
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
+struct terminal {
+	struct termios termios;
+};
+
+/* Disable buffering and echoing, save previous settings to oldterm. */
+void terminal_adjust(struct terminal *oldterm);
+
+/* Restore terminal settings form oldterm. */
+void terminal_restore(const struct terminal *oldterm);
+
 struct cmdline {
 	int s_in;
 	int s_out;
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
+	struct terminal oldterm;
 };
 
 #endif
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index 5e4b734d6..e73666f15 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -37,18 +37,11 @@ struct cmdline *
 cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 {
 	struct cmdline *cl;
-	struct termios oldterm, term;
-
-	tcgetattr(0, &oldterm);
-	memcpy(&term, &oldterm, sizeof(term));
-	term.c_lflag &= ~(ICANON | ECHO | ISIG);
-	tcsetattr(0, TCSANOW, &term);
-	setbuf(stdin, NULL);
 
 	cl = cmdline_new(ctx, prompt, 0, 1);
 
-	if (cl)
-		memcpy(&cl->oldterm, &oldterm, sizeof(term));
+	if (cl != NULL)
+		terminal_adjust(&cl->oldterm);
 
 	return cl;
 }
@@ -56,8 +49,8 @@ cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 void
 cmdline_stdin_exit(struct cmdline *cl)
 {
-	if (!cl)
+	if (cl == NULL)
 		return;
 
-	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
+	terminal_restore(&cl->oldterm);
 }
diff --git a/lib/librte_cmdline/cmdline_vt100.c b/lib/librte_cmdline/cmdline_vt100.c
index 662fc7345..bb968dd5f 100644
--- a/lib/librte_cmdline/cmdline_vt100.c
+++ b/lib/librte_cmdline/cmdline_vt100.c
@@ -10,7 +10,6 @@
 #include <string.h>
 #include <stdarg.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include "cmdline_vt100.h"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 7fc54ff1a..5c9e8886d 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,4 +25,8 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
+if not is_windows
+	sources += files('cmdline_os_unix.c')
+endif
+
 deps += ['net']
-- 
2.25.4


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

* [dpdk-dev] [PATCH 3/7] cmdline: add internal wrappers for character input
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Olivier Matz

poll(3) is a purely Unix facility, so it cannot be directly used by
common code. read(2) is limited in device support outside of Unix.
Create wrapper functions and implement them for Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 12 +++---------
 lib/librte_cmdline/cmdline_os_unix.c | 20 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h |  6 ++++++
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 6f3fdd598..a04719998 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <poll.h>
 #include <errno.h>
 #include <netinet/in.h>
 
@@ -185,7 +184,6 @@ cmdline_quit(struct cmdline *cl)
 int
 cmdline_poll(struct cmdline *cl)
 {
-	struct pollfd pfd;
 	int status;
 	ssize_t read_status;
 	char c;
@@ -195,16 +193,12 @@ cmdline_poll(struct cmdline *cl)
 	else if (cl->rdl.status == RDLINE_EXITED)
 		return RDLINE_EXITED;
 
-	pfd.fd = cl->s_in;
-	pfd.events = POLLIN;
-	pfd.revents = 0;
-
-	status = poll(&pfd, 1, 0);
+	status = cmdline_poll_char(cl);
 	if (status < 0)
 		return status;
 	else if (status > 0) {
 		c = -1;
-		read_status = read(cl->s_in, &c, 1);
+		read_status = cmdline_read_char(cl, &c);
 		if (read_status < 0)
 			return read_status;
 
@@ -226,7 +220,7 @@ cmdline_interact(struct cmdline *cl)
 
 	c = -1;
 	while (1) {
-		if (read(cl->s_in, &c, 1) <= 0)
+		if (cmdline_read_char(cl, &c) <= 0)
 			break;
 		if (cmdline_in(cl, &c, 1) < 0)
 			break;
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index ca47bd19f..865a89ddd 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -2,7 +2,9 @@
  * Copyright (c) 2020 Dmitry Kozlyuk
  */
 
+#include <poll.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "cmdline_private.h"
 
@@ -25,3 +27,21 @@ terminal_restore(const struct terminal *oldterm)
 {
 	tcsetattr(fileno(stdin), TCSANOW, &oldterm->termios);
 }
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	struct pollfd pfd;
+
+	pfd.fd = cl->s_in;
+	pfd.events = POLLIN;
+	pfd.revents = 0;
+
+	return poll(&pfd, 1, 0);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	return read(cl->s_in, c, 1);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index adc552845..ecfeb89f6 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -29,4 +29,10 @@ struct cmdline {
 	struct terminal oldterm;
 };
 
+/* Check if a single character can be read from input. */
+int cmdline_poll_char(struct cmdline *cl);
+
+/* Read one character from input. */
+ssize_t cmdline_read_char(struct cmdline *cl, char *c);
+
 #endif
-- 
2.25.4


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

* [dpdk-dev] [PATCH 4/7] cmdline: add internal wrapper for vdprintf
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (2 preceding siblings ...)
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Olivier Matz

Add internal wrapper for vdprintf(3) that is only available on Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 2 +-
 lib/librte_cmdline/cmdline_os_unix.c | 6 ++++++
 lib/librte_cmdline/cmdline_private.h | 8 ++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index a04719998..00b9e6b2e 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -132,7 +132,7 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 	if (cl->s_out < 0)
 		return;
 	va_start(ap, fmt);
-	vdprintf(cl->s_out, fmt, ap);
+	cmdline_vdprintf(cl->s_out, fmt, ap);
 	va_end(ap);
 }
 
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index 865a89ddd..2052cd254 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -45,3 +45,9 @@ cmdline_read_char(struct cmdline *cl, char *c)
 {
 	return read(cl->s_in, c, 1);
 }
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	return vdprintf(fd, format, op);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index ecfeb89f6..338d3d55c 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -7,6 +7,10 @@
 
 #include <termios.h>
 
+#include <stdarg.h>
+
+#include <rte_common.h>
+
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
@@ -35,4 +39,8 @@ int cmdline_poll_char(struct cmdline *cl);
 /* Read one character from input. */
 ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 
+/* vdprintf(3) */
+__rte_format_printf(2, 0)
+int cmdline_vdprintf(int fd, const char *format, va_list op);
+
 #endif
-- 
2.25.4


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

* [dpdk-dev] [PATCH 5/7] eal/windows: improve compatibility networking headers
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (3 preceding siblings ...)
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 6/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Harini Ramakrishnan,
	Omar Cardona, Pallavi Kadam, Ranjit Menon

Extend compatibility header system to support librte_cmdline.

pthread.h has to include windows.h, which exposes struct in_addr, etc.
conflicting with compatibility headers. WIN32_LEAN_AND_MEAN macro
is required to disable this behavior. Use rte_windows.h to define
WIN32_LEAN_AND_MEAN for pthread library.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---

Assumming __attribute__((stdcall)) is rare enough so that the following
checkpatch complaint can be ignored:

    Warning in /lib/librte_eal/windows/include/arpa/inet.h:
    Using compiler attribute directly

 lib/librte_eal/windows/include/arpa/inet.h  | 30 +++++++++++++++++++++
 lib/librte_eal/windows/include/netinet/in.h | 11 ++++++++
 lib/librte_eal/windows/include/pthread.h    |  2 +-
 lib/librte_eal/windows/include/sys/socket.h | 24 +++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

diff --git a/lib/librte_eal/windows/include/arpa/inet.h b/lib/librte_eal/windows/include/arpa/inet.h
new file mode 100644
index 000000000..96b698438
--- /dev/null
+++ b/lib/librte_eal/windows/include/arpa/inet.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _ARPA_INET_H_
+#define _ARPA_INET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+int
+inet_pton(int af, const char *src, void *dst);
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+const char *
+inet_ntop(int af, const void *src, char *dst, socklen_t size);
+
+#endif /* _ARPA_INET_H_ */
diff --git a/lib/librte_eal/windows/include/netinet/in.h b/lib/librte_eal/windows/include/netinet/in.h
index 489b587c3..ed46163b5 100644
--- a/lib/librte_eal/windows/include/netinet/in.h
+++ b/lib/librte_eal/windows/include/netinet/in.h
@@ -5,6 +5,8 @@
 #ifndef _IN_H_
 #define _IN_H_
 
+#include <sys/socket.h>
+
 #define IPPROTO_IP 0
 #define IPPROTO_HOPOPTS 0
 #define	IPPROTO_IPV4 4             /* IPv4 encapsulation */
@@ -21,5 +23,14 @@
 #define	IPPROTO_DSTOPTS 60         /* IP6 destination option */
 #define IPPROTO_SCTP 132           /* Stream Control Transmission Protocol */
 
+#define INET6_ADDRSTRLEN 46
+
+struct in_addr {
+	uint32_t s_addr;
+};
+
+struct in6_addr {
+	uint8_t s6_addr[16];
+};
 
 #endif
diff --git a/lib/librte_eal/windows/include/pthread.h b/lib/librte_eal/windows/include/pthread.h
index e2274cf4e..b4dbee9d8 100644
--- a/lib/librte_eal/windows/include/pthread.h
+++ b/lib/librte_eal/windows/include/pthread.h
@@ -16,8 +16,8 @@
 extern "C" {
 #endif
 
-#include <windows.h>
 #include <rte_common.h>
+#include <rte_windows.h>
 
 #define PTHREAD_BARRIER_SERIAL_THREAD TRUE
 
diff --git a/lib/librte_eal/windows/include/sys/socket.h b/lib/librte_eal/windows/include/sys/socket.h
new file mode 100644
index 000000000..9536cf8e6
--- /dev/null
+++ b/lib/librte_eal/windows/include/sys/socket.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _SYS_SOCKET_H_
+#define _SYS_SOCKET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <stddef.h>
+
+#define AF_INET  2
+#define AF_INET6 23
+
+typedef size_t socklen_t;
+
+#endif /* _SYS_SOCKET_H_ */
-- 
2.25.4


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

* [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (4 preceding siblings ...)
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-06-28 14:20   ` Fady Bader
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Thomas Monjalon, Olivier Matz

Implement terminal handling, input polling, and vdprintf() for Windows.

Because Windows I/O model differs fundamentally from Unix and there is
no concept of character device, polling is simulated depending on the
underlying inpue device. Supporting non-terminal input is useful for
automated testing.

Windows emulation of VT100 uses "ESC [ E" for newline instead of
standard "ESC E", so a workaround is added.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build                      |   2 +
 lib/librte_cmdline/cmdline.c            |   5 +
 lib/librte_cmdline/cmdline_os_windows.c | 207 ++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h    |  15 ++
 lib/librte_cmdline/cmdline_socket.c     |   4 +
 lib/librte_cmdline/cmdline_vt100.h      |   4 +
 lib/librte_cmdline/meson.build          |   4 +-
 lib/meson.build                         |   1 +
 8 files changed, 241 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c

diff --git a/config/meson.build b/config/meson.build
index d3f05f878..733b5e310 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -269,6 +269,8 @@ if is_windows
 		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
 	endif
 
+	add_project_link_arguments('-lws2_32', language: 'c')
+
 	# Contrary to docs, VirtualAlloc2() is exported by mincore.lib
 	# in Windows SDK, while MinGW exports it by advapi32.a.
 	if is_ms_linker
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 00b9e6b2e..c0ddb5f23 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,9 +13,14 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <netinet/in.h>
+#include <unistd.h>
 
 #include <rte_string_fns.h>
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define write _write
+#endif
+
 #include "cmdline.h"
 #include "cmdline_private.h"
 
diff --git a/lib/librte_cmdline/cmdline_os_windows.c b/lib/librte_cmdline/cmdline_os_windows.c
new file mode 100644
index 000000000..9736f6531
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_windows.c
@@ -0,0 +1,207 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <io.h>
+
+#include <rte_os.h>
+
+#include "cmdline_private.h"
+
+/* Missing from some MinGW-w64 distributions. */
+#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING
+#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004
+#endif
+
+#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT
+#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200
+#endif
+
+void
+terminal_adjust(struct terminal *oldterm)
+{
+	HANDLE handle;
+	DWORD mode;
+
+	ZeroMemory(oldterm, sizeof(*oldterm));
+
+	/* Detect console input, set it up and make it emulate VT100. */
+	handle = GetStdHandle(STD_INPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		oldterm->is_console_input = 1;
+		oldterm->input_mode = mode;
+
+		mode &= ~(
+			ENABLE_LINE_INPUT |      /* no line buffering */
+			ENABLE_ECHO_INPUT |      /* no echo */
+			ENABLE_PROCESSED_INPUT | /* pass Ctrl+C to program */
+			ENABLE_MOUSE_INPUT |     /* no mouse events */
+			ENABLE_WINDOW_INPUT);    /* no window resize events */
+		mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+		SetConsoleMode(handle, mode);
+	}
+
+	/* Detect console output and make it emulate VT100. */
+	handle = GetStdHandle(STD_OUTPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		oldterm->is_console_output = 1;
+		oldterm->output_mode = mode;
+
+		mode &= ~ENABLE_WRAP_AT_EOL_OUTPUT;
+		mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+		SetConsoleMode(handle, mode);
+	}
+}
+
+void
+terminal_restore(const struct terminal *oldterm)
+{
+	if (oldterm->is_console_input) {
+		HANDLE handle = GetStdHandle(STD_INPUT_HANDLE);
+		SetConsoleMode(handle, oldterm->input_mode);
+	}
+
+	if (oldterm->is_console_output) {
+		HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);
+		SetConsoleMode(handle, oldterm->output_mode);
+	}
+}
+
+static int
+cmdline_is_key_down(const INPUT_RECORD *record)
+{
+	return (record->EventType == KEY_EVENT) &&
+		record->Event.KeyEvent.bKeyDown;
+}
+
+static int
+cmdline_poll_char_console(HANDLE handle)
+{
+	INPUT_RECORD record;
+	DWORD events;
+
+	if (!PeekConsoleInput(handle, &record, 1, &events)) {
+		/* Simulate poll(3) behavior on EOF. */
+		return (GetLastError() == ERROR_HANDLE_EOF) ? 1 : -1;
+	}
+
+	if ((events == 0) || !cmdline_is_key_down(&record))
+		return 0;
+
+	return 1;
+}
+
+static int
+cmdline_poll_char_file(struct cmdline *cl, HANDLE handle)
+{
+	DWORD type = GetFileType(handle);
+
+	/* Since console is handled by cmdline_poll_char_console(),
+	 * this is either a serial port or input handle had been replaced.
+	 */
+	if (type == FILE_TYPE_CHAR)
+		return cmdline_poll_char_console(handle);
+
+	/* PeekNamedPipe() can handle all pipes and also sockets. */
+	if (type == FILE_TYPE_PIPE) {
+		DWORD bytes_avail;
+		if (!PeekNamedPipe(handle, NULL, 0, NULL, &bytes_avail, NULL))
+			return (GetLastError() == ERROR_BROKEN_PIPE) ? 1 : -1;
+		return bytes_avail ? 1 : 0;
+	}
+
+	/* There is no straightforward way to peek a file in Windows
+	 * I/O model. Read the byte, if it is not the end of file,
+	 * buffer it for subsequent read. This will not work with
+	 * a file being appended and probably some other edge cases.
+	 */
+	if (type == FILE_TYPE_DISK) {
+		char c;
+		int ret;
+
+		ret = _read(cl->s_in, &c, sizeof(c));
+		if (ret == 1) {
+			cl->repeat_count = 1;
+			cl->repeated_char = c;
+		}
+		return ret;
+	}
+
+	/* GetFileType() failed or file of unknown type,
+	 * which we do not know how to peek anyway.
+	 */
+	return -1;
+}
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	HANDLE handle = (HANDLE)_get_osfhandle(cl->s_in);
+	return cl->oldterm.is_console_input ?
+		cmdline_poll_char_console(handle) :
+		cmdline_poll_char_file(cl, handle);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	HANDLE handle;
+	INPUT_RECORD record;
+	KEY_EVENT_RECORD *key;
+	DWORD events;
+
+	if (!cl->oldterm.is_console_input)
+		return _read(cl->s_in, c, 1);
+
+	/* Return repeated strokes from previous event. */
+	if (cl->repeat_count > 0) {
+		*c = cl->repeated_char;
+		cl->repeat_count--;
+		return 1;
+	}
+
+	handle = (HANDLE)_get_osfhandle(cl->s_in);
+	key = &record.Event.KeyEvent;
+	do {
+		if (!ReadConsoleInput(handle, &record, 1, &events)) {
+			if (GetLastError() == ERROR_HANDLE_EOF) {
+				*c = EOF;
+				return 0;
+			}
+			return -1;
+		}
+	} while (!cmdline_is_key_down(&record));
+
+	*c = key->uChar.AsciiChar;
+
+	/* Save repeated strokes from a single event. */
+	if (key->wRepeatCount > 1) {
+		cl->repeated_char = *c;
+		cl->repeat_count = key->wRepeatCount - 1;
+	}
+
+	return 1;
+}
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	int copy, ret;
+	FILE *file;
+
+	copy = _dup(fd);
+	if (copy < 0)
+		return -1;
+
+	file = _fdopen(copy, "a");
+	if (file == NULL) {
+		_close(copy);
+		return -1;
+	}
+
+	ret = vfprintf(file, format, op);
+
+	fclose(file); /* also closes copy */
+
+	return ret;
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 338d3d55c..1e05ec376 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -5,7 +5,11 @@
 #ifndef _CMDLINE_PRIVATE_H_
 #define _CMDLINE_PRIVATE_H_
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <rte_windows.h>
+#else
 #include <termios.h>
+#endif
 
 #include <stdarg.h>
 
@@ -15,7 +19,14 @@
 #include <cmdline_parse.h>
 
 struct terminal {
+#ifndef RTE_EXEC_ENV_WINDOWS
 	struct termios termios;
+#else
+	DWORD input_mode;
+	DWORD output_mode;
+	int is_console_input;
+	int is_console_output;
+#endif
 };
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
@@ -31,6 +42,10 @@ struct cmdline {
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
 	struct terminal oldterm;
+#ifdef RTE_EXEC_ENV_WINDOWS
+	char repeated_char;
+	WORD repeat_count;
+#endif
 };
 
 /* Check if a single character can be read from input. */
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index e73666f15..c5f483413 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -16,6 +16,10 @@
 #include "cmdline_private.h"
 #include "cmdline_socket.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define open _open
+#endif
+
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
 {
diff --git a/lib/librte_cmdline/cmdline_vt100.h b/lib/librte_cmdline/cmdline_vt100.h
index e33e67ed8..be9ae8e1c 100644
--- a/lib/librte_cmdline/cmdline_vt100.h
+++ b/lib/librte_cmdline/cmdline_vt100.h
@@ -31,7 +31,11 @@ extern "C" {
 #define vt100_multi_right  "\033\133%uC"
 #define vt100_multi_left   "\033\133%uD"
 #define vt100_suppr        "\033\133\063\176"
+#ifndef RTE_EXEC_ENV_WINDOWS
 #define vt100_home         "\033M\033E"
+#else
+#define vt100_home         "\033M\033[E"
+#endif
 #define vt100_word_left    "\033\142"
 #define vt100_word_right   "\033\146"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 5c9e8886d..5009b3354 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,7 +25,9 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
-if not is_windows
+if is_windows
+	sources += files('cmdline_os_windows.c')
+else
 	sources += files('cmdline_os_unix.c')
 endif
 
diff --git a/lib/meson.build b/lib/meson.build
index 40e452025..5b72a2d9e 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -40,6 +40,7 @@ if is_windows
 		'kvargs','eal',
 		'ring',
 		'mempool', 'mbuf', 'pci', 'net',
+		'cmdline',
 	] # only supported libraries for windows
 endif
 
-- 
2.25.4


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

* [dpdk-dev] [PATCH 7/7] examples/cmdline: build on Windows
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (5 preceding siblings ...)
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 6/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-06-20 21:05 ` Dmitry Kozlyuk
  2020-07-17 22:16 ` [dpdk-dev] [PATCH 0/7] cmdline: support Windows Narcisa Ana Maria Vasile
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-20 21:05 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Dmitry Kozlyuk, Olivier Matz

Enable cmdline sample application as all dependencies are met.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 examples/cmdline/commands.c | 1 -
 examples/cmdline/main.c     | 1 -
 examples/meson.build        | 6 +++---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index 0e2232f03..f43eacfba 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <netinet/in.h>
-#include <termios.h>
 #ifdef RTE_EXEC_ENV_FREEBSD
 #include <sys/socket.h>
 #endif
diff --git a/examples/cmdline/main.c b/examples/cmdline/main.c
index f2f2e5a2f..bb7954245 100644
--- a/examples/cmdline/main.c
+++ b/examples/cmdline/main.c
@@ -8,7 +8,6 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
-#include <termios.h>
 #include <sys/queue.h>
 
 #include <cmdline_rdline.h>
diff --git a/examples/meson.build b/examples/meson.build
index 3b540012f..96920219d 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -81,9 +81,9 @@ foreach example: examples
 
 	ext_deps = [execinfo]
 	includes = [include_directories(example)]
-	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
-	if is_windows
-		deps = ['eal'] # only supported lib on Windows currently
+	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
+	if not is_windows
+		deps += ['ethdev'] # not currently supported
 	endif
 	subdir(example)
 
-- 
2.25.4


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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 6/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-06-28 14:20   ` Fady Bader
  2020-06-29  6:23     ` Ranjit Menon
  0 siblings, 1 reply; 50+ messages in thread
From: Fady Bader @ 2020-06-28 14:20 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Tal Shnaiderman,
	Thomas Monjalon, Olivier Matz

Hi Dmitry,
I'm trying to run test-pmd on Windows and I ran into this error with cmdline.

The error log message is :
In file included from ../app/test-pmd/cmdline_flow.c:23:
..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64' redeclared as different kind of symbol
  INT64

In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/winnt.h:150,
                 from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/minwindef.h:163,
                 from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windef.h:8,
                 from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windows.h:69,
                 from ..\lib/librte_eal/windows/include/rte_windows.h:22,
                 from ..\lib/librte_eal/windows/include/pthread.h:20,
                 from ..\lib/librte_eal/include/rte_per_lcore.h:25,
                 from ..\lib/librte_eal/include/rte_errno.h:18,
                 from ..\lib\librte_ethdev/rte_ethdev.h:156,
                 from ../app/test-pmd/cmdline_flow.c:18:
C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was here
   __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;

The same error is for the other types defined in cmdline_numtype.

This problem with windows.h is popping in many places and some of them are 
cmdline and test-pmd and librte_net. 
We should find a way to exclude windows.h from the unneeded places, is there any
suggestions on how it can be done ?

> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Sent: Sunday, June 21, 2020 12:05 AM
> To: dev@dpdk.org
> Cc: Dmitry Malloy <dmitrym@microsoft.com>; Narcisa Ana Maria Vasile
> <Narcisa.Vasile@microsoft.com>; Fady Bader <fady@mellanox.com>; Tal
> Shnaiderman <talshn@mellanox.com>; Dmitry Kozlyuk
> <dmitry.kozliuk@gmail.com>; Thomas Monjalon <thomas@monjalon.net>;
> Olivier Matz <olivier.matz@6wind.com>
> Subject: [PATCH 6/7] cmdline: support Windows
> 
> Implement terminal handling, input polling, and vdprintf() for Windows.
> 
> Because Windows I/O model differs fundamentally from Unix and there is no
> concept of character device, polling is simulated depending on the underlying
> inpue device. Supporting non-terminal input is useful for automated testing.
> 
> Windows emulation of VT100 uses "ESC [ E" for newline instead of standard "ESC
> E", so a workaround is added.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  config/meson.build                      |   2 +
>  lib/librte_cmdline/cmdline.c            |   5 +
>  lib/librte_cmdline/cmdline_os_windows.c | 207 ++++++++++++++++++++++++
>  lib/librte_cmdline/cmdline_private.h    |  15 ++
>  lib/librte_cmdline/cmdline_socket.c     |   4 +
>  lib/librte_cmdline/cmdline_vt100.h      |   4 +
>  lib/librte_cmdline/meson.build          |   4 +-
>  lib/meson.build                         |   1 +
>  8 files changed, 241 insertions(+), 1 deletion(-)  create mode 100644
> lib/librte_cmdline/cmdline_os_windows.c
> 
> diff --git a/config/meson.build b/config/meson.build index d3f05f878..733b5e310
> 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -269,6 +269,8 @@ if is_windows
>  		add_project_arguments('-D__USE_MINGW_ANSI_STDIO',
> language: 'c')
>  	endif
> 
> +	add_project_link_arguments('-lws2_32', language: 'c')
> +
>  	# Contrary to docs, VirtualAlloc2() is exported by mincore.lib
>  	# in Windows SDK, while MinGW exports it by advapi32.a.
>  	if is_ms_linker
> diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c index
> 00b9e6b2e..c0ddb5f23 100644
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -13,9 +13,14 @@
>  #include <fcntl.h>
>  #include <errno.h>
>  #include <netinet/in.h>
> +#include <unistd.h>
> 
>  #include <rte_string_fns.h>
> 
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +#define write _write
> +#endif
> +
>  #include "cmdline.h"
>  #include "cmdline_private.h"
> 
> diff --git a/lib/librte_cmdline/cmdline_os_windows.c
> b/lib/librte_cmdline/cmdline_os_windows.c
> new file mode 100644
> index 000000000..9736f6531
> --- /dev/null
> +++ b/lib/librte_cmdline/cmdline_os_windows.c
> @@ -0,0 +1,207 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2020 Dmitry Kozlyuk
> + */
> +
> +#include <io.h>
> +
> +#include <rte_os.h>
> +
> +#include "cmdline_private.h"
> +
> +/* Missing from some MinGW-w64 distributions. */ #ifndef
> +ENABLE_VIRTUAL_TERMINAL_PROCESSING
> +#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004 #endif
> +
> +#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT
> +#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200 #endif
> +
> +void
> +terminal_adjust(struct terminal *oldterm) {
> +	HANDLE handle;
> +	DWORD mode;
> +
> +	ZeroMemory(oldterm, sizeof(*oldterm));
> +
> +	/* Detect console input, set it up and make it emulate VT100. */
> +	handle = GetStdHandle(STD_INPUT_HANDLE);
> +	if (GetConsoleMode(handle, &mode)) {
> +		oldterm->is_console_input = 1;
> +		oldterm->input_mode = mode;
> +
> +		mode &= ~(
> +			ENABLE_LINE_INPUT |      /* no line buffering */
> +			ENABLE_ECHO_INPUT |      /* no echo */
> +			ENABLE_PROCESSED_INPUT | /* pass Ctrl+C to program
> */
> +			ENABLE_MOUSE_INPUT |     /* no mouse events */
> +			ENABLE_WINDOW_INPUT);    /* no window resize events
> */
> +		mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
> +		SetConsoleMode(handle, mode);
> +	}
> +
> +	/* Detect console output and make it emulate VT100. */
> +	handle = GetStdHandle(STD_OUTPUT_HANDLE);
> +	if (GetConsoleMode(handle, &mode)) {
> +		oldterm->is_console_output = 1;
> +		oldterm->output_mode = mode;
> +
> +		mode &= ~ENABLE_WRAP_AT_EOL_OUTPUT;
> +		mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
> +		SetConsoleMode(handle, mode);
> +	}
> +}
> +
> +void
> +terminal_restore(const struct terminal *oldterm) {
> +	if (oldterm->is_console_input) {
> +		HANDLE handle = GetStdHandle(STD_INPUT_HANDLE);
> +		SetConsoleMode(handle, oldterm->input_mode);
> +	}
> +
> +	if (oldterm->is_console_output) {
> +		HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);
> +		SetConsoleMode(handle, oldterm->output_mode);
> +	}
> +}
> +
> +static int
> +cmdline_is_key_down(const INPUT_RECORD *record) {
> +	return (record->EventType == KEY_EVENT) &&
> +		record->Event.KeyEvent.bKeyDown;
> +}
> +
> +static int
> +cmdline_poll_char_console(HANDLE handle) {
> +	INPUT_RECORD record;
> +	DWORD events;
> +
> +	if (!PeekConsoleInput(handle, &record, 1, &events)) {
> +		/* Simulate poll(3) behavior on EOF. */
> +		return (GetLastError() == ERROR_HANDLE_EOF) ? 1 : -1;
> +	}
> +
> +	if ((events == 0) || !cmdline_is_key_down(&record))
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int
> +cmdline_poll_char_file(struct cmdline *cl, HANDLE handle) {
> +	DWORD type = GetFileType(handle);
> +
> +	/* Since console is handled by cmdline_poll_char_console(),
> +	 * this is either a serial port or input handle had been replaced.
> +	 */
> +	if (type == FILE_TYPE_CHAR)
> +		return cmdline_poll_char_console(handle);
> +
> +	/* PeekNamedPipe() can handle all pipes and also sockets. */
> +	if (type == FILE_TYPE_PIPE) {
> +		DWORD bytes_avail;
> +		if (!PeekNamedPipe(handle, NULL, 0, NULL, &bytes_avail, NULL))
> +			return (GetLastError() == ERROR_BROKEN_PIPE) ? 1 : -1;
> +		return bytes_avail ? 1 : 0;
> +	}
> +
> +	/* There is no straightforward way to peek a file in Windows
> +	 * I/O model. Read the byte, if it is not the end of file,
> +	 * buffer it for subsequent read. This will not work with
> +	 * a file being appended and probably some other edge cases.
> +	 */
> +	if (type == FILE_TYPE_DISK) {
> +		char c;
> +		int ret;
> +
> +		ret = _read(cl->s_in, &c, sizeof(c));
> +		if (ret == 1) {
> +			cl->repeat_count = 1;
> +			cl->repeated_char = c;
> +		}
> +		return ret;
> +	}
> +
> +	/* GetFileType() failed or file of unknown type,
> +	 * which we do not know how to peek anyway.
> +	 */
> +	return -1;
> +}
> +
> +int
> +cmdline_poll_char(struct cmdline *cl)
> +{
> +	HANDLE handle = (HANDLE)_get_osfhandle(cl->s_in);
> +	return cl->oldterm.is_console_input ?
> +		cmdline_poll_char_console(handle) :
> +		cmdline_poll_char_file(cl, handle);
> +}
> +
> +ssize_t
> +cmdline_read_char(struct cmdline *cl, char *c) {
> +	HANDLE handle;
> +	INPUT_RECORD record;
> +	KEY_EVENT_RECORD *key;
> +	DWORD events;
> +
> +	if (!cl->oldterm.is_console_input)
> +		return _read(cl->s_in, c, 1);
> +
> +	/* Return repeated strokes from previous event. */
> +	if (cl->repeat_count > 0) {
> +		*c = cl->repeated_char;
> +		cl->repeat_count--;
> +		return 1;
> +	}
> +
> +	handle = (HANDLE)_get_osfhandle(cl->s_in);
> +	key = &record.Event.KeyEvent;
> +	do {
> +		if (!ReadConsoleInput(handle, &record, 1, &events)) {
> +			if (GetLastError() == ERROR_HANDLE_EOF) {
> +				*c = EOF;
> +				return 0;
> +			}
> +			return -1;
> +		}
> +	} while (!cmdline_is_key_down(&record));
> +
> +	*c = key->uChar.AsciiChar;
> +
> +	/* Save repeated strokes from a single event. */
> +	if (key->wRepeatCount > 1) {
> +		cl->repeated_char = *c;
> +		cl->repeat_count = key->wRepeatCount - 1;
> +	}
> +
> +	return 1;
> +}
> +
> +int
> +cmdline_vdprintf(int fd, const char *format, va_list op) {
> +	int copy, ret;
> +	FILE *file;
> +
> +	copy = _dup(fd);
> +	if (copy < 0)
> +		return -1;
> +
> +	file = _fdopen(copy, "a");
> +	if (file == NULL) {
> +		_close(copy);
> +		return -1;
> +	}
> +
> +	ret = vfprintf(file, format, op);
> +
> +	fclose(file); /* also closes copy */
> +
> +	return ret;
> +}
> diff --git a/lib/librte_cmdline/cmdline_private.h
> b/lib/librte_cmdline/cmdline_private.h
> index 338d3d55c..1e05ec376 100644
> --- a/lib/librte_cmdline/cmdline_private.h
> +++ b/lib/librte_cmdline/cmdline_private.h
> @@ -5,7 +5,11 @@
>  #ifndef _CMDLINE_PRIVATE_H_
>  #define _CMDLINE_PRIVATE_H_
> 
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +#include <rte_windows.h>
> +#else
>  #include <termios.h>
> +#endif
> 
>  #include <stdarg.h>
> 
> @@ -15,7 +19,14 @@
>  #include <cmdline_parse.h>
> 
>  struct terminal {
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  	struct termios termios;
> +#else
> +	DWORD input_mode;
> +	DWORD output_mode;
> +	int is_console_input;
> +	int is_console_output;
> +#endif
>  };
> 
>  /* Disable buffering and echoing, save previous settings to oldterm. */ @@ -31,6
> +42,10 @@ struct cmdline {
>  	struct rdline rdl;
>  	char prompt[RDLINE_PROMPT_SIZE];
>  	struct terminal oldterm;
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +	char repeated_char;
> +	WORD repeat_count;
> +#endif
>  };
> 
>  /* Check if a single character can be read from input. */ diff --git
> a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
> index e73666f15..c5f483413 100644
> --- a/lib/librte_cmdline/cmdline_socket.c
> +++ b/lib/librte_cmdline/cmdline_socket.c
> @@ -16,6 +16,10 @@
>  #include "cmdline_private.h"
>  #include "cmdline_socket.h"
> 
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +#define open _open
> +#endif
> +
>  struct cmdline *
>  cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char
> *path)  { diff --git a/lib/librte_cmdline/cmdline_vt100.h
> b/lib/librte_cmdline/cmdline_vt100.h
> index e33e67ed8..be9ae8e1c 100644
> --- a/lib/librte_cmdline/cmdline_vt100.h
> +++ b/lib/librte_cmdline/cmdline_vt100.h
> @@ -31,7 +31,11 @@ extern "C" {
>  #define vt100_multi_right  "\033\133%uC"
>  #define vt100_multi_left   "\033\133%uD"
>  #define vt100_suppr        "\033\133\063\176"
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  #define vt100_home         "\033M\033E"
> +#else
> +#define vt100_home         "\033M\033[E"
> +#endif
>  #define vt100_word_left    "\033\142"
>  #define vt100_word_right   "\033\146"
> 
> diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
> index 5c9e8886d..5009b3354 100644
> --- a/lib/librte_cmdline/meson.build
> +++ b/lib/librte_cmdline/meson.build
> @@ -25,7 +25,9 @@ headers = files('cmdline.h',
>  	'cmdline_cirbuf.h',
>  	'cmdline_parse_portlist.h')
> 
> -if not is_windows
> +if is_windows
> +	sources += files('cmdline_os_windows.c') else
>  	sources += files('cmdline_os_unix.c')
>  endif
> 
> diff --git a/lib/meson.build b/lib/meson.build index 40e452025..5b72a2d9e
> 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -40,6 +40,7 @@ if is_windows
>  		'kvargs','eal',
>  		'ring',
>  		'mempool', 'mbuf', 'pci', 'net',
> +		'cmdline',
>  	] # only supported libraries for windows  endif
> 
> --
> 2.25.4


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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-28 14:20   ` Fady Bader
@ 2020-06-29  6:23     ` Ranjit Menon
  2020-06-29  7:42       ` Dmitry Kozlyuk
  0 siblings, 1 reply; 50+ messages in thread
From: Ranjit Menon @ 2020-06-29  6:23 UTC (permalink / raw)
  To: Fady Bader, Dmitry Kozlyuk, dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Tal Shnaiderman,
	Thomas Monjalon, Olivier Matz


On 6/28/2020 7:20 AM, Fady Bader wrote:
> Hi Dmitry,
> I'm trying to run test-pmd on Windows and I ran into this error with cmdline.
>
> The error log message is :
> In file included from ../app/test-pmd/cmdline_flow.c:23:
> ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64' redeclared as different kind of symbol
>    INT64
>
> In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/winnt.h:150,
>                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/minwindef.h:163,
>                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windef.h:8,
>                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windows.h:69,
>                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
>                   from ..\lib/librte_eal/windows/include/pthread.h:20,
>                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
>                   from ..\lib/librte_eal/include/rte_errno.h:18,
>                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
>                   from ../app/test-pmd/cmdline_flow.c:18:
> C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was here
>     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
>
> The same error is for the other types defined in cmdline_numtype.
>
> This problem with windows.h is popping in many places and some of them are
> cmdline and test-pmd and librte_net.
> We should find a way to exclude windows.h from the unneeded places, is there any
> suggestions on how it can be done ?

We ran into this same issue when working with the code that is on the 
draft repo.

The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types in 
Windows headers for integer types. We found that it is easier to change 
the enum in cmdline_parse_num.h than try to play with the include order 
of headers. AFAIK, the enums were only used to determine the type in a 
series of switch() statements in librte_cmdline, so we simply renamed 
the enums. Not sure, if that will be acceptable here.

<snip>

ranjit m.


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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-29  6:23     ` Ranjit Menon
@ 2020-06-29  7:42       ` Dmitry Kozlyuk
  2020-06-29  8:12         ` Tal Shnaiderman
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-29  7:42 UTC (permalink / raw)
  To: Ranjit Menon
  Cc: Fady Bader, dev, Dmitry Malloy, Narcisa Ana Maria Vasile,
	Tal Shnaiderman, Thomas Monjalon, Olivier Matz

On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:
> On 6/28/2020 7:20 AM, Fady Bader wrote:
> > Hi Dmitry,
> > I'm trying to run test-pmd on Windows and I ran into this error with cmdline.
> >
> > The error log message is :
> > In file included from ../app/test-pmd/cmdline_flow.c:23:
> > ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64' redeclared as different kind of symbol
> >    INT64
> >
> > In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/winnt.h:150,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/minwindef.h:163,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windef.h:8,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windows.h:69,
> >                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
> >                   from ..\lib/librte_eal/windows/include/pthread.h:20,
> >                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
> >                   from ..\lib/librte_eal/include/rte_errno.h:18,
> >                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
> >                   from ../app/test-pmd/cmdline_flow.c:18:
> > C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was here
> >     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
> >
> > The same error is for the other types defined in cmdline_numtype.
> >
> > This problem with windows.h is popping in many places and some of them are
> > cmdline and test-pmd and librte_net.
> > We should find a way to exclude windows.h from the unneeded places, is there any
> > suggestions on how it can be done ?  
> 
> We ran into this same issue when working with the code that is on the 
> draft repo.
> 
> The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types in 
> Windows headers for integer types. We found that it is easier to change 
> the enum in cmdline_parse_num.h than try to play with the include order 
> of headers. AFAIK, the enums were only used to determine the type in a 
> series of switch() statements in librte_cmdline, so we simply renamed 
> the enums. Not sure, if that will be acceptable here.

+1 for renaming enum values. It's not a problem of librte_cmdline itself but a
problem of its consumption on Windows, however renaming enum values doesn't
break ABI and winn make librte_cmdline API "namespaced".

I don't see a clean way not to expose windows.h, because pthread.h depends on
it, and if we hide implementation, librte_eal would have to export pthread
symbols on Windows, which is a hack (or is it?).

-- 
Dmitry Kozlyuk

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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-29  7:42       ` Dmitry Kozlyuk
@ 2020-06-29  8:12         ` Tal Shnaiderman
  2020-06-29 23:56           ` Dmitry Kozlyuk
  0 siblings, 1 reply; 50+ messages in thread
From: Tal Shnaiderman @ 2020-06-29  8:12 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Ranjit Menon
  Cc: Fady Bader, dev, Dmitry Malloy, Narcisa Ana Maria Vasile,
	Thomas Monjalon, Olivier Matz

> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Subject: Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
> 
> On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:
> > On 6/28/2020 7:20 AM, Fady Bader wrote:
> > > Hi Dmitry,
> > > I'm trying to run test-pmd on Windows and I ran into this error with
> cmdline.
> > >
> > > The error log message is :
> > > In file included from ../app/test-pmd/cmdline_flow.c:23:
> > > ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64'
> redeclared as different kind of symbol
> > >    INT64
> > >
> > > In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-
> mingw32/include/winnt.h:150,
> > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-
> mingw32/include/minwindef.h:163,
> > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-
> mingw32/include/windef.h:8,
> > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-
> mingw32/include/windows.h:69,
> > >                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
> > >                   from ..\lib/librte_eal/windows/include/pthread.h:20,
> > >                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
> > >                   from ..\lib/librte_eal/include/rte_errno.h:18,
> > >                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
> > >                   from ../app/test-pmd/cmdline_flow.c:18:
> > > C:/mingw-w64/x86_64/mingw64/x86_64-w64-
> mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was
> here
> > >     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
> > >
> > > The same error is for the other types defined in cmdline_numtype.
> > >
> > > This problem with windows.h is popping in many places and some of
> > > them are cmdline and test-pmd and librte_net.
> > > We should find a way to exclude windows.h from the unneeded places,
> > > is there any suggestions on how it can be done ?
> >
> > We ran into this same issue when working with the code that is on the
> > draft repo.
> >
> > The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types
> > in Windows headers for integer types. We found that it is easier to
> > change the enum in cmdline_parse_num.h than try to play with the
> > include order of headers. AFAIK, the enums were only used to determine
> > the type in a series of switch() statements in librte_cmdline, so we
> > simply renamed the enums. Not sure, if that will be acceptable here.
> 
> +1 for renaming enum values. It's not a problem of librte_cmdline itself
> +but a
> problem of its consumption on Windows, however renaming enum values
> doesn't break ABI and winn make librte_cmdline API "namespaced".
> 
> I don't see a clean way not to expose windows.h, because pthread.h
> depends on it, and if we hide implementation, librte_eal would have to
> export pthread symbols on Windows, which is a hack (or is it?).

test_pmd redefine BOOLEAN and PATTERN in the index enum, I'm not sure how many more conflicts we will face because of this huge include.

Also, DPDK applications will inherit it unknowingly, not sure if this is common for windows libraries.

> 
> --
> Dmitry Kozlyuk

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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-29  8:12         ` Tal Shnaiderman
@ 2020-06-29 23:56           ` Dmitry Kozlyuk
  2020-07-08  1:09             ` Dmitry Kozlyuk
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-06-29 23:56 UTC (permalink / raw)
  To: Tal Shnaiderman
  Cc: Ranjit Menon, Fady Bader, dev, Dmitry Malloy,
	Narcisa Ana Maria Vasile, Thomas Monjalon, Olivier Matz

On Mon, 29 Jun 2020 08:12:51 +0000, Tal Shnaiderman wrote:
> > From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Subject: Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
> > 
> > On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:  
> > > On 6/28/2020 7:20 AM, Fady Bader wrote:  
> > > > Hi Dmitry,
> > > > I'm trying to run test-pmd on Windows and I ran into this error with  
> > cmdline.  
> > > >
> > > > The error log message is :
> > > > In file included from ../app/test-pmd/cmdline_flow.c:23:
> > > > ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64'  
> > redeclared as different kind of symbol  
> > > >    INT64
> > > >
> > > > In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/winnt.h:150,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/minwindef.h:163,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/windef.h:8,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/windows.h:69,  
> > > >                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
> > > >                   from ..\lib/librte_eal/windows/include/pthread.h:20,
> > > >                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
> > > >                   from ..\lib/librte_eal/include/rte_errno.h:18,
> > > >                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
> > > >                   from ../app/test-pmd/cmdline_flow.c:18:
> > > > C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was
> > here  
> > > >     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
> > > >
> > > > The same error is for the other types defined in cmdline_numtype.
> > > >
> > > > This problem with windows.h is popping in many places and some of
> > > > them are cmdline and test-pmd and librte_net.
> > > > We should find a way to exclude windows.h from the unneeded places,
> > > > is there any suggestions on how it can be done ?  
> > >
> > > We ran into this same issue when working with the code that is on the
> > > draft repo.
> > >
> > > The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types
> > > in Windows headers for integer types. We found that it is easier to
> > > change the enum in cmdline_parse_num.h than try to play with the
> > > include order of headers. AFAIK, the enums were only used to determine
> > > the type in a series of switch() statements in librte_cmdline, so we
> > > simply renamed the enums. Not sure, if that will be acceptable here.  
> > 
> > +1 for renaming enum values. It's not a problem of librte_cmdline itself
> > +but a
> > problem of its consumption on Windows, however renaming enum values
> > doesn't break ABI and winn make librte_cmdline API "namespaced".
> > 
> > I don't see a clean way not to expose windows.h, because pthread.h
> > depends on it, and if we hide implementation, librte_eal would have to
> > export pthread symbols on Windows, which is a hack (or is it?).  
> 
> test_pmd redefine BOOLEAN and PATTERN in the index enum, I'm not sure how many more conflicts we will face because of this huge include.
>
> Also, DPDK applications will inherit it unknowingly, not sure if this is common for windows libraries.

I never hit these particular conflicts, but you're right that there will be
more, e.g. I remember particularly nasty clashes in failsafe PMD, unrelated
to cmdline token names.


We could take the same approach as with networking headers: copy required
declarations instead of including them from SDK. Here's a list of what
pthread.h uses:

CloseHandle
CreateThread
DeleteSynchronizationBarrier
EnterSynchronizationBarrier
GetThreadAffinityMask
InitializeSynchronizationBarrier
OpenThread
SetPriorityClass
SetThreadAffinityMask
SetThreadPriority
TerminateThread

Windows has strict compatibility policy, so prototypes are unlikely to ever
change. None of the used functions takes string parameters, thus not affected
by A/W macros. Looks a bit messy, but it's limited in scope at least.


An external pthread library would solve the problem, but as I've reported
earlier, I failed to find a good one: [1] and [3] are tied to MinGW, although
of high quality, [2] seems outdated.

[1]: Wnpthreads:
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/
[2] pthreads-win32: https://sourceware.org/pthreads-win32/
[3] mcfgthread: https://github.com/lhmouse/mcfgthread

-- 
Dmitry Kozlyuk

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

* Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
  2020-06-29 23:56           ` Dmitry Kozlyuk
@ 2020-07-08  1:09             ` Dmitry Kozlyuk
  0 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-08  1:09 UTC (permalink / raw)
  To: Tal Shnaiderman
  Cc: Ranjit Menon, Fady Bader, dev, Dmitry Malloy,
	Narcisa Ana Maria Vasile, Thomas Monjalon, Olivier Matz

On Tue, 30 Jun 2020 02:56:20 +0300, Dmitry Kozlyuk wrote:
> On Mon, 29 Jun 2020 08:12:51 +0000, Tal Shnaiderman wrote:
> > > From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > Subject: Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
> > > 
> > > On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:    
[snip]
> > > > The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types
> > > > in Windows headers for integer types. We found that it is easier to
> > > > change the enum in cmdline_parse_num.h than try to play with the
> > > > include order of headers. AFAIK, the enums were only used to determine
> > > > the type in a series of switch() statements in librte_cmdline, so we
> > > > simply renamed the enums. Not sure, if that will be acceptable here.    
> > > 
> > > +1 for renaming enum values. It's not a problem of librte_cmdline itself
> > > +but a
> > > problem of its consumption on Windows, however renaming enum values
> > > doesn't break ABI and winn make librte_cmdline API "namespaced".
> > > 
[snip]
> > 
> > test_pmd redefine BOOLEAN and PATTERN in the index enum, I'm not sure how many more conflicts we will face because of this huge include.
> >
> > Also, DPDK applications will inherit it unknowingly, not sure if this is common for windows libraries.  
> 
> I never hit these particular conflicts, but you're right that there will be
> more, e.g. I remember particularly nasty clashes in failsafe PMD, unrelated
> to cmdline token names.

Still, I'd go for renaming, with or without additional steps to hide
<windows.h>. Although I wouldn't include it in this series: renaming will
touch numerous places and require much more reviewers.

> We could take the same approach as with networking headers: copy required
> declarations instead of including them from SDK. Here's a list of what
> pthread.h uses:

While this will resolve the issue for DPDK code, applications using DPDK
headers can easily hit it by including <windows.h> on their own. On the other
hand, they can always split translation units and I don't know how practical
it is to use system and DPDK networking headers at the same time.

-- 
Dmitry Kozlyuk

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

* Re: [dpdk-dev] [PATCH 0/7] cmdline: support Windows
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (6 preceding siblings ...)
  2020-06-20 21:05 ` [dpdk-dev] [PATCH 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
@ 2020-07-17 22:16 ` Narcisa Ana Maria Vasile
  2020-07-30 18:08 ` Kadam, Pallavi
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
  9 siblings, 0 replies; 50+ messages in thread
From: Narcisa Ana Maria Vasile @ 2020-07-17 22:16 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Omar Cardona

On Sun, Jun 21, 2020 at 12:05:03AM +0300, Dmitry Kozlyuk wrote:
> This patchset enables librte_cmdline on Windows. To do that, it creates
> a number of wrappers for OS-dependent terminal handling and I/O.
> Considered alternative was to revive [1] and use libedit (Unix-only)
> for terminal handling. However, testing revealed that WinEditLine [2]
> is not a drop-in replacement for libedit, so this solution wouldn't be
> universal.
> 
> Note: not all dependency series are up-to-date at the time of
> submission, so this version may require hacking to test.
> 
> [1]: http://patchwork.dpdk.org/patch/38561
> [2]: http://mingweditline.sourceforge.net
> 
> ---
> Depends-on: series-10512 ("Windows bus/pci support ")
> Depends-on: series-10244 ("[v4] eal/windows: ring build on Windows")
> Depends-on: series-10282 ("build mempool on Windows ")
> Depends-on: series-10382 ("compile librte_net for windows")
> 
Tested-by: Narcisa Vasile <navasile@linux.microsoft.com>

Compiled with mingw and tested using the sample cmdline app.
With clang (9.0.0), I'm getting the "conflicting types for '_m_prefetchw'" error,
given the dependency on librte_net patch. 

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

* Re: [dpdk-dev] [PATCH 0/7] cmdline: support Windows
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (7 preceding siblings ...)
  2020-07-17 22:16 ` [dpdk-dev] [PATCH 0/7] cmdline: support Windows Narcisa Ana Maria Vasile
@ 2020-07-30 18:08 ` Kadam, Pallavi
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
  9 siblings, 0 replies; 50+ messages in thread
From: Kadam, Pallavi @ 2020-07-30 18:08 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader, Tal Shnaiderman


Hi Dmitry,

On 6/20/2020 2:05 PM, Dmitry Kozlyuk wrote:
> This patchset enables librte_cmdline on Windows. To do that, it creates
> a number of wrappers for OS-dependent terminal handling and I/O.
> Considered alternative was to revive [1] and use libedit (Unix-only)
> for terminal handling. However, testing revealed that WinEditLine [2]
> is not a drop-in replacement for libedit, so this solution wouldn't be
> universal.
> 
> Note: not all dependency series are up-to-date at the time of
> submission, so this version may require hacking to test.
> 
> [1]: http://patchwork.dpdk.org/patch/38561
> [2]: http://mingweditline.sourceforge.net
> 
> ---
> Depends-on: series-10512 ("Windows bus/pci support ")
> Depends-on: series-10244 ("[v4] eal/windows: ring build on Windows")
> Depends-on: series-10282 ("build mempool on Windows ")
> Depends-on: series-10382 ("compile librte_net for windows")
> 
> Dmitry Kozlyuk (7):
>    cmdline: make implementation opaque
>    cmdline: add internal wrappers for terminal handling
>    cmdline: add internal wrappers for character input
>    cmdline: add internal wrapper for vdprintf
>    eal/windows: improve compatibility networking headers
>    cmdline: support Windows
>    examples/cmdline: build on Windows
> 
>   app/test-cmdline/commands.c                 |   8 +-
>   app/test/test_cmdline_lib.c                 |  42 ++--
>   config/meson.build                          |   2 +
>   examples/cmdline/commands.c                 |   1 -
>   examples/cmdline/main.c                     |   1 -
>   examples/meson.build                        |   6 +-
>   lib/librte_cmdline/Makefile                 |   4 +
>   lib/librte_cmdline/cmdline.c                |  29 +--
>   lib/librte_cmdline/cmdline.h                |  15 +-
>   lib/librte_cmdline/cmdline_os_unix.c        |  53 +++++
>   lib/librte_cmdline/cmdline_os_windows.c     | 207 ++++++++++++++++++++
>   lib/librte_cmdline/cmdline_parse.c          |   4 +-
>   lib/librte_cmdline/cmdline_private.h        |  61 ++++++
>   lib/librte_cmdline/cmdline_socket.c         |  25 +--
>   lib/librte_cmdline/cmdline_vt100.c          |   1 -
>   lib/librte_cmdline/cmdline_vt100.h          |   4 +
>   lib/librte_cmdline/meson.build              |   6 +
>   lib/librte_cmdline/rte_cmdline_version.map  |   8 +
>   lib/librte_eal/windows/include/arpa/inet.h  |  30 +++
>   lib/librte_eal/windows/include/netinet/in.h |  11 ++
>   lib/librte_eal/windows/include/pthread.h    |   2 +-
>   lib/librte_eal/windows/include/sys/socket.h |  24 +++
>   lib/meson.build                             |   1 +
>   23 files changed, 470 insertions(+), 75 deletions(-)
>   create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
>   create mode 100644 lib/librte_cmdline/cmdline_os_windows.c
>   create mode 100644 lib/librte_cmdline/cmdline_private.h
>   create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
>   create mode 100644 lib/librte_eal/windows/include/sys/socket.h
> 
Will this series be rebased with the latest in order to apply it cleanly?
Thanks,

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

* [dpdk-dev] [PATCH v2 0/7] cmdline: support Windows
  2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
                   ` (8 preceding siblings ...)
  2020-07-30 18:08 ` Kadam, Pallavi
@ 2020-07-30 21:06 ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
                     ` (7 more replies)
  9 siblings, 8 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk

This patchset enables librte_cmdline on Windows. To do that, it creates
a number of wrappers for OS-dependent terminal handling and I/O.
Considered alternative was to revive [1] and use libedit (Unix-only)
for terminal handling. However, testing revealed that WinEditLine [2]
is not a drop-in replacement for libedit, so this solution wouldn't be
universal.

[1]: http://patchwork.dpdk.org/patch/38561
[2]: http://mingweditline.sourceforge.net

---
Depends-on: series-10382 ("compile librte_net for windows")

v2: rebase on ToT, remove already merged dependencies.

Dmitry Kozlyuk (7):
  cmdline: make implementation opaque
  cmdline: add internal wrappers for terminal handling
  cmdline: add internal wrappers for character input
  cmdline: add internal wrapper for vdprintf
  eal/windows: improve compatibility networking headers
  cmdline: support Windows
  examples/cmdline: build on Windows

 app/test-cmdline/commands.c                 |   8 +-
 app/test/test_cmdline_lib.c                 |  42 ++--
 config/meson.build                          |   2 +
 examples/cmdline/commands.c                 |   1 -
 examples/cmdline/main.c                     |   1 -
 examples/meson.build                        |   6 +-
 lib/librte_cmdline/Makefile                 |   4 +
 lib/librte_cmdline/cmdline.c                |  29 +--
 lib/librte_cmdline/cmdline.h                |  15 +-
 lib/librte_cmdline/cmdline_os_unix.c        |  53 +++++
 lib/librte_cmdline/cmdline_os_windows.c     | 207 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_parse.c          |   4 +-
 lib/librte_cmdline/cmdline_private.h        |  61 ++++++
 lib/librte_cmdline/cmdline_socket.c         |  25 +--
 lib/librte_cmdline/cmdline_vt100.c          |   1 -
 lib/librte_cmdline/cmdline_vt100.h          |   4 +
 lib/librte_cmdline/meson.build              |   6 +
 lib/librte_cmdline/rte_cmdline_version.map  |   8 +
 lib/librte_eal/windows/include/arpa/inet.h  |  30 +++
 lib/librte_eal/windows/include/netinet/in.h |  12 ++
 lib/librte_eal/windows/include/sys/socket.h |  24 +++
 lib/meson.build                             |   1 +
 22 files changed, 470 insertions(+), 74 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c
 create mode 100644 lib/librte_cmdline/cmdline_private.h
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-08-05  9:31     ` Kinsella, Ray
  2020-09-17 13:34     ` Olivier Matz
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
                     ` (6 subsequent siblings)
  7 siblings, 2 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Olivier Matz,
	Ray Kinsella, Neil Horman

struct cmdline exposes platform-specific members it contains, most
notably struct termios that is only available on Unix. Make the
structure opaque.

Remove tests checking struct cmdline content as meaningless.

Add cmdline_get_rdline() to access history buffer.
The new function is currently used only in tests.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 app/test-cmdline/commands.c                |  8 +++--
 app/test/test_cmdline_lib.c                | 42 +++++++++-------------
 lib/librte_cmdline/cmdline.c               | 10 ++++--
 lib/librte_cmdline/cmdline.h               | 15 ++++----
 lib/librte_cmdline/cmdline_parse.c         |  4 +--
 lib/librte_cmdline/cmdline_private.h       | 22 ++++++++++++
 lib/librte_cmdline/cmdline_socket.c        |  6 ++--
 lib/librte_cmdline/rte_cmdline_version.map |  8 +++++
 8 files changed, 68 insertions(+), 47 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_private.h

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d67c0ca6a..ff7ac973e 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -294,8 +294,10 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(cl->rdl.history_buf));
+			sizeof(rdl->history_buf));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
@@ -326,7 +328,9 @@ cmd_clear_history_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
-	rdline_clear_history(&cl->rdl);
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
+	rdline_clear_history(rdl);
 }
 
 cmdline_parse_token_string_t cmd_clear_history_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index dec465da5..a7f5a7f7f 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
 static int
 test_cmdline_parse_fns(void)
 {
-	struct cmdline cl;
+	struct cmdline *cl;
+	cmdline_parse_ctx_t ctx;
 	int i = 0;
 	char dst[CMDLINE_TEST_BUFSIZE];
 
+	cl = cmdline_new(&ctx, "prompt", -1, -1);
+	if (cl == NULL) {
+		printf("Error: cannot create cmdline to test parse fns!\n");
+		return -1;
+	}
+
 	if (cmdline_parse(NULL, "buffer") >= 0)
 		goto error;
-	if (cmdline_parse(&cl, NULL) >= 0)
+	if (cmdline_parse(cl, NULL) >= 0)
 		goto error;
 
 	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
 		goto error;
 
 	return 0;
@@ -166,11 +173,11 @@ static int
 test_cmdline_fns(void)
 {
 	cmdline_parse_ctx_t ctx;
-	struct cmdline cl, *tmp;
+	struct cmdline *cl;
 
 	memset(&ctx, 0, sizeof(ctx));
-	tmp = cmdline_new(&ctx, "test", -1, -1);
-	if (tmp == NULL)
+	cl = cmdline_new(&ctx, "test", -1, -1);
+	if (cl == NULL)
 		goto error;
 
 	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
@@ -179,7 +186,7 @@ test_cmdline_fns(void)
 		goto error;
 	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
-	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
+	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
 	if (cmdline_write_char(NULL, 0) >= 0)
 		goto error;
@@ -188,31 +195,14 @@ test_cmdline_fns(void)
 	cmdline_set_prompt(NULL, "prompt");
 	cmdline_free(NULL);
 	cmdline_printf(NULL, "format");
-	/* this should fail as stream handles are invalid */
-	cmdline_printf(tmp, "format");
 	cmdline_interact(NULL);
 	cmdline_quit(NULL);
 
-	/* check if void calls change anything when they should fail */
-	cl = *tmp;
-
-	cmdline_printf(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_set_prompt(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-
-	cmdline_free(tmp);
-
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
 	return -1;
-mismatch:
-	printf("Error: data changed!\n");
-	return -1;
 }
 
 /* test library functions. the point of these tests is not so much to test
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index cfd703e5b..6f3fdd598 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,14 +13,12 @@
 #include <fcntl.h>
 #include <poll.h>
 #include <errno.h>
-#include <termios.h>
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
 
 static void
 cmdline_valid_buffer(struct rdline *rdl, const char *buf,
@@ -103,6 +101,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	return cl;
 }
 
+struct rdline*
+cmdline_get_rdline(struct cmdline *cl)
+{
+	return &cl->rdl;
+}
+
 void
 cmdline_free(struct cmdline *cl)
 {
diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 243f99d20..96674dfda 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -8,8 +8,8 @@
 #define _CMDLINE_H_
 
 #include <rte_common.h>
+#include <rte_compat.h>
 
-#include <termios.h>
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
@@ -23,14 +23,7 @@
 extern "C" {
 #endif
 
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
+struct cmdline;
 
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
@@ -40,6 +33,10 @@ void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 
+__rte_experimental
+struct rdline *
+cmdline_get_rdline(struct cmdline *cl);
+
 /**
  * This function is nonblocking equivalent of ``cmdline_interact()``. It polls
  * *cl* for one character and interpret it. If return value is *RDLINE_EXITED*
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index b57b30e8f..ea0979158 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -10,15 +10,13 @@
 #include <string.h>
 #include <inttypes.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_rdline.h"
-#include "cmdline_parse.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
 
 #ifdef RTE_LIBRTE_CMDLINE_DEBUG
 #define debug_printf printf
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
new file mode 100644
index 000000000..3b1c70e9f
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _CMDLINE_PRIVATE_H_
+#define _CMDLINE_PRIVATE_H_
+
+#include <termios.h>
+
+#include <cmdline_rdline.h>
+#include <cmdline_parse.h>
+
+struct cmdline {
+	int s_in;
+	int s_out;
+	cmdline_parse_ctx_t *ctx;
+	struct rdline rdl;
+	char prompt[RDLINE_PROMPT_SIZE];
+	struct termios oldterm;
+};
+
+#endif
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index ecb3d82b6..5e4b734d6 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -11,12 +11,10 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <termios.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
-#include "cmdline_socket.h"
 #include "cmdline.h"
+#include "cmdline_private.h"
+#include "cmdline_socket.h"
 
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
diff --git a/lib/librte_cmdline/rte_cmdline_version.map b/lib/librte_cmdline/rte_cmdline_version.map
index 95fce812f..135ecc71f 100644
--- a/lib/librte_cmdline/rte_cmdline_version.map
+++ b/lib/librte_cmdline/rte_cmdline_version.map
@@ -69,3 +69,11 @@ DPDK_20.0 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	cmdline_get_rdline;
+
+	local: *;
+};
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 2/7] cmdline: add internal wrappers for terminal handling
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Olivier Matz

Extract struct terminal and associated functions that set up, save, and
restore terminal parameters. Use existing code as Unix implementation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/Makefile          |  4 ++++
 lib/librte_cmdline/cmdline_os_unix.c | 27 +++++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h | 12 +++++++++++-
 lib/librte_cmdline/cmdline_socket.c  | 15 ++++-----------
 lib/librte_cmdline/cmdline_vt100.c   |  1 -
 lib/librte_cmdline/meson.build       |  4 ++++
 6 files changed, 50 insertions(+), 13 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c

diff --git a/lib/librte_cmdline/Makefile b/lib/librte_cmdline/Makefile
index 619d9a242..3d8e84c07 100644
--- a/lib/librte_cmdline/Makefile
+++ b/lib/librte_cmdline/Makefile
@@ -23,6 +23,10 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_vt100.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_socket.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_parse_portlist.c
 
+ifneq ($(CONFIG_RTE_EXEC_ENV_WINDOWS),y)
+SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_os_unix.c
+endif
+
 LDLIBS += -lrte_net -lrte_eal
 
 # install includes
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
new file mode 100644
index 000000000..ca47bd19f
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <string.h>
+
+#include "cmdline_private.h"
+
+void
+terminal_adjust(struct terminal *oldterm)
+{
+	struct termios term;
+
+	tcgetattr(0, &oldterm->termios);
+
+	memcpy(&term, &oldterm->termios, sizeof(term));
+	term.c_lflag &= ~(ICANON | ECHO | ISIG);
+	tcsetattr(0, TCSANOW, &term);
+
+	setbuf(stdin, NULL);
+}
+
+void
+terminal_restore(const struct terminal *oldterm)
+{
+	tcsetattr(fileno(stdin), TCSANOW, &oldterm->termios);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 3b1c70e9f..adc552845 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -10,13 +10,23 @@
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
+struct terminal {
+	struct termios termios;
+};
+
+/* Disable buffering and echoing, save previous settings to oldterm. */
+void terminal_adjust(struct terminal *oldterm);
+
+/* Restore terminal settings form oldterm. */
+void terminal_restore(const struct terminal *oldterm);
+
 struct cmdline {
 	int s_in;
 	int s_out;
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
+	struct terminal oldterm;
 };
 
 #endif
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index 5e4b734d6..e73666f15 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -37,18 +37,11 @@ struct cmdline *
 cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 {
 	struct cmdline *cl;
-	struct termios oldterm, term;
-
-	tcgetattr(0, &oldterm);
-	memcpy(&term, &oldterm, sizeof(term));
-	term.c_lflag &= ~(ICANON | ECHO | ISIG);
-	tcsetattr(0, TCSANOW, &term);
-	setbuf(stdin, NULL);
 
 	cl = cmdline_new(ctx, prompt, 0, 1);
 
-	if (cl)
-		memcpy(&cl->oldterm, &oldterm, sizeof(term));
+	if (cl != NULL)
+		terminal_adjust(&cl->oldterm);
 
 	return cl;
 }
@@ -56,8 +49,8 @@ cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 void
 cmdline_stdin_exit(struct cmdline *cl)
 {
-	if (!cl)
+	if (cl == NULL)
 		return;
 
-	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
+	terminal_restore(&cl->oldterm);
 }
diff --git a/lib/librte_cmdline/cmdline_vt100.c b/lib/librte_cmdline/cmdline_vt100.c
index 662fc7345..bb968dd5f 100644
--- a/lib/librte_cmdline/cmdline_vt100.c
+++ b/lib/librte_cmdline/cmdline_vt100.c
@@ -10,7 +10,6 @@
 #include <string.h>
 #include <stdarg.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include "cmdline_vt100.h"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 7fc54ff1a..5c9e8886d 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,4 +25,8 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
+if not is_windows
+	sources += files('cmdline_os_unix.c')
+endif
+
 deps += ['net']
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 3/7] cmdline: add internal wrappers for character input
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Olivier Matz

poll(3) is a purely Unix facility, so it cannot be directly used by
common code. read(2) is limited in device support outside of Unix.
Create wrapper functions and implement them for Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 12 +++---------
 lib/librte_cmdline/cmdline_os_unix.c | 20 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h |  6 ++++++
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 6f3fdd598..a04719998 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <poll.h>
 #include <errno.h>
 #include <netinet/in.h>
 
@@ -185,7 +184,6 @@ cmdline_quit(struct cmdline *cl)
 int
 cmdline_poll(struct cmdline *cl)
 {
-	struct pollfd pfd;
 	int status;
 	ssize_t read_status;
 	char c;
@@ -195,16 +193,12 @@ cmdline_poll(struct cmdline *cl)
 	else if (cl->rdl.status == RDLINE_EXITED)
 		return RDLINE_EXITED;
 
-	pfd.fd = cl->s_in;
-	pfd.events = POLLIN;
-	pfd.revents = 0;
-
-	status = poll(&pfd, 1, 0);
+	status = cmdline_poll_char(cl);
 	if (status < 0)
 		return status;
 	else if (status > 0) {
 		c = -1;
-		read_status = read(cl->s_in, &c, 1);
+		read_status = cmdline_read_char(cl, &c);
 		if (read_status < 0)
 			return read_status;
 
@@ -226,7 +220,7 @@ cmdline_interact(struct cmdline *cl)
 
 	c = -1;
 	while (1) {
-		if (read(cl->s_in, &c, 1) <= 0)
+		if (cmdline_read_char(cl, &c) <= 0)
 			break;
 		if (cmdline_in(cl, &c, 1) < 0)
 			break;
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index ca47bd19f..865a89ddd 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -2,7 +2,9 @@
  * Copyright (c) 2020 Dmitry Kozlyuk
  */
 
+#include <poll.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "cmdline_private.h"
 
@@ -25,3 +27,21 @@ terminal_restore(const struct terminal *oldterm)
 {
 	tcsetattr(fileno(stdin), TCSANOW, &oldterm->termios);
 }
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	struct pollfd pfd;
+
+	pfd.fd = cl->s_in;
+	pfd.events = POLLIN;
+	pfd.revents = 0;
+
+	return poll(&pfd, 1, 0);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	return read(cl->s_in, c, 1);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index adc552845..ecfeb89f6 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -29,4 +29,10 @@ struct cmdline {
 	struct terminal oldterm;
 };
 
+/* Check if a single character can be read from input. */
+int cmdline_poll_char(struct cmdline *cl);
+
+/* Read one character from input. */
+ssize_t cmdline_read_char(struct cmdline *cl, char *c);
+
 #endif
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 4/7] cmdline: add internal wrapper for vdprintf
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
                     ` (2 preceding siblings ...)
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Olivier Matz

Add internal wrapper for vdprintf(3) that is only available on Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 2 +-
 lib/librte_cmdline/cmdline_os_unix.c | 6 ++++++
 lib/librte_cmdline/cmdline_private.h | 8 ++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index a04719998..00b9e6b2e 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -132,7 +132,7 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 	if (cl->s_out < 0)
 		return;
 	va_start(ap, fmt);
-	vdprintf(cl->s_out, fmt, ap);
+	cmdline_vdprintf(cl->s_out, fmt, ap);
 	va_end(ap);
 }
 
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index 865a89ddd..2052cd254 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -45,3 +45,9 @@ cmdline_read_char(struct cmdline *cl, char *c)
 {
 	return read(cl->s_in, c, 1);
 }
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	return vdprintf(fd, format, op);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index ecfeb89f6..338d3d55c 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -7,6 +7,10 @@
 
 #include <termios.h>
 
+#include <stdarg.h>
+
+#include <rte_common.h>
+
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
@@ -35,4 +39,8 @@ int cmdline_poll_char(struct cmdline *cl);
 /* Read one character from input. */
 ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 
+/* vdprintf(3) */
+__rte_format_printf(2, 0)
+int cmdline_vdprintf(int fd, const char *format, va_list op);
+
 #endif
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 5/7] eal/windows: improve compatibility networking headers
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
                     ` (3 preceding siblings ...)
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 6/7] cmdline: support Windows Dmitry Kozlyuk
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk,
	Harini Ramakrishnan, Omar Cardona, Ranjit Menon

Extend compatibility header system to support librte_cmdline.

pthread.h has to include windows.h, which exposes struct in_addr, etc.
conflicting with compatibility headers. WIN32_LEAN_AND_MEAN macro
is required to disable this behavior. Use rte_windows.h to define
WIN32_LEAN_AND_MEAN for pthread library.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/windows/include/arpa/inet.h  | 30 +++++++++++++++++++++
 lib/librte_eal/windows/include/netinet/in.h | 12 +++++++++
 lib/librte_eal/windows/include/sys/socket.h | 24 +++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

diff --git a/lib/librte_eal/windows/include/arpa/inet.h b/lib/librte_eal/windows/include/arpa/inet.h
new file mode 100644
index 000000000..96b698438
--- /dev/null
+++ b/lib/librte_eal/windows/include/arpa/inet.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _ARPA_INET_H_
+#define _ARPA_INET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+int
+inet_pton(int af, const char *src, void *dst);
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+const char *
+inet_ntop(int af, const void *src, char *dst, socklen_t size);
+
+#endif /* _ARPA_INET_H_ */
diff --git a/lib/librte_eal/windows/include/netinet/in.h b/lib/librte_eal/windows/include/netinet/in.h
index 2be25c8be..f7c486be6 100644
--- a/lib/librte_eal/windows/include/netinet/in.h
+++ b/lib/librte_eal/windows/include/netinet/in.h
@@ -5,6 +5,8 @@
 #ifndef _IN_H_
 #define _IN_H_
 
+#include <sys/socket.h>
+
 #define IPPROTO_IP 0               /* Dummy for IP */
 #define IPPROTO_HOPOPTS 0          /* IPv6 Hop-by-Hop options */
 #define IPPROTO_IPIP 4             /* IPIP tunnels (for compatibility) */
@@ -20,4 +22,14 @@
 #define IPPROTO_DSTOPTS 60         /* IPv6 destination option */
 #define IPPROTO_SCTP 132           /* Stream Control Transmission Protocol */
 
+#define INET6_ADDRSTRLEN 46
+
+struct in_addr {
+	uint32_t s_addr;
+};
+
+struct in6_addr {
+	uint8_t s6_addr[16];
+};
+
 #endif
diff --git a/lib/librte_eal/windows/include/sys/socket.h b/lib/librte_eal/windows/include/sys/socket.h
new file mode 100644
index 000000000..9536cf8e6
--- /dev/null
+++ b/lib/librte_eal/windows/include/sys/socket.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _SYS_SOCKET_H_
+#define _SYS_SOCKET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <stddef.h>
+
+#define AF_INET  2
+#define AF_INET6 23
+
+typedef size_t socklen_t;
+
+#endif /* _SYS_SOCKET_H_ */
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 6/7] cmdline: support Windows
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
                     ` (4 preceding siblings ...)
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Thomas Monjalon,
	Olivier Matz

Implement terminal handling, input polling, and vdprintf() for Windows.

Because Windows I/O model differs fundamentally from Unix and there is
no concept of character device, polling is simulated depending on the
underlying inpue device. Supporting non-terminal input is useful for
automated testing.

Windows emulation of VT100 uses "ESC [ E" for newline instead of
standard "ESC E", so a workaround is added.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build                      |   2 +
 lib/librte_cmdline/cmdline.c            |   5 +
 lib/librte_cmdline/cmdline_os_windows.c | 207 ++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h    |  15 ++
 lib/librte_cmdline/cmdline_socket.c     |   4 +
 lib/librte_cmdline/cmdline_vt100.h      |   4 +
 lib/librte_cmdline/meson.build          |   4 +-
 lib/meson.build                         |   1 +
 8 files changed, 241 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c

diff --git a/config/meson.build b/config/meson.build
index cff8b33dd..2d1b6fab2 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -270,6 +270,8 @@ if is_windows
 		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
 	endif
 
+	add_project_link_arguments('-lws2_32', language: 'c')
+
 	# Contrary to docs, VirtualAlloc2() is exported by mincore.lib
 	# in Windows SDK, while MinGW exports it by advapi32.a.
 	if is_ms_linker
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 00b9e6b2e..c0ddb5f23 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,9 +13,14 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <netinet/in.h>
+#include <unistd.h>
 
 #include <rte_string_fns.h>
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define write _write
+#endif
+
 #include "cmdline.h"
 #include "cmdline_private.h"
 
diff --git a/lib/librte_cmdline/cmdline_os_windows.c b/lib/librte_cmdline/cmdline_os_windows.c
new file mode 100644
index 000000000..9736f6531
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_windows.c
@@ -0,0 +1,207 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <io.h>
+
+#include <rte_os.h>
+
+#include "cmdline_private.h"
+
+/* Missing from some MinGW-w64 distributions. */
+#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING
+#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004
+#endif
+
+#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT
+#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200
+#endif
+
+void
+terminal_adjust(struct terminal *oldterm)
+{
+	HANDLE handle;
+	DWORD mode;
+
+	ZeroMemory(oldterm, sizeof(*oldterm));
+
+	/* Detect console input, set it up and make it emulate VT100. */
+	handle = GetStdHandle(STD_INPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		oldterm->is_console_input = 1;
+		oldterm->input_mode = mode;
+
+		mode &= ~(
+			ENABLE_LINE_INPUT |      /* no line buffering */
+			ENABLE_ECHO_INPUT |      /* no echo */
+			ENABLE_PROCESSED_INPUT | /* pass Ctrl+C to program */
+			ENABLE_MOUSE_INPUT |     /* no mouse events */
+			ENABLE_WINDOW_INPUT);    /* no window resize events */
+		mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+		SetConsoleMode(handle, mode);
+	}
+
+	/* Detect console output and make it emulate VT100. */
+	handle = GetStdHandle(STD_OUTPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		oldterm->is_console_output = 1;
+		oldterm->output_mode = mode;
+
+		mode &= ~ENABLE_WRAP_AT_EOL_OUTPUT;
+		mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+		SetConsoleMode(handle, mode);
+	}
+}
+
+void
+terminal_restore(const struct terminal *oldterm)
+{
+	if (oldterm->is_console_input) {
+		HANDLE handle = GetStdHandle(STD_INPUT_HANDLE);
+		SetConsoleMode(handle, oldterm->input_mode);
+	}
+
+	if (oldterm->is_console_output) {
+		HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);
+		SetConsoleMode(handle, oldterm->output_mode);
+	}
+}
+
+static int
+cmdline_is_key_down(const INPUT_RECORD *record)
+{
+	return (record->EventType == KEY_EVENT) &&
+		record->Event.KeyEvent.bKeyDown;
+}
+
+static int
+cmdline_poll_char_console(HANDLE handle)
+{
+	INPUT_RECORD record;
+	DWORD events;
+
+	if (!PeekConsoleInput(handle, &record, 1, &events)) {
+		/* Simulate poll(3) behavior on EOF. */
+		return (GetLastError() == ERROR_HANDLE_EOF) ? 1 : -1;
+	}
+
+	if ((events == 0) || !cmdline_is_key_down(&record))
+		return 0;
+
+	return 1;
+}
+
+static int
+cmdline_poll_char_file(struct cmdline *cl, HANDLE handle)
+{
+	DWORD type = GetFileType(handle);
+
+	/* Since console is handled by cmdline_poll_char_console(),
+	 * this is either a serial port or input handle had been replaced.
+	 */
+	if (type == FILE_TYPE_CHAR)
+		return cmdline_poll_char_console(handle);
+
+	/* PeekNamedPipe() can handle all pipes and also sockets. */
+	if (type == FILE_TYPE_PIPE) {
+		DWORD bytes_avail;
+		if (!PeekNamedPipe(handle, NULL, 0, NULL, &bytes_avail, NULL))
+			return (GetLastError() == ERROR_BROKEN_PIPE) ? 1 : -1;
+		return bytes_avail ? 1 : 0;
+	}
+
+	/* There is no straightforward way to peek a file in Windows
+	 * I/O model. Read the byte, if it is not the end of file,
+	 * buffer it for subsequent read. This will not work with
+	 * a file being appended and probably some other edge cases.
+	 */
+	if (type == FILE_TYPE_DISK) {
+		char c;
+		int ret;
+
+		ret = _read(cl->s_in, &c, sizeof(c));
+		if (ret == 1) {
+			cl->repeat_count = 1;
+			cl->repeated_char = c;
+		}
+		return ret;
+	}
+
+	/* GetFileType() failed or file of unknown type,
+	 * which we do not know how to peek anyway.
+	 */
+	return -1;
+}
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	HANDLE handle = (HANDLE)_get_osfhandle(cl->s_in);
+	return cl->oldterm.is_console_input ?
+		cmdline_poll_char_console(handle) :
+		cmdline_poll_char_file(cl, handle);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	HANDLE handle;
+	INPUT_RECORD record;
+	KEY_EVENT_RECORD *key;
+	DWORD events;
+
+	if (!cl->oldterm.is_console_input)
+		return _read(cl->s_in, c, 1);
+
+	/* Return repeated strokes from previous event. */
+	if (cl->repeat_count > 0) {
+		*c = cl->repeated_char;
+		cl->repeat_count--;
+		return 1;
+	}
+
+	handle = (HANDLE)_get_osfhandle(cl->s_in);
+	key = &record.Event.KeyEvent;
+	do {
+		if (!ReadConsoleInput(handle, &record, 1, &events)) {
+			if (GetLastError() == ERROR_HANDLE_EOF) {
+				*c = EOF;
+				return 0;
+			}
+			return -1;
+		}
+	} while (!cmdline_is_key_down(&record));
+
+	*c = key->uChar.AsciiChar;
+
+	/* Save repeated strokes from a single event. */
+	if (key->wRepeatCount > 1) {
+		cl->repeated_char = *c;
+		cl->repeat_count = key->wRepeatCount - 1;
+	}
+
+	return 1;
+}
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	int copy, ret;
+	FILE *file;
+
+	copy = _dup(fd);
+	if (copy < 0)
+		return -1;
+
+	file = _fdopen(copy, "a");
+	if (file == NULL) {
+		_close(copy);
+		return -1;
+	}
+
+	ret = vfprintf(file, format, op);
+
+	fclose(file); /* also closes copy */
+
+	return ret;
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 338d3d55c..1e05ec376 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -5,7 +5,11 @@
 #ifndef _CMDLINE_PRIVATE_H_
 #define _CMDLINE_PRIVATE_H_
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <rte_windows.h>
+#else
 #include <termios.h>
+#endif
 
 #include <stdarg.h>
 
@@ -15,7 +19,14 @@
 #include <cmdline_parse.h>
 
 struct terminal {
+#ifndef RTE_EXEC_ENV_WINDOWS
 	struct termios termios;
+#else
+	DWORD input_mode;
+	DWORD output_mode;
+	int is_console_input;
+	int is_console_output;
+#endif
 };
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
@@ -31,6 +42,10 @@ struct cmdline {
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
 	struct terminal oldterm;
+#ifdef RTE_EXEC_ENV_WINDOWS
+	char repeated_char;
+	WORD repeat_count;
+#endif
 };
 
 /* Check if a single character can be read from input. */
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index e73666f15..c5f483413 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -16,6 +16,10 @@
 #include "cmdline_private.h"
 #include "cmdline_socket.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define open _open
+#endif
+
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
 {
diff --git a/lib/librte_cmdline/cmdline_vt100.h b/lib/librte_cmdline/cmdline_vt100.h
index e33e67ed8..be9ae8e1c 100644
--- a/lib/librte_cmdline/cmdline_vt100.h
+++ b/lib/librte_cmdline/cmdline_vt100.h
@@ -31,7 +31,11 @@ extern "C" {
 #define vt100_multi_right  "\033\133%uC"
 #define vt100_multi_left   "\033\133%uD"
 #define vt100_suppr        "\033\133\063\176"
+#ifndef RTE_EXEC_ENV_WINDOWS
 #define vt100_home         "\033M\033E"
+#else
+#define vt100_home         "\033M\033[E"
+#endif
 #define vt100_word_left    "\033\142"
 #define vt100_word_right   "\033\146"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 5c9e8886d..5009b3354 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,7 +25,9 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
-if not is_windows
+if is_windows
+	sources += files('cmdline_os_windows.c')
+else
 	sources += files('cmdline_os_unix.c')
 endif
 
diff --git a/lib/meson.build b/lib/meson.build
index 6bbaf242a..8430057c6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -41,6 +41,7 @@ if is_windows
 		'eal',
 		'ring',
 		'mempool', 'mbuf', 'pci', 'net',
+		'cmdline',
 	] # only supported libraries for windows
 endif
 
-- 
2.25.4


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

* [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
                     ` (5 preceding siblings ...)
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 6/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-07-30 21:06   ` Dmitry Kozlyuk
  2020-09-26  1:33     ` [dpdk-dev] [EXTERNAL] " Narcisa Ana Maria Vasile
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
  7 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-07-30 21:06 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Dmitry Kozlyuk, Olivier Matz

Enable cmdline sample application as all dependencies are met.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 examples/cmdline/commands.c | 1 -
 examples/cmdline/main.c     | 1 -
 examples/meson.build        | 6 +++---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index 0e2232f03..f43eacfba 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <netinet/in.h>
-#include <termios.h>
 #ifdef RTE_EXEC_ENV_FREEBSD
 #include <sys/socket.h>
 #endif
diff --git a/examples/cmdline/main.c b/examples/cmdline/main.c
index f2f2e5a2f..bb7954245 100644
--- a/examples/cmdline/main.c
+++ b/examples/cmdline/main.c
@@ -8,7 +8,6 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
-#include <termios.h>
 #include <sys/queue.h>
 
 #include <cmdline_rdline.h>
diff --git a/examples/meson.build b/examples/meson.build
index eb13e8210..2b8ebce3b 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -79,9 +79,9 @@ foreach example: examples
 
 	ext_deps = [execinfo]
 	includes = [include_directories(example)]
-	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
-	if is_windows
-		deps = ['eal'] # only supported lib on Windows currently
+	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
+	if not is_windows
+		deps += ['ethdev'] # not currently supported
 	endif
 	subdir(example)
 
-- 
2.25.4


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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
@ 2020-08-05  9:31     ` Kinsella, Ray
  2020-08-05 11:17       ` Dmitry Kozlyuk
  2020-09-17 13:34     ` Olivier Matz
  1 sibling, 1 reply; 50+ messages in thread
From: Kinsella, Ray @ 2020-08-05  9:31 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Olivier Matz, Neil Horman



On 30/07/2020 22:06, Dmitry Kozlyuk wrote:
> struct cmdline exposes platform-specific members it contains, most
> notably struct termios that is only available on Unix. Make the
> structure opaque.
> 
> Remove tests checking struct cmdline content as meaningless.
> 
> Add cmdline_get_rdline() to access history buffer.
> The new function is currently used only in tests.

Should it be INTERNAL then? Is it useful outside of the test cases?

> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  app/test-cmdline/commands.c                |  8 +++--
>  app/test/test_cmdline_lib.c                | 42 +++++++++-------------
>  lib/librte_cmdline/cmdline.c               | 10 ++++--
>  lib/librte_cmdline/cmdline.h               | 15 ++++----
>  lib/librte_cmdline/cmdline_parse.c         |  4 +--
>  lib/librte_cmdline/cmdline_private.h       | 22 ++++++++++++
>  lib/librte_cmdline/cmdline_socket.c        |  6 ++--
>  lib/librte_cmdline/rte_cmdline_version.map |  8 +++++
>  8 files changed, 68 insertions(+), 47 deletions(-)
>  create mode 100644 lib/librte_cmdline/cmdline_private.h
> 
> diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
> index d67c0ca6a..ff7ac973e 100644
> --- a/app/test-cmdline/commands.c
> +++ b/app/test-cmdline/commands.c
> @@ -294,8 +294,10 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
>  		struct cmdline *cl,
>  		__rte_unused void *data)
>  {
> +	struct rdline *rdl = cmdline_get_rdline(cl);
> +
>  	cmdline_printf(cl, "History buffer size: %zu\n",
> -			sizeof(cl->rdl.history_buf));
> +			sizeof(rdl->history_buf));
>  }
>  
>  cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
> @@ -326,7 +328,9 @@ cmd_clear_history_parsed(__rte_unused void *parsed_result,
>  		struct cmdline *cl,
>  		__rte_unused void *data)
>  {
> -	rdline_clear_history(&cl->rdl);
> +	struct rdline *rdl = cmdline_get_rdline(cl);
> +
> +	rdline_clear_history(rdl);
>  }
>  
>  cmdline_parse_token_string_t cmd_clear_history_tok =
> diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
> index dec465da5..a7f5a7f7f 100644
> --- a/app/test/test_cmdline_lib.c
> +++ b/app/test/test_cmdline_lib.c
> @@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
>  static int
>  test_cmdline_parse_fns(void)
>  {
> -	struct cmdline cl;
> +	struct cmdline *cl;
> +	cmdline_parse_ctx_t ctx;
>  	int i = 0;
>  	char dst[CMDLINE_TEST_BUFSIZE];
>  
> +	cl = cmdline_new(&ctx, "prompt", -1, -1);
> +	if (cl == NULL) {
> +		printf("Error: cannot create cmdline to test parse fns!\n");
> +		return -1;
> +	}
> +
>  	if (cmdline_parse(NULL, "buffer") >= 0)
>  		goto error;
> -	if (cmdline_parse(&cl, NULL) >= 0)
> +	if (cmdline_parse(cl, NULL) >= 0)
>  		goto error;
>  
>  	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
>  		goto error;
>  
>  	return 0;
> @@ -166,11 +173,11 @@ static int
>  test_cmdline_fns(void)
>  {
>  	cmdline_parse_ctx_t ctx;
> -	struct cmdline cl, *tmp;
> +	struct cmdline *cl;
>  
>  	memset(&ctx, 0, sizeof(ctx));
> -	tmp = cmdline_new(&ctx, "test", -1, -1);
> -	if (tmp == NULL)
> +	cl = cmdline_new(&ctx, "test", -1, -1);
> +	if (cl == NULL)
>  		goto error;
>  
>  	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
> @@ -179,7 +186,7 @@ test_cmdline_fns(void)
>  		goto error;
>  	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
>  		goto error;
> -	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
> +	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
>  		goto error;
>  	if (cmdline_write_char(NULL, 0) >= 0)
>  		goto error;
> @@ -188,31 +195,14 @@ test_cmdline_fns(void)
>  	cmdline_set_prompt(NULL, "prompt");
>  	cmdline_free(NULL);
>  	cmdline_printf(NULL, "format");
> -	/* this should fail as stream handles are invalid */
> -	cmdline_printf(tmp, "format");
>  	cmdline_interact(NULL);
>  	cmdline_quit(NULL);
>  
> -	/* check if void calls change anything when they should fail */
> -	cl = *tmp;
> -
> -	cmdline_printf(&cl, NULL);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -	cmdline_set_prompt(&cl, NULL);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -
> -	cmdline_free(tmp);
> -
>  	return 0;
>  
>  error:
>  	printf("Error: function accepted null parameter!\n");
>  	return -1;
> -mismatch:
> -	printf("Error: data changed!\n");
> -	return -1;
>  }
>  
>  /* test library functions. the point of these tests is not so much to test
> diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
> index cfd703e5b..6f3fdd598 100644
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -13,14 +13,12 @@
>  #include <fcntl.h>
>  #include <poll.h>
>  #include <errno.h>
> -#include <termios.h>
>  #include <netinet/in.h>
>  
>  #include <rte_string_fns.h>
>  
> -#include "cmdline_parse.h"
> -#include "cmdline_rdline.h"
>  #include "cmdline.h"
> +#include "cmdline_private.h"
>  
>  static void
>  cmdline_valid_buffer(struct rdline *rdl, const char *buf,
> @@ -103,6 +101,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
>  	return cl;
>  }
>  
> +struct rdline*
> +cmdline_get_rdline(struct cmdline *cl)
> +{
> +	return &cl->rdl;
> +}
> +
>  void
>  cmdline_free(struct cmdline *cl)
>  {
> diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
> index 243f99d20..96674dfda 100644
> --- a/lib/librte_cmdline/cmdline.h
> +++ b/lib/librte_cmdline/cmdline.h
> @@ -8,8 +8,8 @@
>  #define _CMDLINE_H_
>  
>  #include <rte_common.h>
> +#include <rte_compat.h>
>  
> -#include <termios.h>
>  #include <cmdline_rdline.h>
>  #include <cmdline_parse.h>
>  
> @@ -23,14 +23,7 @@
>  extern "C" {
>  #endif
>  
> -struct cmdline {
> -	int s_in;
> -	int s_out;
> -	cmdline_parse_ctx_t *ctx;
> -	struct rdline rdl;
> -	char prompt[RDLINE_PROMPT_SIZE];
> -	struct termios oldterm;
> -};
> +struct cmdline;
>  
>  struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
>  void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
> @@ -40,6 +33,10 @@ void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
>  int cmdline_in(struct cmdline *cl, const char *buf, int size);
>  int cmdline_write_char(struct rdline *rdl, char c);
>  
> +__rte_experimental
> +struct rdline *
> +cmdline_get_rdline(struct cmdline *cl);
> +
>  /**
>   * This function is nonblocking equivalent of ``cmdline_interact()``. It polls
>   * *cl* for one character and interpret it. If return value is *RDLINE_EXITED*
> diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
> index b57b30e8f..ea0979158 100644
> --- a/lib/librte_cmdline/cmdline_parse.c
> +++ b/lib/librte_cmdline/cmdline_parse.c
> @@ -10,15 +10,13 @@
>  #include <string.h>
>  #include <inttypes.h>
>  #include <ctype.h>
> -#include <termios.h>
>  
>  #include <netinet/in.h>
>  
>  #include <rte_string_fns.h>
>  
> -#include "cmdline_rdline.h"
> -#include "cmdline_parse.h"
>  #include "cmdline.h"
> +#include "cmdline_private.h"
>  
>  #ifdef RTE_LIBRTE_CMDLINE_DEBUG
>  #define debug_printf printf
> diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
> new file mode 100644
> index 000000000..3b1c70e9f
> --- /dev/null
> +++ b/lib/librte_cmdline/cmdline_private.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2020 Dmitry Kozlyuk
> + */
> +
> +#ifndef _CMDLINE_PRIVATE_H_
> +#define _CMDLINE_PRIVATE_H_
> +
> +#include <termios.h>
> +
> +#include <cmdline_rdline.h>
> +#include <cmdline_parse.h>
> +
> +struct cmdline {
> +	int s_in;
> +	int s_out;
> +	cmdline_parse_ctx_t *ctx;
> +	struct rdline rdl;
> +	char prompt[RDLINE_PROMPT_SIZE];
> +	struct termios oldterm;
> +};
> +
> +#endif
> diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
> index ecb3d82b6..5e4b734d6 100644
> --- a/lib/librte_cmdline/cmdline_socket.c
> +++ b/lib/librte_cmdline/cmdline_socket.c
> @@ -11,12 +11,10 @@
>  #include <stdarg.h>
>  #include <inttypes.h>
>  #include <fcntl.h>
> -#include <termios.h>
>  
> -#include "cmdline_parse.h"
> -#include "cmdline_rdline.h"
> -#include "cmdline_socket.h"
>  #include "cmdline.h"
> +#include "cmdline_private.h"
> +#include "cmdline_socket.h"
>  
>  struct cmdline *
>  cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
> diff --git a/lib/librte_cmdline/rte_cmdline_version.map b/lib/librte_cmdline/rte_cmdline_version.map
> index 95fce812f..135ecc71f 100644
> --- a/lib/librte_cmdline/rte_cmdline_version.map
> +++ b/lib/librte_cmdline/rte_cmdline_version.map
> @@ -69,3 +69,11 @@ DPDK_20.0 {
>  
>  	local: *;
>  };
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	cmdline_get_rdline;
> +
> +	local: *;
> +};
> 

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-08-05  9:31     ` Kinsella, Ray
@ 2020-08-05 11:17       ` Dmitry Kozlyuk
  2020-09-30  8:11         ` Kinsella, Ray
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-08-05 11:17 UTC (permalink / raw)
  To: Kinsella, Ray
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Olivier Matz, Neil Horman

On Wed, 5 Aug 2020 10:31:31 +0100, Kinsella, Ray wrote:
> On 30/07/2020 22:06, Dmitry Kozlyuk wrote:
> > struct cmdline exposes platform-specific members it contains, most
> > notably struct termios that is only available on Unix. Make the
> > structure opaque.
> > 
> > Remove tests checking struct cmdline content as meaningless.
> > 
> > Add cmdline_get_rdline() to access history buffer.
> > The new function is currently used only in tests.  
> 
> Should it be INTERNAL then? Is it useful outside of the test cases?

There are already exposed rdline_*() functions that require struct rdline
pointer, which is now only accessible via this function for struct cmdline
instances. Thus, public API would be broken with INTERNAL for such use cases.


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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
  2020-08-05  9:31     ` Kinsella, Ray
@ 2020-09-17 13:34     ` Olivier Matz
  2020-09-17 17:05       ` Stephen Hemminger
                         ` (2 more replies)
  1 sibling, 3 replies; 50+ messages in thread
From: Olivier Matz @ 2020-09-17 13:34 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Ray Kinsella, Neil Horman

Hi Dmitry,

On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
> struct cmdline exposes platform-specific members it contains, most
> notably struct termios that is only available on Unix. Make the
> structure opaque.
> 
> Remove tests checking struct cmdline content as meaningless.
> 
> Add cmdline_get_rdline() to access history buffer.
> The new function is currently used only in tests.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

First, please forgive me for the very late feedback. It is all the more
problematic because I think this patch introduces an ABI breakage, that
should have been announced.

Making cmdline struct opaque clearly goes in the right direction, as
it will prevent future ABI breakage.

In my opinion, we could accept the patch for 20.11, knowing it reduce
the risk of future ABI breakage, and that cmdline is not a core
component of DPDK. However it has to be discussed and accepted by other
people.

Else, the patch would be delayed for 21.11. From what I see from the
other patches, it looks possible to keep cmdline struct public without
breaking the existing ABI by adding #ifdef CONFIG_RTE_EXEC_ENV_WINDOWS,
is it correct?

One minor comment below:

> --- a/app/test/test_cmdline_lib.c
> +++ b/app/test/test_cmdline_lib.c
> @@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
>  static int
>  test_cmdline_parse_fns(void)
>  {
> -	struct cmdline cl;
> +	struct cmdline *cl;
> +	cmdline_parse_ctx_t ctx;
>  	int i = 0;
>  	char dst[CMDLINE_TEST_BUFSIZE];
>  
> +	cl = cmdline_new(&ctx, "prompt", -1, -1);
> +	if (cl == NULL) {
> +		printf("Error: cannot create cmdline to test parse fns!\n");
> +		return -1;
> +	}
> +
>  	if (cmdline_parse(NULL, "buffer") >= 0)
>  		goto error;
> -	if (cmdline_parse(&cl, NULL) >= 0)
> +	if (cmdline_parse(cl, NULL) >= 0)
>  		goto error;
>  
>  	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
>  		goto error;
> -	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
> +	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
>  		goto error;
>  
>  	return 0;
> @@ -166,11 +173,11 @@ static int
>  test_cmdline_fns(void)
>  {
>  	cmdline_parse_ctx_t ctx;
> -	struct cmdline cl, *tmp;
> +	struct cmdline *cl;
>  
>  	memset(&ctx, 0, sizeof(ctx));
> -	tmp = cmdline_new(&ctx, "test", -1, -1);
> -	if (tmp == NULL)
> +	cl = cmdline_new(&ctx, "test", -1, -1);
> +	if (cl == NULL)
>  		goto error;
>  
>  	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
> @@ -179,7 +186,7 @@ test_cmdline_fns(void)
>  		goto error;
>  	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
>  		goto error;
> -	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
> +	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
>  		goto error;
>  	if (cmdline_write_char(NULL, 0) >= 0)
>  		goto error;
> @@ -188,31 +195,14 @@ test_cmdline_fns(void)
>  	cmdline_set_prompt(NULL, "prompt");
>  	cmdline_free(NULL);
>  	cmdline_printf(NULL, "format");
> -	/* this should fail as stream handles are invalid */
> -	cmdline_printf(tmp, "format");
>  	cmdline_interact(NULL);
>  	cmdline_quit(NULL);
>  
> -	/* check if void calls change anything when they should fail */
> -	cl = *tmp;
> -
> -	cmdline_printf(&cl, NULL);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -	cmdline_set_prompt(&cl, NULL);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
> -
> -	cmdline_free(tmp);
> -
>  	return 0;
>  
>  error:
>  	printf("Error: function accepted null parameter!\n");
>  	return -1;
> -mismatch:
> -	printf("Error: data changed!\n");
> -	return -1;

cmdline_free(cl) is missing.

before your patch, cmdline_free(tmp) was already missing in error case
by the way.


Thanks,
Olivier

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-17 13:34     ` Olivier Matz
@ 2020-09-17 17:05       ` Stephen Hemminger
  2020-09-18  8:33         ` Bruce Richardson
  2020-09-18 13:23         ` Kinsella, Ray
  2020-09-17 23:13       ` Dmitry Kozlyuk
  2020-09-18 13:31       ` Kinsella, Ray
  2 siblings, 2 replies; 50+ messages in thread
From: Stephen Hemminger @ 2020-09-17 17:05 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Dmitry Kozlyuk, dev, Dmitry Malloy, Narcisa Ana Maria Vasile,
	Fady Bader, Tal Shnaiderman, Kadam, Pallavi, Ray Kinsella,
	Neil Horman

On Thu, 17 Sep 2020 15:34:43 +0200
Olivier Matz <olivier.matz@6wind.com> wrote:

> Hi Dmitry,
> 
> On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
> > struct cmdline exposes platform-specific members it contains, most
> > notably struct termios that is only available on Unix. Make the
> > structure opaque.
> > 
> > Remove tests checking struct cmdline content as meaningless.
> > 
> > Add cmdline_get_rdline() to access history buffer.
> > The new function is currently used only in tests.
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>  
> 
> First, please forgive me for the very late feedback. It is all the more
> problematic because I think this patch introduces an ABI breakage, that
> should have been announced.

Since 20.11 is a API/ABI breaking release, I think breaking ABI
is okay without announcement. What matters more is if that API would
need to be impacted. API changes need some announcement.

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-17 13:34     ` Olivier Matz
  2020-09-17 17:05       ` Stephen Hemminger
@ 2020-09-17 23:13       ` Dmitry Kozlyuk
  2020-09-18 13:31       ` Kinsella, Ray
  2 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-17 23:13 UTC (permalink / raw)
  To: Olivier Matz
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Ray Kinsella, Neil Horman

Hi Olivier, thanks for the review.

> In my opinion, we could accept the patch for 20.11, knowing it reduce
> the risk of future ABI breakage, and that cmdline is not a core
> component of DPDK. However it has to be discussed and accepted by other
> people.
> 
> Else, the patch would be delayed for 21.11. From what I see from the
> other patches, it looks possible to keep cmdline struct public without
> breaking the existing ABI by adding #ifdef CONFIG_RTE_EXEC_ENV_WINDOWS,
> is it correct?

Windows team is now working on getting testpmd, tests, and examples
functional. librte_cmdline is essential for that, so delaying to 21.11 is the
worst option. If unannounced ABI breakage is deemed intolerable, I'd certainly
go with #ifdef.

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-17 17:05       ` Stephen Hemminger
@ 2020-09-18  8:33         ` Bruce Richardson
  2020-09-18 12:13           ` Ferruh Yigit
  2020-09-18 13:23         ` Kinsella, Ray
  1 sibling, 1 reply; 50+ messages in thread
From: Bruce Richardson @ 2020-09-18  8:33 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Olivier Matz, Dmitry Kozlyuk, dev, Dmitry Malloy,
	Narcisa Ana Maria Vasile, Fady Bader, Tal Shnaiderman, Kadam,
	Pallavi, Ray Kinsella, Neil Horman

On Thu, Sep 17, 2020 at 10:05:48AM -0700, Stephen Hemminger wrote:
> On Thu, 17 Sep 2020 15:34:43 +0200
> Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> > Hi Dmitry,
> > 
> > On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
> > > struct cmdline exposes platform-specific members it contains, most
> > > notably struct termios that is only available on Unix. Make the
> > > structure opaque.
> > > 
> > > Remove tests checking struct cmdline content as meaningless.
> > > 
> > > Add cmdline_get_rdline() to access history buffer.
> > > The new function is currently used only in tests.
> > > 
> > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>  
> > 
> > First, please forgive me for the very late feedback. It is all the more
> > problematic because I think this patch introduces an ABI breakage, that
> > should have been announced.
> 
> Since 20.11 is a API/ABI breaking release, I think breaking ABI
> is okay without announcement. What matters more is if that API would
> need to be impacted. API changes need some announcement.

This is something that we need to get a clear decision from technical board
on, I think, since there are some other proposed ABI changes in patches
that were not pre-announced, e.g. changing the lpm structure.

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-18  8:33         ` Bruce Richardson
@ 2020-09-18 12:13           ` Ferruh Yigit
  0 siblings, 0 replies; 50+ messages in thread
From: Ferruh Yigit @ 2020-09-18 12:13 UTC (permalink / raw)
  To: Bruce Richardson, Stephen Hemminger
  Cc: Olivier Matz, Dmitry Kozlyuk, dev, Dmitry Malloy,
	Narcisa Ana Maria Vasile, Fady Bader, Tal Shnaiderman, Kadam,
	Pallavi, Ray Kinsella, Neil Horman

On 9/18/2020 9:33 AM, Bruce Richardson wrote:
> On Thu, Sep 17, 2020 at 10:05:48AM -0700, Stephen Hemminger wrote:
>> On Thu, 17 Sep 2020 15:34:43 +0200
>> Olivier Matz <olivier.matz@6wind.com> wrote:
>>
>>> Hi Dmitry,
>>>
>>> On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
>>>> struct cmdline exposes platform-specific members it contains, most
>>>> notably struct termios that is only available on Unix. Make the
>>>> structure opaque.
>>>>
>>>> Remove tests checking struct cmdline content as meaningless.
>>>>
>>>> Add cmdline_get_rdline() to access history buffer.
>>>> The new function is currently used only in tests.
>>>>
>>>> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
>>>
>>> First, please forgive me for the very late feedback. It is all the more
>>> problematic because I think this patch introduces an ABI breakage, that
>>> should have been announced.
>>
>> Since 20.11 is a API/ABI breaking release, I think breaking ABI
>> is okay without announcement. What matters more is if that API would
>> need to be impacted. API changes need some announcement.
> 
> This is something that we need to get a clear decision from technical board
> on, I think, since there are some other proposed ABI changes in patches
> that were not pre-announced, e.g. changing the lpm structure.
> 

And we accepted another in ethdev one, but that library already has 
bunch of deprecation notices on it and one more change won't has much 
affect.

Overall looks OK to me to accept minor changes without deprecation 
notices, big ones can be investigated case by case, and +1 to have some 
guidance from the techboard.

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-17 17:05       ` Stephen Hemminger
  2020-09-18  8:33         ` Bruce Richardson
@ 2020-09-18 13:23         ` Kinsella, Ray
  1 sibling, 0 replies; 50+ messages in thread
From: Kinsella, Ray @ 2020-09-18 13:23 UTC (permalink / raw)
  To: Stephen Hemminger, Olivier Matz
  Cc: Dmitry Kozlyuk, dev, Dmitry Malloy, Narcisa Ana Maria Vasile,
	Fady Bader, Tal Shnaiderman, Kadam, Pallavi, Neil Horman



On 17/09/2020 18:05, Stephen Hemminger wrote:
> On Thu, 17 Sep 2020 15:34:43 +0200
> Olivier Matz <olivier.matz@6wind.com> wrote:
> 
>> Hi Dmitry,
>>
>> On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
>>> struct cmdline exposes platform-specific members it contains, most
>>> notably struct termios that is only available on Unix. Make the
>>> structure opaque.
>>>
>>> Remove tests checking struct cmdline content as meaningless.
>>>
>>> Add cmdline_get_rdline() to access history buffer.
>>> The new function is currently used only in tests.
>>>
>>> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>  
>>
>> First, please forgive me for the very late feedback. It is all the more
>> problematic because I think this patch introduces an ABI breakage, that
>> should have been announced.
> 
> Since 20.11 is a API/ABI breaking release, I think breaking ABI
> is okay without announcement. What matters more is if that API would
> need to be impacted. API changes need some announcement.

Right but 20.11 being an ABI breaking release, 
still doesn't waive the need to observe the deprecation notice, 3-acks from Maintainers etc.


 

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-17 13:34     ` Olivier Matz
  2020-09-17 17:05       ` Stephen Hemminger
  2020-09-17 23:13       ` Dmitry Kozlyuk
@ 2020-09-18 13:31       ` Kinsella, Ray
  2 siblings, 0 replies; 50+ messages in thread
From: Kinsella, Ray @ 2020-09-18 13:31 UTC (permalink / raw)
  To: Olivier Matz, Dmitry Kozlyuk
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Neil Horman



On 17/09/2020 14:34, Olivier Matz wrote:
> Hi Dmitry,
> 
> On Fri, Jul 31, 2020 at 12:06:45AM +0300, Dmitry Kozlyuk wrote:
>> struct cmdline exposes platform-specific members it contains, most
>> notably struct termios that is only available on Unix. Make the
>> structure opaque.
>>
>> Remove tests checking struct cmdline content as meaningless.
>>
>> Add cmdline_get_rdline() to access history buffer.
>> The new function is currently used only in tests.
>>
>> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> First, please forgive me for the very late feedback. It is all the more
> problematic because I think this patch introduces an ABI breakage, that
> should have been announced.

I think this is more a theoretical ABI breakage, than a real ABI breakage.
As the consuming application is never likely to access the structure, 
As in most cases, I doubt it ever has any knowledge of the structure. 

My 2c, is that I think it is fine not consider this as ABI breakage. 

> 
> Making cmdline struct opaque clearly goes in the right direction, as
> it will prevent future ABI breakage.
> 
> In my opinion, we could accept the patch for 20.11, knowing it reduce
> the risk of future ABI breakage, and that cmdline is not a core
> component of DPDK. However it has to be discussed and accepted by other
> people.
> 
> Else, the patch would be delayed for 21.11. From what I see from the
> other patches, it looks possible to keep cmdline struct public without
> breaking the existing ABI by adding #ifdef CONFIG_RTE_EXEC_ENV_WINDOWS,
> is it correct?
> 
> One minor comment below:
> 
>> --- a/app/test/test_cmdline_lib.c
>> +++ b/app/test/test_cmdline_lib.c
>> @@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
>>  static int
>>  test_cmdline_parse_fns(void)
>>  {
>> -	struct cmdline cl;
>> +	struct cmdline *cl;
>> +	cmdline_parse_ctx_t ctx;
>>  	int i = 0;
>>  	char dst[CMDLINE_TEST_BUFSIZE];
>>  
>> +	cl = cmdline_new(&ctx, "prompt", -1, -1);
>> +	if (cl == NULL) {
>> +		printf("Error: cannot create cmdline to test parse fns!\n");
>> +		return -1;
>> +	}
>> +
>>  	if (cmdline_parse(NULL, "buffer") >= 0)
>>  		goto error;
>> -	if (cmdline_parse(&cl, NULL) >= 0)
>> +	if (cmdline_parse(cl, NULL) >= 0)
>>  		goto error;
>>  
>>  	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
>>  		goto error;
>> -	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
>> +	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
>>  		goto error;
>> -	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
>> +	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
>>  		goto error;
>> -	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
>> +	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
>>  		goto error;
>>  
>>  	return 0;
>> @@ -166,11 +173,11 @@ static int
>>  test_cmdline_fns(void)
>>  {
>>  	cmdline_parse_ctx_t ctx;
>> -	struct cmdline cl, *tmp;
>> +	struct cmdline *cl;
>>  
>>  	memset(&ctx, 0, sizeof(ctx));
>> -	tmp = cmdline_new(&ctx, "test", -1, -1);
>> -	if (tmp == NULL)
>> +	cl = cmdline_new(&ctx, "test", -1, -1);
>> +	if (cl == NULL)
>>  		goto error;
>>  
>>  	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
>> @@ -179,7 +186,7 @@ test_cmdline_fns(void)
>>  		goto error;
>>  	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
>>  		goto error;
>> -	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
>> +	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
>>  		goto error;
>>  	if (cmdline_write_char(NULL, 0) >= 0)
>>  		goto error;
>> @@ -188,31 +195,14 @@ test_cmdline_fns(void)
>>  	cmdline_set_prompt(NULL, "prompt");
>>  	cmdline_free(NULL);
>>  	cmdline_printf(NULL, "format");
>> -	/* this should fail as stream handles are invalid */
>> -	cmdline_printf(tmp, "format");
>>  	cmdline_interact(NULL);
>>  	cmdline_quit(NULL);
>>  
>> -	/* check if void calls change anything when they should fail */
>> -	cl = *tmp;
>> -
>> -	cmdline_printf(&cl, NULL);
>> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
>> -	cmdline_set_prompt(&cl, NULL);
>> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
>> -	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
>> -	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
>> -
>> -	cmdline_free(tmp);
>> -
>>  	return 0;
>>  
>>  error:
>>  	printf("Error: function accepted null parameter!\n");
>>  	return -1;
>> -mismatch:
>> -	printf("Error: data changed!\n");
>> -	return -1;
> 
> cmdline_free(cl) is missing.
> 
> before your patch, cmdline_free(tmp) was already missing in error case
> by the way.
> 
> 
> Thanks,
> Olivier
> 

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

* Re: [dpdk-dev] [EXTERNAL] [PATCH v2 7/7] examples/cmdline: build on Windows
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
@ 2020-09-26  1:33     ` Narcisa Ana Maria Vasile
  2020-09-26  6:03       ` Khoa To
  0 siblings, 1 reply; 50+ messages in thread
From: Narcisa Ana Maria Vasile @ 2020-09-26  1:33 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev, Khoa To

Adding Khoa

-----Original Message-----
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 
Sent: Thursday, July 30, 2020 2:07 PM
To: dev@dpdk.org
Cc: Dmitry Malloy (MESHCHANINOV) <dmitrym@microsoft.com>; Narcisa Ana Maria Vasile <Narcisa.Vasile@microsoft.com>; Fady Bader <fady@mellanox.com>; Tal Shnaiderman <talshn@mellanox.com>; Kadam, Pallavi <pallavi.kadam@intel.com>; Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>; Olivier Matz <olivier.matz@6wind.com>
Subject: [EXTERNAL] [PATCH v2 7/7] examples/cmdline: build on Windows

Enable cmdline sample application as all dependencies are met.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 examples/cmdline/commands.c | 1 -
 examples/cmdline/main.c     | 1 -
 examples/meson.build        | 6 +++---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c index 0e2232f03..f43eacfba 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <netinet/in.h>
-#include <termios.h>
 #ifdef RTE_EXEC_ENV_FREEBSD
 #include <sys/socket.h>
 #endif
diff --git a/examples/cmdline/main.c b/examples/cmdline/main.c index f2f2e5a2f..bb7954245 100644
--- a/examples/cmdline/main.c
+++ b/examples/cmdline/main.c
@@ -8,7 +8,6 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
-#include <termios.h>
 #include <sys/queue.h>
 
 #include <cmdline_rdline.h>
diff --git a/examples/meson.build b/examples/meson.build index eb13e8210..2b8ebce3b 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -79,9 +79,9 @@ foreach example: examples
 
 	ext_deps = [execinfo]
 	includes = [include_directories(example)]
-	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
-	if is_windows
-		deps = ['eal'] # only supported lib on Windows currently
+	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
+	if not is_windows
+		deps += ['ethdev'] # not currently supported
 	endif
 	subdir(example)
 
--
2.25.4


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

* Re: [dpdk-dev] [EXTERNAL] [PATCH v2 7/7] examples/cmdline: build on Windows
  2020-09-26  1:33     ` [dpdk-dev] [EXTERNAL] " Narcisa Ana Maria Vasile
@ 2020-09-26  6:03       ` Khoa To
  0 siblings, 0 replies; 50+ messages in thread
From: Khoa To @ 2020-09-26  6:03 UTC (permalink / raw)
  To: Narcisa Ana Maria Vasile, Dmitry Kozlyuk, dev


> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> diff --git a/examples/meson.build b/examples/meson.build index
> eb13e8210..2b8ebce3b 100644
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -79,9 +79,9 @@ foreach example: examples
> 
>  	ext_deps = [execinfo]
>  	includes = [include_directories(example)]
> -	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
> -	if is_windows
> -		deps = ['eal'] # only supported lib on Windows currently
> +	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
> +	if not is_windows
> +		deps += ['ethdev'] # not currently supported
>  	endif

'ethdev' is now supported on Windows, so I think you can just have 'ethdev' in the common deps list.

>  	subdir(example)
> 
> --
> 2.25.4



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

* [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows
  2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
                     ` (6 preceding siblings ...)
  2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
@ 2020-09-28 21:50   ` Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
                       ` (7 more replies)
  7 siblings, 8 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit, Dmitry Kozlyuk

This patchset enables librte_cmdline on Windows. To do that, it creates
a number of wrappers for OS-dependent terminal handling and I/O.
Considered alternative was to revive [1] and use libedit (Unix-only)
for terminal handling. However, testing revealed that WinEditLine [2]
is not a drop-in replacement for libedit, so this solution wouldn't be
universal.

[1]: http://patchwork.dpdk.org/patch/38561
[2]: http://mingweditline.sourceforge.net

v3:
    * Add #ifdef workaround to keep API/ABI for Unices (Olivier).
    * Fix missing cmdline_free() in test (Olivier).
    * Rebase on ToT (Khoa).

Dmitry Kozlyuk (7):
  cmdline: make implementation logically opaque
  cmdline: add internal wrappers for terminal handling
  cmdline: add internal wrappers for character input
  cmdline: add internal wrapper for vdprintf
  eal/windows: improve compatibility networking headers
  cmdline: support Windows
  examples/cmdline: build on Windows

 app/test-cmdline/commands.c                 |   8 +-
 app/test/test_cmdline_lib.c                 |  44 ++---
 config/meson.build                          |   2 +
 doc/guides/rel_notes/deprecation.rst        |   4 +
 examples/cmdline/commands.c                 |   1 -
 examples/cmdline/main.c                     |   1 -
 examples/meson.build                        |   6 +-
 lib/librte_cmdline/cmdline.c                |  30 +--
 lib/librte_cmdline/cmdline.h                |  18 +-
 lib/librte_cmdline/cmdline_os_unix.c        |  53 +++++
 lib/librte_cmdline/cmdline_os_windows.c     | 207 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_parse.c          |   5 +-
 lib/librte_cmdline/cmdline_private.h        |  53 +++++
 lib/librte_cmdline/cmdline_socket.c         |  25 +--
 lib/librte_cmdline/cmdline_vt100.c          |   1 -
 lib/librte_cmdline/cmdline_vt100.h          |   4 +
 lib/librte_cmdline/meson.build              |   6 +
 lib/librte_cmdline/rte_cmdline_version.map  |   8 +
 lib/librte_eal/windows/include/arpa/inet.h  |  30 +++
 lib/librte_eal/windows/include/netinet/in.h |  12 ++
 lib/librte_eal/windows/include/sys/socket.h |  24 +++
 lib/meson.build                             |   1 +
 22 files changed, 475 insertions(+), 68 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c
 create mode 100644 lib/librte_cmdline/cmdline_private.h
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-09-30  8:12       ` Kinsella, Ray
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
                       ` (6 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Olivier Matz, Neil Horman

struct cmdline exposes platform-specific members it contains, most
notably struct termios that is only available on Unix. While ABI
considerations prevent from hinding the definition on already supported
platforms, struct cmdline is considered logically opaque from now on.
Add a deprecation notice targeted at 20.11.

* Remove tests checking struct cmdline content as meaningless.

* Fix missing cmdline_free() in unit test.

* Add cmdline_get_rdline() to access history buffer indirectly.
  The new function is currently used only in tests.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Suggested-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-cmdline/commands.c                |  8 +++-
 app/test/test_cmdline_lib.c                | 44 +++++++++-------------
 doc/guides/rel_notes/deprecation.rst       |  4 ++
 lib/librte_cmdline/cmdline.c               |  9 +++--
 lib/librte_cmdline/cmdline.h               | 18 ++++++++-
 lib/librte_cmdline/cmdline_parse.c         |  3 --
 lib/librte_cmdline/cmdline_socket.c        |  5 +--
 lib/librte_cmdline/rte_cmdline_version.map |  8 ++++
 8 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d67c0ca6a..ff7ac973e 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -294,8 +294,10 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(cl->rdl.history_buf));
+			sizeof(rdl->history_buf));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
@@ -326,7 +328,9 @@ cmd_clear_history_parsed(__rte_unused void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
-	rdline_clear_history(&cl->rdl);
+	struct rdline *rdl = cmdline_get_rdline(cl);
+
+	rdline_clear_history(rdl);
 }
 
 cmdline_parse_token_string_t cmd_clear_history_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index dec465da5..bd72df0da 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -46,22 +46,29 @@ complete_buffer(__rte_unused struct rdline *rdl,
 static int
 test_cmdline_parse_fns(void)
 {
-	struct cmdline cl;
+	struct cmdline *cl;
+	cmdline_parse_ctx_t ctx;
 	int i = 0;
 	char dst[CMDLINE_TEST_BUFSIZE];
 
+	cl = cmdline_new(&ctx, "prompt", -1, -1);
+	if (cl == NULL) {
+		printf("Error: cannot create cmdline to test parse fns!\n");
+		return -1;
+	}
+
 	if (cmdline_parse(NULL, "buffer") >= 0)
 		goto error;
-	if (cmdline_parse(&cl, NULL) >= 0)
+	if (cmdline_parse(cl, NULL) >= 0)
 		goto error;
 
 	if (cmdline_complete(NULL, "buffer", &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, NULL, &i, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, NULL, &i, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", NULL, dst, sizeof(dst)) >= 0)
 		goto error;
-	if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
+	if (cmdline_complete(cl, "buffer", &i, NULL, sizeof(dst)) >= 0)
 		goto error;
 
 	return 0;
@@ -166,11 +173,11 @@ static int
 test_cmdline_fns(void)
 {
 	cmdline_parse_ctx_t ctx;
-	struct cmdline cl, *tmp;
+	struct cmdline *cl;
 
 	memset(&ctx, 0, sizeof(ctx));
-	tmp = cmdline_new(&ctx, "test", -1, -1);
-	if (tmp == NULL)
+	cl = cmdline_new(&ctx, "test", -1, -1);
+	if (cl == NULL)
 		goto error;
 
 	if (cmdline_new(NULL, "prompt", 0, 0) != NULL)
@@ -179,7 +186,7 @@ test_cmdline_fns(void)
 		goto error;
 	if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
-	if (cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
+	if (cmdline_in(cl, NULL, CMDLINE_TEST_BUFSIZE) >= 0)
 		goto error;
 	if (cmdline_write_char(NULL, 0) >= 0)
 		goto error;
@@ -188,30 +195,15 @@ test_cmdline_fns(void)
 	cmdline_set_prompt(NULL, "prompt");
 	cmdline_free(NULL);
 	cmdline_printf(NULL, "format");
-	/* this should fail as stream handles are invalid */
-	cmdline_printf(tmp, "format");
 	cmdline_interact(NULL);
 	cmdline_quit(NULL);
 
-	/* check if void calls change anything when they should fail */
-	cl = *tmp;
-
-	cmdline_printf(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_set_prompt(&cl, NULL);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-	cmdline_in(&cl, NULL, CMDLINE_TEST_BUFSIZE);
-	if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch;
-
-	cmdline_free(tmp);
-
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
-	return -1;
-mismatch:
-	printf("Error: data changed!\n");
+	if (cl != NULL)
+		cmdline_free(cl);
 	return -1;
 }
 
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 9691f2c57..7ae013c27 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -255,3 +255,7 @@ Deprecation Notices
   ``make``. Given environments are too much variables for such a simple script,
   it will be removed in DPDK 20.11.
   Some useful parts may be converted into specific scripts.
+
+* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
+  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
+  original structure will be kept until DPDK 21.11.
\ No newline at end of file
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index cfd703e5b..41f50cc56 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,13 +13,10 @@
 #include <fcntl.h>
 #include <poll.h>
 #include <errno.h>
-#include <termios.h>
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
 #include "cmdline.h"
 
 static void
@@ -103,6 +100,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	return cl;
 }
 
+struct rdline*
+cmdline_get_rdline(struct cmdline *cl)
+{
+	return &cl->rdl;
+}
+
 void
 cmdline_free(struct cmdline *cl)
 {
diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 243f99d20..c29762dda 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -7,9 +7,13 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+#include <termios.h>
+#endif
+
 #include <rte_common.h>
+#include <rte_compat.h>
 
-#include <termios.h>
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
 
@@ -23,6 +27,8 @@
 extern "C" {
 #endif
 
+#ifndef RTE_EXEC_ENV_WINDOWS
+
 struct cmdline {
 	int s_in;
 	int s_out;
@@ -32,6 +38,12 @@ struct cmdline {
 	struct termios oldterm;
 };
 
+#else
+
+struct cmdline;
+
+#endif /* RTE_EXEC_ENV_WINDOWS */
+
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
@@ -40,6 +52,10 @@ void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 
+__rte_experimental
+struct rdline *
+cmdline_get_rdline(struct cmdline *cl);
+
 /**
  * This function is nonblocking equivalent of ``cmdline_interact()``. It polls
  * *cl* for one character and interpret it. If return value is *RDLINE_EXITED*
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index b57b30e8f..f120f19dd 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -10,14 +10,11 @@
 #include <string.h>
 #include <inttypes.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline_rdline.h"
-#include "cmdline_parse.h"
 #include "cmdline.h"
 
 #ifdef RTE_LIBRTE_CMDLINE_DEBUG
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index ecb3d82b6..6c89d2171 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -11,12 +11,9 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <termios.h>
 
-#include "cmdline_parse.h"
-#include "cmdline_rdline.h"
-#include "cmdline_socket.h"
 #include "cmdline.h"
+#include "cmdline_socket.h"
 
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
diff --git a/lib/librte_cmdline/rte_cmdline_version.map b/lib/librte_cmdline/rte_cmdline_version.map
index a99104457..9df027215 100644
--- a/lib/librte_cmdline/rte_cmdline_version.map
+++ b/lib/librte_cmdline/rte_cmdline_version.map
@@ -69,3 +69,11 @@ DPDK_21 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	cmdline_get_rdline;
+
+	local: *;
+};
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 2/7] cmdline: add internal wrappers for terminal handling
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
                       ` (5 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Olivier Matz

Add functions that set up, save, and restore terminal parameters.
Use existing code as Unix implementation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline_os_unix.c | 27 +++++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h | 16 ++++++++++++++++
 lib/librte_cmdline/cmdline_socket.c  | 16 +++++-----------
 lib/librte_cmdline/cmdline_vt100.c   |  1 -
 lib/librte_cmdline/meson.build       |  4 ++++
 5 files changed, 52 insertions(+), 12 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
 create mode 100644 lib/librte_cmdline/cmdline_private.h

diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
new file mode 100644
index 000000000..d50eb1c65
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <string.h>
+
+#include "cmdline_private.h"
+
+void
+terminal_adjust(struct cmdline *cl)
+{
+	struct termios term;
+
+	tcgetattr(0, &cl->oldterm);
+
+	memcpy(&term, &cl->oldterm, sizeof(term));
+	term.c_lflag &= ~(ICANON | ECHO | ISIG);
+	tcsetattr(0, TCSANOW, &term);
+
+	setbuf(stdin, NULL);
+}
+
+void
+terminal_restore(const struct cmdline *cl)
+{
+	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
new file mode 100644
index 000000000..50326e8a4
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _CMDLINE_PRIVATE_H_
+#define _CMDLINE_PRIVATE_H_
+
+#include <cmdline.h>
+
+/* Disable buffering and echoing, save previous settings to oldterm. */
+void terminal_adjust(struct cmdline *cl);
+
+/* Restore terminal settings form oldterm. */
+void terminal_restore(const struct cmdline *cl);
+
+#endif
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index 6c89d2171..998e8ade2 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -13,6 +13,7 @@
 #include <fcntl.h>
 
 #include "cmdline.h"
+#include "cmdline_private.h"
 #include "cmdline_socket.h"
 
 struct cmdline *
@@ -36,18 +37,11 @@ struct cmdline *
 cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 {
 	struct cmdline *cl;
-	struct termios oldterm, term;
-
-	tcgetattr(0, &oldterm);
-	memcpy(&term, &oldterm, sizeof(term));
-	term.c_lflag &= ~(ICANON | ECHO | ISIG);
-	tcsetattr(0, TCSANOW, &term);
-	setbuf(stdin, NULL);
 
 	cl = cmdline_new(ctx, prompt, 0, 1);
 
-	if (cl)
-		memcpy(&cl->oldterm, &oldterm, sizeof(term));
+	if (cl != NULL)
+		terminal_adjust(cl);
 
 	return cl;
 }
@@ -55,8 +49,8 @@ cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 void
 cmdline_stdin_exit(struct cmdline *cl)
 {
-	if (!cl)
+	if (cl == NULL)
 		return;
 
-	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
+	terminal_restore(cl);
 }
diff --git a/lib/librte_cmdline/cmdline_vt100.c b/lib/librte_cmdline/cmdline_vt100.c
index 662fc7345..bb968dd5f 100644
--- a/lib/librte_cmdline/cmdline_vt100.c
+++ b/lib/librte_cmdline/cmdline_vt100.c
@@ -10,7 +10,6 @@
 #include <string.h>
 #include <stdarg.h>
 #include <ctype.h>
-#include <termios.h>
 
 #include "cmdline_vt100.h"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 7fc54ff1a..5c9e8886d 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,4 +25,8 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
+if not is_windows
+	sources += files('cmdline_os_unix.c')
+endif
+
 deps += ['net']
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 3/7] cmdline: add internal wrappers for character input
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
                       ` (4 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Olivier Matz

poll(3) is a purely Unix facility, so it cannot be directly used by
common code. read(2) is limited in device support outside of Unix.
Create wrapper functions and implement them for Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 14 ++++----------
 lib/librte_cmdline/cmdline_os_unix.c | 20 ++++++++++++++++++++
 lib/librte_cmdline/cmdline_private.h |  6 ++++++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 41f50cc56..0b2e1e30b 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -11,13 +11,12 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <fcntl.h>
-#include <poll.h>
 #include <errno.h>
 #include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
-#include "cmdline.h"
+#include "cmdline_private.h"
 
 static void
 cmdline_valid_buffer(struct rdline *rdl, const char *buf,
@@ -184,7 +183,6 @@ cmdline_quit(struct cmdline *cl)
 int
 cmdline_poll(struct cmdline *cl)
 {
-	struct pollfd pfd;
 	int status;
 	ssize_t read_status;
 	char c;
@@ -194,16 +192,12 @@ cmdline_poll(struct cmdline *cl)
 	else if (cl->rdl.status == RDLINE_EXITED)
 		return RDLINE_EXITED;
 
-	pfd.fd = cl->s_in;
-	pfd.events = POLLIN;
-	pfd.revents = 0;
-
-	status = poll(&pfd, 1, 0);
+	status = cmdline_poll_char(cl);
 	if (status < 0)
 		return status;
 	else if (status > 0) {
 		c = -1;
-		read_status = read(cl->s_in, &c, 1);
+		read_status = cmdline_read_char(cl, &c);
 		if (read_status < 0)
 			return read_status;
 
@@ -225,7 +219,7 @@ cmdline_interact(struct cmdline *cl)
 
 	c = -1;
 	while (1) {
-		if (read(cl->s_in, &c, 1) <= 0)
+		if (cmdline_read_char(cl, &c) <= 0)
 			break;
 		if (cmdline_in(cl, &c, 1) < 0)
 			break;
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index d50eb1c65..a4916c197 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -2,7 +2,9 @@
  * Copyright (c) 2020 Dmitry Kozlyuk
  */
 
+#include <poll.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "cmdline_private.h"
 
@@ -25,3 +27,21 @@ terminal_restore(const struct cmdline *cl)
 {
 	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
 }
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	struct pollfd pfd;
+
+	pfd.fd = cl->s_in;
+	pfd.events = POLLIN;
+	pfd.revents = 0;
+
+	return poll(&pfd, 1, 0);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	return read(cl->s_in, c, 1);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 50326e8a4..ac10de4f8 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -13,4 +13,10 @@ void terminal_adjust(struct cmdline *cl);
 /* Restore terminal settings form oldterm. */
 void terminal_restore(const struct cmdline *cl);
 
+/* Check if a single character can be read from input. */
+int cmdline_poll_char(struct cmdline *cl);
+
+/* Read one character from input. */
+ssize_t cmdline_read_char(struct cmdline *cl, char *c);
+
 #endif
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 4/7] cmdline: add internal wrapper for vdprintf
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (2 preceding siblings ...)
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
                       ` (3 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Olivier Matz

Add internal wrapper for vdprintf(3) that is only available on Unix.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.c         | 2 +-
 lib/librte_cmdline/cmdline_os_unix.c | 6 ++++++
 lib/librte_cmdline/cmdline_private.h | 8 ++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 0b2e1e30b..49770869b 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -131,7 +131,7 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 	if (cl->s_out < 0)
 		return;
 	va_start(ap, fmt);
-	vdprintf(cl->s_out, fmt, ap);
+	cmdline_vdprintf(cl->s_out, fmt, ap);
 	va_end(ap);
 }
 
diff --git a/lib/librte_cmdline/cmdline_os_unix.c b/lib/librte_cmdline/cmdline_os_unix.c
index a4916c197..64a945a34 100644
--- a/lib/librte_cmdline/cmdline_os_unix.c
+++ b/lib/librte_cmdline/cmdline_os_unix.c
@@ -45,3 +45,9 @@ cmdline_read_char(struct cmdline *cl, char *c)
 {
 	return read(cl->s_in, c, 1);
 }
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	return vdprintf(fd, format, op);
+}
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index ac10de4f8..4d9ea33f0 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -5,6 +5,10 @@
 #ifndef _CMDLINE_PRIVATE_H_
 #define _CMDLINE_PRIVATE_H_
 
+#include <stdarg.h>
+
+#include <rte_common.h>
+
 #include <cmdline.h>
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
@@ -19,4 +23,8 @@ int cmdline_poll_char(struct cmdline *cl);
 /* Read one character from input. */
 ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 
+/* vdprintf(3) */
+__rte_format_printf(2, 0)
+int cmdline_vdprintf(int fd, const char *format, va_list op);
+
 #endif
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 5/7] eal/windows: improve compatibility networking headers
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (3 preceding siblings ...)
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (2 subsequent siblings)
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam

Extend compatibility header system to support librte_cmdline.

pthread.h has to include windows.h, which exposes struct in_addr, etc.
conflicting with compatibility headers. WIN32_LEAN_AND_MEAN macro
is required to disable this behavior. Use rte_windows.h to define
WIN32_LEAN_AND_MEAN for pthread library.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/windows/include/arpa/inet.h  | 30 +++++++++++++++++++++
 lib/librte_eal/windows/include/netinet/in.h | 12 +++++++++
 lib/librte_eal/windows/include/sys/socket.h | 24 +++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 create mode 100644 lib/librte_eal/windows/include/sys/socket.h

diff --git a/lib/librte_eal/windows/include/arpa/inet.h b/lib/librte_eal/windows/include/arpa/inet.h
new file mode 100644
index 000000000..96b698438
--- /dev/null
+++ b/lib/librte_eal/windows/include/arpa/inet.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _ARPA_INET_H_
+#define _ARPA_INET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+int
+inet_pton(int af, const char *src, void *dst);
+
+/* defined in ws2_32.dll */
+__attribute__((stdcall))
+const char *
+inet_ntop(int af, const void *src, char *dst, socklen_t size);
+
+#endif /* _ARPA_INET_H_ */
diff --git a/lib/librte_eal/windows/include/netinet/in.h b/lib/librte_eal/windows/include/netinet/in.h
index 534a2d99a..be1469ec1 100644
--- a/lib/librte_eal/windows/include/netinet/in.h
+++ b/lib/librte_eal/windows/include/netinet/in.h
@@ -5,6 +5,8 @@
 #ifndef _IN_H_
 #define _IN_H_
 
+#include <sys/socket.h>
+
 #define IPPROTO_IP         0
 #define IPPROTO_HOPOPTS    0
 #define IPPROTO_ICMP       1
@@ -22,4 +24,14 @@
 #define IPPROTO_DSTOPTS   60
 #define IPPROTO_SCTP     132
 
+#define INET6_ADDRSTRLEN 46
+
+struct in_addr {
+	uint32_t s_addr;
+};
+
+struct in6_addr {
+	uint8_t s6_addr[16];
+};
+
 #endif
diff --git a/lib/librte_eal/windows/include/sys/socket.h b/lib/librte_eal/windows/include/sys/socket.h
new file mode 100644
index 000000000..9536cf8e6
--- /dev/null
+++ b/lib/librte_eal/windows/include/sys/socket.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#ifndef _SYS_SOCKET_H_
+#define _SYS_SOCKET_H_
+
+/**
+ * @file
+ *
+ * Compatibility header
+ *
+ * Although symbols declared here are present on Windows,
+ * including <winsock2.h> would expose too much macros breaking common code.
+ */
+
+#include <stddef.h>
+
+#define AF_INET  2
+#define AF_INET6 23
+
+typedef size_t socklen_t;
+
+#endif /* _SYS_SOCKET_H_ */
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (4 preceding siblings ...)
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-10-14 22:31       ` Thomas Monjalon
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
  2020-10-05 15:33     ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Olivier Matz
  7 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Bruce Richardson, Olivier Matz

Implement terminal handling, input polling, and vdprintf() for Windows.

Because Windows I/O model differs fundamentally from Unix and there is
no concept of character device, polling is simulated depending on the
underlying input device. Supporting non-terminal input is useful for
automated testing.

Windows emulation of VT100 uses "ESC [ E" for newline instead of
standard "ESC E", so add a workaround.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build                      |   2 +
 lib/librte_cmdline/cmdline.c            |   5 +
 lib/librte_cmdline/cmdline_os_windows.c | 207 ++++++++++++++++++++++++
 lib/librte_cmdline/cmdline_parse.c      |   2 +-
 lib/librte_cmdline/cmdline_private.h    |  23 +++
 lib/librte_cmdline/cmdline_socket.c     |   4 +
 lib/librte_cmdline/cmdline_vt100.h      |   4 +
 lib/librte_cmdline/meson.build          |   4 +-
 lib/meson.build                         |   1 +
 9 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_cmdline/cmdline_os_windows.c

diff --git a/config/meson.build b/config/meson.build
index 69f2aeb60..bd382bd7b 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -276,6 +276,8 @@ if is_windows
 		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
 	endif
 
+	add_project_link_arguments('-lws2_32', language: 'c')
+
 	# Contrary to docs, VirtualAlloc2() is exported by mincore.lib
 	# in Windows SDK, while MinGW exports it by advapi32.a.
 	if is_ms_linker
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 49770869b..dd6229b56 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -13,11 +13,16 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <netinet/in.h>
+#include <unistd.h>
 
 #include <rte_string_fns.h>
 
 #include "cmdline_private.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define write _write
+#endif
+
 static void
 cmdline_valid_buffer(struct rdline *rdl, const char *buf,
 		     __rte_unused unsigned int size)
diff --git a/lib/librte_cmdline/cmdline_os_windows.c b/lib/librte_cmdline/cmdline_os_windows.c
new file mode 100644
index 000000000..e9585c9ee
--- /dev/null
+++ b/lib/librte_cmdline/cmdline_os_windows.c
@@ -0,0 +1,207 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Dmitry Kozlyuk
+ */
+
+#include <io.h>
+
+#include <rte_os.h>
+
+#include "cmdline_private.h"
+
+/* Missing from some MinGW-w64 distributions. */
+#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING
+#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004
+#endif
+
+#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT
+#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200
+#endif
+
+void
+terminal_adjust(struct cmdline *cl)
+{
+	HANDLE handle;
+	DWORD mode;
+
+	ZeroMemory(&cl->oldterm, sizeof(cl->oldterm));
+
+	/* Detect console input, set it up and make it emulate VT100. */
+	handle = GetStdHandle(STD_INPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		cl->oldterm.is_console_input = 1;
+		cl->oldterm.input_mode = mode;
+
+		mode &= ~(
+			ENABLE_LINE_INPUT |      /* no line buffering */
+			ENABLE_ECHO_INPUT |      /* no echo */
+			ENABLE_PROCESSED_INPUT | /* pass Ctrl+C to program */
+			ENABLE_MOUSE_INPUT |     /* no mouse events */
+			ENABLE_WINDOW_INPUT);    /* no window resize events */
+		mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+		SetConsoleMode(handle, mode);
+	}
+
+	/* Detect console output and make it emulate VT100. */
+	handle = GetStdHandle(STD_OUTPUT_HANDLE);
+	if (GetConsoleMode(handle, &mode)) {
+		cl->oldterm.is_console_output = 1;
+		cl->oldterm.output_mode = mode;
+
+		mode &= ~ENABLE_WRAP_AT_EOL_OUTPUT;
+		mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+		SetConsoleMode(handle, mode);
+	}
+}
+
+void
+terminal_restore(const struct cmdline *cl)
+{
+	if (cl->oldterm.is_console_input) {
+		HANDLE handle = GetStdHandle(STD_INPUT_HANDLE);
+		SetConsoleMode(handle, cl->oldterm.input_mode);
+	}
+
+	if (cl->oldterm.is_console_output) {
+		HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);
+		SetConsoleMode(handle, cl->oldterm.output_mode);
+	}
+}
+
+static int
+cmdline_is_key_down(const INPUT_RECORD *record)
+{
+	return (record->EventType == KEY_EVENT) &&
+		record->Event.KeyEvent.bKeyDown;
+}
+
+static int
+cmdline_poll_char_console(HANDLE handle)
+{
+	INPUT_RECORD record;
+	DWORD events;
+
+	if (!PeekConsoleInput(handle, &record, 1, &events)) {
+		/* Simulate poll(3) behavior on EOF. */
+		return (GetLastError() == ERROR_HANDLE_EOF) ? 1 : -1;
+	}
+
+	if ((events == 0) || !cmdline_is_key_down(&record))
+		return 0;
+
+	return 1;
+}
+
+static int
+cmdline_poll_char_file(struct cmdline *cl, HANDLE handle)
+{
+	DWORD type = GetFileType(handle);
+
+	/* Since console is handled by cmdline_poll_char_console(),
+	 * this is either a serial port or input handle had been replaced.
+	 */
+	if (type == FILE_TYPE_CHAR)
+		return cmdline_poll_char_console(handle);
+
+	/* PeekNamedPipe() can handle all pipes and also sockets. */
+	if (type == FILE_TYPE_PIPE) {
+		DWORD bytes_avail;
+		if (!PeekNamedPipe(handle, NULL, 0, NULL, &bytes_avail, NULL))
+			return (GetLastError() == ERROR_BROKEN_PIPE) ? 1 : -1;
+		return bytes_avail ? 1 : 0;
+	}
+
+	/* There is no straightforward way to peek a file in Windows
+	 * I/O model. Read the byte, if it is not the end of file,
+	 * buffer it for subsequent read. This will not work with
+	 * a file being appended and probably some other edge cases.
+	 */
+	if (type == FILE_TYPE_DISK) {
+		char c;
+		int ret;
+
+		ret = _read(cl->s_in, &c, sizeof(c));
+		if (ret == 1) {
+			cl->repeat_count = 1;
+			cl->repeated_char = c;
+		}
+		return ret;
+	}
+
+	/* GetFileType() failed or file of unknown type,
+	 * which we do not know how to peek anyway.
+	 */
+	return -1;
+}
+
+int
+cmdline_poll_char(struct cmdline *cl)
+{
+	HANDLE handle = (HANDLE)_get_osfhandle(cl->s_in);
+	return cl->oldterm.is_console_input ?
+		cmdline_poll_char_console(handle) :
+		cmdline_poll_char_file(cl, handle);
+}
+
+ssize_t
+cmdline_read_char(struct cmdline *cl, char *c)
+{
+	HANDLE handle;
+	INPUT_RECORD record;
+	KEY_EVENT_RECORD *key;
+	DWORD events;
+
+	if (!cl->oldterm.is_console_input)
+		return _read(cl->s_in, c, 1);
+
+	/* Return repeated strokes from previous event. */
+	if (cl->repeat_count > 0) {
+		*c = cl->repeated_char;
+		cl->repeat_count--;
+		return 1;
+	}
+
+	handle = (HANDLE)_get_osfhandle(cl->s_in);
+	key = &record.Event.KeyEvent;
+	do {
+		if (!ReadConsoleInput(handle, &record, 1, &events)) {
+			if (GetLastError() == ERROR_HANDLE_EOF) {
+				*c = EOF;
+				return 0;
+			}
+			return -1;
+		}
+	} while (!cmdline_is_key_down(&record));
+
+	*c = key->uChar.AsciiChar;
+
+	/* Save repeated strokes from a single event. */
+	if (key->wRepeatCount > 1) {
+		cl->repeated_char = *c;
+		cl->repeat_count = key->wRepeatCount - 1;
+	}
+
+	return 1;
+}
+
+int
+cmdline_vdprintf(int fd, const char *format, va_list op)
+{
+	int copy, ret;
+	FILE *file;
+
+	copy = _dup(fd);
+	if (copy < 0)
+		return -1;
+
+	file = _fdopen(copy, "a");
+	if (file == NULL) {
+		_close(copy);
+		return -1;
+	}
+
+	ret = vfprintf(file, format, op);
+
+	fclose(file); /* also closes copy */
+
+	return ret;
+}
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index f120f19dd..fe366841c 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -15,7 +15,7 @@
 
 #include <rte_string_fns.h>
 
-#include "cmdline.h"
+#include "cmdline_private.h"
 
 #ifdef RTE_LIBRTE_CMDLINE_DEBUG
 #define debug_printf printf
diff --git a/lib/librte_cmdline/cmdline_private.h b/lib/librte_cmdline/cmdline_private.h
index 4d9ea33f0..a8a6ee9e6 100644
--- a/lib/librte_cmdline/cmdline_private.h
+++ b/lib/librte_cmdline/cmdline_private.h
@@ -8,9 +8,32 @@
 #include <stdarg.h>
 
 #include <rte_common.h>
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <rte_windows.h>
+#endif
 
 #include <cmdline.h>
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+struct terminal {
+	DWORD input_mode;
+	DWORD output_mode;
+	int is_console_input;
+	int is_console_output;
+};
+
+struct cmdline {
+	int s_in;
+	int s_out;
+	cmdline_parse_ctx_t *ctx;
+	struct rdline rdl;
+	char prompt[RDLINE_PROMPT_SIZE];
+	struct terminal oldterm;
+	char repeated_char;
+	WORD repeat_count;
+};
+#endif
+
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
 
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index 998e8ade2..0fe149700 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -16,6 +16,10 @@
 #include "cmdline_private.h"
 #include "cmdline_socket.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#define open _open
+#endif
+
 struct cmdline *
 cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
 {
diff --git a/lib/librte_cmdline/cmdline_vt100.h b/lib/librte_cmdline/cmdline_vt100.h
index e33e67ed8..be9ae8e1c 100644
--- a/lib/librte_cmdline/cmdline_vt100.h
+++ b/lib/librte_cmdline/cmdline_vt100.h
@@ -31,7 +31,11 @@ extern "C" {
 #define vt100_multi_right  "\033\133%uC"
 #define vt100_multi_left   "\033\133%uD"
 #define vt100_suppr        "\033\133\063\176"
+#ifndef RTE_EXEC_ENV_WINDOWS
 #define vt100_home         "\033M\033E"
+#else
+#define vt100_home         "\033M\033[E"
+#endif
 #define vt100_word_left    "\033\142"
 #define vt100_word_right   "\033\146"
 
diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
index 5c9e8886d..5009b3354 100644
--- a/lib/librte_cmdline/meson.build
+++ b/lib/librte_cmdline/meson.build
@@ -25,7 +25,9 @@ headers = files('cmdline.h',
 	'cmdline_cirbuf.h',
 	'cmdline_parse_portlist.h')
 
-if not is_windows
+if is_windows
+	sources += files('cmdline_os_windows.c')
+else
 	sources += files('cmdline_os_unix.c')
 endif
 
diff --git a/lib/meson.build b/lib/meson.build
index d8b358e5f..ed66590a7 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -42,6 +42,7 @@ if is_windows
 		'eal',
 		'ring',
 		'mempool', 'mbuf', 'net', 'meter', 'ethdev', 'pci',
+		'cmdline',
 	] # only supported libraries for windows
 endif
 
-- 
2.28.0


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

* [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (5 preceding siblings ...)
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-09-28 21:50     ` Dmitry Kozlyuk
  2020-10-14 22:33       ` Thomas Monjalon
  2020-10-05 15:33     ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Olivier Matz
  7 siblings, 1 reply; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-28 21:50 UTC (permalink / raw)
  To: dev
  Cc: Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Dmitry Kozlyuk, Olivier Matz

Enable cmdline sample application as all dependencies are met.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 examples/cmdline/commands.c | 1 -
 examples/cmdline/main.c     | 1 -
 examples/meson.build        | 6 +++---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index 0e2232f03..f43eacfba 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -11,7 +11,6 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <netinet/in.h>
-#include <termios.h>
 #ifdef RTE_EXEC_ENV_FREEBSD
 #include <sys/socket.h>
 #endif
diff --git a/examples/cmdline/main.c b/examples/cmdline/main.c
index f2f2e5a2f..bb7954245 100644
--- a/examples/cmdline/main.c
+++ b/examples/cmdline/main.c
@@ -8,7 +8,6 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
-#include <termios.h>
 #include <sys/queue.h>
 
 #include <cmdline_rdline.h>
diff --git a/examples/meson.build b/examples/meson.build
index eb13e8210..2b8ebce3b 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -79,9 +79,9 @@ foreach example: examples
 
 	ext_deps = [execinfo]
 	includes = [include_directories(example)]
-	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
-	if is_windows
-		deps = ['eal'] # only supported lib on Windows currently
+	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
+	if not is_windows
+		deps += ['ethdev'] # not currently supported
 	endif
 	subdir(example)
 
-- 
2.28.0


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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-08-05 11:17       ` Dmitry Kozlyuk
@ 2020-09-30  8:11         ` Kinsella, Ray
  2020-09-30 15:26           ` Dmitry Kozlyuk
  0 siblings, 1 reply; 50+ messages in thread
From: Kinsella, Ray @ 2020-09-30  8:11 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Olivier Matz, Neil Horman



On 05/08/2020 12:17, Dmitry Kozlyuk wrote:
> On Wed, 5 Aug 2020 10:31:31 +0100, Kinsella, Ray wrote:
>> On 30/07/2020 22:06, Dmitry Kozlyuk wrote:
>>> struct cmdline exposes platform-specific members it contains, most
>>> notably struct termios that is only available on Unix. Make the
>>> structure opaque.
>>>
>>> Remove tests checking struct cmdline content as meaningless.
>>>
>>> Add cmdline_get_rdline() to access history buffer.
>>> The new function is currently used only in tests.  
>>
>> Should it be INTERNAL then? Is it useful outside of the test cases?
> 
> There are already exposed rdline_*() functions that require struct rdline
> pointer, which is now only accessible via this function for struct cmdline
> instances. Thus, public API would be broken with INTERNAL for such use cases.
> 

Right but that runs a bit contrary to what you said in the commit log.

"Add cmdline_get_rdline() to access history buffer.
The new function is currently used only in tests."

In anycase, given the elapse in time since my feedback.
I will ACK the changes to MAP file. 

Ray K

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

* Re: [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
@ 2020-09-30  8:12       ` Kinsella, Ray
  0 siblings, 0 replies; 50+ messages in thread
From: Kinsella, Ray @ 2020-09-30  8:12 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: Khoa To, Stephen Hemminger, Ferruh Yigit, Olivier Matz, Neil Horman



On 28/09/2020 22:50, Dmitry Kozlyuk wrote:
> struct cmdline exposes platform-specific members it contains, most
> notably struct termios that is only available on Unix. While ABI
> considerations prevent from hinding the definition on already supported
> platforms, struct cmdline is considered logically opaque from now on.
> Add a deprecation notice targeted at 20.11.
> 
> * Remove tests checking struct cmdline content as meaningless.
> 
> * Fix missing cmdline_free() in unit test.
> 
> * Add cmdline_get_rdline() to access history buffer indirectly.
>   The new function is currently used only in tests.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Suggested-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  app/test-cmdline/commands.c                |  8 +++-
>  app/test/test_cmdline_lib.c                | 44 +++++++++-------------
>  doc/guides/rel_notes/deprecation.rst       |  4 ++
>  lib/librte_cmdline/cmdline.c               |  9 +++--
>  lib/librte_cmdline/cmdline.h               | 18 ++++++++-
>  lib/librte_cmdline/cmdline_parse.c         |  3 --
>  lib/librte_cmdline/cmdline_socket.c        |  5 +--
>  lib/librte_cmdline/rte_cmdline_version.map |  8 ++++
>  8 files changed, 60 insertions(+), 39 deletions(-)

Acked-by: Ray Kinsella <mdr@ashroe.eu>

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

* Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque
  2020-09-30  8:11         ` Kinsella, Ray
@ 2020-09-30 15:26           ` Dmitry Kozlyuk
  0 siblings, 0 replies; 50+ messages in thread
From: Dmitry Kozlyuk @ 2020-09-30 15:26 UTC (permalink / raw)
  To: Kinsella, Ray
  Cc: dev, Dmitry Malloy, Narcisa Ana Maria Vasile, Fady Bader,
	Tal Shnaiderman, Kadam, Pallavi, Olivier Matz, Neil Horman

On Wed, 30 Sep 2020 09:11:41 +0100, Kinsella, Ray wrote:
> On 05/08/2020 12:17, Dmitry Kozlyuk wrote:
> > On Wed, 5 Aug 2020 10:31:31 +0100, Kinsella, Ray wrote:  
> >> On 30/07/2020 22:06, Dmitry Kozlyuk wrote:  
> >>> struct cmdline exposes platform-specific members it contains, most
> >>> notably struct termios that is only available on Unix. Make the
> >>> structure opaque.
> >>>
> >>> Remove tests checking struct cmdline content as meaningless.
> >>>
> >>> Add cmdline_get_rdline() to access history buffer.
> >>> The new function is currently used only in tests.    
> >>
> >> Should it be INTERNAL then? Is it useful outside of the test cases?  
> > 
> > There are already exposed rdline_*() functions that require struct rdline
> > pointer, which is now only accessible via this function for struct cmdline
> > instances. Thus, public API would be broken with INTERNAL for such use cases.
> >   
> 
> Right but that runs a bit contrary to what you said in the commit log.
> 
> "Add cmdline_get_rdline() to access history buffer.
> The new function is currently used only in tests."
> 
> In anycase, given the elapse in time since my feedback.
> I will ACK the changes to MAP file. 

I rather meant logical breakage that technical one: no functions are removed,
yes, but if existing applications use cmdline::rdline, they won't have a
legitimate way of getting it anymore. The comment you're quoting was before
the workaround, but the WA exists only to keep ABI and we'd prefer the users
to remove direct accesses of the structure fields by 21.11. To do that, we
give them cmdline_get_rdline() and make it public, not internal.

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

* Re: [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows
  2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
                       ` (6 preceding siblings ...)
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
@ 2020-10-05 15:33     ` Olivier Matz
  2020-10-14 22:41       ` Thomas Monjalon
  7 siblings, 1 reply; 50+ messages in thread
From: Olivier Matz @ 2020-10-05 15:33 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit

Hi Dmitry,

On Tue, Sep 29, 2020 at 12:50:45AM +0300, Dmitry Kozlyuk wrote:
> This patchset enables librte_cmdline on Windows. To do that, it creates
> a number of wrappers for OS-dependent terminal handling and I/O.
> Considered alternative was to revive [1] and use libedit (Unix-only)
> for terminal handling. However, testing revealed that WinEditLine [2]
> is not a drop-in replacement for libedit, so this solution wouldn't be
> universal.
> 
> [1]: http://patchwork.dpdk.org/patch/38561
> [2]: http://mingweditline.sourceforge.net
> 
> v3:
>     * Add #ifdef workaround to keep API/ABI for Unices (Olivier).
>     * Fix missing cmdline_free() in test (Olivier).
>     * Rebase on ToT (Khoa).
> 
> Dmitry Kozlyuk (7):
>   cmdline: make implementation logically opaque
>   cmdline: add internal wrappers for terminal handling
>   cmdline: add internal wrappers for character input
>   cmdline: add internal wrapper for vdprintf
>   eal/windows: improve compatibility networking headers
>   cmdline: support Windows
>   examples/cmdline: build on Windows
> 
>  app/test-cmdline/commands.c                 |   8 +-
>  app/test/test_cmdline_lib.c                 |  44 ++---
>  config/meson.build                          |   2 +
>  doc/guides/rel_notes/deprecation.rst        |   4 +
>  examples/cmdline/commands.c                 |   1 -
>  examples/cmdline/main.c                     |   1 -
>  examples/meson.build                        |   6 +-
>  lib/librte_cmdline/cmdline.c                |  30 +--
>  lib/librte_cmdline/cmdline.h                |  18 +-
>  lib/librte_cmdline/cmdline_os_unix.c        |  53 +++++
>  lib/librte_cmdline/cmdline_os_windows.c     | 207 ++++++++++++++++++++
>  lib/librte_cmdline/cmdline_parse.c          |   5 +-
>  lib/librte_cmdline/cmdline_private.h        |  53 +++++
>  lib/librte_cmdline/cmdline_socket.c         |  25 +--
>  lib/librte_cmdline/cmdline_vt100.c          |   1 -
>  lib/librte_cmdline/cmdline_vt100.h          |   4 +
>  lib/librte_cmdline/meson.build              |   6 +
>  lib/librte_cmdline/rte_cmdline_version.map  |   8 +
>  lib/librte_eal/windows/include/arpa/inet.h  |  30 +++
>  lib/librte_eal/windows/include/netinet/in.h |  12 ++
>  lib/librte_eal/windows/include/sys/socket.h |  24 +++
>  lib/meson.build                             |   1 +
>  22 files changed, 475 insertions(+), 68 deletions(-)
>  create mode 100644 lib/librte_cmdline/cmdline_os_unix.c
>  create mode 100644 lib/librte_cmdline/cmdline_os_windows.c
>  create mode 100644 lib/librte_cmdline/cmdline_private.h
>  create mode 100644 lib/librte_eal/windows/include/arpa/inet.h
>  create mode 100644 lib/librte_eal/windows/include/sys/socket.h

For series:
Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks!

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

* Re: [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows Dmitry Kozlyuk
@ 2020-10-14 22:31       ` Thomas Monjalon
  0 siblings, 0 replies; 50+ messages in thread
From: Thomas Monjalon @ 2020-10-14 22:31 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Bruce Richardson, Olivier Matz

28/09/2020 23:50, Dmitry Kozlyuk:
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -13,11 +13,16 @@
>  #include <fcntl.h>
>  #include <errno.h>
>  #include <netinet/in.h>
> +#include <unistd.h>

unistd is already included few lines above.




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

* Re: [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows
  2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
@ 2020-10-14 22:33       ` Thomas Monjalon
  0 siblings, 0 replies; 50+ messages in thread
From: Thomas Monjalon @ 2020-10-14 22:33 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Olivier Matz

28/09/2020 23:50, Dmitry Kozlyuk:
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -79,9 +79,9 @@ foreach example: examples
>  
>  	ext_deps = [execinfo]
>  	includes = [include_directories(example)]
> -	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
> -	if is_windows
> -		deps = ['eal'] # only supported lib on Windows currently
> +	deps = ['eal', 'mempool', 'net', 'mbuf', 'cmdline']
> +	if not is_windows
> +		deps += ['ethdev'] # not currently supported
>  	endif

ethdev is supported on Windows now.



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

* Re: [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows
  2020-10-05 15:33     ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Olivier Matz
@ 2020-10-14 22:41       ` Thomas Monjalon
  0 siblings, 0 replies; 50+ messages in thread
From: Thomas Monjalon @ 2020-10-14 22:41 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Kinsella, Ray, Khoa To, Stephen Hemminger, Ferruh Yigit,
	Olivier Matz

> > Dmitry Kozlyuk (7):
> >   cmdline: make implementation logically opaque
> >   cmdline: add internal wrappers for terminal handling
> >   cmdline: add internal wrappers for character input
> >   cmdline: add internal wrapper for vdprintf
> >   eal/windows: improve compatibility networking headers
> >   cmdline: support Windows
> >   examples/cmdline: build on Windows
> 
> For series:
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied with few changes described in the thread, thanks.





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

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

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] cmdline: support Windows Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-06-28 14:20   ` Fady Bader
2020-06-29  6:23     ` Ranjit Menon
2020-06-29  7:42       ` Dmitry Kozlyuk
2020-06-29  8:12         ` Tal Shnaiderman
2020-06-29 23:56           ` Dmitry Kozlyuk
2020-07-08  1:09             ` Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-07-17 22:16 ` [dpdk-dev] [PATCH 0/7] cmdline: support Windows Narcisa Ana Maria Vasile
2020-07-30 18:08 ` Kadam, Pallavi
2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
2020-08-05  9:31     ` Kinsella, Ray
2020-08-05 11:17       ` Dmitry Kozlyuk
2020-09-30  8:11         ` Kinsella, Ray
2020-09-30 15:26           ` Dmitry Kozlyuk
2020-09-17 13:34     ` Olivier Matz
2020-09-17 17:05       ` Stephen Hemminger
2020-09-18  8:33         ` Bruce Richardson
2020-09-18 12:13           ` Ferruh Yigit
2020-09-18 13:23         ` Kinsella, Ray
2020-09-17 23:13       ` Dmitry Kozlyuk
2020-09-18 13:31       ` Kinsella, Ray
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-09-26  1:33     ` [dpdk-dev] [EXTERNAL] " Narcisa Ana Maria Vasile
2020-09-26  6:03       ` Khoa To
2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
2020-09-30  8:12       ` Kinsella, Ray
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-10-14 22:31       ` Thomas Monjalon
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-10-14 22:33       ` Thomas Monjalon
2020-10-05 15:33     ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Olivier Matz
2020-10-14 22:41       ` 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).