[dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface

[David Hunt <david.hunt@intel.com>]

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

This patch implements a separate FIFO for each cpu core to improve the
previous functionality where anyone with access to the FIFO could affect
any core on the system. By using appropriate permissions, fifo interfaces
can be configured to only affect the particular cores.

Because each FIFO is per core, the following fields have been removed
from the command JSON format: core_list, resource_id, name.

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Tested-by: David Hunt <david.hunt@intel.com>

---
v2:
* updated handling vm_name (use proper buff size)
* rebase to master changes

v3:
* improvement to coding style

v4:
* rebase to tip of master

v5:
* merged docs into same patch as the code, as per mailing list policy
* made changes out of review by Anatoly.
---
 .../sample_app_ug/vm_power_management.rst     | 61 +++---------
 examples/vm_power_manager/channel_manager.c   | 90 +++++++++++++-----
 examples/vm_power_manager/channel_manager.h   |  7 +-
 examples/vm_power_manager/channel_monitor.c   | 92 +++++++++++++------
 examples/vm_power_manager/main.c              |  2 +-
 5 files changed, 150 insertions(+), 102 deletions(-)

diff --git a/doc/guides/sample_app_ug/vm_power_management.rst b/doc/guides/sample_app_ug/vm_power_management.rst
index 5d9a26172..0ffff835e 100644
--- a/doc/guides/sample_app_ug/vm_power_management.rst
+++ b/doc/guides/sample_app_ug/vm_power_management.rst
@@ -380,9 +380,16 @@ parsing functionality will not be present in the app.
 
 Sending a command or policy to the power manager application is achieved by
 simply opening a fifo file, writing a JSON string to that fifo, and closing
-the file.
+the file. In actual implementation every core has own dedicated fifo[0..n],
+where n is number of the last available core.
+Having a dedicated fifo file per core allows using standard filesystem permissions
+to ensure a given container can only write JSON commands into fifos it is allowed
+to use.
 
-The fifo is at /tmp/powermonitor/fifo
+The fifo is at /tmp/powermonitor/fifo[0..n]
+
+For example all cmds put to the /tmp/powermonitor/fifo7, will have
+effect only on CPU[7].
 
 The JSON string can be a policy or instruction, and takes the following
 format:
@@ -405,19 +412,6 @@ arrays, etc. Examples of policies follow later in this document. The allowed
 names and value types are as follows:
 
 
-:Pair Name: "name"
-:Description: Name of the VM or Host. Allows the parser to associate the
-  policy with the relevant VM or Host OS.
-:Type: string
-:Values: any valid string
-:Required: yes
-:Example:
-
-    .. code-block:: javascript
-
-      "name", "ubuntu2"
-
-
 :Pair Name: "command"
 :Description: The type of packet we're sending to the power manager. We can be
   creating or destroying a policy, or sending a direct command to adjust
@@ -509,17 +503,6 @@ names and value types are as follows:
 
     "max_packet_thresh": 500000
 
-:Pair Name: "core_list"
-:Description: The cores to which to apply the policy.
-:Type: array of integers
-:Values: array with list of virtual CPUs.
-:Required: only policy CREATE/DESTROY
-:Example:
-
-  .. code-block:: javascript
-
-    "core_list":[ 10, 11 ]
-
 :Pair Name: "workload"
 :Description: When our policy is of type WORKLOAD, we need to specify how
   heavy our workload is.
@@ -566,17 +549,6 @@ names and value types are as follows:
 
     "unit", "SCALE_MAX"
 
-:Pair Name: "resource_id"
-:Description: The core to which to apply the power command.
-:Type: integer
-:Values: valid core id for VM or host OS.
-:Required: only POWER instruction
-:Example:
-
-  .. code-block:: javascript
-
-    "resource_id": 10
-
 JSON API Examples
 ~~~~~~~~~~~~~~~~~
 
@@ -585,12 +557,10 @@ Profile create example:
   .. code-block:: javascript
 
     {"policy": {
-      "name": "ubuntu",
       "command": "create",
       "policy_type": "TIME",
       "busy_hours":[ 17, 18, 19, 20, 21, 22, 23 ],
-      "quiet_hours":[ 2, 3, 4, 5, 6 ],
-      "core_list":[ 11 ]
+      "quiet_hours":[ 2, 3, 4, 5, 6 ]
     }}
 
 Profile destroy example:
@@ -598,8 +568,7 @@ Profile destroy example:
   .. code-block:: javascript
 
     {"policy": {
-      "name": "ubuntu",
-      "command": "destroy",
+      "command": "destroy"
     }}
 
 Power command example:
@@ -607,18 +576,16 @@ Power command example:
   .. code-block:: javascript
 
     {"instruction": {
-      "name": "ubuntu",
       "command": "power",
-      "unit": "SCALE_MAX",
-      "resource_id": 10
+      "unit": "SCALE_MAX"
     }}
 
 To send a JSON string to the Power Manager application, simply paste the
-example JSON string into a text file and cat it into the fifo:
+example JSON string into a text file and cat it into the proper fifo:
 
   .. code-block:: console
 
-    cat file.json >/tmp/powermonitor/fifo
+    cat file.json >/tmp/powermonitor/fifo[0..n]
 
 The console of the Power Manager application should indicate the command that
 was just received via the fifo.
diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 0f5f9e287..56ee699ab 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -345,10 +345,22 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr,
 	return 0;
 }
 
