DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface
@ 2019-04-08 10:45 Lukasz Krakowiak
  2019-04-08 10:45 ` [dpdk-dev] [PATCH 1/2] " Lukasz Krakowiak
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Lukasz Krakowiak @ 2019-04-08 10:45 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Lukasz Krakowiak

This patch implement a separate FIFO for each cpu core.
Dependency to commit:  5c7c96571b7b7051ecc286f36be0b3d95b49995e
(power: update for handling fifo path string).

Lukasz Krakowiak (2):
  power: add fifo per core for JSON interface
  doc: update according to the fifo per core impl

 .../sample_app_ug/vm_power_management.rst     | 53 +++--------
 examples/vm_power_manager/channel_manager.c   | 85 +++++++++++------
 examples/vm_power_manager/channel_manager.h   |  7 +-
 examples/vm_power_manager/channel_monitor.c   | 91 +++++++++++++------
 examples/vm_power_manager/main.c              |  2 +-
 5 files changed, 140 insertions(+), 98 deletions(-)

-- 
2.19.2.windows.1


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

* [dpdk-dev] [PATCH 1/2] power: add fifo per core for JSON interface
  2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
@ 2019-04-08 10:45 ` " Lukasz Krakowiak
  2019-04-08 10:45 ` [dpdk-dev] [PATCH 2/2] doc: update according to the fifo per core impl Lukasz Krakowiak
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Lukasz Krakowiak @ 2019-04-08 10:45 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Lukasz Krakowiak

This patch implement a separate FIFO for each cpu core.
For proper handling JSON interface, removed fields from cmds:
core_list, resource_id, name.

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
---
 examples/vm_power_manager/channel_manager.c | 85 +++++++++++++------
 examples/vm_power_manager/channel_manager.h |  7 +-
 examples/vm_power_manager/channel_monitor.c | 91 +++++++++++++++------
 examples/vm_power_manager/main.c            |  2 +-
 4 files changed, 129 insertions(+), 56 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index f1dd8cbf3..01ad12b3a 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,40 +546,59 @@ 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;
 
-	fifo_path(socket_path, sizeof(socket_path));
-
-	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);
+	do {
+		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
+			continue;
 
-	if (setup_host_channel_info(&chan_info, 0) < 0) {
-		rte_free(chan_info);
-		return 0;
-	}
-	num_channels_enabled++;
+		ret = fifo_path(socket_path, sizeof(socket_path),
+							num_channels_enabled);
+		if (ret < 0)
+			return 0;
+
+		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));
+			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;
+		}
+		snprintf(chan_info->channel_path,
+			sizeof(chan_info->channel_path),
+			"%s", socket_path);
+
+		if (setup_host_channel_info(&chan_info,
+			num_channels_enabled) < 0) {
+			rte_free(chan_info);
+			return 0;
+		}
+	} while (++num_channels_enabled <= ci->core_count);
 
 	return num_channels_enabled;
 }
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index c3cdce492..7fddaf6a6 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -28,6 +28,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)
@@ -212,13 +215,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 7892d75de..8ac4e6217 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -28,6 +28,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"
@@ -50,7 +51,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[POWER_MGR_MAX_CPUS];
 
 #ifdef USE_JANSSON
 
@@ -129,13 +130,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
+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)
+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));
 
@@ -146,19 +179,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")) {
-			strcpy(pkt->vm_name, json_string_value(value));
 		} else if (!strcmp(key, "command")) {
 			char command[32];
 			snprintf(command, 32, "%s", json_string_value(value));
@@ -221,16 +258,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);
@@ -268,13 +295,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, UNIX_PATH_MAX);
+		pkt->resource_id = resource_id;
 	}
 	return 0;
 }
@@ -424,13 +459,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;
@@ -445,7 +480,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]);
@@ -463,13 +498,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;
@@ -790,6 +825,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 {
@@ -826,13 +863,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,
@@ -889,9 +928,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 893bf4cdd..9f24cf69b 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -421,7 +421,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.19.2.windows.1


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

* [dpdk-dev] [PATCH 2/2] doc: update according to the fifo per core impl
  2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
  2019-04-08 10:45 ` [dpdk-dev] [PATCH 1/2] " Lukasz Krakowiak
