[dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Extend guest channel API to allow bidirectional
communication. Modify power manager host and guest
side to communicate in both directions.

---

v5:
* enhance logging

v4:
* [vm_power_manager] treat 0 as valid socket id
* [guest_manager] use user level logs
* correct code formatting

v3:
* fix global_fds[lcore_id] comparison to invalid value
* check 0 to verify if read function actually read some data
* define _NACK cmd instead of _NAK
* simplify rte_power_guest_channel_receive_msg func logic

v2:
* send ack only if power operation return positive value
* log diffent error for unexpected incoming command and
  error during ack/nak cmd sending


Marcin Hajkowski (4):
  power: fix invalid socket indicator value
  power: extend guest channel API for reading
  power: process incoming confirmation cmds
  power: send confirmation cmd to vm guest

 examples/vm_power_manager/channel_monitor.c   | 68 +++++++++++++++--
 examples/vm_power_manager/guest_cli/Makefile  |  1 +
 .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
 lib/librte_power/channel_commands.h           |  5 ++
 lib/librte_power/guest_channel.c              | 72 ++++++++++++++++--
 lib/librte_power/guest_channel.h              | 35 +++++++++
 lib/librte_power/rte_power_version.map        |  1 +
 7 files changed, 230 insertions(+), 25 deletions(-)

-- 
2.17.2

[dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Extend guest channel API to allow bidirectional
communication. Modify power manager host and guest
side to communicate in both directions.

---

v5:
* enhance logging

v4:
* [vm_power_manager] treat 0 as valid socket id
* [guest_manager] use user level logs
* correct code formatting

v3:
* fix global_fds[lcore_id] comparison to invalid value
* check 0 to verify if read function actually read some data
* define _NACK cmd instead of _NAK
* simplify rte_power_guest_channel_receive_msg func logic

v2:
* send ack only if power operation return positive value
* log diffent error for unexpected incoming command and
  error during ack/nak cmd sending


Marcin Hajkowski (4):
  power: fix invalid socket indicator value
  power: extend guest channel API for reading
  power: process incoming confirmation cmds
  power: send confirmation cmd to vm guest

 examples/vm_power_manager/channel_monitor.c   | 68 +++++++++++++++--
 examples/vm_power_manager/guest_cli/Makefile  |  1 +
 .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
 lib/librte_power/channel_commands.h           |  5 ++
 lib/librte_power/guest_channel.c              | 72 ++++++++++++++++--
 lib/librte_power/guest_channel.h              | 35 +++++++++
 lib/librte_power/rte_power_version.map        |  1 +
 7 files changed, 230 insertions(+), 25 deletions(-)

-- 
2.17.2

[dpdk-dev] [PATCH v5 1/4] power: fix invalid socket indicator value

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Currently 0 is being used for not connected slot indication.
This is not consistent with linux doc which identifies 0 as valid
(connected) slot, thus modification was done to change it.

Fixes: cd0d5547 ("power: vm communication channels in guest")
Cc: stable@dpdk.org

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_power/guest_channel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index c17ea46b4..9cf7d2cb2 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -19,7 +19,7 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
-static int global_fds[RTE_MAX_LCORE];
+static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
 guest_channel_host_connect(const char *path, unsigned int lcore_id)
@@ -35,7 +35,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 		return -1;
 	}
 	/* check if path is already open */
-	if (global_fds[lcore_id] != 0) {
+	if (global_fds[lcore_id] != -1) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is already open with fd %d\n",
 				lcore_id, global_fds[lcore_id]);
 		return -1;
@@ -84,7 +84,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 	return 0;
 error:
 	close(fd);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 	return -1;
 }
 
@@ -100,7 +100,7 @@ guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id)
 		return -1;
 	}
 
-	if (global_fds[lcore_id] == 0) {
+	if (global_fds[lcore_id] < 0) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
 		return -1;
 	}
@@ -134,8 +134,8 @@ guest_channel_host_disconnect(unsigned int lcore_id)
 				lcore_id, RTE_MAX_LCORE-1);
 		return;
 	}
-	if (global_fds[lcore_id] == 0)
+	if (global_fds[lcore_id] < 0)
 		return;
 	close(global_fds[lcore_id]);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 }
-- 
2.17.2

[dpdk-dev] [PATCH v5 1/4] power: fix invalid socket indicator value

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Currently 0 is being used for not connected slot indication.
This is not consistent with linux doc which identifies 0 as valid
(connected) slot, thus modification was done to change it.

Fixes: cd0d5547 ("power: vm communication channels in guest")
Cc: stable@dpdk.org

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_power/guest_channel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index c17ea46b4..9cf7d2cb2 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -19,7 +19,7 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
-static int global_fds[RTE_MAX_LCORE];
+static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
 guest_channel_host_connect(const char *path, unsigned int lcore_id)
@@ -35,7 +35,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 		return -1;
 	}
 	/* check if path is already open */
-	if (global_fds[lcore_id] != 0) {
+	if (global_fds[lcore_id] != -1) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is already open with fd %d\n",
 				lcore_id, global_fds[lcore_id]);
 		return -1;
@@ -84,7 +84,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 	return 0;
 error:
 	close(fd);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 	return -1;
 }
 
@@ -100,7 +100,7 @@ guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id)
 		return -1;
 	}
 
-	if (global_fds[lcore_id] == 0) {
+	if (global_fds[lcore_id] < 0) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
 		return -1;
 	}
@@ -134,8 +134,8 @@ guest_channel_host_disconnect(unsigned int lcore_id)
 				lcore_id, RTE_MAX_LCORE-1);
 		return;
 	}
-	if (global_fds[lcore_id] == 0)
+	if (global_fds[lcore_id] < 0)
 		return;
 	close(global_fds[lcore_id]);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 }
-- 
2.17.2

[dpdk-dev] [PATCH v5 2/4] power: extend guest channel API for reading

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Added new experimental API rte_power_guest_channel_receive_msg
which gives possibility to receive messages send to guest.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 lib/librte_power/channel_commands.h    |  5 +++
 lib/librte_power/guest_channel.c       | 60 ++++++++++++++++++++++++++
 lib/librte_power/guest_channel.h       | 35 +++++++++++++++
 lib/librte_power/rte_power_version.map |  1 +
 4 files changed, 101 insertions(+)

diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h
index e7b93a797..33fd53a6d 100644
--- a/lib/librte_power/channel_commands.h
+++ b/lib/librte_power/channel_commands.h
@@ -28,6 +28,11 @@ extern "C" {
 #define CPU_POWER_SCALE_MIN     4
 #define CPU_POWER_ENABLE_TURBO  5
 #define CPU_POWER_DISABLE_TURBO 6
+
+/* Generic Power Command Response */
+#define CPU_POWER_CMD_ACK       1
+#define CPU_POWER_CMD_NACK      2
+
 #define HOURS 24
 
 #define MAX_VFS 10
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 9cf7d2cb2..888423891 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -10,6 +10,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <errno.h>
+#include <poll.h>
 
 
 #include <rte_log.h>
@@ -19,6 +20,9 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
+/* Timeout for incoming message in milliseconds. */
+#define TIMEOUT 10
+
 static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
@@ -125,6 +129,62 @@ int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 	return guest_channel_send_msg(pkt, lcore_id);
 }
 
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	int ret;
+	struct pollfd fds;
+	void *buffer = pkt;
+	int buffer_len = sizeof(*pkt);
+
+	fds.fd = global_fds[lcore_id];
+	fds.events = POLLIN;
+
+	ret = poll(&fds, 1, TIMEOUT);
+	if (ret == 0) {
+		RTE_LOG(DEBUG, GUEST_CHANNEL, "Timeout occurred during poll function.\n");
+		return -1;
+	} else if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Error occurred during poll function: %s\n",
+				strerror(ret));
+		return -1;
+	}
+
+	if (lcore_id >= RTE_MAX_LCORE) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
+				lcore_id, RTE_MAX_LCORE-1);
+		return -1;
+	}
+
+	if (global_fds[lcore_id] < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = read(global_fds[lcore_id],
+				buffer, buffer_len);
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -1;
+		}
+		if (ret == 0) {
+			RTE_LOG(ERR, GUEST_CHANNEL, "Expected more data, but connection has been closed.\n");
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+
+	return 0;
+}
+
+int rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	return power_guest_channel_read_msg(pkt, lcore_id);
+}
 
 void
 guest_channel_host_disconnect(unsigned int lcore_id)
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index 373d39898..7c385df39 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -68,6 +68,41 @@ int guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id);
 int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 			unsigned int lcore_id);
 