-static void
-fifo_path(char *dst, unsigned int len)
+static int
+fifo_path(char *dst, unsigned int len, unsigned int id)
 {
-	snprintf(dst, len, "%sfifo", CHANNEL_MGR_SOCKET_PATH);
+	int cnt;
+
+	cnt = snprintf(dst, len, "%s%s%d", CHANNEL_MGR_SOCKET_PATH,
+			CHANNEL_MGR_FIFO_PATTERN_NAME, id);
+
+	if ((cnt < 0) || (cnt > (int)len - 1)) {
+		RTE_LOG(ERR, CHANNEL_MANAGER, "Could not create proper "
+			"string for fifo path\n");
+
+		return -1;
+	}
+
+	return 0;
 }
 
 static int
@@ -534,42 +546,70 @@ add_channels(const char *vm_name, unsigned *channel_list,
 }
 
 int
-add_host_channel(void)
+add_host_channels(void)
 {
 	struct channel_info *chan_info;
 	char socket_path[PATH_MAX];
 	int num_channels_enabled = 0;
 	int ret;
+	struct core_info *ci;
+	struct channel_info *chan_infos[RTE_MAX_LCORE];
+	int i;
 
-	fifo_path(socket_path, sizeof(socket_path));
+	for (i = 0; i < RTE_MAX_LCORE; i++)
+		chan_infos[i] = NULL;
 
-	ret = mkfifo(socket_path, 0660);
-	if ((errno != EEXIST) && (ret < 0)) {
-		RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot create fifo '%s' error: "
-				"%s\n", socket_path, strerror(errno));
+	ci = get_core_info();
+	if (ci == NULL) {
+		RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot allocate memory for core_info\n");
 		return 0;
 	}
 
-	if (access(socket_path, F_OK) < 0) {
-		RTE_LOG(ERR, CHANNEL_MANAGER, "Channel path '%s' error: "
-				"%s\n", socket_path, strerror(errno));
-		return 0;
-	}
-	chan_info = rte_malloc(NULL, sizeof(*chan_info), 0);
-	if (chan_info == NULL) {
-		RTE_LOG(ERR, CHANNEL_MANAGER, "Error allocating memory for "
-				"channel '%s'\n", socket_path);
-		return 0;
-	}
-	rte_strlcpy(chan_info->channel_path, socket_path, UNIX_PATH_MAX);
+	for (i = 0; i < ci->core_count; i++) {
+		if (ci->cd[i].global_enabled_cpus == 0)
+			continue;
 
-	if (setup_host_channel_info(&chan_info, 0) < 0) {
-		rte_free(chan_info);
-		return 0;
+		ret = fifo_path(socket_path, sizeof(socket_path), i);
+		if (ret < 0)
+			goto error;
+
+		ret = mkfifo(socket_path, 0660);
+		RTE_LOG(DEBUG, CHANNEL_MANAGER, "TRY CREATE fifo '%s'\n",
+			socket_path);
+		if ((errno != EEXIST) && (ret < 0)) {
+			RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot create fifo '%s' error: "
+					"%s\n", socket_path, strerror(errno));
+			goto error;
+		}
+		chan_info = rte_malloc(NULL, sizeof(*chan_info), 0);
+		if (chan_info == NULL) {
+			RTE_LOG(ERR, CHANNEL_MANAGER, "Error allocating memory for "
+					"channel '%s'\n", socket_path);
+			goto error;
+		}
+		chan_infos[i] = chan_info;
+		rte_strlcpy(chan_info->channel_path, socket_path,
+				sizeof(chan_info->channel_path));
+
+		if (setup_host_channel_info(&chan_info, i) < 0) {
+			rte_free(chan_info);
+			chan_infos[i] = NULL;
+			goto error;
+		}
+		num_channels_enabled++;
 	}
-	num_channels_enabled++;
 
 	return num_channels_enabled;
+error:
+	/* Clean up the channels opened before we hit an error. */
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		if (chan_infos[i] != NULL) {
+			remove_channel_from_monitor(chan_infos[i]);
+			close(chan_infos[i]->fd);
+			rte_free(chan_infos[i]);
+		}
+	}
+	return 0;
 }
 
 int
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index 251d2163c..3766e93c8 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -22,6 +22,9 @@ extern "C" {
 /* File socket directory */
 #define CHANNEL_MGR_SOCKET_PATH     "/tmp/powermonitor/"
 
+/* FIFO file name template */
+#define CHANNEL_MGR_FIFO_PATTERN_NAME   "fifo"
+
 #ifndef UNIX_PATH_MAX
 struct sockaddr_un _sockaddr_un;
 #define UNIX_PATH_MAX sizeof(_sockaddr_un.sun_path)
@@ -206,13 +209,13 @@ int add_channels(const char *vm_name, unsigned *channel_list,
 		unsigned num_channels);
 
 /**
- * Set up a fifo by which host applications can send command an policies
+ * Set up fifos by which host applications can send command an policies
  * through a fifo to the vm_power_manager
  *
  * @return
  *  - 0 for success
  */
-int add_host_channel(void);
+int add_host_channels(void);
 
 /**
  * Remove a channel definition from the channel manager. This must only be
diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index aab19ba57..b9a326e7d 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -29,6 +29,7 @@
 #include <rte_cycles.h>
 #include <rte_ethdev.h>
 #include <rte_pmd_i40e.h>
+#include <rte_string_fns.h>
 
 #include <libvirt/libvirt.h>
 #include "channel_monitor.h"
@@ -51,7 +52,7 @@ static volatile unsigned run_loop = 1;
 static int global_event_fd;
 static unsigned int policy_is_set;
 static struct epoll_event *global_events_list;
-static struct policy policies[MAX_CLIENTS];
+static struct policy policies[RTE_MAX_LCORE];
 
 #ifdef USE_JANSSON
 
@@ -130,13 +131,45 @@ set_policy_mac(struct channel_packet *pkt, int idx, char *mac)
 	return 0;
 }
 
+static char*
+get_resource_name_from_chn_path(const char *channel_path)
+{
+	char *substr = NULL;
+
+	substr = strstr(channel_path, CHANNEL_MGR_FIFO_PATTERN_NAME);
+
+	return substr;
+}
 
 static int
-parse_json_to_pkt(json_t *element, struct channel_packet *pkt)
+get_resource_id_from_vmname(const char *vm_name)
+{
+	int result = -1;
+	int off = 0;
+
+	if (vm_name == NULL)
+		return -1;
+
+	while (vm_name[off] != '\0') {
+		if (isdigit(vm_name[off]))
+			break;
+		off++;
+	}
+	result = atoi(&vm_name[off]);
+	if ((result == 0) && (vm_name[off] != '0'))
+		return -1;
+
+	return result;
+}
+
+static int
+parse_json_to_pkt(json_t *element, struct channel_packet *pkt,
+					const char *vm_name)
 {
 	const char *key;
 	json_t *value;
 	int ret;
+	int resource_id;
 
 	memset(pkt, 0, sizeof(struct channel_packet));
 
@@ -147,20 +180,23 @@ parse_json_to_pkt(json_t *element, struct channel_packet *pkt)
 	pkt->command = PKT_POLICY;
 	pkt->core_type = CORE_TYPE_PHYSICAL;
 
+	if (vm_name == NULL) {
+		RTE_LOG(ERR, CHANNEL_MONITOR,
+			"vm_name is NULL, request rejected !\n");
+		return -1;
+	}
+
 	json_object_foreach(element, key, value) {
 		if (!strcmp(key, "policy")) {
 			/* Recurse in to get the contents of profile */
-			ret = parse_json_to_pkt(value, pkt);
+			ret = parse_json_to_pkt(value, pkt, vm_name);
 			if (ret)
 				return ret;
 		} else if (!strcmp(key, "instruction")) {
 			/* Recurse in to get the contents of instruction */
-			ret = parse_json_to_pkt(value, pkt);
+			ret = parse_json_to_pkt(value, pkt, vm_name);
 			if (ret)
 				return ret;
-		} else if (!strcmp(key, "name")) {
-			strlcpy(pkt->vm_name, json_string_value(value),
-					sizeof(pkt->vm_name));
 		} else if (!strcmp(key, "command")) {
 			char command[32];
 			strlcpy(command, json_string_value(value), 32);
@@ -223,16 +259,6 @@ parse_json_to_pkt(json_t *element, struct channel_packet *pkt)
 						json_array_get(value, i));
 				pkt->timer_policy.quiet_hours[i] = hour;
 			}