@ 2019-04-08 10:45 ` Lukasz Krakowiak
  2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Lukasz Krakowiak @ 2019-04-08 10:45 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Lukasz Krakowiak

Updated power management docs for fifo JSON API.
Removed from JSON API:
* 'name'
* 'resource_id'
* 'core_list'

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
---
 .../sample_app_ug/vm_power_management.rst     | 53 ++++---------------
 1 file changed, 11 insertions(+), 42 deletions(-)

diff --git a/doc/guides/sample_app_ug/vm_power_management.rst b/doc/guides/sample_app_ug/vm_power_management.rst
index 14d432e78..7b2c78ffc 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 jason 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
 ~~~~~~~~~~~~~~~~~
 
@@ -598,7 +570,6 @@ Profile destroy example:
   .. code-block:: javascript
 
     {"profile": {
-      "name": "ubuntu",
       "command": "destroy",
     }}
 
@@ -607,17 +578,15 @@ Power command example:
   .. code-block:: javascript
 
     {"command": {
-      "name": "ubuntu",
       "unit": "SCALE_MAX",
-      "resource_id": 10
     }}
 
 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.
-- 
2.19.2.windows.1


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

* [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface
  2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
  2019-04-08 10:45 ` [dpdk-dev] [PATCH 1/2] " Lukasz Krakowiak
  2019-04-08 10:45 ` [dpdk-dev] [PATCH 2/2] doc: update according to the fifo per core impl Lukasz Krakowiak
@ 2019-04-12 13:54 ` Lukasz Gosiewski
  2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 1/2] " Lukasz Gosiewski
                     ` (2 more replies)
  2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
  2019-06-07  8:50 ` [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Hajkowski, MarcinX
  4 siblings, 3 replies; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-12 13:54 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak

This patch implement a separate FIFO for each cpu core.
Dependency to commit:  5c7c96571b7b7051ecc286f36be0b3d95b49995e
(power: update for handling fifo path string).

Lukasz Krakowiak (2):
  power: add fifo per core for JSON interface
  doc: update according to the fifo per core impl

---
v2:
* rebased on last changes on master
* updated handling vm_name (use proper buff size)

 .../sample_app_ug/vm_power_management.rst     | 53 +++--------
 examples/vm_power_manager/channel_manager.c   | 85 +++++++++++------
 examples/vm_power_manager/channel_manager.h   |  7 +-
 examples/vm_power_manager/channel_monitor.c   | 91 +++++++++++++------
 examples/vm_power_manager/main.c              |  2 +-
 5 files changed, 140 insertions(+), 98 deletions(-)

-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.


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

* [dpdk-dev] [PATCH v2 1/2] power: add fifo per core for JSON interface
  2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
@ 2019-04-12 13:54   ` " Lukasz Gosiewski
  2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
  2019-04-22 20:37   ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Thomas Monjalon
  2 siblings, 0 replies; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-12 13:54 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak, Lukasz Gosiewski

From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>

This patch implement a separate FIFO for each cpu core.
For proper handling JSON interface, removed fields from cmds:
core_list, resource_id, name.

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

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
---
 examples/vm_power_manager/channel_manager.c | 85 +++++++++++++------
 examples/vm_power_manager/channel_manager.h |  7 +-
 examples/vm_power_manager/channel_monitor.c | 91 +++++++++++++++------
 examples/vm_power_manager/main.c            |  2 +-
 4 files changed, 129 insertions(+), 56 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 05c0eea44..d19276839 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -346,10 +346,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
@@ -535,40 +547,59 @@ 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;
 
-	fifo_path(socket_path, sizeof(socket_path));
-
-	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);
+	do {
+		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
+			continue;
 
-	if (setup_host_channel_info(&chan_info, 0) < 0) {
-		rte_free(chan_info);
-		return 0;
-	}
-	num_channels_enabled++;
+		ret = fifo_path(socket_path, sizeof(socket_path),
+							num_channels_enabled);
+		if (ret < 0)
+			return 0;
+
+		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));
+			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;
+		}
+		snprintf(chan_info->channel_path,
+			sizeof(chan_info->channel_path),
+			"%s", socket_path);
+
+		if (setup_host_channel_info(&chan_info,
+			num_channels_enabled) < 0) {
+			rte_free(chan_info);
+			return 0;
+		}
+	} while (++num_channels_enabled <= ci->core_count);
 
 	return num_channels_enabled;
 }
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index c3cdce492..7fddaf6a6 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -28,6 +28,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)
@@ -212,13 +215,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 74df0fe20..41ee8671d 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[POWER_MGR_MAX_CPUS];
 
 #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,19 +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")) {
-			strcpy(pkt->vm_name, json_string_value(value));
 		} else if (!strcmp(key, "command")) {
 			char command[32];
 			strlcpy(command, json_string_value(value), 32);
@@ -222,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);
@@ -270,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;
 }
@@ -426,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;
@@ -447,7 +482,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]);
@@ -465,13 +500,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;
@@ -792,6 +827,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 {
@@ -828,13 +865,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,
@@ -891,9 +930,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 893bf4cdd..9f24cf69b 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -421,7 +421,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

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.


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

* [dpdk-dev] [PATCH v2 2/2] doc: update according to the fifo per core impl
  2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
  2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 1/2] " Lukasz Gosiewski
@ 2019-04-12 13:54   ` Lukasz Gosiewski
  2019-04-22 20:37   ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Thomas Monjalon
  2 siblings, 0 replies; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-12 13:54 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak, Lukasz Gosiewski

From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>

Updated power management docs for fifo JSON API.
Removed from JSON API:
* 'name'
* 'resource_id'
* 'core_list'

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
---
 .../sample_app_ug/vm_power_management.rst     | 53 ++++---------------
 1 file changed, 11 insertions(+), 42 deletions(-)

diff --git a/doc/guides/sample_app_ug/vm_power_management.rst b/doc/guides/sample_app_ug/vm_power_management.rst
index 14d432e78..7b2c78ffc 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 jason 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
 ~~~~~~~~~~~~~~~~~
 
@@ -598,7 +570,6 @@ Profile destroy example:
   .. code-block:: javascript
 
     {"profile": {
-      "name": "ubuntu",
       "command": "destroy",
     }}
 
@@ -607,17 +578,15 @@ Power command example:
   .. code-block:: javascript
 
     {"command": {
-      "name": "ubuntu",
       "unit": "SCALE_MAX",
-      "resource_id": 10
     }}
 
 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.
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.


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

* Re: [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface
  2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
  2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 1/2] " Lukasz Gosiewski
  2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
@ 2019-04-22 20:37   ` Thomas Monjalon
  2 siblings, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2019-04-22 20:37 UTC (permalink / raw)
  To: Lukasz Gosiewski; +Cc: dev, david.hunt, lukaszx.krakowiak

12/04/2019 15:54, Lukasz Gosiewski:
> This patch implement a separate FIFO for each cpu core.
> Dependency to commit:  5c7c96571b7b7051ecc286f36be0b3d95b49995e
> (power: update for handling fifo path string).
> 
> Lukasz Krakowiak (2):
>   power: add fifo per core for JSON interface
>   doc: update according to the fifo per core impl

Deferred to 19.08 because of a lack of review.




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

* [dpdk-dev] [PATCH v3 0/2] add fifo per core for JSON interface
  2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
                   ` (2 preceding siblings ...)
  2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
@ 2019-04-24  8:21 ` " Lukasz Gosiewski
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 1/2] power: " Lukasz Gosiewski
                     ` (2 more replies)
  2019-06-07  8:50 ` [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Hajkowski, MarcinX
  4 siblings, 3 replies; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-24  8:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak, root

From: root <root@localhost.localdomain>

Lukasz Krakowiak (2):
  power: add fifo per core for JSON interface
  doc: update according to the fifo per core impl

 .../sample_app_ug/vm_power_management.rst     | 53 +++--------
 examples/vm_power_manager/channel_manager.c   | 85 +++++++++++------
 examples/vm_power_manager/channel_manager.h   |  7 +-
 examples/vm_power_manager/channel_monitor.c   | 91 +++++++++++++------
 examples/vm_power_manager/main.c              |  2 +-
 5 files changed, 140 insertions(+), 98 deletions(-)

-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 1/2] power: add fifo per core for JSON interface
  2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
@ 2019-04-24  8:21   ` " Lukasz Gosiewski
  2019-06-07  8:26     ` Hajkowski, MarcinX
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
  2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
  2 siblings, 1 reply; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-24  8:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak, Lukasz Gosiewski

From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>

This patch implement a separate FIFO for each cpu core.
For proper handling JSON interface, removed fields from cmds:
core_list, resource_id, name.

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>

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

v3:
* improvement to coding style
---
 examples/vm_power_manager/channel_manager.c | 85 +++++++++++++------
 examples/vm_power_manager/channel_manager.h |  7 +-
 examples/vm_power_manager/channel_monitor.c | 91 +++++++++++++++------
 examples/vm_power_manager/main.c            |  2 +-
 4 files changed, 129 insertions(+), 56 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 05c0eea44..d19276839 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -346,10 +346,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
@@ -535,40 +547,59 @@ 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;
 
-	fifo_path(socket_path, sizeof(socket_path));
-
-	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);
+	do {
+		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
+			continue;
 
-	if (setup_host_channel_info(&chan_info, 0) < 0) {
-		rte_free(chan_info);
-		return 0;
-	}
-	num_channels_enabled++;
+		ret = fifo_path(socket_path, sizeof(socket_path),
+							num_channels_enabled);
+		if (ret < 0)
+			return 0;
+
+		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));
+			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;
+		}
+		snprintf(chan_info->channel_path,
+			sizeof(chan_info->channel_path),
+			"%s", socket_path);
+
+		if (setup_host_channel_info(&chan_info,
+			num_channels_enabled) < 0) {
+			rte_free(chan_info);
+			return 0;
+		}
+	} while (++num_channels_enabled <= ci->core_count);
 
 	return num_channels_enabled;
 }
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index c3cdce492..7fddaf6a6 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -28,6 +28,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)
@@ -212,13 +215,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 74df0fe20..41ee8671d 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[POWER_MGR_MAX_CPUS];
 
 #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,19 +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")) {
-			strcpy(pkt->vm_name, json_string_value(value));
 		} else if (!strcmp(key, "command")) {
 			char command[32];
 			strlcpy(command, json_string_value(value), 32);
@@ -222,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);
@@ -270,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;
 }
@@ -426,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;
@@ -447,7 +482,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]);
@@ -465,13 +500,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;
@@ -792,6 +827,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 {
@@ -828,13 +865,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,
@@ -891,9 +930,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 893bf4cdd..9f24cf69b 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -421,7 +421,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


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

* [dpdk-dev] [PATCH v3 2/2] doc: update according to the fifo per core impl
  2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 1/2] power: " Lukasz Gosiewski
@ 2019-04-24  8:21   ` Lukasz Gosiewski
  2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
  2 siblings, 0 replies; 26+ messages in thread
From: Lukasz Gosiewski @ 2019-04-24  8:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, lukaszx.krakowiak, Lukasz Gosiewski

From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>

Updated power management docs for fifo JSON API.
Removed from JSON API:
* 'name'
* 'resource_id'
* 'core_list'

Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
---
 .../sample_app_ug/vm_power_management.rst     | 53 ++++---------------
 1 file changed, 11 insertions(+), 42 deletions(-)

diff --git a/doc/guides/sample_app_ug/vm_power_management.rst b/doc/guides/sample_app_ug/vm_power_management.rst
index 14d432e78..7b2c78ffc 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 jason 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
 ~~~~~~~~~~~~~~~~~
 
@@ -598,7 +570,6 @@ Profile destroy example:
   .. code-block:: javascript
 
     {"profile": {
-      "name": "ubuntu",
       "command": "destroy",
     }}
 
@@ -607,17 +578,15 @@ Power command example:
   .. code-block:: javascript
 
     {"command": {
-      "name": "ubuntu",
       "unit": "SCALE_MAX",
-      "resource_id": 10
     }}
 
 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.
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3 1/2] power: add fifo per core for JSON interface
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 1/2] power: " Lukasz Gosiewski
@ 2019-06-07  8:26     ` Hajkowski, MarcinX
  0 siblings, 0 replies; 26+ messages in thread
From: Hajkowski, MarcinX @ 2019-06-07  8:26 UTC (permalink / raw)
  To: Gosiewski, LukaszX, Hunt, David
  Cc: dev, Krakowiak, LukaszX, Gosiewski, LukaszX


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lukasz Gosiewski
> Sent: Wednesday, April 24, 2019 10:22 AM
> To: Hunt, David <david.hunt@intel.com>
> Cc: dev@dpdk.org; Krakowiak, LukaszX <lukaszx.krakowiak@intel.com>;
> Gosiewski, LukaszX <lukaszx.gosiewski@intel.com>
> Subject: [dpdk-dev] [PATCH v3 1/2] power: add fifo per core for JSON
> interface
> 
> From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> 
> This patch implement a separate FIFO for each cpu core.
> For proper handling JSON interface, removed fields from cmds:
> core_list, resource_id, name.
> 
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>

I belive there should be information provided that patch depends on http://patchwork.dpdk.org/patch/52186/. 

> 
> ---
> v2:
> * updated handling vm_name (use proper buff size)
> * rebase to master changes
> 
> v3:
> * improvement to coding style
> ---
>  examples/vm_power_manager/channel_manager.c | 85 +++++++++++++-----
> -  examples/vm_power_manager/channel_manager.h |  7 +-
> examples/vm_power_manager/channel_monitor.c | 91 +++++++++++++++---
> ---
>  examples/vm_power_manager/main.c            |  2 +-
>  4 files changed, 129 insertions(+), 56 deletions(-)
> 
> diff --git a/examples/vm_power_manager/channel_manager.c
> b/examples/vm_power_manager/channel_manager.c
> index 05c0eea44..d19276839 100644
> --- a/examples/vm_power_manager/channel_manager.c
> +++ b/examples/vm_power_manager/channel_manager.c
> @@ -346,10 +346,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
> @@ -535,40 +547,59 @@ 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;
> 
> -	fifo_path(socket_path, sizeof(socket_path));
> -
> -	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);
> +	do {
> +		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
> +			continue;
> 
> -	if (setup_host_channel_info(&chan_info, 0) < 0) {
> -		rte_free(chan_info);
> -		return 0;
> -	}
> -	num_channels_enabled++;
> +		ret = fifo_path(socket_path, sizeof(socket_path),
> +
> 	num_channels_enabled);
> +		if (ret < 0)
> +			return 0;
> +
> +		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));
> +			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;
> +		}
> +		snprintf(chan_info->channel_path,
> +			sizeof(chan_info->channel_path),
> +			"%s", socket_path);
> +
> +		if (setup_host_channel_info(&chan_info,
> +			num_channels_enabled) < 0) {
> +			rte_free(chan_info);
> +			return 0;
> +		}
> +	} while (++num_channels_enabled <= ci->core_count);
> 
>  	return num_channels_enabled;
>  }
> diff --git a/examples/vm_power_manager/channel_manager.h
> b/examples/vm_power_manager/channel_manager.h
> index c3cdce492..7fddaf6a6 100644
> --- a/examples/vm_power_manager/channel_manager.h
> +++ b/examples/vm_power_manager/channel_manager.h
> @@ -28,6 +28,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) @@ -212,13
> +215,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 74df0fe20..41ee8671d 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[POWER_MGR_MAX_CPUS];

POWER_MGR_MAX_CPUS was deleted. RTE_MAX_LCORE should be used instead I think.

> 
>  #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,19 +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")) {
> -			strcpy(pkt->vm_name, json_string_value(value));
>  		} else if (!strcmp(key, "command")) {
>  			char command[32];
>  			strlcpy(command, json_string_value(value), 32); @@
> -222,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); @@ -270,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;
>  }
> @@ -426,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;
> @@ -447,7 +482,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]);
> @@ -465,13 +500,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;
> @@ -792,6 +827,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 {
> @@ -828,13 +865,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,
> @@ -891,9 +930,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 893bf4cdd..9f24cf69b 100644
> --- a/examples/vm_power_manager/main.c
> +++ b/examples/vm_power_manager/main.c
> @@ -421,7 +421,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

Tested-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>


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

* Re: [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface
  2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
                   ` (3 preceding siblings ...)
  2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