+/**
+ * Read a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+		unsigned int lcore_id);
+
+/**
+ * Receive a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+int __rte_experimental
+rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_power/rte_power_version.map b/lib/librte_power/rte_power_version.map
index 042917360..69f5ea3f4 100644
--- a/lib/librte_power/rte_power_version.map
+++ b/lib/librte_power/rte_power_version.map
@@ -44,4 +44,5 @@ EXPERIMENTAL {
 	rte_power_empty_poll_stat_update;
 	rte_power_poll_stat_fetch;
 	rte_power_poll_stat_update;
+	rte_power_guest_channel_receive_msg;
 };
-- 
2.17.2

[dpdk-dev] [PATCH v5 2/4] power: extend guest channel API for reading

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Added new experimental API rte_power_guest_channel_receive_msg
which gives possibility to receive messages send to guest.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 lib/librte_power/channel_commands.h    |  5 +++
 lib/librte_power/guest_channel.c       | 60 ++++++++++++++++++++++++++
 lib/librte_power/guest_channel.h       | 35 +++++++++++++++
 lib/librte_power/rte_power_version.map |  1 +
 4 files changed, 101 insertions(+)

diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h
index e7b93a797..33fd53a6d 100644
--- a/lib/librte_power/channel_commands.h
+++ b/lib/librte_power/channel_commands.h
@@ -28,6 +28,11 @@ extern "C" {
 #define CPU_POWER_SCALE_MIN     4
 #define CPU_POWER_ENABLE_TURBO  5
 #define CPU_POWER_DISABLE_TURBO 6
+
+/* Generic Power Command Response */
+#define CPU_POWER_CMD_ACK       1
+#define CPU_POWER_CMD_NACK      2
+
 #define HOURS 24
 
 #define MAX_VFS 10
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 9cf7d2cb2..888423891 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -10,6 +10,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <errno.h>
+#include <poll.h>
 
 
 #include <rte_log.h>
@@ -19,6 +20,9 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
+/* Timeout for incoming message in milliseconds. */
+#define TIMEOUT 10
+
 static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
@@ -125,6 +129,62 @@ int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 	return guest_channel_send_msg(pkt, lcore_id);
 }
 
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	int ret;
+	struct pollfd fds;
+	void *buffer = pkt;
+	int buffer_len = sizeof(*pkt);
+
+	fds.fd = global_fds[lcore_id];
+	fds.events = POLLIN;
+
+	ret = poll(&fds, 1, TIMEOUT);
+	if (ret == 0) {
+		RTE_LOG(DEBUG, GUEST_CHANNEL, "Timeout occurred during poll function.\n");
+		return -1;
+	} else if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Error occurred during poll function: %s\n",
+				strerror(ret));
+		return -1;
+	}
+
+	if (lcore_id >= RTE_MAX_LCORE) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
+				lcore_id, RTE_MAX_LCORE-1);
+		return -1;
+	}
+
+	if (global_fds[lcore_id] < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = read(global_fds[lcore_id],
+				buffer, buffer_len);
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -1;
+		}
+		if (ret == 0) {
+			RTE_LOG(ERR, GUEST_CHANNEL, "Expected more data, but connection has been closed.\n");
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+
+	return 0;
+}
+
+int rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	return power_guest_channel_read_msg(pkt, lcore_id);
+}
 
 void
 guest_channel_host_disconnect(unsigned int lcore_id)
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index 373d39898..7c385df39 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -68,6 +68,41 @@ int guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id);
 int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 			unsigned int lcore_id);
 
