[v3,5/8] examples/power: add json string handling

Message ID 20180914135406.52190-6-david.hunt@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add json power policy interface for containers |

Checks

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

Commit Message

Hunt, David Sept. 14, 2018, 1:54 p.m. UTC
  Add JSON string handling to vm_power_manager for JSON strings received
through the fifo. The format of the JSON strings are detailed in the
next patch, the vm_power_manager user guide documentation updates.

This patch introduces a new dependency on Jansson, a C library for
encoding, decoding and manipulating JSON data. To compile the sample app
you now need to have installed libjansson4 and libjansson-dev (these may
be named slightly differently depending on your Operating System)

Signed-off-by: David Hunt <david.hunt@intel.com>
---
 examples/vm_power_manager/Makefile          |   6 +
 examples/vm_power_manager/channel_monitor.c | 297 +++++++++++++++++---
 2 files changed, 258 insertions(+), 45 deletions(-)
  

Comments

Burakov, Anatoly Sept. 25, 2018, 11:27 a.m. UTC | #1
On 14-Sep-18 2:54 PM, David Hunt wrote:
> Add JSON string handling to vm_power_manager for JSON strings received
> through the fifo. The format of the JSON strings are detailed in the
> next patch, the vm_power_manager user guide documentation updates.
> 
> This patch introduces a new dependency on Jansson, a C library for
> encoding, decoding and manipulating JSON data. To compile the sample app
> you now need to have installed libjansson4 and libjansson-dev (these may
> be named slightly differently depending on your Operating System)
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>
> ---

<snip>