@ 2019-06-07  8:50 ` Hajkowski, MarcinX
  4 siblings, 0 replies; 26+ messages in thread
From: Hajkowski, MarcinX @ 2019-06-07  8:50 UTC (permalink / raw)
  To: Krakowiak, LukaszX, Hunt, David
  Cc: dev, Krakowiak, LukaszX, Gosiewski, LukaszX


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lukasz Krakowiak
> Sent: Monday, April 8, 2019 12:45 PM
> To: Hunt, David <david.hunt@intel.com>
> Cc: dev@dpdk.org; Krakowiak, LukaszX <lukaszx.krakowiak@intel.com>
> Subject: [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface
> 
> This patch implement a separate FIFO for each cpu core.
> Dependency to commit:  5c7c96571b7b7051ecc286f36be0b3d95b49995e
> (power: update for handling fifo path string).
> 
> Lukasz Krakowiak (2):
>   power: add fifo per core for JSON interface
>   doc: update according to the fifo per core impl
> 
>  .../sample_app_ug/vm_power_management.rst     | 53 +++--------
>  examples/vm_power_manager/channel_manager.c   | 85 +++++++++++------
>  examples/vm_power_manager/channel_manager.h   |  7 +-
>  examples/vm_power_manager/channel_monitor.c   | 91 +++++++++++++----
> --
>  examples/vm_power_manager/main.c              |  2 +-
>  5 files changed, 140 insertions(+), 98 deletions(-)
> 
> --
> 2.19.2.windows.1

I belive this patchset should be marked as superseded since new version is provided by Lukasz Gosiewski:
http://patchwork.dpdk.org/cover/53036/

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

* [dpdk-dev] [PATCH v4 0/2] Fifo per core
  2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 1/2] power: " Lukasz Gosiewski
  2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
@ 2019-06-13  9:21   ` Hajkowski
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
                       ` (2 more replies)
  2 siblings, 3 replies; 26+ messages in thread
From: Hajkowski @ 2019-06-13  9:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Marcin Hajkowski

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

This patch implement a separate FIFO for each cpu core.
For proper handling JSON interface, removed fields from cmds:
core_list, resource_id, name.

Please note that this patchset depends on
http://patchwork.dpdk.org/patch/52824/

---
v4:
* changes due to code rebase

v3:
* improvement to coding style

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

Marcin Hajkowski (2):
  power: add fifo per core for JSON interface
  doc: update according to the fifo per core impl

 .../sample_app_ug/vm_power_management.rst     | 61 +++---------
 examples/vm_power_manager/channel_manager.c   | 84 +++++++++++------
 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, 142 insertions(+), 104 deletions(-)

-- 
2.17.2


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

* [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface
  2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
@ 2019-06-13  9:21     ` Hajkowski
  2019-07-08 13:44       ` Burakov, Anatoly
                         ` (2 more replies)
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl Hajkowski
  2019-07-04 20:09     ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Thomas Monjalon
  2 siblings, 3 replies; 26+ messages in thread