+/**
+ * Read a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+		unsigned int lcore_id);
+
+/**
+ * Receive a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+int __rte_experimental
+rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_power/rte_power_version.map b/lib/librte_power/rte_power_version.map
index 042917360..69f5ea3f4 100644
--- a/lib/librte_power/rte_power_version.map
+++ b/lib/librte_power/rte_power_version.map
@@ -44,4 +44,5 @@ EXPERIMENTAL {
 	rte_power_empty_poll_stat_update;
 	rte_power_poll_stat_fetch;
 	rte_power_poll_stat_update;
+	rte_power_guest_channel_receive_msg;
 };
-- 
2.17.2

[dpdk-dev] [PATCH v5 3/4] power: process incoming confirmation cmds

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Extend vm_power_guest to check incoming confirmations
of messages previously sent to host.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 examples/vm_power_manager/guest_cli/Makefile  |  1 +
 .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/examples/vm_power_manager/guest_cli/Makefile b/examples/vm_power_manager/guest_cli/Makefile
index e35a68d0f..67cf08193 100644
--- a/examples/vm_power_manager/guest_cli/Makefile
+++ b/examples/vm_power_manager/guest_cli/Makefile
@@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c
 
 CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 2d9e7689a..49ed7b208 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -27,7 +27,7 @@
 #define CHANNEL_PATH "/dev/virtio-ports/virtio.serial.port.poweragent"
 
 
-#define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
+#define RTE_LOGTYPE_GUEST_CLI RTE_LOGTYPE_USER1
 
 struct cmd_quit_result {
 	cmdline_fixed_string_t quit;
@@ -132,6 +132,32 @@ struct cmd_set_cpu_freq_result {
 	cmdline_fixed_string_t cmd;
 };
 
+static int
+check_response_cmd(unsigned int lcore_id, int *result)
+{
+	struct channel_packet pkt;
+	int ret;
+
+	ret = rte_power_guest_channel_receive_msg(&pkt, lcore_id);
+	if (ret < 0)
+		return -1;
+
+	switch (pkt.command) {
+	case(CPU_POWER_CMD_ACK):
+		*result = 1;
+		break;
+	case(CPU_POWER_CMD_NACK):
+		*result = 0;
+		break;
+	default:
+		RTE_LOG(ERR, GUEST_CLI,
+				"Received invalid response from host, expecting ACK/NACK.\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void
 cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 		       __attribute__((unused)) void *data)
@@ -139,20 +165,31 @@ cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 	int ret = -1;
 	struct cmd_set_cpu_freq_result *res = parsed_result;
 
-	if (!strcmp(res->cmd , "up"))
+	if (!strcmp(res->cmd, "up"))
 		ret = rte_power_freq_up(res->lcore_id);
-	else if (!strcmp(res->cmd , "down"))
+	else if (!strcmp(res->cmd, "down"))
 		ret = rte_power_freq_down(res->lcore_id);
-	else if (!strcmp(res->cmd , "min"))
+	else if (!strcmp(res->cmd, "min"))
 		ret = rte_power_freq_min(res->lcore_id);
-	else if (!strcmp(res->cmd , "max"))
+	else if (!strcmp(res->cmd, "max"))
 		ret = rte_power_freq_max(res->lcore_id);
 	else if (!strcmp(res->cmd, "enable_turbo"))
 		ret = rte_power_freq_enable_turbo(res->lcore_id);
 	else if (!strcmp(res->cmd, "disable_turbo"))
 		ret = rte_power_freq_disable_turbo(res->lcore_id);
-	if (ret != 1)
+
+	if (ret != 1) {
 		cmdline_printf(cl, "Error sending message: %s\n", strerror(ret));
+		return;
+	}
+	int result;
+	ret = check_response_cmd(res->lcore_id, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent message received\n");
+	} else {
+		cmdline_printf(cl, "%s received for message sent to host.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
 }
 
 cmdline_parse_token_string_t cmd_set_cpu_freq =
@@ -185,16 +222,26 @@ struct cmd_send_policy_result {
 };
 
 static inline int
-send_policy(struct channel_packet *pkt)
+send_policy(struct channel_packet *pkt, struct cmdline *cl)
 {
 	int ret;
 
 	ret = rte_power_guest_channel_send_msg(pkt, 1);
-	if (ret == 0)
-		return 1;
-	RTE_LOG(DEBUG, POWER, "Error sending message: %s\n",
-			ret > 0 ? strerror(ret) : "channel not connected");
-	return -1;
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "Error sending message: %s\n",
+				ret > 0 ? strerror(ret) : "channel not connected");
+		return -1;
+	}
+
+	int result;
+	ret = check_response_cmd(1, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent policy received\n");
+	} else {
+		cmdline_printf(cl, "%s for sent policy received.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
+	return 1;
 }
 
 static void
@@ -206,7 +253,7 @@ cmd_send_policy_parsed(void *parsed_result, struct cmdline *cl,
 
 	if (!strcmp(res->cmd, "now")) {
 		printf("Sending Policy down now!\n");
-		ret = send_policy(&policy);
+		ret = send_policy(&policy, cl);
 	}
 	if (ret != 1)
 		cmdline_printf(cl, "Error sending message: %s\n",
-- 
2.17.2

[dpdk-dev] [PATCH v5 3/4] power: process incoming confirmation cmds

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Extend vm_power_guest to check incoming confirmations
of messages previously sent to host.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 examples/vm_power_manager/guest_cli/Makefile  |  1 +
 .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/examples/vm_power_manager/guest_cli/Makefile b/examples/vm_power_manager/guest_cli/Makefile
index e35a68d0f..67cf08193 100644
--- a/examples/vm_power_manager/guest_cli/Makefile
+++ b/examples/vm_power_manager/guest_cli/Makefile
@@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c
 
 CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 2d9e7689a..49ed7b208 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -27,7 +27,7 @@
 #define CHANNEL_PATH "/dev/virtio-ports/virtio.serial.port.poweragent"
 
 
-#define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
+#define RTE_LOGTYPE_GUEST_CLI RTE_LOGTYPE_USER1
 
 struct cmd_quit_result {
 	cmdline_fixed_string_t quit;
@@ -132,6 +132,32 @@ struct cmd_set_cpu_freq_result {
 	cmdline_fixed_string_t cmd;
 };
 
+static int
+check_response_cmd(unsigned int lcore_id, int *result)
+{
+	struct channel_packet pkt;
+	int ret;
+
+	ret = rte_power_guest_channel_receive_msg(&pkt, lcore_id);
+	if (ret < 0)
+		return -1;
+
+	switch (pkt.command) {
+	case(CPU_POWER_CMD_ACK):
+		*result = 1;
+		break;
+	case(CPU_POWER_CMD_NACK):
+		*result = 0;
+		break;
+	default:
+		RTE_LOG(ERR, GUEST_CLI,
+				"Received invalid response from host, expecting ACK/NACK.\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void
 cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 		       __attribute__((unused)) void *data)
@@ -139,20 +165,31 @@ cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 	int ret = -1;
 	struct cmd_set_cpu_freq_result *res = parsed_result;
 
-	if (!strcmp(res->cmd , "up"))
+	if (!strcmp(res->cmd, "up"))
 		ret = rte_power_freq_up(res->lcore_id);
-	else if (!strcmp(res->cmd , "down"))
+	else if (!strcmp(res->cmd, "down"))
 		ret = rte_power_freq_down(res->lcore_id);
-	else if (!strcmp(res->cmd , "min"))
+	else if (!strcmp(res->cmd, "min"))
 		ret = rte_power_freq_min(res->lcore_id);
-	else if (!strcmp(res->cmd , "max"))
+	else if (!strcmp(res->cmd, "max"))
 		ret = rte_power_freq_max(res->lcore_id);
 	else if (!strcmp(res->cmd, "enable_turbo"))
 		ret = rte_power_freq_enable_turbo(res->lcore_id);
 	else if (!strcmp(res->cmd, "disable_turbo"))
 		ret = rte_power_freq_disable_turbo(res->lcore_id);
-	if (ret != 1)
+
+	if (ret != 1) {
 		cmdline_printf(cl, "Error sending message: %s\n", strerror(ret));
+		return;
+	}
+	int result;
+	ret = check_response_cmd(res->lcore_id, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent message received\n");
+	} else {
+		cmdline_printf(cl, "%s received for message sent to host.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
 }
 
 cmdline_parse_token_string_t cmd_set_cpu_freq =
@@ -185,16 +222,26 @@ struct cmd_send_policy_result {
 };
 
 static inline int
-send_policy(struct channel_packet *pkt)
+send_policy(struct channel_packet *pkt, struct cmdline *cl)
 {
 	int ret;
 
 	ret = rte_power_guest_channel_send_msg(pkt, 1);
-	if (ret == 0)
-		return 1;
-	RTE_LOG(DEBUG, POWER, "Error sending message: %s\n",
-			ret > 0 ? strerror(ret) : "channel not connected");
-	return -1;
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "Error sending message: %s\n",
+				ret > 0 ? strerror(ret) : "channel not connected");
+		return -1;
+	}
+
+	int result;
+	ret = check_response_cmd(1, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent policy received\n");
+	} else {
+		cmdline_printf(cl, "%s for sent policy received.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
+	return 1;
 }
 
 static void
@@ -206,7 +253,7 @@ cmd_send_policy_parsed(void *parsed_result, struct cmdline *cl,
 
 	if (!strcmp(res->cmd, "now")) {
 		printf("Sending Policy down now!\n");
-		ret = send_policy(&policy);
+		ret = send_policy(&policy, cl);
 	}
 	if (ret != 1)
 		cmdline_printf(cl, "Error sending message: %s\n",
-- 
2.17.2

[dpdk-dev] [PATCH v5 4/4] power: send confirmation cmd to vm guest

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Use new guest channel API to send confirmation
message for received power command.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 examples/vm_power_manager/channel_monitor.c | 68 +++++++++++++++++++--
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 7892d75de..ed580b36a 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -627,6 +627,41 @@ apply_policy(struct policy *pol)
 		apply_workload_profile(pol);
 }
 
+static int
+write_binary_packet(struct channel_packet *pkt, struct channel_info *chan_info)
+{
+	int ret, buffer_len = sizeof(*pkt);
+	void *buffer = pkt;
+
+	if (chan_info->fd < 0) {
+		RTE_LOG(ERR, CHANNEL_MONITOR, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = write(chan_info->fd, buffer, buffer_len);
+		if (ret == -1) {
+			if (errno == EINTR)
+				continue;
+			RTE_LOG(ERR, CHANNEL_MONITOR, "Write function failed due to %s.\n",
+					strerror(errno));
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+	return 0;
+}
+
+static int
+send_ack_for_received_cmd(struct channel_packet *pkt,
+		struct channel_info *chan_info,
+		uint32_t command)
+{
+	pkt->command = command;
+	return write_binary_packet(pkt, chan_info);
+}
+
 static int
 process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 {
@@ -650,33 +685,54 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 		RTE_LOG(DEBUG, CHANNEL_MONITOR, "Processing requested cmd for cpu:%d\n",
 			core_num);
 
+		bool valid_unit = true;
+		int scale_res;
+
 		switch (pkt->unit) {
 		case(CPU_POWER_SCALE_MIN):
-			power_manager_scale_core_min(core_num);
+			scale_res = power_manager_scale_core_min(core_num);
 			break;
 		case(CPU_POWER_SCALE_MAX):
-			power_manager_scale_core_max(core_num);
+			scale_res = power_manager_scale_core_max(core_num);
 			break;
 		case(CPU_POWER_SCALE_DOWN):
-			power_manager_scale_core_down(core_num);
+			scale_res = power_manager_scale_core_down(core_num);
 			break;
 		case(CPU_POWER_SCALE_UP):
-			power_manager_scale_core_up(core_num);
+			scale_res = power_manager_scale_core_up(core_num);
 			break;
 		case(CPU_POWER_ENABLE_TURBO):
-			power_manager_enable_turbo_core(core_num);
+			scale_res = power_manager_enable_turbo_core(core_num);
 			break;
 		case(CPU_POWER_DISABLE_TURBO):
-			power_manager_disable_turbo_core(core_num);
+			scale_res = power_manager_disable_turbo_core(core_num);
 			break;
 		default:
+			valid_unit = false;
 			break;
 		}
+
+		if (valid_unit) {
+			ret = send_ack_for_received_cmd(pkt,
+					chan_info,
+					scale_res > 0 ?
+						CPU_POWER_CMD_ACK :
+						CPU_POWER_CMD_NACK);
+			if (ret < 0)
+				RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
+		} else
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Unexpected unit type.\n");
+
 	}
 
 	if (pkt->command == PKT_POLICY) {
 		RTE_LOG(INFO, CHANNEL_MONITOR, "Processing policy request %s\n",
 				pkt->vm_name);
+		int ret = send_ack_for_received_cmd(pkt,
+				chan_info,
+				CPU_POWER_CMD_ACK);
+		if (ret < 0)
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
 		update_policy(pkt);
 		policy_is_set = 1;
 	}
-- 
2.17.2

[dpdk-dev] [PATCH v5 4/4] power: send confirmation cmd to vm guest

[Hajkowski]

From: Marcin Hajkowski <marcinx.hajkowski@intel.com>

Use new guest channel API to send confirmation
message for received power command.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
---
 examples/vm_power_manager/channel_monitor.c | 68 +++++++++++++++++++--
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 7892d75de..ed580b36a 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -627,6 +627,41 @@ apply_policy(struct policy *pol)
 		apply_workload_profile(pol);
 }
 
+static int
+write_binary_packet(struct channel_packet *pkt, struct channel_info *chan_info)
+{
+	int ret, buffer_len = sizeof(*pkt);
+	void *buffer = pkt;
+
+	if (chan_info->fd < 0) {
+		RTE_LOG(ERR, CHANNEL_MONITOR, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = write(chan_info->fd, buffer, buffer_len);
+		if (ret == -1) {
+			if (errno == EINTR)
+				continue;
+			RTE_LOG(ERR, CHANNEL_MONITOR, "Write function failed due to %s.\n",
+					strerror(errno));
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+	return 0;
+}
+
+static int
+send_ack_for_received_cmd(struct channel_packet *pkt,
+		struct channel_info *chan_info,
+		uint32_t command)
+{
+	pkt->command = command;
+	return write_binary_packet(pkt, chan_info);
+}
+
 static int
 process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 {
@@ -650,33 +685,54 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 		RTE_LOG(DEBUG, CHANNEL_MONITOR, "Processing requested cmd for cpu:%d\n",
 			core_num);
 
+		bool valid_unit = true;
+		int scale_res;
+
 		switch (pkt->unit) {
 		case(CPU_POWER_SCALE_MIN):
-			power_manager_scale_core_min(core_num);
+			scale_res = power_manager_scale_core_min(core_num);
 			break;
 		case(CPU_POWER_SCALE_MAX):
-			power_manager_scale_core_max(core_num);
+			scale_res = power_manager_scale_core_max(core_num);
 			break;
 		case(CPU_POWER_SCALE_DOWN):
-			power_manager_scale_core_down(core_num);
+			scale_res = power_manager_scale_core_down(core_num);
 			break;
 		case(CPU_POWER_SCALE_UP):
-			power_manager_scale_core_up(core_num);
+			scale_res = power_manager_scale_core_up(core_num);
 			break;
 		case(CPU_POWER_ENABLE_TURBO):
-			power_manager_enable_turbo_core(core_num);
+			scale_res = power_manager_enable_turbo_core(core_num);
 			break;
 		case(CPU_POWER_DISABLE_TURBO):
-			power_manager_disable_turbo_core(core_num);
+			scale_res = power_manager_disable_turbo_core(core_num);
 			break;
 		default:
+			valid_unit = false;
 			break;
 		}
+
+		if (valid_unit) {
+			ret = send_ack_for_received_cmd(pkt,
+					chan_info,
+					scale_res > 0 ?
+						CPU_POWER_CMD_ACK :
+						CPU_POWER_CMD_NACK);
+			if (ret < 0)
+				RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
+		} else
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Unexpected unit type.\n");
+
 	}
 
 	if (pkt->command == PKT_POLICY) {
 		RTE_LOG(INFO, CHANNEL_MONITOR, "Processing policy request %s\n",
 				pkt->vm_name);
+		int ret = send_ack_for_received_cmd(pkt,
+				chan_info,
+				CPU_POWER_CMD_ACK);
+		if (ret < 0)
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
 		update_policy(pkt);
 		policy_is_set = 1;
 	}
-- 
2.17.2

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Thomas Monjalon]

05/04/2019 15:24, Hajkowski:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> 
> Extend guest channel API to allow bidirectional
> communication. Modify power manager host and guest
> side to communicate in both directions.

This patchset is deferred to 19.08 because of a lack
of review.

Note: too many librte_power patches are not reviewed.

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Thomas Monjalon]

05/04/2019 15:24, Hajkowski:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> 
> Extend guest channel API to allow bidirectional
> communication. Modify power manager host and guest
> side to communicate in both directions.

This patchset is deferred to 19.08 because of a lack
of review.

Note: too many librte_power patches are not reviewed.

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Hunt, David]

On 22/4/2019 9:36 PM, Thomas Monjalon wrote:
> 05/04/2019 15:24, Hajkowski:
>> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>>
>> Extend guest channel API to allow bidirectional
>> communication. Modify power manager host and guest
>> side to communicate in both directions.
> This patchset is deferred to 19.08 because of a lack
> of review.
>
> Note: too many librte_power patches are not reviewed.


Hi Thomas,

  The four remaining patch sets for power were initially submitted after 
the v1 deadline, and were not expected to make it into 19.05. We will be 
sure to review for 19.08.

Regards,
Dave.

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Hunt, David]

On 22/4/2019 9:36 PM, Thomas Monjalon wrote:
> 05/04/2019 15:24, Hajkowski:
>> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>>
>> Extend guest channel API to allow bidirectional
>> communication. Modify power manager host and guest
>> side to communicate in both directions.
> This patchset is deferred to 19.08 because of a lack
> of review.
>
> Note: too many librte_power patches are not reviewed.


Hi Thomas,

  The four remaining patch sets for power were initially submitted after 
the v1 deadline, and were not expected to make it into 19.05. We will be 
sure to review for 19.08.

Regards,
Dave.

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Thomas Monjalon]

24/04/2019 10:38, Hunt, David:
> On 22/4/2019 9:36 PM, Thomas Monjalon wrote:
> > 05/04/2019 15:24, Hajkowski:
> >> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> >>
> >> Extend guest channel API to allow bidirectional
> >> communication. Modify power manager host and guest
> >> side to communicate in both directions.
> > This patchset is deferred to 19.08 because of a lack
> > of review.
> >
> > Note: too many librte_power patches are not reviewed.
> 
> 
> Hi Thomas,
> 
>   The four remaining patch sets for power were initially submitted after 
> the v1 deadline, and were not expected to make it into 19.05. We will be 
> sure to review for 19.08.

OK thanks for the confirmation.
In such case, it is better to add "19.08" in the subject prefix
so it is explicit from the beginning. Thanks

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Thomas Monjalon]

24/04/2019 10:38, Hunt, David:
> On 22/4/2019 9:36 PM, Thomas Monjalon wrote:
> > 05/04/2019 15:24, Hajkowski:
> >> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> >>
> >> Extend guest channel API to allow bidirectional
> >> communication. Modify power manager host and guest
> >> side to communicate in both directions.
> > This patchset is deferred to 19.08 because of a lack
> > of review.
> >
> > Note: too many librte_power patches are not reviewed.
> 
> 
> Hi Thomas,
> 
>   The four remaining patch sets for power were initially submitted after 
> the v1 deadline, and were not expected to make it into 19.05. We will be 
> sure to review for 19.08.

OK thanks for the confirmation.
In such case, it is better to add "19.08" in the subject prefix
so it is explicit from the beginning. Thanks

Re: [dpdk-dev] [PATCH v5 0/4] bidirect guest channel

[Thomas Monjalon]

24/04/2019 10:44, Thomas Monjalon:
> 24/04/2019 10:38, Hunt, David:
> > On 22/4/2019 9:36 PM, Thomas Monjalon wrote:
> > > 05/04/2019 15:24, Hajkowski:
> > >> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> > >>
> > >> Extend guest channel API to allow bidirectional
> > >> communication. Modify power manager host and guest
> > >> side to communicate in both directions.
> > > This patchset is deferred to 19.08 because of a lack
> > > of review.
> > >
> > > Note: too many librte_power patches are not reviewed.
> > 
> > 
> > Hi Thomas,
> > 
> >   The four remaining patch sets for power were initially submitted after 
> > the v1 deadline, and were not expected to make it into 19.05. We will be 
> > sure to review for 19.08.
> 
> OK thanks for the confirmation.
> In such case, it is better to add "19.08" in the subject prefix
> so it is explicit from the beginning. Thanks

There was no more review since April.
I guess it will miss 19.08 as well.

Re: [dpdk-dev] [PATCH v5 2/4] power: extend guest channel API for reading

[Hunt, David]

On 05/04/2019 14:24, Hajkowski wrote:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>
> Added new experimental API rte_power_guest_channel_receive_msg
> which gives possibility to receive messages send to guest.
>
> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> ---
>   lib/librte_power/channel_commands.h    |  5 +++
>   lib/librte_power/guest_channel.c       | 60 ++++++++++++++++++++++++++
>   lib/librte_power/guest_channel.h       | 35 +++++++++++++++
>   lib/librte_power/rte_power_version.map |  1 +
>   4 files changed, 101 insertions(+)
>
> diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h
> index e7b93a797..33fd53a6d 100644
> --- a/lib/librte_power/channel_commands.h
> +++ b/lib/librte_power/channel_commands.h
> @@ -28,6 +28,11 @@ extern "C" {
>   #define CPU_POWER_SCALE_MIN     4
>   #define CPU_POWER_ENABLE_TURBO  5
>   #define CPU_POWER_DISABLE_TURBO 6
> +
> +/* Generic Power Command Response */
> +#define CPU_POWER_CMD_ACK       1
> +#define CPU_POWER_CMD_NACK      2
> +
>   #define HOURS 24
>   
>   #define MAX_VFS 10
> diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
> index 9cf7d2cb2..888423891 100644
> --- a/lib/librte_power/guest_channel.c
> +++ b/lib/librte_power/guest_channel.c
> @@ -10,6 +10,7 @@
>   #include <fcntl.h>
>   #include <string.h>
>   #include <errno.h>
> +#include <poll.h>
>   
>   
>   #include <rte_log.h>
> @@ -19,6 +20,9 @@
>   
>   #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
>   
> +/* Timeout for incoming message in milliseconds. */
> +#define TIMEOUT 10
> +
>   static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
>   
>   int
> @@ -125,6 +129,62 @@ int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
>   	return guest_channel_send_msg(pkt, lcore_id);
>   }
>   
> +int power_guest_channel_read_msg(struct channel_packet *pkt,
> +			unsigned int lcore_id)
> +{
> +	int ret;
> +	struct pollfd fds;
> +	void *buffer = pkt;
> +	int buffer_len = sizeof(*pkt);
> +
> +	fds.fd = global_fds[lcore_id];
> +	fds.events = POLLIN;
> +
> +	ret = poll(&fds, 1, TIMEOUT);
> +	if (ret == 0) {
> +		RTE_LOG(DEBUG, GUEST_CHANNEL, "Timeout occurred during poll function.\n");
> +		return -1;
> +	} else if (ret < 0) {
> +		RTE_LOG(ERR, GUEST_CHANNEL, "Error occurred during poll function: %s\n",
> +				strerror(ret));
> +		return -1;
> +	}
> +
> +	if (lcore_id >= RTE_MAX_LCORE) {
> +		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
> +				lcore_id, RTE_MAX_LCORE-1);
> +		return -1;
> +	}
> +
> +	if (global_fds[lcore_id] < 0) {
> +		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
> +		return -1;
> +	}
> +
> +	while (buffer_len > 0) {
> +		ret = read(global_fds[lcore_id],
> +				buffer, buffer_len);
> +		if (ret < 0) {
> +			if (errno == EINTR)
> +				continue;
> +			return -1;
> +		}
> +		if (ret == 0) {
> +			RTE_LOG(ERR, GUEST_CHANNEL, "Expected more data, but connection has been closed.\n");
> +			return -1;
> +		}
> +		buffer = (char *)buffer + ret;
> +		buffer_len -= ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
> +			unsigned int lcore_id)
> +{
> +	return power_guest_channel_read_msg(pkt, lcore_id);
> +}
>   
>   void
>   guest_channel_host_disconnect(unsigned int lcore_id)
> diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
> index 373d39898..7c385df39 100644
> --- a/lib/librte_power/guest_channel.h
> +++ b/lib/librte_power/guest_channel.h
> @@ -68,6 +68,41 @@ int guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id);
>   int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
>   			unsigned int lcore_id);
>   
> +/**
> + * Read a message contained in pkt over the Virtio-Serial
> + * from the host endpoint.
> + *
> + * @param pkt
> + *  Pointer to a populated struct channel_packet
> + *
> + * @param lcore_id
> + *  lcore_id.
> + *
> + * @return
> + *  - 0 on success.
> + *  - Negative on error.
> + */
> +int power_guest_channel_read_msg(struct channel_packet *pkt,
> +		unsigned int lcore_id);
> +
> +/**
> + * Receive a message contained in pkt over the Virtio-Serial
> + * from the host endpoint.
> + *
> + * @param pkt
> + *  Pointer to a populated struct channel_packet
> + *
> + * @param lcore_id
> + *  lcore_id.
> + *
> + * @return
> + *  - 0 on success.
> + *  - Negative on error.
> + */
> +int __rte_experimental
> +rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
> +			unsigned int lcore_id);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/librte_power/rte_power_version.map b/lib/librte_power/rte_power_version.map
> index 042917360..69f5ea3f4 100644
> --- a/lib/librte_power/rte_power_version.map
> +++ b/lib/librte_power/rte_power_version.map
> @@ -44,4 +44,5 @@ EXPERIMENTAL {
>   	rte_power_empty_poll_stat_update;
>   	rte_power_poll_stat_fetch;
>   	rte_power_poll_stat_update;
> +	rte_power_guest_channel_receive_msg;
>   };


