[v3,3/4] power: process incoming confirmation cmds
Checks
Commit Message
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 | 65 +++++++++++++++----
2 files changed, 54 insertions(+), 12 deletions(-)
Comments
On 21-Mar-19 10:55 AM, 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 | 65 +++++++++++++++----
> 2 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/examples/vm_power_manager/guest_cli/Makefile b/examples/vm_power_manager/guest_cli/Makefile
> index a5634eacf..51a5010ab 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..698dd5062 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
> @@ -132,6 +132,26 @@ 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;
> +
> + if (pkt.command != CPU_POWER_CMD_ACK &&
> + pkt.command != CPU_POWER_CMD_NACK) {
> + RTE_LOG(DEBUG, POWER, "Not expected command has been received.\n");
> + return -1;
> + }
Maybe use switch?
E.g.
swithc (pkt.command) {
case ACK:
case NACK:
*result = blah;
break;
default:
log failure;
fail;
}
> +
> + *result = (pkt.command == CPU_POWER_CMD_ACK);
> + return 0;
> +}
> +
> static void
> cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
> __attribute__((unused)) void *data)
> @@ -139,20 +159,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(DEBUG, POWER, "No confirmation for sent msg received\n");
> + } else {
> + cmdline_printf(cl, "Ack for sent msg received with result: %s.\n",
> + result == 1 ? "SUCCESS" : "ERROR");
Here and in other places: others can correct me if i'm wrong, but i'm
pretty sure RTE_LOG(..., POWER) is reserved for logs related to power
library, and are not supposed to be used in user applications. If
logging in an application is required, there are USER log levels.
I can see that the app in question already does that in a few places, so
it's kind of consistent with how things are already, but i would
question the value of consistency in cases like these. Not sure how to
proceed here.
> + }
> }
>
> cmdline_parse_token_string_t cmd_set_cpu_freq =
> @@ -185,16 +216,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(DEBUG, POWER, "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(DEBUG, POWER, "No confirmation for sent policy received\n");
> + } else {
> + cmdline_printf(cl, "Ack for sent policy received with result: %s.\n",
> + result == 1 ? "SUCCESS" : "ERROR");
> + }
> + return 1;
> }
>
> static void
> @@ -206,7 +247,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",
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hajkowski
> Sent: Thursday, March 21, 2019 10:55 AM
> + RTE_LOG(DEBUG, POWER, "No confirmation for sent msg
> received\n");
> + }
As Anatoly mentioned, we should not use POWER log here, instead #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
is defined for GUEST_CHANNEL logs, you should use that for the logs in this file.
Thanks,
Reshma
@@ -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
@@ -132,6 +132,26 @@ 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;
+
+ if (pkt.command != CPU_POWER_CMD_ACK &&
+ pkt.command != CPU_POWER_CMD_NACK) {
+ RTE_LOG(DEBUG, POWER, "Not expected command has been received.\n");
+ return -1;
+ }
+
+ *result = (pkt.command == CPU_POWER_CMD_ACK);
+ return 0;
+}
+
static void
cmd_set_cpu_freq_parsed(void *parsed_result, struct cmdline *cl,
__attribute__((unused)) void *data)
@@ -139,20 +159,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(DEBUG, POWER, "No confirmation for sent msg received\n");
+ } else {
+ cmdline_printf(cl, "Ack for sent msg received with result: %s.\n",
+ result == 1 ? "SUCCESS" : "ERROR");
+ }
}
cmdline_parse_token_string_t cmd_set_cpu_freq =
@@ -185,16 +216,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(DEBUG, POWER, "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(DEBUG, POWER, "No confirmation for sent policy received\n");
+ } else {
+ cmdline_printf(cl, "Ack for sent policy received with result: %s.\n",
+ result == 1 ? "SUCCESS" : "ERROR");
+ }
+ return 1;
}
static void
@@ -206,7 +247,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",