diff mbox series

[4/4] power: send confirmation cmd to vm guest

Message ID 20190314144752.13812-5-marcinx.hajkowski@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series bidirect guest channel |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Marcin Hajkowski March 14, 2019, 2:47 p.m. UTC 
  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 | 64 +++++++++++++++++++--
 1 file changed, 58 insertions(+), 6 deletions(-)

Comments

Pattan, Reshma March 15, 2019, 4:29 p.m. UTC | #1 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hajkowski
>  static int
>  process_request(struct channel_packet *pkt, struct channel_info *chan_info)  {
> @@ -645,33 +678,52 @@ process_request(struct channel_packet *pkt, struct
> channel_info *chan_info)

power_manager_* functions returns 0, -1 and 1. So, below  might  not be correct. 

> +						scale_res ?
> CPU_POWER_CMD_ACK : CPU_POWER_CMD_NAK);

Thanks,
Reshma
Pattan, Reshma March 15, 2019, 5:13 p.m. UTC | #2 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hajkowski

> +		int ret = -1;
> +		if (valid_unit)
> +			ret = send_ack_for_received_cmd(pkt,
> +						chan_info,
> +						scale_res ?

> CPU_POWER_CMD_ACK : CPU_POWER_CMD_NAK);
> +		if (ret < 0)
> +			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during
> sending ack command "
> +					"or unexpected unit type.\n");
> +
>  	}

This if check should go inside for if(valid_unit)?,  otherwise this would applicable for valid_unit == false also.
Also do we need to handle valid_unit ==false case? Do we need to send back  message to guest saying invalid command.

Thanks,
Reshma
Marcin Hajkowski March 18, 2019, 12:40 p.m. UTC | #3 
Logging  error in case of invalid command is enough in my opinion. In this case we distinguish between invalid command (log error) and unsuccessful power operation (NAK cmd). 
I agree though that error log might be more precise (separate for error during sending ACK/NAK and separate for  invalid command).

Best Regards,
Marcin

-----Original Message-----
From: Pattan, Reshma 
Sent: Friday, March 15, 2019 6:14 PM
To: Hajkowski, MarcinX <marcinx.hajkowski@intel.com>; Hunt, David <david.hunt@intel.com>
Cc: dev@dpdk.org; Hajkowski, MarcinX <marcinx.hajkowski@intel.com>
Subject: RE: [dpdk-dev] [PATCH 4/4] power: send confirmation cmd to vm guest



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hajkowski

> +		int ret = -1;
> +		if (valid_unit)
> +			ret = send_ack_for_received_cmd(pkt,
> +						chan_info,
> +						scale_res ?

> CPU_POWER_CMD_ACK : CPU_POWER_CMD_NAK);
> +		if (ret < 0)
> +			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during
> sending ack command "
> +					"or unexpected unit type.\n");
> +
>  	}

This if check should go inside for if(valid_unit)?,  otherwise this would applicable for valid_unit == false also.
Also do we need to handle valid_unit ==false case? Do we need to send back  message to guest saying invalid command.

Thanks,
Reshma
diff mbox series

Patch

diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 1a3a0fa76..4d8568d87 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -627,6 +627,39 @@  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;
+			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)
 {
@@ -645,33 +678,52 @@  process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 		else
 			core_num = pkt->resource_id;
 
+		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;
 		}
+
+		int ret = -1;
+		if (valid_unit)
+			ret = send_ack_for_received_cmd(pkt,
+						chan_info,
+						scale_res ? CPU_POWER_CMD_ACK : CPU_POWER_CMD_NAK);
+		if (ret < 0)
+			RTE_LOG(DEBUG, CHANNEL_MONITOR, "Error during sending ack command "
+					"or 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;
 	}