From: Hajkowski @ 2019-06-13  9:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Marcin Hajkowski, Lukasz Krakowiak, Lukasz Gosiewski

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

This patch implement a separate FIFO for each cpu core.
For proper handling JSON interface, removed fields from cmds:
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>
---
 examples/vm_power_manager/channel_manager.c | 84 +++++++++++++------
 examples/vm_power_manager/channel_manager.h |  7 +-
 examples/vm_power_manager/channel_monitor.c | 92 +++++++++++++++------
 examples/vm_power_manager/main.c            |  2 +-
 4 files changed, 128 insertions(+), 57 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 81bdf1b84..1a1379bfc 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -346,10 +346,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
@@ -535,40 +547,58 @@ 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;
 
-	fifo_path(socket_path, sizeof(socket_path));
-
-	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);
+	do {
+		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
+			continue;
 
-	if (setup_host_channel_info(&chan_info, 0) < 0) {
-		rte_free(chan_info);
-		return 0;
-	}
-	num_channels_enabled++;
+		ret = fifo_path(socket_path, sizeof(socket_path),
+							num_channels_enabled);
+		if (ret < 0)
+			return 0;
+
+		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));
+			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;
+		}
+		strlcpy(chan_info->channel_path, socket_path,
+				sizeof(chan_info->channel_path));
+
+		if (setup_host_channel_info(&chan_info,
+			num_channels_enabled) < 0) {
+			rte_free(chan_info);
+			return 0;
+		}
+	} while (++num_channels_enabled <= ci->core_count);
 
 	return num_channels_enabled;
 }
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.2


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