I tested this with the later patches in the set, and the acknowledges 
successfully get back to the guest from the host in the form of an Ack 
or Nack for various commands sent from the guest.

Tested-by: David Hunt <david.hunt@intel.com>

Re: [dpdk-dev] [PATCH v5 3/4] power: process incoming confirmation cmds

[Hunt, David]

On 05/04/2019 14:24, Hajkowski wrote:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>
> Extend vm_power_guest to check incoming confirmations
> of messages previously sent to host.
>
> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> ---
>   examples/vm_power_manager/guest_cli/Makefile  |  1 +
>   .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
>   2 files changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/examples/vm_power_manager/guest_cli/Makefile b/examples/vm_power_manager/guest_cli/Makefile
> index e35a68d0f..67cf08193 100644
> --- a/examples/vm_power_manager/guest_cli/Makefile
> +++ b/examples/vm_power_manager/guest_cli/Makefile
> @@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c
>   
>   CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>   CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>   
>   # workaround for a gcc bug with noreturn attribute
>   # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> index 2d9e7689a..49ed7b208 100644
> --- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> +++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> @@ -27,7 +27,7 @@
>   #define CHANNEL_PATH "/dev/virtio-ports/virtio.serial.port.poweragent"
>   
>   
> -#define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
> +#define RTE_LOGTYPE_GUEST_CLI RTE_LOGTYPE_USER1
>   
>   struct cmd_quit_result {
>   	cmdline_fixed_string_t quit;
> @@ -132,6 +132,32 @@ struct cmd_set_cpu_freq_result {
>   	cmdline_fixed_string_t cmd;
>   };
>   
> +static int
> +check_response_cmd(unsigned int lcore_id, int *result)
> +{
> +	struct channel_packet pkt;
> +	int ret;
> +
> +	ret = rte_power_guest_channel_receive_msg(&pkt, lcore_id);
> +	if (ret < 0)
> +		return -1;
> +
> +	switch (pkt.command) {
> +	case(CPU_POWER_CMD_ACK):
> +		*result = 1;
> +		break;
> +	case(CPU_POWER_CMD_NACK):
> +		*result = 0;
> +		break;
> +	default:
> +		RTE_LOG(ERR, GUEST_CLI,
> +				"Received invalid response from host, expecting ACK/NACK.\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   static void
>   cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
>   		       __attribute__((unused)) void *data)
> @@ -139,20 +165,31 @@ cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
>   	int ret = -1;
>   	struct cmd_set_cpu_freq_result *res = parsed_result;
>   
> -	if (!strcmp(res->cmd , "up"))
> +	if (!strcmp(res->cmd, "up"))
>   		ret = rte_power_freq_up(res->lcore_id);
> -	else if (!strcmp(res->cmd , "down"))
> +	else if (!strcmp(res->cmd, "down"))
>   		ret = rte_power_freq_down(res->lcore_id);
> -	else if (!strcmp(res->cmd , "min"))
> +	else if (!strcmp(res->cmd, "min"))
>   		ret = rte_power_freq_min(res->lcore_id);
> -	else if (!strcmp(res->cmd , "max"))
> +	else if (!strcmp(res->cmd, "max"))
>   		ret = rte_power_freq_max(res->lcore_id);
>   	else if (!strcmp(res->cmd, "enable_turbo"))
>   		ret = rte_power_freq_enable_turbo(res->lcore_id);
>   	else if (!strcmp(res->cmd, "disable_turbo"))
>   		ret = rte_power_freq_disable_turbo(res->lcore_id);
> -	if (ret != 1)
> +
> +	if (ret != 1) {
>   		cmdline_printf(cl, "Error sending message: %s\n", strerror(ret));
> +		return;
> +	}
> +	int result;
> +	ret = check_response_cmd(res->lcore_id, &result);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent message received\n");
> +	} else {
> +		cmdline_printf(cl, "%s received for message sent to host.\n",
> +				result == 1 ? "ACK" : "NACK");
> +	}
>   }
>   
>   cmdline_parse_token_string_t cmd_set_cpu_freq =
> @@ -185,16 +222,26 @@ struct cmd_send_policy_result {
>   };
>   
>   static inline int
> -send_policy(struct channel_packet *pkt)
> +send_policy(struct channel_packet *pkt, struct cmdline *cl)
>   {
>   	int ret;
>   
>   	ret = rte_power_guest_channel_send_msg(pkt, 1);
> -	if (ret == 0)
> -		return 1;
> -	RTE_LOG(DEBUG, POWER, "Error sending message: %s\n",
> -			ret > 0 ? strerror(ret) : "channel not connected");
> -	return -1;
> +	if (ret < 0) {
> +		RTE_LOG(ERR, GUEST_CLI, "Error sending message: %s\n",
> +				ret > 0 ? strerror(ret) : "channel not connected");
> +		return -1;
> +	}
> +
> +	int result;
> +	ret = check_response_cmd(1, &result);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent policy received\n");
> +	} else {
> +		cmdline_printf(cl, "%s for sent policy received.\n",
> +				result == 1 ? "ACK" : "NACK");
> +	}
> +	return 1;
>   }
>   
>   static void
> @@ -206,7 +253,7 @@ cmd_send_policy_parsed(void *parsed_result, struct cmdline *cl,
>   
>   	if (!strcmp(res->cmd, "now")) {
>   		printf("Sending Policy down now!\n");
> -		ret = send_policy(&policy);
> +		ret = send_policy(&policy, cl);
>   	}
>   	if (ret != 1)
>   		cmdline_printf(cl, "Error sending message: %s\n",



I tested this with the later patches in the set, and the acknowledges 
successfully get back to the guest from the host in the form of an Ack 
or Nack for various commands sent from the guest.

Tested-by: David Hunt <david.hunt@intel.com>

Re: [dpdk-dev] [PATCH v5 4/4] power: send confirmation cmd to vm guest

[Hunt, David]

On 05/04/2019 14:24, Hajkowski wrote:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>
> Use new guest channel API to send confirmation
> message for received power command.
>
> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> ---
>   examples/vm_power_manager/channel_monitor.c | 68 +++++++++++++++++++--
>   1 file changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
> index 7892d75de..ed580b36a 100644
> --- a/examples/vm_power_manager/channel_monitor.c
> +++ b/examples/vm_power_manager/channel_monitor.c
> @@ -627,6 +627,41 @@ apply_policy(struct policy *pol)
>   		apply_workload_profile(pol);
>   }
>   
> +static int
> +write_binary_packet(struct channel_packet *pkt, struct channel_info *chan_info)
> +{
> +	int ret, buffer_len = sizeof(*pkt);
> +	void *buffer = pkt;
> +
> +	if (chan_info->fd < 0) {
> +		RTE_LOG(ERR, CHANNEL_MONITOR, "Channel is not connected\n");
> +		return -1;
> +	}
> +
> +	while (buffer_len > 0) {
> +		ret = write(chan_info->fd, buffer, buffer_len);
> +		if (ret == -1) {
> +			if (errno == EINTR)
> +				continue;
> +			RTE_LOG(ERR, CHANNEL_MONITOR, "Write function failed due to %s.\n",
> +					strerror(errno));
> +			return -1;
> +		}
> +		buffer = (char *)buffer + ret;
> +		buffer_len -= ret;
> +	}
> +	return 0;
> +}
> +
> +static int
> +send_ack_for_received_cmd(struct channel_packet *pkt,
> +		struct channel_info *chan_info,
> +		uint32_t command)
> +{
> +	pkt->command = command;
> +	return write_binary_packet(pkt, chan_info);
> +}
> +
>   static int
>   process_request(struct channel_packet *pkt, struct channel_info *chan_info)
>   {
> @@ -650,33 +685,54 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)
>   		RTE_LOG(DEBUG, CHANNEL_MONITOR, "Processing requested cmd for cpu:%d\n",
>   			core_num);
>   
> +		bool valid_unit = true;
> +		int scale_res;
> +
>   		switch (pkt->unit) {
>   		case(CPU_POWER_SCALE_MIN):
> -			power_manager_scale_core_min(core_num);
> +			scale_res = power_manager_scale_core_min(core_num);
>   			break;
>   		case(CPU_POWER_SCALE_MAX):
> -			power_manager_scale_core_max(core_num);
> +			scale_res = power_manager_scale_core_max(core_num);
>   			break;
>   		case(CPU_POWER_SCALE_DOWN):
> -			power_manager_scale_core_down(core_num);
> +			scale_res = power_manager_scale_core_down(core_num);
>   			break;
>   		case(CPU_POWER_SCALE_UP):
> -			power_manager_scale_core_up(core_num);
> +			scale_res = power_manager_scale_core_up(core_num);
>   			break;
>   		case(CPU_POWER_ENABLE_TURBO):
> -			power_manager_enable_turbo_core(core_num);
> +			scale_res = power_manager_enable_turbo_core(core_num);
>   			break;
>   		case(CPU_POWER_DISABLE_TURBO):
> -			power_manager_disable_turbo_core(core_num);
> +			scale_res = power_manager_disable_turbo_core(core_num);
>   			break;
>   		default:
> +			valid_unit = false;
>   			break;
>   		}
> +
> +		if (valid_unit) {
> +			ret = send_ack_for_received_cmd(pkt,
> +					chan_info,
> +					scale_res > 0 ?
> +						CPU_POWER_CMD_ACK :
> +						CPU_POWER_CMD_NACK);
> +			if (ret < 0)
> +				RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
> +		} else
> +			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Unexpected unit type.\n");
> +
>   	}
>   
>   	if (pkt->command == PKT_POLICY) {
>   		RTE_LOG(INFO, CHANNEL_MONITOR, "Processing policy request %s\n",
>   				pkt->vm_name);
> +		int ret = send_ack_for_received_cmd(pkt,
> +				chan_info,
> +				CPU_POWER_CMD_ACK);
> +		if (ret < 0)
> +			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
>   		update_policy(pkt);
>   		policy_is_set = 1;
>   	}