-		} else if (!strcmp(key, "core_list")) {
-			unsigned int i;
-			size_t size = json_array_size(value);
-
-			for (i = 0; i < size; i++) {
-				int core = (int)json_integer_value(
-						json_array_get(value, i));
-				pkt->vcpu_to_control[i] = core;
-			}
-			pkt->num_vcpu = size;
 		} else if (!strcmp(key, "mac_list")) {
 			unsigned int i;
 			size_t size = json_array_size(value);
@@ -271,13 +297,21 @@ parse_json_to_pkt(json_t *element, struct channel_packet *pkt)
 					"Invalid command received in JSON\n");
 				return -1;
 			}
-		} else if (!strcmp(key, "resource_id")) {
-			pkt->resource_id = (uint32_t)json_integer_value(value);
 		} else {
 			RTE_LOG(ERR, CHANNEL_MONITOR,
 				"Unknown key received in JSON string: %s\n",
 				key);
 		}
+
+		resource_id = get_resource_id_from_vmname(vm_name);
+		if (resource_id < 0) {
+			RTE_LOG(ERR, CHANNEL_MONITOR,
+				"Could not get resource_id from vm_name:%s\n",
+				vm_name);
+			return -1;
+		}
+		rte_strlcpy(pkt->vm_name, vm_name, VM_MAX_NAME_SZ);
+		pkt->resource_id = resource_id;
 	}
 	return 0;
 }
