From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BDB6CA00E6 for ; Tue, 9 Jul 2019 17:08:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 13AB14C8D; Tue, 9 Jul 2019 17:08:17 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id E1184322C for ; Tue, 9 Jul 2019 17:08:14 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jul 2019 08:08:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,470,1557212400"; d="scan'208";a="317053007" Received: from silpixa00399952.ir.intel.com (HELO silpixa00399952.ger.corp.intel.com) ([10.237.222.88]) by orsmga004.jf.intel.com with ESMTP; 09 Jul 2019 08:08:11 -0700 From: David Hunt To: dev@dpdk.org Cc: david.hunt@intel.com, lukaszx.gosiewski@intel.com, anatoly.burakov@intel.com, Marcin Hajkowski , Lukasz Krakowiak Date: Tue, 9 Jul 2019 16:07:53 +0100 Message-Id: <20190709150753.15732-1-david.hunt@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190613092117.7252-2-marcinx.hajkowski@intel.com> References: <20190613092117.7252-2-marcinx.hajkowski@intel.com> Subject: [dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Marcin Hajkowski 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 Signed-off-by: Lukasz Gosiewski Signed-off-by: Marcin Hajkowski Tested-by: David Hunt --- 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 #include #include +#include #include #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