Using the guest_cli sample app in a VM, sending commands to the 
vm_power_manager app on the host, we now have Acks and Nacks coming from 
the host to the VM to conform the execution of the guest requests.

vmpower(guest)> set_cpu_freq 3 down
ACK received for message sent to host.
vmpower(guest)> set_cpu_freq 3 up
ACK received for message sent to host.
vmpower(guest)> set_cpu_freq 3 up
NACK received for message sent to host.

(NACK because we're already at the maxumum frequency)

And in the host vm_power_manager command line, we can see when a guest 
request cannot be processed:
POWER: Turbo is off, frequency can't be scaled up more 31

Patchset looks good functionally.

Tested-by: David Hunt <david.hunt@intel.com>

Re: [dpdk-dev] [PATCH v5 4/4] power: send confirmation cmd to vm guest

[Daly, Lee]

> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> >
> > Use new guest channel API to send confirmation message for received
> > power command.
> >
> > Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> > ---
> >   examples/vm_power_manager/channel_monitor.c | 68
> +++++++++++++++++++--
> >   1 file changed, 62 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/vm_power_manager/channel_monitor.c
> > b/examples/vm_power_manager/channel_monitor.c
> > index 7892d75de..ed580b36a 100644
> > --- a/examples/vm_power_manager/channel_monitor.c
> > +++ b/examples/vm_power_manager/channel_monitor.c


<...>

> 
> Using the guest_cli sample app in a VM, sending commands to the
> vm_power_manager app on the host, we now have Acks and Nacks coming
> from the host to the VM to conform the execution of the guest requests.
> 
> vmpower(guest)> set_cpu_freq 3 down
> ACK received for message sent to host.
> vmpower(guest)> set_cpu_freq 3 up
> ACK received for message sent to host.
> vmpower(guest)> set_cpu_freq 3 up
> NACK received for message sent to host.
> 
> (NACK because we're already at the maxumum frequency)
> 
> And in the host vm_power_manager command line, we can see when a
> guest request cannot be processed:
> POWER: Turbo is off, frequency can't be scaled up more 31
> 
> Patchset looks good functionally.
> 
> Tested-by: David Hunt <david.hunt@intel.com>
 Acked-by: Lee Daly <lee.daly@intel.com>

Re: [dpdk-dev] [PATCH v5 3/4] power: process incoming confirmation cmds

[Daly, Lee]

> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> >
> > Extend vm_power_guest to check incoming confirmations of messages
> > previously sent to host.
> >
> > Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> > ---
> >   examples/vm_power_manager/guest_cli/Makefile  |  1 +
> >   .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
> >   2 files changed, 61 insertions(+), 13 deletions(-)
> >
> > diff --git a/examples/vm_power_manager/guest_cli/Makefile
> > b/examples/vm_power_manager/guest_cli/Makefile
> > index e35a68d0f..67cf08193 100644
> > --- a/examples/vm_power_manager/guest_cli/Makefile
> > +++ b/examples/vm_power_manager/guest_cli/Makefile
> > @@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c


<...>

> I tested this with the later patches in the set, and the acknowledges
> successfully get back to the guest from the host in the form of an Ack or Nack
> for various commands sent from the guest.
> 
> Tested-by: David Hunt <david.hunt@intel.com>

Acked-by: Lee Daly <lee.daly@intel.com>

Re: [dpdk-dev] [PATCH v5 2/4] power: extend guest channel API for reading

[Daly, Lee]

> On 05/04/2019 14:24, Hajkowski wrote:
> > From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> >
> > Added new experimental API rte_power_guest_channel_receive_msg
> > which gives possibility to receive messages send to guest.
> >
> > Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> > ---
> >   lib/librte_power/channel_commands.h    |  5 +++
> >   lib/librte_power/guest_channel.c       | 60 ++++++++++++++++++++++++++
> >   lib/librte_power/guest_channel.h       | 35 +++++++++++++++
> >   lib/librte_power/rte_power_version.map |  1 +
> >   4 files changed, 101 insertions(+)
> >
> > diff --git a/lib/librte_power/channel_commands.h
> > b/lib/librte_power/channel_commands.h


<...>

> > diff --git a/lib/librte_power/rte_power_version.map
> > b/lib/librte_power/rte_power_version.map
> > index 042917360..69f5ea3f4 100644
> > --- a/lib/librte_power/rte_power_version.map
> > +++ b/lib/librte_power/rte_power_version.map
> > @@ -44,4 +44,5 @@ EXPERIMENTAL {
> >   	rte_power_empty_poll_stat_update;
> >   	rte_power_poll_stat_fetch;
> >   	rte_power_poll_stat_update;
> > +	rte_power_guest_channel_receive_msg;
> >   };
Nit Pick, new api should be added in alphabetical order, i.e after "rte_power_empty_poll_stat_update" .

> 
> 
> I tested this with the later patches in the set, and the acknowledges
> successfully get back to the guest from the host in the form of an Ack or Nack
> for various commands sent from the guest.
> 
> Tested-by: David Hunt <david.hunt@intel.com>
> 

Feel free to add my ack after the above minor change. 
Acked-by: Lee Daly <lee.daly@intel.com>

[dpdk-dev] [PATCH v6 0/4] bidirectional guest channel

[Marcin Hajkowski]

Extend guest channel API to allow bidirectional communication. Modify power manager host and guest side to communicate in both directions.

---

v6:
* put map file api's in alphabetical order
* fix checkpatch issue around experimental tag

v5:
* enhance logging

v4:
* [vm_power_manager] treat 0 as valid socket id
* [guest_manager] use user level logs
* correct code formatting

v3:
* fix global_fds[lcore_id] comparison to invalid value
* check 0 to verify if read function actually read some data
* define _NACK cmd instead of _NAK
* simplify rte_power_guest_channel_receive_msg func logic

v2:
* send ack only if power operation return positive value
* log diffent error for unexpected incoming command and
  error during ack/nak cmd sending


Marcin Hajkowski (4):
[PATCH v6 1/4] power: fix invalid socket indicator value
[PATCH v6 2/4] power: extend guest channel API for reading
[PATCH v6 3/4] power: process incoming confirmation cmds
[PATCH v6 4/4] power: send confirmation cmd to vm guest

[dpdk-dev] [PATCH v6 1/4] power: fix invalid socket indicator value

[Marcin Hajkowski]

Currently 0 is being used for not connected slot indication.
This is not consistent with linux doc which identifies 0 as valid
(connected) slot, thus modification was done to change it.

Fixes: cd0d5547 ("power: vm communication channels in guest")
Cc: stable@dpdk.org

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_power/guest_channel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index c17ea46b4..9cf7d2cb2 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -19,7 +19,7 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
-static int global_fds[RTE_MAX_LCORE];
+static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
 guest_channel_host_connect(const char *path, unsigned int lcore_id)
@@ -35,7 +35,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 		return -1;
 	}
 	/* check if path is already open */
-	if (global_fds[lcore_id] != 0) {
+	if (global_fds[lcore_id] != -1) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is already open with fd %d\n",
 				lcore_id, global_fds[lcore_id]);
 		return -1;
@@ -84,7 +84,7 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
 	return 0;
 error:
 	close(fd);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 	return -1;
 }
 