* [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl
  2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
@ 2019-06-13  9:21     ` Hajkowski
  2019-07-08 13:34       ` Burakov, Anatoly
  2019-07-04 20:09     ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Thomas Monjalon
  2 siblings, 1 reply; 26+ messages in thread
From: Hajkowski @ 2019-06-13  9:21 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Marcin Hajkowski, Lukasz Krakowiak, Lukasz Gosiewski

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

Updated power management docs for fifo JSON API.
Removed from JSON API:
* 'name'
* 'resource_id'
* 'core_list'

Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
---
 .../sample_app_ug/vm_power_management.rst     | 61 +++++--------------
 1 file changed, 14 insertions(+), 47 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.
-- 
2.17.2


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

* Re: [dpdk-dev] [PATCH v4 0/2] Fifo per core
  2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl Hajkowski
@ 2019-07-04 20:09     ` Thomas Monjalon
  2 siblings, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2019-07-04 20:09 UTC (permalink / raw)
  To: Hajkowski, david.hunt; +Cc: dev

13/06/2019 11:21, Hajkowski:
> Marcin Hajkowski (2):
>   power: add fifo per core for JSON interface
>   doc: update according to the fifo per core impl

We are missing a review of these patches.



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

* Re: [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl Hajkowski
@ 2019-07-08 13:34       ` Burakov, Anatoly
  2019-07-08 15:46         ` Hunt, David
  0 siblings, 1 reply; 26+ messages in thread
From: Burakov, Anatoly @ 2019-07-08 13:34 UTC (permalink / raw)
  To: Hajkowski, david.hunt; +Cc: dev, Lukasz Krakowiak, Lukasz Gosiewski

On 13-Jun-19 10:21 AM, Hajkowski wrote:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> 
> Updated power management docs for fifo JSON API.
> Removed from JSON API:
> * 'name'
> * 'resource_id'
> * 'core_list'
> 
> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
> ---

I think current policy is, doc updates should be part of the patch that 
introduces the changes.

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
@ 2019-07-08 13:44       ` Burakov, Anatoly
  2019-07-09 15:07       ` [dpdk-dev] [PATCH v5] " David Hunt
  2019-07-09 15:21       ` [dpdk-dev] [PATCH v6] " David Hunt
  2 siblings, 0 replies; 26+ messages in thread
From: Burakov, Anatoly @ 2019-07-08 13:44 UTC (permalink / raw)
  To: Hajkowski, david.hunt; +Cc: dev, Lukasz Krakowiak, Lukasz Gosiewski

On 13-Jun-19 10:21 AM, Hajkowski wrote:
> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
> 
> This patch implement a separate FIFO for each cpu core.
> For proper handling JSON interface, removed fields from cmds:
> 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>
> ---

<snip>

> -		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);
> +	do {
> +		if (ci->cd[num_channels_enabled].global_enabled_cpus == 0)
> +			continue;
>   
> -	if (setup_host_channel_info(&chan_info, 0) < 0) {
> -		rte_free(chan_info);
> -		return 0;
> -	}
> -	num_channels_enabled++;
> +		ret = fifo_path(socket_path, sizeof(socket_path),
> +							num_channels_enabled);
> +		if (ret < 0)
> +			return 0;

So if we encounter *any* failure, *all* channels become invalid? Should 
we at least roll back the changes we've made by this point? This is 
consistent with previous behavior so maybe not in this patch, but still...

> +
> +		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));
> +			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;
> +		}