@@ -427,13 +461,13 @@ update_policy(struct channel_packet *pkt)
 {
 
 	unsigned int updated = 0;
-	int i;
+	unsigned int i;
 
 
 	RTE_LOG(INFO, CHANNEL_MONITOR,
 			"Applying policy for %s\n", pkt->vm_name);
 
-	for (i = 0; i < MAX_CLIENTS; i++) {
+	for (i = 0; i < RTE_DIM(policies); i++) {
 		if (strcmp(policies[i].pkt.vm_name, pkt->vm_name) == 0) {
 			/* Copy the contents of *pkt into the policy.pkt */
 			policies[i].pkt = *pkt;
@@ -451,7 +485,7 @@ update_policy(struct channel_packet *pkt)
 		}
 	}
 	if (!updated) {
-		for (i = 0; i < MAX_CLIENTS; i++) {
+		for (i = 0; i < RTE_DIM(policies); i++) {
 			if (policies[i].enabled == 0) {
 				policies[i].pkt = *pkt;
 				get_pcpu_to_control(&policies[i]);
@@ -474,13 +508,13 @@ update_policy(struct channel_packet *pkt)
 static int
 remove_policy(struct channel_packet *pkt __rte_unused)
 {
-	int i;
+	unsigned int i;
 
 	/*
 	 * Disabling the policy is simply a case of setting
 	 * enabled to 0
 	 */
-	for (i = 0; i < MAX_CLIENTS; i++) {
+	for (i = 0; i < RTE_DIM(policies); i++) {
 		if (strcmp(policies[i].pkt.vm_name, pkt->vm_name) == 0) {
 			policies[i].enabled = 0;
 			return 0;
@@ -801,6 +835,8 @@ read_json_packet(struct channel_info *chan_info)
 	int n_bytes, ret;
 	json_t *root;
 	json_error_t error;
+	const char *resource_name;
+
 
 	/* read opening brace to closing brace */
 	do {
@@ -832,13 +868,15 @@ read_json_packet(struct channel_info *chan_info)
 		root = json_loads(json_data, 0, &error);
 
 		if (root) {
+			resource_name = get_resource_name_from_chn_path(
+				chan_info->channel_path);
 			/*
 			 * Because our data is now in the json
 			 * object, we can overwrite the pkt
 			 * with a channel_packet struct, using
 			 * parse_json_to_pkt()
 			 */
-			ret = parse_json_to_pkt(root, &pkt);
+			ret = parse_json_to_pkt(root, &pkt, resource_name);
 			json_decref(root);
 			if (ret) {
 				RTE_LOG(ERR, CHANNEL_MONITOR,
@@ -895,9 +933,9 @@ run_channel_monitor(void)
 		}
 		rte_delay_us(time_period_ms*1000);
 		if (policy_is_set) {
-			int j;
+			unsigned int j;
 
-			for (j = 0; j < MAX_CLIENTS; j++) {
+			for (j = 0; j < RTE_DIM(policies); j++) {
 				if (policies[j].enabled == 1)
 					apply_policy(&policies[j]);
 			}
diff --git a/examples/vm_power_manager/main.c b/examples/vm_power_manager/main.c
index bc15cb64e..54c704610 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -434,7 +434,7 @@ main(int argc, char **argv)
 		return -1;
 	}
 
-	add_host_channel();
+	add_host_channels();
 
 	printf("Running core monitor on lcore id %d\n", lcore_id);
 	rte_eal_remote_launch(run_core_monitor, NULL, lcore_id);
-- 
2.17.1