From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 67BD01559 for ; Tue, 4 Sep 2018 09:33:37 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2018 00:33:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,328,1531810800"; d="scan'208";a="260496110" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga006.fm.intel.com with ESMTP; 04 Sep 2018 00:31:43 -0700 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 4 Sep 2018 00:31:43 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 4 Sep 2018 00:31:42 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.226]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.150]) with mapi id 14.03.0319.002; Tue, 4 Sep 2018 15:31:39 +0800 From: "Yao, Lei A" To: "Hunt, David" , "dev@dpdk.org" CC: "Mcnamara, John" , "Hunt, David" Thread-Topic: [dpdk-dev] [PATCH v1 4/7] examples/power: add host channel to power manager Thread-Index: AQHUQFAR2pddv/BIqka1+84uHcJ0aaTfwDCQ Date: Tue, 4 Sep 2018 07:31:39 +0000 Message-ID: <2DBBFF226F7CF64BAFCA79B681719D953A53C342@shsmsx102.ccr.corp.intel.com> References: <20180830105422.1198-1-david.hunt@intel.com> <20180830105422.1198-5-david.hunt@intel.com> In-Reply-To: <20180830105422.1198-5-david.hunt@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzQ4N2YyZjgtODMzNi00M2Q1LThkODgtNDk0ZGMzYWQyYTg0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZWRETXRRQm1uXC9LaTFoS1ZJVFVHbXlFTUtkSHAzRjN1TWJxcGFLZXR6S1BkU0xNXC9Wbk92VjI5eWprZ2lWdjdBIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 4/7] examples/power: add host channel to power manager 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: , X-List-Received-Date: Tue, 04 Sep 2018 07:33:38 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Hunt > Sent: Thursday, August 30, 2018 6:54 PM > To: dev@dpdk.org > Cc: Mcnamara, John ; Hunt, David > > Subject: [dpdk-dev] [PATCH v1 4/7] examples/power: add host channel to > power manager >=20 > This patch adds a fifo channel to the vm_power_manager app through which > we can send commands and polices. Intended for sending JSON strings. > The fifo is at /tmp/powermonitor/fifo.0 >=20 > Signed-off-by: David Hunt > --- > examples/vm_power_manager/channel_manager.c | 108 > +++++++++++++++ > examples/vm_power_manager/channel_manager.h | 17 ++- > examples/vm_power_manager/channel_monitor.c | 146 > +++++++++++++++----- > examples/vm_power_manager/main.c | 2 + > 4 files changed, 238 insertions(+), 35 deletions(-) >=20 > diff --git a/examples/vm_power_manager/channel_manager.c > b/examples/vm_power_manager/channel_manager.c > index 2bb8641d3..bcd106be1 100644 > --- a/examples/vm_power_manager/channel_manager.c > +++ b/examples/vm_power_manager/channel_manager.c > @@ -13,6 +13,7 @@ >=20 > #include > #include > +#include > #include > #include >=20 > @@ -284,6 +285,38 @@ open_non_blocking_channel(struct channel_info > *info) > return 0; > } >=20 > +static int > +open_host_channel(struct channel_info *info) > +{ > + int flags; > + > + info->fd =3D open(info->channel_path, O_RDWR | O_RSYNC); > + if (info->fd =3D=3D -1) { > + RTE_LOG(ERR, CHANNEL_MANAGER, "Error(%s) opening fifo > for '%s'\n", > + strerror(errno), > + info->channel_path); > + return -1; > + } > + > + /* Get current flags */ > + flags =3D fcntl(info->fd, F_GETFL, 0); > + if (flags < 0) { > + RTE_LOG(WARNING, CHANNEL_MANAGER, "Error(%s) fcntl > get flags socket for" > + "'%s'\n", strerror(errno), info- > >channel_path); > + return 1; > + } > + /* Set to Non Blocking */ > + flags |=3D O_NONBLOCK; > + if (fcntl(info->fd, F_SETFL, flags) < 0) { > + RTE_LOG(WARNING, CHANNEL_MANAGER, > + "Error(%s) setting non-blocking " > + "socket for '%s'\n", > + strerror(errno), info->channel_path); > + return -1; > + } > + return 0; > +} > + > static int > setup_channel_info(struct virtual_machine_info **vm_info_dptr, > struct channel_info **chan_info_dptr, unsigned > channel_num) > @@ -294,6 +327,7 @@ setup_channel_info(struct virtual_machine_info > **vm_info_dptr, > chan_info->channel_num =3D channel_num; > chan_info->priv_info =3D (void *)vm_info; > chan_info->status =3D CHANNEL_MGR_CHANNEL_DISCONNECTED; > + chan_info->type =3D CHANNEL_TYPE_BINARY; > if (open_non_blocking_channel(chan_info) < 0) { > RTE_LOG(ERR, CHANNEL_MANAGER, "Could not open > channel: " > "'%s' for VM '%s'\n", > @@ -316,6 +350,35 @@ setup_channel_info(struct virtual_machine_info > **vm_info_dptr, > return 0; > } >=20 > +static int > +setup_host_channel_info(struct channel_info **chan_info_dptr, > + unsigned int channel_num) > +{ > + struct channel_info *chan_info =3D *chan_info_dptr; > + > + chan_info->channel_num =3D channel_num; > + chan_info->priv_info =3D (void *)0; > + chan_info->status =3D CHANNEL_MGR_CHANNEL_DISCONNECTED; > + chan_info->type =3D CHANNEL_TYPE_JSON; > + sprintf(chan_info->channel_path, "%sfifo.0", > CHANNEL_MGR_SOCKET_PATH); > + > + if (open_host_channel(chan_info) < 0) { > + RTE_LOG(ERR, CHANNEL_MANAGER, "Could not open host > channel: " > + "'%s'\n", > + chan_info->channel_path); > + return -1; > + } > + if (add_channel_to_monitor(&chan_info) < 0) { > + RTE_LOG(ERR, CHANNEL_MANAGER, "Could add channel: " > + "'%s' to epoll ctl\n", > + chan_info->channel_path); > + return -1; > + > + } > + chan_info->status =3D CHANNEL_MGR_CHANNEL_CONNECTED; > + return 0; > +} > + > int > add_all_channels(const char *vm_name) > { > @@ -470,6 +533,51 @@ add_channels(const char *vm_name, unsigned > *channel_list, > return num_channels_enabled; > } >=20 > +int > +add_host_channel(void) > +{ > + struct channel_info *chan_info; > + char socket_path[PATH_MAX]; > + int num_channels_enabled =3D 0; > + int ret; > + > + snprintf(socket_path, sizeof(socket_path), "%sfifo.%u", > + CHANNEL_MGR_SOCKET_PATH, 0); > + > + errno =3D 0; > + ret =3D mkfifo(socket_path, 0666); > + if ((errno !=3D EEXIST) && (ret < 0)) { > + printf(" %d %d, %d\n", ret, EEXIST, errno); > + RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot create fifo '%s' > error: " > + "%s\n", socket_path, strerror(errno)); > + return 0; > + } > + > + errno =3D 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 =3D rte_malloc(NULL, sizeof(*chan_info), > + RTE_CACHE_LINE_SIZE); > + if (chan_info =3D=3D 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), "%sfifo.%u", > + CHANNEL_MGR_SOCKET_PATH, 0); > + if (setup_host_channel_info(&chan_info, 0) < 0) { > + rte_free(chan_info); > + return 0; > + } > + num_channels_enabled++; > + > + return num_channels_enabled; > +} > + > int > remove_channel(struct channel_info **chan_info_dptr) > { > diff --git a/examples/vm_power_manager/channel_manager.h > b/examples/vm_power_manager/channel_manager.h > index 872ec6140..c157cc22b 100644 > --- a/examples/vm_power_manager/channel_manager.h > +++ b/examples/vm_power_manager/channel_manager.h > @@ -37,7 +37,7 @@ struct sockaddr_un _sockaddr_un; > #define UNIX_PATH_MAX sizeof(_sockaddr_un.sun_path) > #endif >=20 > -#define MAX_VMS 4 > +#define MAX_VMS 64 > #define MAX_VCPUS 20 >=20 >=20 > @@ -54,6 +54,11 @@ enum channel_status > { CHANNEL_MGR_CHANNEL_DISCONNECTED =3D 0, > CHANNEL_MGR_CHANNEL_DISABLED, > CHANNEL_MGR_CHANNEL_PROCESSING}; >=20 > +/* Communication Channel Type */ > +enum channel_type { CHANNEL_TYPE_BINARY =3D 0, > + CHANNEL_TYPE_INI, > + CHANNEL_TYPE_JSON}; > + > /* VM libvirt(qemu/KVM) connection status */ > enum vm_status { CHANNEL_MGR_VM_INACTIVE =3D 0, > CHANNEL_MGR_VM_ACTIVE}; >=20 > @@ -66,6 +71,7 @@ struct channel_info { > volatile uint32_t status; /**< Connection status(enum > channel_status) */ > int fd; /**< AF_UNIX socket fd */ > unsigned channel_num; /**< > CHANNEL_MGR_SOCKET_PATH/.channel_num */ > + enum channel_type type; /**< Binary, ini, json, etc. */ > void *priv_info; /**< Pointer to private info, do not modif= y */ > }; >=20 > @@ -226,6 +232,15 @@ int add_all_channels(const char *vm_name); > int add_channels(const char *vm_name, unsigned *channel_list, > unsigned num_channels); >=20 > +/** > + * Set up a fifo 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); > + > /** > * Remove a channel definition from the channel manager. This must only = be > * called from the channel monitor thread. > diff --git a/examples/vm_power_manager/channel_monitor.c > b/examples/vm_power_manager/channel_monitor.c > index f180d74e6..0ffa1112a 100644 > --- a/examples/vm_power_manager/channel_monitor.c > +++ b/examples/vm_power_manager/channel_monitor.c > @@ -85,6 +85,33 @@ core_share_status(int pNo) > } > } >=20 > + > +static int > +pcpu_monitor(struct policy *pol, struct core_info *ci, int pcpu, int cou= nt) > +{ > + int ret =3D 0; > + > + if (pol->pkt.policy_to_use =3D=3D BRANCH_RATIO) { > + ci->cd[pcpu].oob_enabled =3D 1; > + ret =3D add_core_to_monitor(pcpu); > + if (ret =3D=3D 0) > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "Monitoring pcpu %d OOB for %s\n", > + pcpu, pol->pkt.vm_name); > + else > + RTE_LOG(ERR, CHANNEL_MONITOR, > + "Error monitoring pcpu %d OOB > for %s\n", > + pcpu, pol->pkt.vm_name); > + > + } else { > + pol->core_share[count].pcpu =3D pcpu; > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "Monitoring pcpu %d for %s\n", > + pcpu, pol->pkt.vm_name); > + } > + return ret; > +} > + > static void > get_pcpu_to_control(struct policy *pol) > { > @@ -94,34 +121,46 @@ get_pcpu_to_control(struct policy *pol) > int pcpu, count; > uint64_t mask_u64b; > struct core_info *ci; > - int ret; >=20 > ci =3D get_core_info(); >=20 > - RTE_LOG(INFO, CHANNEL_MONITOR, "Looking for pcpu for %s\n", > - pol->pkt.vm_name); > - get_info_vm(pol->pkt.vm_name, &info); > - > - for (count =3D 0; count < pol->pkt.num_vcpu; count++) { > - mask_u64b =3D info.pcpu_mask[pol- > >pkt.vcpu_to_control[count]]; > - for (pcpu =3D 0; mask_u64b; mask_u64b &=3D ~(1ULL << pcpu++)) > { > - if ((mask_u64b >> pcpu) & 1) { > - if (pol->pkt.policy_to_use =3D=3D BRANCH_RATIO) > { > - ci->cd[pcpu].oob_enabled =3D 1; > - ret =3D add_core_to_monitor(pcpu); > - if (ret =3D=3D 0) > - printf("Monitoring pcpu %d > via Branch Ratio\n", > - pcpu); > - else > - printf("Failed to start OOB > Monitoring pcpu %d\n", > - pcpu); > - > - } else { > - pol->core_share[count].pcpu =3D pcpu; > - printf("Monitoring pcpu %d\n", pcpu); > - } > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "Looking for pcpu for %s\n", pol->pkt.vm_name); > + > + /* > + * So now that we're handling virtual and physical cores, we need to > + * differenciate between them when adding them to the branch > monitor. > + * Virtual cores need to be converted to physical cores. > + */ > + > + > + > + > + if (pol->pkt.core_type =3D=3D VIRTUAL) { > + /* > + * If the cores in the policy are virtual, we need to map them > + * to physical core. We look up the vm info and use that for > + * the mapping. > + */ > + get_info_vm(pol->pkt.vm_name, &info); > + for (count =3D 0; count < pol->pkt.num_vcpu; count++) { > + mask_u64b =3D > + info.pcpu_mask[pol- > >pkt.vcpu_to_control[count]]; > + for (pcpu =3D 0; mask_u64b; > + mask_u64b &=3D ~(1ULL << pcpu++)) { > + if ((mask_u64b >> pcpu) & 1) > + pcpu_monitor(pol, ci, pcpu, count); > } > } > + } else { > + /* > + * If the cores in the policy are physical, we just use > + * those core id's directly. > + */ > + for (count =3D 0; count < pol->pkt.num_vcpu; count++) { > + pcpu =3D pol->pkt.vcpu_to_control[count]; > + pcpu_monitor(pol, ci, pcpu, count); > + } > } > } >=20 > @@ -160,8 +199,13 @@ update_policy(struct channel_packet *pkt) > unsigned int updated =3D 0; > int i; >=20 > + > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "Updating policy for %s\n", pkt->vm_name); > + > for (i =3D 0; i < MAX_VMS; i++) { > if (strcmp(policies[i].pkt.vm_name, pkt->vm_name) =3D=3D 0) { I suggest add warning log here when no VM can match the policy name which we send through the fifo.0. Otherwise, the user can't aware the=20 policy won't be applied. =20 > + /* Copy the contents of *pkt into the policy.pkt */ > policies[i].pkt =3D *pkt; > get_pcpu_to_control(&policies[i]); > if (get_pfid(&policies[i]) =3D=3D -1) { > @@ -189,6 +233,24 @@ update_policy(struct channel_packet *pkt) > return 0; > } >=20 > +static int > +remove_policy(struct channel_packet *pkt __rte_unused) > +{ > + int i; > + > + /* > + * Disabling the policy is simply a case of setting > + * enabled to 0 > + */ > + for (i =3D 0; i < MAX_VMS; i++) { > + if (strcmp(policies[i].pkt.vm_name, pkt->vm_name) =3D=3D 0) { > + policies[i].enabled =3D 0; > + return 0; > + } > + } > + return -1; > +} > + > static uint64_t > get_pkt_diff(struct policy *pol) > { > @@ -346,7 +408,6 @@ apply_policy(struct policy *pol) > apply_workload_profile(pol); > } >=20 > - > static int > process_request(struct channel_packet *pkt, struct channel_info > *chan_info) > { > @@ -355,6 +416,8 @@ process_request(struct channel_packet *pkt, struct > channel_info *chan_info) > if (chan_info =3D=3D NULL) > return -1; >=20 > + RTE_LOG(INFO, CHANNEL_MONITOR, "Processing Request %s\n", > pkt->vm_name); > + > if (rte_atomic32_cmpset(&(chan_info->status), > CHANNEL_MGR_CHANNEL_CONNECTED, > CHANNEL_MGR_CHANNEL_PROCESSING) =3D=3D 0) > return -1; > @@ -362,10 +425,12 @@ process_request(struct channel_packet *pkt, > struct channel_info *chan_info) > if (pkt->command =3D=3D CPU_POWER) { > core_mask =3D get_pcpus_mask(chan_info, pkt->resource_id); > if (core_mask =3D=3D 0) { > - RTE_LOG(ERR, CHANNEL_MONITOR, "Error get > physical CPU mask for " > - "channel '%s' using vCPU(%u)\n", chan_info- > >channel_path, > - (unsigned)pkt->unit); > - return -1; > + /* > + * Core mask will be 0 in the case where > + * hypervisor is not available so we're working in > + * the host, so use the core as the mask. > + */ > + core_mask =3D 1 << pkt->resource_id; > } > if (__builtin_popcountll(core_mask) =3D=3D 1) { >=20 > @@ -421,12 +486,20 @@ process_request(struct channel_packet *pkt, > struct channel_info *chan_info) > } >=20 > if (pkt->command =3D=3D PKT_POLICY) { > - RTE_LOG(INFO, CHANNEL_MONITOR, "\nProcessing Policy > request from Guest\n"); > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "\nProcessing Policy request\n"); > update_policy(pkt); > policy_is_set =3D 1; > } >=20 > - /* Return is not checked as channel status may have been set to > DISABLED > + if (pkt->command =3D=3D PKT_POLICY_REMOVE) { > + RTE_LOG(INFO, CHANNEL_MONITOR, > + "Removing policy %s\n", pkt->vm_name); > + remove_policy(pkt); > + } > + > + /* > + * Return is not checked as channel status may have been set to > DISABLED > * from management thread > */ > rte_atomic32_cmpset(&(chan_info->status), > CHANNEL_MGR_CHANNEL_PROCESSING, > @@ -448,13 +521,16 @@ add_channel_to_monitor(struct channel_info > **chan_info) > "to epoll\n", info->channel_path); > return -1; > } > + RTE_LOG(ERR, CHANNEL_MONITOR, "Added channel '%s' " > + "to monitor\n", info->channel_path); > return 0; > } >=20 > int > remove_channel_from_monitor(struct channel_info *chan_info) > { > - if (epoll_ctl(global_event_fd, EPOLL_CTL_DEL, chan_info->fd, NULL) > < 0) { > + if (epoll_ctl(global_event_fd, EPOLL_CTL_DEL, > + chan_info->fd, NULL) < 0) { > RTE_LOG(ERR, CHANNEL_MONITOR, "Unable to remove > channel '%s' " > "from epoll\n", chan_info->channel_path); > return -1; > @@ -467,11 +543,13 @@ channel_monitor_init(void) > { > global_event_fd =3D epoll_create1(0); > if (global_event_fd =3D=3D 0) { > - RTE_LOG(ERR, CHANNEL_MONITOR, "Error creating epoll > context with " > - "error %s\n", strerror(errno)); > + RTE_LOG(ERR, CHANNEL_MONITOR, > + "Error creating epoll context with error %s\n", > + strerror(errno)); > return -1; > } > - global_events_list =3D rte_malloc("epoll_events", > sizeof(*global_events_list) > + global_events_list =3D rte_malloc("epoll_events", > + sizeof(*global_events_list) > * MAX_EVENTS, RTE_CACHE_LINE_SIZE); > if (global_events_list =3D=3D NULL) { > RTE_LOG(ERR, CHANNEL_MONITOR, "Unable to rte_malloc > for " > diff --git a/examples/vm_power_manager/main.c > b/examples/vm_power_manager/main.c > index 58c5fa45c..893bf4cdd 100644 > --- a/examples/vm_power_manager/main.c > +++ b/examples/vm_power_manager/main.c > @@ -421,6 +421,8 @@ main(int argc, char **argv) > return -1; > } >=20 > + add_host_channel(); > + > printf("Running core monitor on lcore id %d\n", lcore_id); > rte_eal_remote_launch(run_core_monitor, NULL, lcore_id); >=20 > -- > 2.17.1