I believe this is not needed. Trying to do this here is a TOCTOU issue, 
and if the access fails on open later, you handle that and free the 
channel info anyway, so this check is essentially useless.

> +		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;
> +		}
> +		strlcpy(chan_info->channel_path, socket_path,
> +				sizeof(chan_info->channel_path));

should this be rte_strlcpy?

> +
> +		if (setup_host_channel_info(&chan_info,
> +			num_channels_enabled) < 0) {
> +			rte_free(chan_info);
> +			return 0;
> +		}
> +	} while (++num_channels_enabled <= ci->core_count);

This looks like a for-loop, why is `while` used here? I mean, i don't 
care either way, it's just a for-loop would have been a more obvious 
choice...


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl
  2019-07-08 13:34       ` Burakov, Anatoly
@ 2019-07-08 15:46         ` Hunt, David
  0 siblings, 0 replies; 26+ messages in thread
From: Hunt, David @ 2019-07-08 15:46 UTC (permalink / raw)
  To: Burakov, Anatoly, Hajkowski; +Cc: dev, Lukasz Krakowiak, Lukasz Gosiewski

Hi Anatoly,

On 08/07/2019 14:34, Burakov, Anatoly wrote:

> On 13-Jun-19 10:21 AM, Hajkowski wrote:
>> From: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>>
>> Updated power management docs for fifo JSON API.
>> Removed from JSON API:
>> * 'name'
>> * 'resource_id'
>> * 'core_list'
>>
>> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski@intel.com>
>> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
>> Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski@intel.com>
>> ---
>
> I think current policy is, doc updates should be part of the patch 
> that introduces the changes.
>