>   void channel_monitor_exit(void)
>   {
>   	run_loop = 0;
> @@ -124,18 +259,11 @@ get_pcpu_to_control(struct policy *pol)
>   
>   	ci = get_core_info();
>   
> -	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.
>   	 */
> -
> -
> -
> -

Now you're removing those newlines you added in previous commit :)

>   	if (pol->pkt.core_type == CORE_TYPE_VIRTUAL) {
>   		/*
>   		 * If the cores in the policy are virtual, we need to map them
> @@ -295,8 +423,6 @@ apply_traffic_profile(struct policy *pol)
>   
>   	diff = get_pkt_diff(pol);
>   
> -	RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
> -

Here and in a few other places: these log message removals look to be 
unrelated to this commit. Also, in my experience, more logging is better 
than less logging, especially when something goes wrong - maybe instead 
of removing them, just switch the level to debug?

>   	if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
>   		for (count = 0; count < pol->pkt.num_vcpu; count++) {
>   			if (pol->core_share[count].status != 1)
> @@ -340,9 +466,6 @@ apply_time_profile(struct policy *pol)
>   				if (pol->core_share[count].status != 1) {
>   					power_manager_scale_core_max(
>   						pol->core_share[count].pcpu);
> -				RTE_LOG(INFO, CHANNEL_MONITOR,

<snip>

> +		int idx = 0;
> +		int indent = 0;
> +		do {
> +			n_bytes = read(chan_info->fd, &json_data[idx], 1);
> +			if (n_bytes == 0)
> +				break;
> +			if (json_data[idx] == '{')
> +				indent++;
> +			if (json_data[idx] == '}')
> +				indent--;

What happens if someone sends a string with a "{" or "}" inside?

> +			if ((indent > 0) || (idx >> 0))

idx > 0?

> +				idx++;
> +			if (indent == 0)
> +				json_data[idx] = 0;
> +			if (idx >= MAX_JSON_STRING_LEN)
> +				break;

This looks like a potential overflow to me, because you increment idx, 
check if it's >= max, but still write into that location if indent == 0 
on previous line.

> +		} while (indent > 0);
> +
> +		if (indent > 0)
> +			/*
> +			 * We've broken out of the read loop without getting
> +			 * a closing brace, so throw away the datai

I think "data" is plural already, no need to invent a new word :)

> +			 */
> +			json_data[idx] = 0;

idx could potentially be overflown already due to code above?

> +
> +		if (strlen(json_data) == 0)
> +			continue;
> +
> +		printf("got [%s]\n", json_data);
> +

<snip>

>   void
>   run_channel_monitor(void)
>   {
> @@ -570,11 +785,16 @@ run_channel_monitor(void)
>   		if (!run_loop)
>   			break;
>   		for (i = 0; i < n_events; i++) {
> +			if (!global_events_list[i].data.ptr) {
> +				fflush(stdout);
> +				continue;
> +			}

This change looks unrelated to this commit.

>   			struct channel_info *chan_info = (struct channel_info *)
>   					global_events_list[i].data.ptr;
>   			if ((global_events_list[i].events & EPOLLERR) ||
  
Hunt, David Sept. 25, 2018, 2 p.m. UTC | #2
Hi Anatoly,


On 25/9/2018 12:27 PM, Burakov, Anatoly wrote:
> On 14-Sep-18 2:54 PM, David Hunt wrote:
>> Add JSON string handling to vm_power_manager for JSON strings received
>> through the fifo. The format of the JSON strings are detailed in the
>> next patch, the vm_power_manager user guide documentation updates.
>>
>> This patch introduces a new dependency on Jansson, a C library for
>> encoding, decoding and manipulating JSON data. To compile the sample app
>> you now need to have installed libjansson4 and libjansson-dev (these may
>> be named slightly differently depending on your Operating System)
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>
> <snip>
>
>>   void channel_monitor_exit(void)
>>   {
>>       run_loop = 0;
>> @@ -124,18 +259,11 @@ get_pcpu_to_control(struct policy *pol)
>>         ci = get_core_info();
>>   -    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.
>>        */
>> -
>> -
>> -
>> -
>
> Now you're removing those newlines you added in previous commit :)
>

Fixed in previous patch in the next version.


>>       if (pol->pkt.core_type == CORE_TYPE_VIRTUAL) {
>>           /*
>>            * If the cores in the policy are virtual, we need to map them
>> @@ -295,8 +423,6 @@ apply_traffic_profile(struct policy *pol)
>>         diff = get_pkt_diff(pol);
>>   -    RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
>> -
>
> Here and in a few other places: these log message removals look to be 
> unrelated to this commit. Also, in my experience, more logging is 
> better than less logging, especially when something goes wrong - maybe 
> instead of removing them, just switch the level to debug?
>

Changed to Debug instead. Was causing quite a verbose output.

>>       if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
>>           for (count = 0; count < pol->pkt.num_vcpu; count++) {
>>               if (pol->core_share[count].status != 1)
>> @@ -340,9 +466,6 @@ apply_time_profile(struct policy *pol)
>>                   if (pol->core_share[count].status != 1) {
>>                       power_manager_scale_core_max(
>>                           pol->core_share[count].pcpu);
>> -                RTE_LOG(INFO, CHANNEL_MONITOR,
>
> <snip>
>
>> +        int idx = 0;
>> +        int indent = 0;
>> +        do {
>> +            n_bytes = read(chan_info->fd, &json_data[idx], 1);
>> +            if (n_bytes == 0)
>> +                break;
>> +            if (json_data[idx] == '{')
>> +                indent++;
>> +            if (json_data[idx] == '}')
>> +                indent--;
>
> What happens if someone sends a string with a "{" or "}" inside?
>

If we get to the end of the buffer without a "}", it calls the library 
to convert, will fail, and move on.  No damage done (I hope).
Also, a short un-terminated (by "}") string will also exit when no 
characters read.
So any invalid JSON string that's send to Jansson will fail to parse, 
and the application will be ready for the next (hopefully valid) JSON 
string.

>> +            if ((indent > 0) || (idx >> 0))
>
> idx > 0?
>

Yes, will fix.

>> +                idx++;
>> +            if (indent == 0)
>> +                json_data[idx] = 0;
>> +            if (idx >= MAX_JSON_STRING_LEN)
>> +                break;
>
> This looks like a potential overflow to me, because you increment idx, 
> check if it's >= max, but still write into that location if indent == 
> 0 on previous line.
>

Yes, will exit at MAX_JSON_STRING_LEN-1

>> +        } while (indent > 0);
>> +
>> +        if (indent > 0)
>> +            /*
>> +             * We've broken out of the read loop without getting
>> +             * a closing brace, so throw away the datai
>
> I think "data" is plural already, no need to invent a new word :)
>
>> +             */
>> +            json_data[idx] = 0;
>
> idx could potentially be overflown already due to code above?
>

Typo fixed in next version.

>> +
>> +        if (strlen(json_data) == 0)
>> +            continue;
>> +
>> +        printf("got [%s]\n", json_data);
>> +
>
> <snip>
>
>>   void
>>   run_channel_monitor(void)
>>   {
>> @@ -570,11 +785,16 @@ run_channel_monitor(void)
>>           if (!run_loop)
>>               break;
>>           for (i = 0; i < n_events; i++) {
>> +            if (!global_events_list[i].data.ptr) {
>> +                fflush(stdout);
>> +                continue;
>> +            }
>
> This change looks unrelated to this commit.

There was a few printfs in there, this change will be removed altogether 
in next version.

>
>>               struct channel_info *chan_info = (struct channel_info *)
>>                       global_events_list[i].data.ptr;
>>               if ((global_events_list[i].events & EPOLLERR) ||
>

Thanks,
Dave.
  
Burakov, Anatoly Sept. 25, 2018, 2:15 p.m. UTC | #3
On 25-Sep-18 3:00 PM, Hunt, David wrote:
>> Now you're removing those newlines you added in previous commit :)
>>
> 
> Fixed in previous patch in the next version.
> 
> 
>>>       if (pol->pkt.core_type == CORE_TYPE_VIRTUAL) {
>>>           /*
>>>            * If the cores in the policy are virtual, we need to map them
>>> @@ -295,8 +423,6 @@ apply_traffic_profile(struct policy *pol)
>>>         diff = get_pkt_diff(pol);
>>>   -    RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
>>> -
>>
>> Here and in a few other places: these log message removals look to be 
>> unrelated to this commit. Also, in my experience, more logging is 
>> better than less logging, especially when something goes wrong - maybe 
>> instead of removing them, just switch the level to debug?
>>
> 
> Changed to Debug instead. Was causing quite a verbose output.

Hopefully in a separate commit :)

> 
>>>       if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
>>>           for (count = 0; count < pol->pkt.num_vcpu; count++) {
>>>               if (pol->core_share[count].status != 1)
>>> @@ -340,9 +466,6 @@ apply_time_profile(struct policy *pol)
>>>                   if (pol->core_share[count].status != 1) {
>>>                       power_manager_scale_core_max(
>>>                           pol->core_share[count].pcpu);
>>> -                RTE_LOG(INFO, CHANNEL_MONITOR,
>>
>> <snip>
>>
>>> +        int idx = 0;
>>> +        int indent = 0;
>>> +        do {
>>> +            n_bytes = read(chan_info->fd, &json_data[idx], 1);
>>> +            if (n_bytes == 0)
>>> +                break;
>>> +            if (json_data[idx] == '{')
>>> +                indent++;
>>> +            if (json_data[idx] == '}')
>>> +                indent--;
>>
>> What happens if someone sends a string with a "{" or "}" inside?
>>
> 
> If we get to the end of the buffer without a "}", it calls the library 
> to convert, will fail, and move on.  No damage done (I hope).
> Also, a short un-terminated (by "}") string will also exit when no 
> characters read.
> So any invalid JSON string that's send to Jansson will fail to parse, 
> and the application will be ready for the next (hopefully valid) JSON 
> string.

No, what i meant is something like this:

{ "json_value": "{"}

According to JSON validator, this is a valid JSON string, but it will 
break your code :)
  
Hunt, David Sept. 25, 2018, 3:15 p.m. UTC | #4
On 25/9/2018 3:15 PM, Burakov, Anatoly wrote:
> On 25-Sep-18 3:00 PM, Hunt, David wrote:
>>> Now you're removing those newlines you added in previous commit :)
>>>
>>
>> Fixed in previous patch in the next version.
>>
>>
>>>>       if (pol->pkt.core_type == CORE_TYPE_VIRTUAL) {
>>>>           /*
>>>>            * If the cores in the policy are virtual, we need to map 
>>>> them
>>>> @@ -295,8 +423,6 @@ apply_traffic_profile(struct policy *pol)
>>>>         diff = get_pkt_diff(pol);
>>>>   -    RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
>>>> -
>>>
>>> Here and in a few other places: these log message removals look to 
>>> be unrelated to this commit. Also, in my experience, more logging is 
>>> better than less logging, especially when something goes wrong - 
>>> maybe instead of removing them, just switch the level to debug?
>>>
>>
>> Changed to Debug instead. Was causing quite a verbose output.
>
> Hopefully in a separate commit :)
>

Of Course. :)

>>
>>>>       if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
>>>>           for (count = 0; count < pol->pkt.num_vcpu; count++) {
>>>>               if (pol->core_share[count].status != 1)
>>>> @@ -340,9 +466,6 @@ apply_time_profile(struct policy *pol)
>>>>                   if (pol->core_share[count].status != 1) {
>>>>                       power_manager_scale_core_max(
>>>>                           pol->core_share[count].pcpu);
>>>> -                RTE_LOG(INFO, CHANNEL_MONITOR,
>>>
>>> <snip>
>>>
>>>> +        int idx = 0;
>>>> +        int indent = 0;
>>>> +        do {
>>>> +            n_bytes = read(chan_info->fd, &json_data[idx], 1);
>>>> +            if (n_bytes == 0)
>>>> +                break;
>>>> +            if (json_data[idx] == '{')
>>>> +                indent++;
>>>> +            if (json_data[idx] == '}')
>>>> +                indent--;
>>>
>>> What happens if someone sends a string with a "{" or "}" inside?
>>>
>>
>> If we get to the end of the buffer without a "}", it calls the 
>> library to convert, will fail, and move on.  No damage done (I hope).
>> Also, a short un-terminated (by "}") string will also exit when no 
>> characters read.
>> So any invalid JSON string that's send to Jansson will fail to parse, 
>> and the application will be ready for the next (hopefully valid) JSON 
>> string.
>
> No, what i meant is something like this:
>
> { "json_value": "{"}
>
> According to JSON validator, this is a valid JSON string, but it will 
> break your code :)
>

You are correct if this code was designed to be a general purpose JSON 
string reader. However, it's only designed to take in strings for this 
sample application, and they do not expect any brace characters embedded 
within quotes. So I think it's OK for this use case. Patches welcome, 
though! :)

Thanks,
Dave.
  
Burakov, Anatoly Sept. 25, 2018, 3:31 p.m. UTC | #5
On 25-Sep-18 4:15 PM, Hunt, David wrote:

>>>> What happens if someone sends a string with a "{" or "}" inside?
>>>>
>>>
>>> If we get to the end of the buffer without a "}", it calls the 
>>> library to convert, will fail, and move on.  No damage done (I hope).
>>> Also, a short un-terminated (by "}") string will also exit when no 
>>> characters read.
>>> So any invalid JSON string that's send to Jansson will fail to parse, 
>>> and the application will be ready for the next (hopefully valid) JSON 
>>> string.
>>
>> No, what i meant is something like this:
>>
>> { "json_value": "{"}
>>
>> According to JSON validator, this is a valid JSON string, but it will 
>> break your code :)
>>
> 
> You are correct if this code was designed to be a general purpose JSON 
> string reader. However, it's only designed to take in strings for this 
> sample application, and they do not expect any brace characters embedded 
> within quotes. So I think it's OK for this use case. Patches welcome, 
> though! :)
> 

Fair enough :)

> Thanks,
> Dave.
> 
> 
>
  

Patch

diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile
index 13a5205ba..50147c05d 100644
--- a/examples/vm_power_manager/Makefile
+++ b/examples/vm_power_manager/Makefile
@@ -31,6 +31,12 @@  CFLAGS += $(WERROR_FLAGS)
 
 LDLIBS += -lvirt
 
+JANSSON := $(shell pkg-config --exists jansson; echo $$?)
+ifeq ($(JANSSON), 0)
+LDLIBS += $(shell pkg-config --libs jansson)
+CFLAGS += -DUSE_JANSSON
+endif
+
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 
 ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 17383e9d2..655f5e279 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -9,11 +9,18 @@ 
 #include <signal.h>
 #include <errno.h>
 #include <string.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/epoll.h>
 #include <sys/queue.h>
 #include <sys/time.h>
-
+#include <sys/socket.h>
+#include <sys/select.h>
+#ifdef USE_JANSSON
+#include <jansson.h>
+#else
+#pragma message "Jansson dev libs unavailable, not including JSON parsing"
+#endif
 #include <rte_log.h>
 #include <rte_memory.h>
 #include <rte_malloc.h>
@@ -35,6 +42,8 @@ 
 
 uint64_t vsi_pkt_count_prev[384];
 uint64_t rdtsc_prev[384];
+#define MAX_JSON_STRING_LEN 1024
+char json_data[MAX_JSON_STRING_LEN];
 
 double time_period_ms = 1;
 static volatile unsigned run_loop = 1;
@@ -43,6 +52,132 @@  static unsigned int policy_is_set;
 static struct epoll_event *global_events_list;
 static struct policy policies[MAX_VMS];
 
+#ifdef USE_JANSSON
+static int
+parse_json_to_pkt(json_t *element, struct channel_packet *pkt)
+{
+	const char *key;
+	json_t *value;
+	int ret;
+
+	memset(pkt, 0, sizeof(struct channel_packet));
+
+	pkt->nb_mac_to_monitor = 0;
+	pkt->t_boost_status.tbEnabled = false;
+	pkt->workload = LOW;
+	pkt->policy_to_use = TIME;
+	pkt->command = PKT_POLICY;
+	pkt->core_type = CORE_TYPE_PHYSICAL;
+
+	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);
+			if (ret)
+				return ret;
+		} else if (!strcmp(key, "instruction")) {
+			/* Recurse in to get the contents of instruction */
+			ret = parse_json_to_pkt(value, pkt);
+			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));
+			if (!strcmp(command, "power")) {
+				pkt->command = CPU_POWER;
+			} else if (!strcmp(command, "create")) {
+				pkt->command = PKT_POLICY;
+			} else if (!strcmp(command, "destroy")) {
+				pkt->command = PKT_POLICY_REMOVE;
+			} else {
+				RTE_LOG(ERR, CHANNEL_MONITOR,
+					"Invalid command received in JSON\n");
+				return -1;
+			}
+		} else if (!strcmp(key, "policy_type")) {
+			char command[32];
+			snprintf(command, 32, "%s", json_string_value(value));
+			if (!strcmp(command, "TIME")) {
+				pkt->policy_to_use = TIME;
+			} else if (!strcmp(command, "TRAFFIC")) {
+				pkt->policy_to_use = TRAFFIC;
+			} else if (!strcmp(command, "WORKLOAD")) {
+				pkt->policy_to_use = WORKLOAD;
+			} else if (!strcmp(command, "BRANCH_RATIO")) {
+				pkt->policy_to_use = BRANCH_RATIO;
+			} else {
+				RTE_LOG(ERR, CHANNEL_MONITOR,
+					"Wrong policy_type received in JSON\n");
+				return -1;
+			}
+		} else if (!strcmp(key, "busy_hours")) {
+			unsigned int i;
+			size_t size = json_array_size(value);
+
+			for (i = 0; i < size; i++) {
+				int hour = (int)json_integer_value(
+						json_array_get(value, i));
+				pkt->timer_policy.busy_hours[i] = hour;
+			}
+		} else if (!strcmp(key, "quiet_hours")) {
+			unsigned int i;
+			size_t size = json_array_size(value);
+
+			for (i = 0; i < size; i++) {
+				int hour = (int)json_integer_value(
+						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, "avg_packet_thresh")) {
+			pkt->traffic_policy.avg_max_packet_thresh =
+					(uint32_t)json_integer_value(value);
+		} else if (!strcmp(key, "max_packet_thresh")) {
+			pkt->traffic_policy.max_max_packet_thresh =
+					(uint32_t)json_integer_value(value);
+		} else if (!strcmp(key, "unit")) {
+			char unit[32];
+			snprintf(unit, 32, "%s", json_string_value(value));
+			if (!strcmp(unit, "SCALE_UP")) {
+				pkt->unit = CPU_POWER_SCALE_UP;
+			} else if (!strcmp(unit, "SCALE_DOWN")) {
+				pkt->unit = CPU_POWER_SCALE_DOWN;
+			} else if (!strcmp(unit, "SCALE_MAX")) {
+				pkt->unit = CPU_POWER_SCALE_MAX;
+			} else if (!strcmp(unit, "SCALE_MIN")) {
+				pkt->unit = CPU_POWER_SCALE_MIN;
+			} else if (!strcmp(unit, "ENABLE_TURBO")) {
+				pkt->unit = CPU_POWER_ENABLE_TURBO;
+			} else if (!strcmp(unit, "DISABLE_TURBO")) {
+				pkt->unit = CPU_POWER_DISABLE_TURBO;
+			} else {
+				RTE_LOG(ERR, CHANNEL_MONITOR,
+					"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);
+		}
+	}
+	return 0;
+}
+#endif
+
 void channel_monitor_exit(void)
 {
 	run_loop = 0;
@@ -124,18 +259,11 @@  get_pcpu_to_control(struct policy *pol)
 
 	ci = get_core_info();
 
-	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 == CORE_TYPE_VIRTUAL) {
 		/*
 		 * If the cores in the policy are virtual, we need to map them
@@ -295,8 +423,6 @@  apply_traffic_profile(struct policy *pol)
 
 	diff = get_pkt_diff(pol);
 
-	RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
-
 	if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
 		for (count = 0; count < pol->pkt.num_vcpu; count++) {
 			if (pol->core_share[count].status != 1)
@@ -340,9 +466,6 @@  apply_time_profile(struct policy *pol)
 				if (pol->core_share[count].status != 1) {
 					power_manager_scale_core_max(
 						pol->core_share[count].pcpu);
-				RTE_LOG(INFO, CHANNEL_MONITOR,
-					"Scaling up core %d to max\n",
-					pol->core_share[count].pcpu);
 				}
 			}
 			break;
@@ -352,9 +475,6 @@  apply_time_profile(struct policy *pol)
 				if (pol->core_share[count].status != 1) {
 					power_manager_scale_core_min(
 						pol->core_share[count].pcpu);
-				RTE_LOG(INFO, CHANNEL_MONITOR,
-					"Scaling down core %d to min\n",
-					pol->core_share[count].pcpu);
 			}
 		}
 			break;
@@ -416,8 +536,6 @@  process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 	if (chan_info == NULL)
 		return -1;
 
-	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) == 0)
 		return -1;
@@ -486,8 +604,8 @@  process_request(struct channel_packet *pkt, struct channel_info *chan_info)
 	}
 
 	if (pkt->command == PKT_POLICY) {
-		RTE_LOG(INFO, CHANNEL_MONITOR,
-				"\nProcessing Policy request\n");
+		RTE_LOG(INFO, CHANNEL_MONITOR, "Processing policy request %s\n",
+				pkt->vm_name);
 		update_policy(pkt);
 		policy_is_set = 1;
 	}
@@ -559,6 +677,103 @@  channel_monitor_init(void)
 	return 0;
 }
 
+static void
+read_binary_packet(struct channel_info *chan_info)
+{
+	struct channel_packet pkt;
+	void *buffer = &pkt;
+	int buffer_len = sizeof(pkt);
+	int n_bytes, err = 0;
+
+	while (buffer_len > 0) {
+		n_bytes = read(chan_info->fd,
+				buffer, buffer_len);
+		if (n_bytes == buffer_len)
+			break;
+		if (n_bytes == -1) {
+			err = errno;
+			RTE_LOG(DEBUG, CHANNEL_MONITOR,
+				"Received error on "
+				"channel '%s' read: %s\n",
+				chan_info->channel_path,
+				strerror(err));
+			remove_channel(&chan_info);
+			break;
+		}
+		buffer = (char *)buffer + n_bytes;
+		buffer_len -= n_bytes;
+	}
+	if (!err)
+		process_request(&pkt, chan_info);
+}
+
+#ifdef USE_JANSSON
+static void
+read_json_packet(struct channel_info *chan_info)
+{
+	struct channel_packet pkt;
+	int n_bytes, ret;
+	json_t *root;
+	json_error_t error;
+
+	/* read opening brace to closing brace */
+	do {
+		int idx = 0;
+		int indent = 0;
+		do {
+			n_bytes = read(chan_info->fd, &json_data[idx], 1);
+			if (n_bytes == 0)
+				break;
+			if (json_data[idx] == '{')
+				indent++;
+			if (json_data[idx] == '}')
+				indent--;
+			if ((indent > 0) || (idx >> 0))
+				idx++;
+			if (indent == 0)
+				json_data[idx] = 0;
+			if (idx >= MAX_JSON_STRING_LEN)
+				break;
+		} while (indent > 0);
+
+		if (indent > 0)
+			/*
+			 * We've broken out of the read loop without getting
+			 * a closing brace, so throw away the datai
+			 */
+			json_data[idx] = 0;
+
+		if (strlen(json_data) == 0)
+			continue;
+
+		printf("got [%s]\n", json_data);
+
+		root = json_loads(json_data, 0, &error);
+
+		if (root) {
+			/*
+			 * 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);
+			json_decref(root);
+			if (ret) {
+				RTE_LOG(ERR, CHANNEL_MONITOR,
+					"Error validating JSON profile data\n");
+				break;
+			}
+			process_request(&pkt, chan_info);
+		} else {
+			RTE_LOG(ERR, CHANNEL_MONITOR,
+					"JSON error on line %d: %s\n",
+					error.line, error.text);
+		}
+	} while (n_bytes > 0);
+}
+#endif
+
 void
 run_channel_monitor(void)
 {
@@ -570,11 +785,16 @@  run_channel_monitor(void)
 		if (!run_loop)
 			break;
 		for (i = 0; i < n_events; i++) {
+			if (!global_events_list[i].data.ptr) {
+				fflush(stdout);
+				continue;
+			}
 			struct channel_info *chan_info = (struct channel_info *)
 					global_events_list[i].data.ptr;
 			if ((global_events_list[i].events & EPOLLERR) ||
 				(global_events_list[i].events & EPOLLHUP)) {
-				RTE_LOG(DEBUG, CHANNEL_MONITOR, "Remote closed connection for "
+				RTE_LOG(INFO, CHANNEL_MONITOR,
+						"Remote closed connection for "
 						"channel '%s'\n",
 						chan_info->channel_path);
 				remove_channel(&chan_info);
@@ -582,31 +802,18 @@  run_channel_monitor(void)
 			}
 			if (global_events_list[i].events & EPOLLIN) {
 
-				int n_bytes, err = 0;
-				struct channel_packet pkt;
-				void *buffer = &pkt;
-				int buffer_len = sizeof(pkt);
-
-				while (buffer_len > 0) {
-					n_bytes = read(chan_info->fd,
-							buffer, buffer_len);
-					if (n_bytes == buffer_len)
-						break;
-					if (n_bytes == -1) {
-						err = errno;
-						RTE_LOG(DEBUG, CHANNEL_MONITOR,
-							"Received error on "
-							"channel '%s' read: %s\n",
-							chan_info->channel_path,
-							strerror(err));
-						remove_channel(&chan_info);
-						break;
-					}
-					buffer = (char *)buffer + n_bytes;
-					buffer_len -= n_bytes;
+				switch (chan_info->type) {
+				case CHANNEL_TYPE_BINARY:
+					read_binary_packet(chan_info);
+					break;
+#ifdef USE_JANSSON
+				case CHANNEL_TYPE_JSON:
+					read_json_packet(chan_info);
+					break;
+#endif
+				default:
+					break;
 				}
-				if (!err)
-					process_request(&pkt, chan_info);
 			}
 		}
 		rte_delay_us(time_period_ms*1000);