@@ -100,7 +100,7 @@ guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id)
 		return -1;
 	}
 
-	if (global_fds[lcore_id] == 0) {
+	if (global_fds[lcore_id] < 0) {
 		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
 		return -1;
 	}
@@ -134,8 +134,8 @@ guest_channel_host_disconnect(unsigned int lcore_id)
 				lcore_id, RTE_MAX_LCORE-1);
 		return;
 	}
-	if (global_fds[lcore_id] == 0)
+	if (global_fds[lcore_id] < 0)
 		return;
 	close(global_fds[lcore_id]);
-	global_fds[lcore_id] = 0;
+	global_fds[lcore_id] = -1;
 }
-- 
2.17.1

[dpdk-dev] [PATCH v6 2/4] power: extend guest channel API for reading

[Marcin Hajkowski]

Added new experimental API rte_power_guest_channel_receive_msg
which gives possibility to receive messages send to guest.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Tested-by: David Hunt <david.hunt@intel.com>
Acked-by: Lee Daly <lee.daly@intel.com>
---
 lib/librte_power/channel_commands.h    |  5 +++
 lib/librte_power/guest_channel.c       | 60 ++++++++++++++++++++++++++
 lib/librte_power/guest_channel.h       | 36 ++++++++++++++++
 lib/librte_power/rte_power_version.map |  1 +
 4 files changed, 102 insertions(+)

diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h
index eca8ff70c..d81216b42 100644
--- a/lib/librte_power/channel_commands.h
+++ b/lib/librte_power/channel_commands.h
@@ -25,6 +25,11 @@ extern "C" {
 #define CPU_POWER_SCALE_MIN     4
 #define CPU_POWER_ENABLE_TURBO  5
 #define CPU_POWER_DISABLE_TURBO 6
+
+/* Generic Power Command Response */
+#define CPU_POWER_CMD_ACK       1
+#define CPU_POWER_CMD_NACK      2
+
 #define HOURS 24
 
 #define MAX_VFS 10
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 9cf7d2cb2..888423891 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -10,6 +10,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <errno.h>
+#include <poll.h>
 
 
 #include <rte_log.h>
@@ -19,6 +20,9 @@
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
+/* Timeout for incoming message in milliseconds. */
+#define TIMEOUT 10
+
 static int global_fds[RTE_MAX_LCORE] = { [0 ... RTE_MAX_LCORE-1] = -1 };
 
 int
@@ -125,6 +129,62 @@ int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 	return guest_channel_send_msg(pkt, lcore_id);
 }
 
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	int ret;
+	struct pollfd fds;
+	void *buffer = pkt;
+	int buffer_len = sizeof(*pkt);
+
+	fds.fd = global_fds[lcore_id];
+	fds.events = POLLIN;
+
+	ret = poll(&fds, 1, TIMEOUT);
+	if (ret == 0) {
+		RTE_LOG(DEBUG, GUEST_CHANNEL, "Timeout occurred during poll function.\n");
+		return -1;
+	} else if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Error occurred during poll function: %s\n",
+				strerror(ret));
+		return -1;
+	}
+
+	if (lcore_id >= RTE_MAX_LCORE) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
+				lcore_id, RTE_MAX_LCORE-1);
+		return -1;
+	}
+
+	if (global_fds[lcore_id] < 0) {
+		RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = read(global_fds[lcore_id],
+				buffer, buffer_len);
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -1;
+		}
+		if (ret == 0) {
+			RTE_LOG(ERR, GUEST_CHANNEL, "Expected more data, but connection has been closed.\n");
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+
+	return 0;
+}
+
+int rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id)
+{
+	return power_guest_channel_read_msg(pkt, lcore_id);
+}
 
 void
 guest_channel_host_disconnect(unsigned int lcore_id)
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/guest_channel.h
index 373d39898..61e142289 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/guest_channel.h
@@ -68,6 +68,42 @@ int guest_channel_send_msg(struct channel_packet *pkt, unsigned int lcore_id);
 int rte_power_guest_channel_send_msg(struct channel_packet *pkt,
 			unsigned int lcore_id);
 
+/**
+ * Read a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+int power_guest_channel_read_msg(struct channel_packet *pkt,
+		unsigned int lcore_id);
+
+/**
+ * Receive a message contained in pkt over the Virtio-Serial
+ * from the host endpoint.
+ *
+ * @param pkt
+ *  Pointer to a populated struct channel_packet
+ *
+ * @param lcore_id
+ *  lcore_id.
+ *
+ * @return
+ *  - 0 on success.
+ *  - Negative on error.
+ */
+__rte_experimental
+int
+rte_power_guest_channel_receive_msg(struct channel_packet *pkt,
+			unsigned int lcore_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_power/rte_power_version.map b/lib/librte_power/rte_power_version.map
index 042917360..772983813 100644
--- a/lib/librte_power/rte_power_version.map
+++ b/lib/librte_power/rte_power_version.map
@@ -42,6 +42,7 @@ EXPERIMENTAL {
 	rte_power_empty_poll_stat_free;
 	rte_power_empty_poll_stat_init;
 	rte_power_empty_poll_stat_update;
+	rte_power_guest_channel_receive_msg;
 	rte_power_poll_stat_fetch;
 	rte_power_poll_stat_update;
 };
-- 
2.17.1

[dpdk-dev] [PATCH v6 3/4] power: process incoming confirmation cmds

[Marcin Hajkowski]

Extend vm_power_guest to check incoming confirmations
of messages previously sent to host.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Tested-by: David Hunt <david.hunt@intel.com>
Acked-by: Lee Daly <lee.daly@intel.com>
---
 examples/vm_power_manager/guest_cli/Makefile  |  1 +
 .../guest_cli/vm_power_cli_guest.c            | 73 +++++++++++++++----
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/examples/vm_power_manager/guest_cli/Makefile b/examples/vm_power_manager/guest_cli/Makefile
index e35a68d0f..67cf08193 100644
--- a/examples/vm_power_manager/guest_cli/Makefile
+++ b/examples/vm_power_manager/guest_cli/Makefile
@@ -18,6 +18,7 @@ SRCS-y := main.c vm_power_cli_guest.c parse.c
 
 CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 78c3aa0a1..cdb241801 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -27,7 +27,7 @@
 #define CHANNEL_PATH "/dev/virtio-ports/virtio.serial.port.poweragent"
 
 
-#define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
+#define RTE_LOGTYPE_GUEST_CLI RTE_LOGTYPE_USER1
 
 struct cmd_quit_result {
 	cmdline_fixed_string_t quit;
@@ -132,6 +132,32 @@ struct cmd_set_cpu_freq_result {
 	cmdline_fixed_string_t cmd;
 };
 
+static int
+check_response_cmd(unsigned int lcore_id, int *result)
+{
+	struct channel_packet pkt;
+	int ret;
+
+	ret = rte_power_guest_channel_receive_msg(&pkt, lcore_id);
+	if (ret < 0)
+		return -1;
+
+	switch (pkt.command) {
+	case(CPU_POWER_CMD_ACK):
+		*result = 1;
+		break;
+	case(CPU_POWER_CMD_NACK):
+		*result = 0;
+		break;
+	default:
+		RTE_LOG(ERR, GUEST_CLI,
+				"Received invalid response from host, expecting ACK/NACK.\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void
 cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 		       __attribute__((unused)) void *data)
@@ -139,20 +165,31 @@ cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
 	int ret = -1;
 	struct cmd_set_cpu_freq_result *res = parsed_result;
 
-	if (!strcmp(res->cmd , "up"))
+	if (!strcmp(res->cmd, "up"))
 		ret = rte_power_freq_up(res->lcore_id);
-	else if (!strcmp(res->cmd , "down"))
+	else if (!strcmp(res->cmd, "down"))
 		ret = rte_power_freq_down(res->lcore_id);
-	else if (!strcmp(res->cmd , "min"))
+	else if (!strcmp(res->cmd, "min"))
 		ret = rte_power_freq_min(res->lcore_id);
-	else if (!strcmp(res->cmd , "max"))
+	else if (!strcmp(res->cmd, "max"))
 		ret = rte_power_freq_max(res->lcore_id);
 	else if (!strcmp(res->cmd, "enable_turbo"))
 		ret = rte_power_freq_enable_turbo(res->lcore_id);
 	else if (!strcmp(res->cmd, "disable_turbo"))
 		ret = rte_power_freq_disable_turbo(res->lcore_id);
-	if (ret != 1)
+
+	if (ret != 1) {
 		cmdline_printf(cl, "Error sending message: %s\n", strerror(ret));
+		return;
+	}
+	int result;
+	ret = check_response_cmd(res->lcore_id, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent message received\n");
+	} else {
+		cmdline_printf(cl, "%s received for message sent to host.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
 }
 
 cmdline_parse_token_string_t cmd_set_cpu_freq =
@@ -185,16 +222,26 @@ struct cmd_send_policy_result {
 };
 
 static inline int
-send_policy(struct channel_packet *pkt)
+send_policy(struct channel_packet *pkt, struct cmdline *cl)
 {
 	int ret;
 
 	ret = rte_power_guest_channel_send_msg(pkt, 1);
-	if (ret == 0)
-		return 1;
-	RTE_LOG(DEBUG, POWER, "Error sending message: %s\n",
-			ret > 0 ? strerror(ret) : "channel not connected");
-	return -1;
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "Error sending message: %s\n",
+				ret > 0 ? strerror(ret) : "channel not connected");
+		return -1;
+	}
+
+	int result;
+	ret = check_response_cmd(1, &result);
+	if (ret < 0) {
+		RTE_LOG(ERR, GUEST_CLI, "No confirmation for sent policy received\n");
+	} else {
+		cmdline_printf(cl, "%s for sent policy received.\n",
+				result == 1 ? "ACK" : "NACK");
+	}
+	return 1;
 }
 
 static void
@@ -206,7 +253,7 @@ cmd_send_policy_parsed(void *parsed_result, struct cmdline *cl,
 
 	if (!strcmp(res->cmd, "now")) {
 		printf("Sending Policy down now!\n");
-		ret = send_policy(&policy);
+		ret = send_policy(&policy, cl);
 	}
 	if (ret != 1)
 		cmdline_printf(cl, "Error sending message: %s\n",
-- 
2.17.1

[dpdk-dev] [PATCH v6 4/4] power: send confirmation cmd to vm guest

[Marcin Hajkowski]

Use new guest channel API to send confirmation
message for received power command.

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Tested-by: David Hunt <david.hunt@intel.com>
Acked-by: Lee Daly <lee.daly@intel.com>
---
 examples/vm_power_manager/channel_monitor.c | 68 +++++++++++++++++++--
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index fe6088a18..0c73fac55 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -671,6 +671,41 @@ apply_policy(struct policy *pol)
 		apply_workload_profile(pol);
 }
 
+static int
+write_binary_packet(struct channel_packet *pkt, struct channel_info *chan_info)
+{
+	int ret, buffer_len = sizeof(*pkt);
+	void *buffer = pkt;
+
+	if (chan_info->fd < 0) {
+		RTE_LOG(ERR, CHANNEL_MONITOR, "Channel is not connected\n");
+		return -1;
+	}
+
+	while (buffer_len > 0) {
+		ret = write(chan_info->fd, buffer, buffer_len);
+		if (ret == -1) {
+			if (errno == EINTR)
+				continue;
+			RTE_LOG(ERR, CHANNEL_MONITOR, "Write function failed due to %s.\n",
+					strerror(errno));
+			return -1;
+		}
+		buffer = (char *)buffer + ret;
+		buffer_len -= ret;
+	}
+	return 0;
+}
+
+static int
+send_ack_for_received_cmd(struct channel_packet *pkt,
+		struct channel_info *chan_info,
+		uint32_t command)
+{
+	pkt->command = command;
+	return write_binary_packet(pkt, chan_info);
+}
+
 static int
 process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 {
@@ -694,33 +729,54 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 		RTE_LOG(DEBUG, CHANNEL_MONITOR, "Processing requested cmd for cpu:%d\n",
 			core_num);
 
+		bool valid_unit = true;
+		int scale_res;
+
 		switch (pkt->unit) {
 		case(CPU_POWER_SCALE_MIN):
-			power_manager_scale_core_min(core_num);
+			scale_res = power_manager_scale_core_min(core_num);
 			break;
 		case(CPU_POWER_SCALE_MAX):
-			power_manager_scale_core_max(core_num);
+			scale_res = power_manager_scale_core_max(core_num);
 			break;
 		case(CPU_POWER_SCALE_DOWN):
-			power_manager_scale_core_down(core_num);
+			scale_res = power_manager_scale_core_down(core_num);
 			break;
 		case(CPU_POWER_SCALE_UP):
-			power_manager_scale_core_up(core_num);
+			scale_res = power_manager_scale_core_up(core_num);
 			break;
 		case(CPU_POWER_ENABLE_TURBO):
-			power_manager_enable_turbo_core(core_num);
+			scale_res = power_manager_enable_turbo_core(core_num);
 			break;
 		case(CPU_POWER_DISABLE_TURBO):
-			power_manager_disable_turbo_core(core_num);
+			scale_res = power_manager_disable_turbo_core(core_num);
 			break;
 		default:
+			valid_unit = false;
 			break;
 		}
+
+		if (valid_unit) {
+			ret = send_ack_for_received_cmd(pkt,
+					chan_info,
+					scale_res > 0 ?
+						CPU_POWER_CMD_ACK :
+						CPU_POWER_CMD_NACK);
+			if (ret < 0)
+				RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
+		} else
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Unexpected unit type.\n");
+
 	}
 
 	if (pkt->command == PKT_POLICY) {
 		RTE_LOG(INFO, CHANNEL_MONITOR, "Processing policy request %s\n",
 				pkt->vm_name);
+		int ret = send_ack_for_received_cmd(pkt,
+				chan_info,
+				CPU_POWER_CMD_ACK);
+		if (ret < 0)
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command.\n");
 		update_policy(pkt);
 		policy_is_set = 1;
 	}
-- 
2.17.1

Re: [dpdk-dev] [PATCH v6 0/4] bidirectional guest channel

[Thomas Monjalon]

27/09/2019 10:42, Marcin Hajkowski:
> Marcin Hajkowski (4):
> [PATCH v6 1/4] power: fix invalid socket indicator value
> [PATCH v6 2/4] power: extend guest channel API for reading
> [PATCH v6 3/4] power: process incoming confirmation cmds
> [PATCH v6 4/4] power: send confirmation cmd to vm guest

Applied, thanks