Thanks, I'll squash into a single patch and re-spin.

Rgds,
Dave.


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

* [dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
  2019-07-08 13:44       ` Burakov, Anatoly
@ 2019-07-09 15:07       ` " David Hunt
  2019-07-09 15:12         ` Burakov, Anatoly
  2019-07-09 15:21       ` [dpdk-dev] [PATCH v6] " David Hunt
  2 siblings, 1 reply; 26+ messages in thread
From: David Hunt @ 2019-07-09 15:07 UTC (permalink / raw)
  To: dev
  Cc: david.hunt, lukaszx.gosiewski, anatoly.burakov, Marcin Hajkowski,
	Lukasz Krakowiak

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


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

* Re: [dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface
  2019-07-09 15:07       ` [dpdk-dev] [PATCH v5] " David Hunt
@ 2019-07-09 15:12         ` Burakov, Anatoly
  2019-07-09 15:23           ` Hunt, David
  0 siblings, 1 reply; 26+ messages in thread
From: Burakov, Anatoly @ 2019-07-09 15:12 UTC (permalink / raw)
  To: David Hunt, dev; +Cc: lukaszx.gosiewski, Marcin Hajkowski, Lukasz Krakowiak

On 09-Jul-19 4:07 PM, David Hunt wrote:
> 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.
> ---

<snip>

> -	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++) {

You're going up to RTE_MAX_LCORE here, but up to ci->core_count in the 
original loop. Is that intentional?

Other than that,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH v6] power: add fifo per core for JSON interface
  2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
  2019-07-08 13:44       ` Burakov, Anatoly
  2019-07-09 15:07       ` [dpdk-dev] [PATCH v5] " David Hunt
@ 2019-07-09 15:21       ` " David Hunt
  2019-07-09 15:27         ` Hunt, David
  2019-07-10 21:55         ` Thomas Monjalon
  2 siblings, 2 replies; 26+ messages in thread
From: David Hunt @ 2019-07-09 15:21 UTC (permalink / raw)
  To: dev
  Cc: david.hunt, lukaszx.gosiewski, anatoly.burakov, Marcin Hajkowski,
	Lukasz Krakowiak

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>
Acked-by: Anatoly Burakov <anatoly.burakov@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.

v6:
* Slight optimisation in the range of a for loop.
---
 .../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..2c1332257 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 < ci->core_count; 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


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

* Re: [dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface
  2019-07-09 15:12         ` Burakov, Anatoly
@ 2019-07-09 15:23           ` Hunt, David
  0 siblings, 0 replies; 26+ messages in thread
From: Hunt, David @ 2019-07-09 15:23 UTC (permalink / raw)
  To: Burakov, Anatoly, dev
  Cc: lukaszx.gosiewski, Marcin Hajkowski, Lukasz Krakowiak

Hi Anatoly,

On 09/07/2019 16:12, Burakov, Anatoly wrote:
> On 09-Jul-19 4:07 PM, David Hunt wrote:
>> 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.
>> ---
>
> <snip>
>
>> -    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++) {
>
> You're going up to RTE_MAX_LCORE here, but up to ci->core_count in the 
> original loop. Is that intentional?
>
> Other than that,
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>

Just to be totally clean, I've fixed the loop limit, and respun as v6. :)

Thanks for the review.

Rgds,

Dave.




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

* Re: [dpdk-dev] [PATCH v6] power: add fifo per core for JSON interface
  2019-07-09 15:21       ` [dpdk-dev] [PATCH v6] " David Hunt
@ 2019-07-09 15:27         ` Hunt, David
  2019-07-09 15:38           ` Thomas Monjalon
  2019-07-10 21:55         ` Thomas Monjalon
  1 sibling, 1 reply; 26+ messages in thread
From: Hunt, David @ 2019-07-09 15:27 UTC (permalink / raw)
  To: thomas
  Cc: dev, lukaszx.gosiewski, anatoly.burakov, Marcin Hajkowski,
	Lukasz Krakowiak

Hi Thomas,

    Fyi, I am unable mark v4 as superseded in patchwork 
(http://patches.dpdk.org/project/dpdk/list/?series=4997), although I've 
done that with v5, v6 is the latest version.

Rgds,
Dave.


On 09/07/2019 16:21, David Hunt wrote:
> 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>
> Acked-by: Anatoly Burakov <anatoly.burakov@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.
>
> v6:
> * Slight optimisation in the range of a for loop.
> ---
>   .../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..2c1332257 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 < ci->core_count; 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);

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

* Re: [dpdk-dev] [PATCH v6] power: add fifo per core for JSON interface
  2019-07-09 15:27         ` Hunt, David
@ 2019-07-09 15:38           ` Thomas Monjalon
  0 siblings, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2019-07-09 15:38 UTC (permalink / raw)
  To: Hunt, David
  Cc: dev, lukaszx.gosiewski, anatoly.burakov, Marcin Hajkowski,
	Lukasz Krakowiak

09/07/2019 17:27, Hunt, David:
> Hi Thomas,
> 
>     Fyi, I am unable mark v4 as superseded in patchwork 
> (http://patches.dpdk.org/project/dpdk/list/?series=4997), although I've 
> done that with v5, v6 is the latest version.

OK, done, thanks




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

* Re: [dpdk-dev] [PATCH v6] power: add fifo per core for JSON interface
  2019-07-09 15:21       ` [dpdk-dev] [PATCH v6] " David Hunt
  2019-07-09 15:27         ` Hunt, David
@ 2019-07-10 21:55         ` Thomas Monjalon
  1 sibling, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2019-07-10 21:55 UTC (permalink / raw)
  To: David Hunt, Marcin Hajkowski
  Cc: dev, lukaszx.gosiewski, anatoly.burakov, Lukasz Krakowiak

09/07/2019 17:21, David Hunt:
> 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>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks

PS: I removed a duplicated include of rte_string_fns.h



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

end of thread, back to index

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 10:45 [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Lukasz Krakowiak
2019-04-08 10:45 ` [dpdk-dev] [PATCH 1/2] " Lukasz Krakowiak
2019-04-08 10:45 ` [dpdk-dev] [PATCH 2/2] doc: update according to the fifo per core impl Lukasz Krakowiak
2019-04-12 13:54 ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Lukasz Gosiewski
2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 1/2] " Lukasz Gosiewski
2019-04-12 13:54   ` [dpdk-dev] [PATCH v2 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
2019-04-22 20:37   ` [dpdk-dev] [PATCH v2 0/2] power: add fifo per core for JSON interface Thomas Monjalon
2019-04-24  8:21 ` [dpdk-dev] [PATCH v3 0/2] " Lukasz Gosiewski
2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 1/2] power: " Lukasz Gosiewski
2019-06-07  8:26     ` Hajkowski, MarcinX
2019-04-24  8:21   ` [dpdk-dev] [PATCH v3 2/2] doc: update according to the fifo per core impl Lukasz Gosiewski
2019-06-13  9:21   ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Hajkowski
2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 1/2] power: add fifo per core for JSON interface Hajkowski
2019-07-08 13:44       ` Burakov, Anatoly
2019-07-09 15:07       ` [dpdk-dev] [PATCH v5] " David Hunt
2019-07-09 15:12         ` Burakov, Anatoly
2019-07-09 15:23           ` Hunt, David
2019-07-09 15:21       ` [dpdk-dev] [PATCH v6] " David Hunt
2019-07-09 15:27         ` Hunt, David
2019-07-09 15:38           ` Thomas Monjalon
2019-07-10 21:55         ` Thomas Monjalon
2019-06-13  9:21     ` [dpdk-dev] [PATCH v4 2/2] doc: update according to the fifo per core impl Hajkowski
2019-07-08 13:34       ` Burakov, Anatoly
2019-07-08 15:46         ` Hunt, David
2019-07-04 20:09     ` [dpdk-dev] [PATCH v4 0/2] Fifo per core Thomas Monjalon
2019-06-07  8:50 ` [dpdk-dev] [PATCH 0/2] power: add fifo per core for JSON interface Hajkowski, MarcinX

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox