diff mbox series

[04/11] telemetry: add parser for client socket messages

Message ID 1535026093-101872-5-git-send-email-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series introduce telemetry library | expand

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Power, Ciara Aug. 23, 2018, 12:08 p.m. UTC
This patch adds the parser file. This is used to parse any
messages that are received on any of the client sockets.

Currently, the unregister functionality works using the parser.
Functionality relating to getting statistic values for certain ports
will be added in a subsequent patch, however the parsing involved
for that command is added in this patch.

Some of the parser code included is in preparation for future
functionality, that is not implemented yet in this patchset.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Signed-off-by: Brian Archbold <brian.archbold@intel.com>
---
 lib/librte_telemetry/Makefile                 |   1 +
 lib/librte_telemetry/meson.build              |   4 +-
 lib/librte_telemetry/rte_telemetry.c          |   9 +
 lib/librte_telemetry/rte_telemetry_internal.h |   3 +
 lib/librte_telemetry/rte_telemetry_parser.c   | 585 ++++++++++++++++++++++++++
 lib/librte_telemetry/rte_telemetry_parser.h   |  13 +
 6 files changed, 613 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_telemetry/rte_telemetry_parser.c
 create mode 100644 lib/librte_telemetry/rte_telemetry_parser.h

Comments

Gaëtan Rivet Aug. 30, 2018, 11:57 p.m. UTC | #1
Hi,

On Thu, Aug 23, 2018 at 01:08:06PM +0100, Ciara Power wrote:
> This patch adds the parser file. This is used to parse any
> messages that are received on any of the client sockets.
> 
> Currently, the unregister functionality works using the parser.
> Functionality relating to getting statistic values for certain ports
> will be added in a subsequent patch, however the parsing involved
> for that command is added in this patch.
> 
> Some of the parser code included is in preparation for future
> functionality, that is not implemented yet in this patchset.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> ---
>  lib/librte_telemetry/Makefile                 |   1 +
>  lib/librte_telemetry/meson.build              |   4 +-
>  lib/librte_telemetry/rte_telemetry.c          |   9 +
>  lib/librte_telemetry/rte_telemetry_internal.h |   3 +
>  lib/librte_telemetry/rte_telemetry_parser.c   | 585 ++++++++++++++++++++++++++
>  lib/librte_telemetry/rte_telemetry_parser.h   |  13 +
>  6 files changed, 613 insertions(+), 2 deletions(-)
>  create mode 100644 lib/librte_telemetry/rte_telemetry_parser.c
>  create mode 100644 lib/librte_telemetry/rte_telemetry_parser.h
> 
> diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
> index bda3788..df8fdd9 100644
> --- a/lib/librte_telemetry/Makefile
> +++ b/lib/librte_telemetry/Makefile
> @@ -19,6 +19,7 @@ LIBABIVER := 1
>  
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
> +SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += rte_telemetry_parser.c
>  
>  # export include files
>  SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
> diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
> index 0ccfa36..7450f96 100644
> --- a/lib/librte_telemetry/meson.build
> +++ b/lib/librte_telemetry/meson.build
> @@ -1,8 +1,8 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2018 Intel Corporation
>  
> -sources = files('rte_telemetry.c')
> -headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
> +sources = files('rte_telemetry.c', 'rte_telemetry_parser.c')
> +headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h')
>  deps += ['metrics', 'ethdev']
>  cflags += '-DALLOW_EXPERIMENTAL_API'
>  jansson = cc.find_library('jansson', required: true)
> diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
> index e9dd022..c6c6612 100644
> --- a/lib/librte_telemetry/rte_telemetry.c
> +++ b/lib/librte_telemetry/rte_telemetry.c
> @@ -15,6 +15,7 @@
>  
>  #include "rte_telemetry.h"
>  #include "rte_telemetry_internal.h"
> +#include "rte_telemetry_parser.h"
>  
>  #define BUF_SIZE 1024
>  #define ACTION_POST 1
> @@ -272,6 +273,8 @@ rte_telemetry_accept_new_client(struct telemetry_impl *telemetry)
>  static int32_t
>  rte_telemetry_read_client_sockets(struct telemetry_impl *telemetry)
>  {
> +	int ret;
> +
>  	telemetry_client *client;
>  	TAILQ_FOREACH(client, &telemetry->client_list_head, client_list) {
>  		char client_buf[BUF_SIZE];
> @@ -279,6 +282,12 @@ rte_telemetry_read_client_sockets(struct telemetry_impl *telemetry)
>  		client_buf[bytes] = '\0';
>  		if (bytes > 0) {
>  			telemetry->request_client = client;
> +			ret = rte_telemetry_parse(telemetry, client_buf);
> +			if (ret < 0) {
> +				TELEMETRY_LOG_WARN("Warning - Parse socket "
> +					"input failed: %i\n", ret);

I see LOG_WARN being always preceded by "Warning - ",
LOG_ERR by "Error - ", and so on.

Wouldn't it be simpler to have the prefix inserted systematically? I
have seen at least one mistake on a LOG_ERR message (in another patch).

Also Shreyansh already remarked about it, but you may have doubled the
newline.

> +				return -1;
> +			}
>  		}
>  	}
>  	return 0;
> diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
> index e3292cf..b057794 100644
> --- a/lib/librte_telemetry/rte_telemetry_internal.h
> +++ b/lib/librte_telemetry/rte_telemetry_internal.h
> @@ -58,4 +58,7 @@ int32_t
>  rte_telemetry_unregister_client(struct telemetry_impl *telemetry,
>  	const char *client_path);
>  
> +int32_t
> +rte_telemetry_check_port_activity(int port_id);
> +
>  #endif
> diff --git a/lib/librte_telemetry/rte_telemetry_parser.c b/lib/librte_telemetry/rte_telemetry_parser.c
> new file mode 100644
> index 0000000..571c991
> --- /dev/null
> +++ b/lib/librte_telemetry/rte_telemetry_parser.c
> @@ -0,0 +1,585 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <jansson.h>
> +
> +#include <rte_metrics.h>
> +#include <rte_common.h>
> +#include <rte_ethdev.h>
> +
> +#include "rte_telemetry_internal.h"
> +
> +#define ACTION_GET 0
> +#define ACTION_DELETE 2

An enum would be cleaner here, I don't see a reason for
these values to be defines.

> +
> +struct command {

struct rte_telemetry_command might be a better name.

> +	char *command_text;

command.command_text? Why not simply text?

> +	int (*comm_func_ptr)(struct telemetry_impl *, int, json_t *);

Function pointers should be typedef for readability.

> +} command;
> +
> +static int32_t
> +rte_telemetry_command_clients(struct telemetry_impl *telemetry, int action,
> +	json_t *data)
> +{
> +	int ret;

blank line missing here.
However, it seems ret is never actually used.
All checks could be performed against the function calls directly,
as the return value is never used itself, -1 is always returned.

> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	if (action != ACTION_DELETE) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
> +			"command\n");
> +		goto einval_fail;
> +	}
> +
> +	if (!json_is_object(data)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
> +			"command\n");
> +		goto einval_fail;
> +	}
> +
> +	json_t *client_path = json_object_get(data, "client_path");

Coding style guide dictates local variables to be defined at start of
scope.

[※]: https://doc.dpdk.org/guides/contributing/coding_style.html#local-variables

> +	if (!json_is_string(client_path)) {
> +		TELEMETRY_LOG_WARN("Warning - Command value is not a string\n");
> +		goto einval_fail;
> +	}
> +
> +	const char *client_path_string = json_string_value(client_path);

This variable is not used afterward, why not write:

if (rte_telemetry_unregister_client(telemetry,
                                json_string_value(client_path))) {
    TELEMETRY_LOG_ERR("Could not unregister client");
    goto einval_fail;
}

> +
> +	ret = rte_telemetry_unregister_client(telemetry, client_path_string);
> +	if (ret < 0) {
> +		TELEMETRY_LOG_ERR("Error - could not unregister client\n");
> +		goto einval_fail;
> +	}
> +
> +	return 0;
> +
> +einval_fail:
> +	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +	if (ret < 0)
> +		TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +	return -1;
> +}
> +
> +static int32_t
> +rte_telemetry_command_ports(struct telemetry_impl *telemetry, int action,
> +	json_t *data)
> +{
> +	int ret;
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	if (!json_is_null(data)) {
> +		TELEMETRY_LOG_WARN("Warning - Data should be NULL JSON object "
> +			"for 'ports' command\n");
> +		goto einval_fail;
> +	}
> +
> +	if (action != ACTION_GET) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
> +			"command\n");
> +		goto einval_fail;
> +	}
> +
> +	return 0;
> +
> +einval_fail:
> +	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +	if (ret < 0)
> +		TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +	return -1;
> +}
> +
> +static int32_t
> +rte_telemetry_command_ports_details(struct telemetry_impl *telemetry,
> +	int action, json_t *data)
> +{
> +	int ret;
> +
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	if (action != ACTION_GET) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
> +			"command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;

Previous functions used gotos for dealing with errors, this one repeats
the pattern. Seems like an oversight.

> +	}
> +
> +	if (!json_is_object(data)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
> +			"command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	json_t *port_ids_json = json_object_get(data, "ports");
> +	if (!json_is_array(port_ids_json)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid Port ID array\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	uint64_t num_port_ids = json_array_size(port_ids_json);
> +	int port_ids[num_port_ids];
> +	RTE_SET_USED(port_ids);
> +	size_t index;
> +	json_t *value;

Declare those at start of scope.

> +
> +	json_array_foreach(port_ids_json, index, value) {
> +		if (!json_is_integer(value)) {
> +			TELEMETRY_LOG_WARN("Warning - Port ID given is "
> +				"invalid\n");
> +			ret = rte_telemetry_send_error_response(telemetry,
> +				-EINVAL);
> +			if (ret < 0)
> +				TELEMETRY_LOG_ERR("Error - Could not send "
> +					"error\n");
> +			return -1;
> +		}
> +		port_ids[index] = json_integer_value(value);
> +	}
> +
> +	return 0;
> +}
> +

Some previous errors are repeated afterward but I won't point each of
them.

[snip]

> +
> +static int32_t
> +rte_telemetry_stat_names_to_ids(struct telemetry_impl *telemetry,
> +	const char * const *stat_names, uint32_t *stat_ids,
> +	uint64_t num_stat_names)
> +{
> +	struct rte_metric_name *names;
> +	int ret;
> +
> +	if (stat_names == NULL) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid stat_names argument\n");
> +		goto einval_fail;
> +	}
> +
> +	if (num_stat_names <= 0) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid num_stat_names "
> +			"argument\n");
> +		goto einval_fail;
> +	}
> +
> +	int num_metrics = rte_metrics_get_names(NULL, 0);
> +	if (num_metrics < 0) {
> +		TELEMETRY_LOG_ERR("Error - Cannot get metrics count\n");
> +		goto eperm_fail;
> +	} else if (num_metrics == 0) {
> +		TELEMETRY_LOG_WARN("Warning - No metrics have been "
> +			"registered\n");
> +		goto eperm_fail;
> +	}
> +
> +	names = malloc(sizeof(struct rte_metric_name) * num_metrics);
> +	if (names == NULL) {
> +		TELEMETRY_LOG_ERR("Error - Cannot allocate memory for names\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -ENOMEM);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	ret = rte_metrics_get_names(names, num_metrics);
> +	if (ret < 0 || ret > num_metrics) {
> +		TELEMETRY_LOG_ERR("Error - Cannot get metrics names\n");
> +		free(names);
> +		goto eperm_fail;
> +	}
> +
> +	uint32_t i, k;
> +	k = 0;
> +	for (i = 0; i < (uint32_t)num_stat_names; i++) {
> +		uint32_t j;
> +		for (j = 0; j < (uint32_t)num_metrics; j++) {
> +			if (strcmp(stat_names[i], names[j].name) == 0) {
> +				stat_ids[k] = j;
> +				k++;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (k != num_stat_names) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid stat names provided\n");

It would be better to provide the user with a list of requested names
that are invalid.

> +		free(names);
> +		goto einval_fail;
> +	}
> +

You are not freeing "names" here, nor are you returning it.

> +	return 0;
> +
> +einval_fail:
> +	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +	if (ret < 0)
> +		TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +	return -1;
> +
> +eperm_fail:
> +	ret = rte_telemetry_send_error_response(telemetry, -EPERM);
> +	if (ret < 0)
> +		TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +	return -1;
> +}
> +
> +int32_t
> +rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
> +	 int action, json_t *data)
> +{
> +	int ret, num_metrics;
> +	struct rte_metric_name *names;
> +
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	if (action != ACTION_GET) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid action for this command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	if (json_is_object(data)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	num_metrics = rte_metrics_get_names(NULL, 0);
> +	if (num_metrics < 0) {
> +		TELEMETRY_LOG_ERR("Error - Cannot get metrics count\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	} else if (num_metrics == 0) {
> +		TELEMETRY_LOG_ERR("Error - No metrics to display (none have been registered)\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EPERM);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	names = malloc(sizeof(struct rte_metric_name) * num_metrics);
> +	if (!names) {
> +		TELEMETRY_LOG_ERR("Error - Cannot allocate memory\n");
> +		ret = rte_telemetry_send_error_response(telemetry,
> +			 -ENOMEM);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	uint64_t num_port_ids = 0;
> +	const char *stat_names[num_metrics];
> +	uint32_t stat_ids[num_metrics];
> +	int p;
> +
> +	RTE_ETH_FOREACH_DEV(p) {
> +		num_port_ids++;
> +	}
> +	if (!num_port_ids) {
> +		TELEMETRY_LOG_WARN("Warning - No active ports\n");
> +		ret = rte_telemetry_send_error_response(telemetry,
> +			-EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		goto fail;
> +	}
> +
> +	ret = rte_metrics_get_names(names, num_metrics);
> +	int i;
> +	for (i = 0; i < num_metrics; i++)
> +		stat_names[i] = names[i].name;
> +
> +	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
> +		num_metrics);
> +	if (ret < 0) {
> +		TELEMETRY_LOG_ERR("Error - Could not convert stat names to IDs\n");
> +		goto fail;
> +	}
> +	return 0;

The retrieved IDs here are not used, is it because this function is
extended in another patch?

You also already use rte_metrics_get_names() to generate a request, that
will itself do the same, compare the two and assign IDs according to
their index in the list.

This could probably be written in a simpler and more concise way.

This is rather awkward to divide the patches this way. It is harder
right now to judge this function and how stat_names_to_ids is
implemented, because the finality of it is not yet available.

It was not necessary to add this command alongside the others, it could
come later.

> +
> +fail:
> +	free(names);
> +	return -1;
> +}
> +
> +int32_t
> +rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
> +	*telemetry, int action, json_t *data)
> +{
> +	int ret;
> +
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	if (action != ACTION_GET) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
> +			"command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	if (!json_is_object(data)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
> +			"command\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	json_t *port_ids_json = json_object_get(data, "ports");
> +	json_t *stat_names_json = json_object_get(data, "stats");
> +	if (!json_is_array(port_ids_json) ||
> +		 !json_is_array(stat_names_json)) {
> +		TELEMETRY_LOG_WARN("Warning - Invalid input data array(s)\n");
> +		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Could not send error\n");
> +		return -1;
> +	}
> +
> +	uint64_t num_port_ids = json_array_size(port_ids_json);
> +	uint32_t port_ids[num_port_ids];
> +	size_t index;
> +	json_t *value;
> +
> +	json_array_foreach(port_ids_json, index, value) {
> +		if (!json_is_integer(value)) {
> +			TELEMETRY_LOG_WARN("Warning - Port ID given is not "
> +				"valid\n");
> +			ret = rte_telemetry_send_error_response(telemetry,
> +				-EINVAL);
> +			if (ret < 0)
> +				TELEMETRY_LOG_ERR("Error - Could not send "
> +					"error\n");
> +			return -1;
> +		}
> +		port_ids[index] = json_integer_value(value);
> +		ret = rte_telemetry_check_port_activity(port_ids[index]);
> +		if (ret < 1) {
> +			ret = rte_telemetry_send_error_response(telemetry,
> +				-EINVAL);
> +			if (ret < 0)
> +				TELEMETRY_LOG_ERR("Error - Could not send "
> +				"error\n");
> +			return -1;
> +		}
> +	}
> +
> +	uint64_t num_stat_names = json_array_size(stat_names_json);
> +	const char *stat_names[num_stat_names];
> +
> +	json_array_foreach(stat_names_json, index, value) {
> +		if (!json_is_string(value)) {
> +			TELEMETRY_LOG_WARN("Warning - Stat Name given is not a "
> +				"string\n");
> +			ret = rte_telemetry_send_error_response(telemetry,
> +				-EINVAL);
> +			if (ret < 0)
> +				TELEMETRY_LOG_ERR("Error - Could not send "
> +					"error\n");
> +			return -1;
> +		}
> +		stat_names[index] = json_string_value(value);
> +	}
> +
> +	uint32_t stat_ids[num_stat_names];
> +	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
> +		num_stat_names);
> +	if (ret < 0) {
> +		TELEMETRY_LOG_ERR("Error - Could not convert stat names to "
> +			"IDs\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +static int32_t
> +rte_telemetry_parse_command(struct telemetry_impl *telemetry, int action,
> +	const char *command, json_t *data)
> +{
> +	int ret;
> +
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
> +		return -1;
> +	}
> +
> +	struct command commands[] = {
> +		{.command_text = "clients",
> +			 .comm_func_ptr = &rte_telemetry_command_clients},
> +		{.command_text = "ports",
> +			 .comm_func_ptr = &rte_telemetry_command_ports},
> +		{.command_text = "ports_details",
> +			 .comm_func_ptr = &rte_telemetry_command_ports_details},
> +		{.command_text = "port_stats",
> +			 .comm_func_ptr = &rte_telemetry_command_port_stats},
> +		{.command_text = "ports_stats_values_by_name",
> +			 .comm_func_ptr =
> +			 &rte_telemetry_command_ports_stats_values_by_name},
> +		{.command_text = "ports_all_stat_values",
> +			 .comm_func_ptr =
> +			 &rte_telemetry_command_ports_all_stat_values}

These command names are unnecessarily verbose, while still not saying
exactly what the commands are.

"clients" only supports "DELETE", but the function name does not reflect
that.  "ports" only supports "GET", but this is not explicit.
And on the other hand,
"rte_telemetry_command_ports_stats_values_by_name" is just too much.

The coding style is wrong as well.

With the proper definitions, something like this could be written:

struct rte_telemetry_command commands[] = {
        {
                .text = "client",
                .fn = {
                        [DELETE] = rte_tlm_client_unregister,
                },
        },
        {
                .text = "port",
                .fn = {
                        [GET] = rte_tlm_port_get,
                },
        },
        ...
};

> +	};
> +
> +	const uint32_t num_commands = sizeof(commands)/sizeof(struct command);

RTE_DIM() should be used for this.

> +	uint32_t i;
> +

Here action should be checked against the enum previously described,
verifying that it is within range.

> +	for (i = 0; i < num_commands; i++) {
> +		if (strcmp(command, commands[i].command_text) == 0) {
> +			int ret = commands[i].comm_func_ptr(telemetry, action,
> +				data);

With the above format, this would become

        if (strcmp(command, commands[i].text) == 0) {
            rte_telemetry_command_cb fn;

            fn = commands[i].fn[action];
            if (fn == NULL) {
                    /* Error invalid command */
                    return -1;
            } else if (fn(telemetry, data)) {
                    /* Error log would already be provided
                     * by the function itself.
                     */
                    return -1;
            }
            return 0;
        }

Regards,
diff mbox series

Patch

diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
index bda3788..df8fdd9 100644
--- a/lib/librte_telemetry/Makefile
+++ b/lib/librte_telemetry/Makefile
@@ -19,6 +19,7 @@  LIBABIVER := 1
 
 # library source files
 SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
+SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += rte_telemetry_parser.c
 
 # export include files
 SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
index 0ccfa36..7450f96 100644
--- a/lib/librte_telemetry/meson.build
+++ b/lib/librte_telemetry/meson.build
@@ -1,8 +1,8 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
-sources = files('rte_telemetry.c')
-headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
+sources = files('rte_telemetry.c', 'rte_telemetry_parser.c')
+headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h')
 deps += ['metrics', 'ethdev']
 cflags += '-DALLOW_EXPERIMENTAL_API'
 jansson = cc.find_library('jansson', required: true)
diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
index e9dd022..c6c6612 100644
--- a/lib/librte_telemetry/rte_telemetry.c
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -15,6 +15,7 @@ 
 
 #include "rte_telemetry.h"
 #include "rte_telemetry_internal.h"
+#include "rte_telemetry_parser.h"
 
 #define BUF_SIZE 1024
 #define ACTION_POST 1
@@ -272,6 +273,8 @@  rte_telemetry_accept_new_client(struct telemetry_impl *telemetry)
 static int32_t
 rte_telemetry_read_client_sockets(struct telemetry_impl *telemetry)
 {
+	int ret;
+
 	telemetry_client *client;
 	TAILQ_FOREACH(client, &telemetry->client_list_head, client_list) {
 		char client_buf[BUF_SIZE];
@@ -279,6 +282,12 @@  rte_telemetry_read_client_sockets(struct telemetry_impl *telemetry)
 		client_buf[bytes] = '\0';
 		if (bytes > 0) {
 			telemetry->request_client = client;
+			ret = rte_telemetry_parse(telemetry, client_buf);
+			if (ret < 0) {
+				TELEMETRY_LOG_WARN("Warning - Parse socket "
+					"input failed: %i\n", ret);
+				return -1;
+			}
 		}
 	}
 	return 0;
diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
index e3292cf..b057794 100644
--- a/lib/librte_telemetry/rte_telemetry_internal.h
+++ b/lib/librte_telemetry/rte_telemetry_internal.h
@@ -58,4 +58,7 @@  int32_t
 rte_telemetry_unregister_client(struct telemetry_impl *telemetry,
 	const char *client_path);
 
+int32_t
+rte_telemetry_check_port_activity(int port_id);
+
 #endif
diff --git a/lib/librte_telemetry/rte_telemetry_parser.c b/lib/librte_telemetry/rte_telemetry_parser.c
new file mode 100644
index 0000000..571c991
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_parser.c
@@ -0,0 +1,585 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+#include <jansson.h>
+
+#include <rte_metrics.h>
+#include <rte_common.h>
+#include <rte_ethdev.h>
+
+#include "rte_telemetry_internal.h"
+
+#define ACTION_GET 0
+#define ACTION_DELETE 2
+
+struct command {
+	char *command_text;
+	int (*comm_func_ptr)(struct telemetry_impl *, int, json_t *);
+} command;
+
+static int32_t
+rte_telemetry_command_clients(struct telemetry_impl *telemetry, int action,
+	json_t *data)
+{
+	int ret;
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (action != ACTION_DELETE) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
+			"command\n");
+		goto einval_fail;
+	}
+
+	if (!json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
+			"command\n");
+		goto einval_fail;
+	}
+
+	json_t *client_path = json_object_get(data, "client_path");
+	if (!json_is_string(client_path)) {
+		TELEMETRY_LOG_WARN("Warning - Command value is not a string\n");
+		goto einval_fail;
+	}
+
+	const char *client_path_string = json_string_value(client_path);
+
+	ret = rte_telemetry_unregister_client(telemetry, client_path_string);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Error - could not unregister client\n");
+		goto einval_fail;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+}
+
+static int32_t
+rte_telemetry_command_ports(struct telemetry_impl *telemetry, int action,
+	json_t *data)
+{
+	int ret;
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (!json_is_null(data)) {
+		TELEMETRY_LOG_WARN("Warning - Data should be NULL JSON object "
+			"for 'ports' command\n");
+		goto einval_fail;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
+			"command\n");
+		goto einval_fail;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+}
+
+static int32_t
+rte_telemetry_command_ports_details(struct telemetry_impl *telemetry,
+	int action, json_t *data)
+{
+	int ret;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
+			"command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	if (!json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
+			"command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	json_t *port_ids_json = json_object_get(data, "ports");
+	if (!json_is_array(port_ids_json)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid Port ID array\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	uint64_t num_port_ids = json_array_size(port_ids_json);
+	int port_ids[num_port_ids];
+	RTE_SET_USED(port_ids);
+	size_t index;
+	json_t *value;
+
+	json_array_foreach(port_ids_json, index, value) {
+		if (!json_is_integer(value)) {
+			TELEMETRY_LOG_WARN("Warning - Port ID given is "
+				"invalid\n");
+			ret = rte_telemetry_send_error_response(telemetry,
+				-EINVAL);
+			if (ret < 0)
+				TELEMETRY_LOG_ERR("Error - Could not send "
+					"error\n");
+			return -1;
+		}
+		port_ids[index] = json_integer_value(value);
+	}
+
+	return 0;
+}
+
+static int32_t
+rte_telemetry_command_port_stats(struct telemetry_impl *telemetry, int action,
+	json_t *data)
+{
+	int ret;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (!json_is_null(data)) {
+		TELEMETRY_LOG_WARN("Warning - Data should be NULL JSON object "
+			"for 'port_stats' command\n");
+		goto einval_fail;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
+			"command\n");
+		goto einval_fail;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+}
+
+static int32_t
+rte_telemetry_stat_names_to_ids(struct telemetry_impl *telemetry,
+	const char * const *stat_names, uint32_t *stat_ids,
+	uint64_t num_stat_names)
+{
+	struct rte_metric_name *names;
+	int ret;
+
+	if (stat_names == NULL) {
+		TELEMETRY_LOG_WARN("Warning - Invalid stat_names argument\n");
+		goto einval_fail;
+	}
+
+	if (num_stat_names <= 0) {
+		TELEMETRY_LOG_WARN("Warning - Invalid num_stat_names "
+			"argument\n");
+		goto einval_fail;
+	}
+
+	int num_metrics = rte_metrics_get_names(NULL, 0);
+	if (num_metrics < 0) {
+		TELEMETRY_LOG_ERR("Error - Cannot get metrics count\n");
+		goto eperm_fail;
+	} else if (num_metrics == 0) {
+		TELEMETRY_LOG_WARN("Warning - No metrics have been "
+			"registered\n");
+		goto eperm_fail;
+	}
+
+	names = malloc(sizeof(struct rte_metric_name) * num_metrics);
+	if (names == NULL) {
+		TELEMETRY_LOG_ERR("Error - Cannot allocate memory for names\n");
+		ret = rte_telemetry_send_error_response(telemetry, -ENOMEM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	ret = rte_metrics_get_names(names, num_metrics);
+	if (ret < 0 || ret > num_metrics) {
+		TELEMETRY_LOG_ERR("Error - Cannot get metrics names\n");
+		free(names);
+		goto eperm_fail;
+	}
+
+	uint32_t i, k;
+	k = 0;
+	for (i = 0; i < (uint32_t)num_stat_names; i++) {
+		uint32_t j;
+		for (j = 0; j < (uint32_t)num_metrics; j++) {
+			if (strcmp(stat_names[i], names[j].name) == 0) {
+				stat_ids[k] = j;
+				k++;
+				break;
+			}
+		}
+	}
+
+	if (k != num_stat_names) {
+		TELEMETRY_LOG_WARN("Warning - Invalid stat names provided\n");
+		free(names);
+		goto einval_fail;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+
+eperm_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EPERM);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+}
+
+int32_t
+rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
+	 int action, json_t *data)
+{
+	int ret, num_metrics;
+	struct rte_metric_name *names;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	if (json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	num_metrics = rte_metrics_get_names(NULL, 0);
+	if (num_metrics < 0) {
+		TELEMETRY_LOG_ERR("Error - Cannot get metrics count\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	} else if (num_metrics == 0) {
+		TELEMETRY_LOG_ERR("Error - No metrics to display (none have been registered)\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EPERM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	names = malloc(sizeof(struct rte_metric_name) * num_metrics);
+	if (!names) {
+		TELEMETRY_LOG_ERR("Error - Cannot allocate memory\n");
+		ret = rte_telemetry_send_error_response(telemetry,
+			 -ENOMEM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	uint64_t num_port_ids = 0;
+	const char *stat_names[num_metrics];
+	uint32_t stat_ids[num_metrics];
+	int p;
+
+	RTE_ETH_FOREACH_DEV(p) {
+		num_port_ids++;
+	}
+	if (!num_port_ids) {
+		TELEMETRY_LOG_WARN("Warning - No active ports\n");
+		ret = rte_telemetry_send_error_response(telemetry,
+			-EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		goto fail;
+	}
+
+	ret = rte_metrics_get_names(names, num_metrics);
+	int i;
+	for (i = 0; i < num_metrics; i++)
+		stat_names[i] = names[i].name;
+
+	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
+		num_metrics);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Error - Could not convert stat names to IDs\n");
+		goto fail;
+	}
+	return 0;
+
+fail:
+	free(names);
+	return -1;
+}
+
+int32_t
+rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
+	*telemetry, int action, json_t *data)
+{
+	int ret;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action for this "
+			"command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	if (!json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid data provided for this "
+			"command\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	json_t *port_ids_json = json_object_get(data, "ports");
+	json_t *stat_names_json = json_object_get(data, "stats");
+	if (!json_is_array(port_ids_json) ||
+		 !json_is_array(stat_names_json)) {
+		TELEMETRY_LOG_WARN("Warning - Invalid input data array(s)\n");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -1;
+	}
+
+	uint64_t num_port_ids = json_array_size(port_ids_json);
+	uint32_t port_ids[num_port_ids];
+	size_t index;
+	json_t *value;
+
+	json_array_foreach(port_ids_json, index, value) {
+		if (!json_is_integer(value)) {
+			TELEMETRY_LOG_WARN("Warning - Port ID given is not "
+				"valid\n");
+			ret = rte_telemetry_send_error_response(telemetry,
+				-EINVAL);
+			if (ret < 0)
+				TELEMETRY_LOG_ERR("Error - Could not send "
+					"error\n");
+			return -1;
+		}
+		port_ids[index] = json_integer_value(value);
+		ret = rte_telemetry_check_port_activity(port_ids[index]);
+		if (ret < 1) {
+			ret = rte_telemetry_send_error_response(telemetry,
+				-EINVAL);
+			if (ret < 0)
+				TELEMETRY_LOG_ERR("Error - Could not send "
+				"error\n");
+			return -1;
+		}
+	}
+
+	uint64_t num_stat_names = json_array_size(stat_names_json);
+	const char *stat_names[num_stat_names];
+
+	json_array_foreach(stat_names_json, index, value) {
+		if (!json_is_string(value)) {
+			TELEMETRY_LOG_WARN("Warning - Stat Name given is not a "
+				"string\n");
+			ret = rte_telemetry_send_error_response(telemetry,
+				-EINVAL);
+			if (ret < 0)
+				TELEMETRY_LOG_ERR("Error - Could not send "
+					"error\n");
+			return -1;
+		}
+		stat_names[index] = json_string_value(value);
+	}
+
+	uint32_t stat_ids[num_stat_names];
+	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
+		num_stat_names);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Error - Could not convert stat names to "
+			"IDs\n");
+		return -1;
+	}
+	return 0;
+}
+
+static int32_t
+rte_telemetry_parse_command(struct telemetry_impl *telemetry, int action,
+	const char *command, json_t *data)
+{
+	int ret;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	struct command commands[] = {
+		{.command_text = "clients",
+			 .comm_func_ptr = &rte_telemetry_command_clients},
+		{.command_text = "ports",
+			 .comm_func_ptr = &rte_telemetry_command_ports},
+		{.command_text = "ports_details",
+			 .comm_func_ptr = &rte_telemetry_command_ports_details},
+		{.command_text = "port_stats",
+			 .comm_func_ptr = &rte_telemetry_command_port_stats},
+		{.command_text = "ports_stats_values_by_name",
+			 .comm_func_ptr =
+			 &rte_telemetry_command_ports_stats_values_by_name},
+		{.command_text = "ports_all_stat_values",
+			 .comm_func_ptr =
+			 &rte_telemetry_command_ports_all_stat_values}
+	};
+
+	const uint32_t num_commands = sizeof(commands)/sizeof(struct command);
+	uint32_t i;
+
+	for (i = 0; i < num_commands; i++) {
+		if (strcmp(command, commands[i].command_text) == 0) {
+			int ret = commands[i].comm_func_ptr(telemetry, action,
+				data);
+			if (ret < 0) {
+				TELEMETRY_LOG_ERR("Error - Command Function "
+					"for %s failed\n",
+					commands[i].command_text);
+				return -1;
+			}
+			return 0;
+		}
+	}
+	TELEMETRY_LOG_WARN("Warning - \"%s\" command not found\n", command);
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+	return -1;
+}
+
+int32_t
+rte_telemetry_parse(struct telemetry_impl *telemetry, char *socket_rx_data)
+{
+	int ret;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - Invalid telemetry argument\n");
+		return -1;
+	}
+
+	json_error_t error;
+	json_t *root = json_loads(socket_rx_data, 0, &error);
+	if (!root) {
+		TELEMETRY_LOG_WARN("Warning - Could not load JSON object from "
+			"data passed in : %s\n", error.text);
+		ret = rte_telemetry_send_error_response(telemetry, -EPERM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -EPERM;
+	} else if (!json_is_object(root)) {
+		TELEMETRY_LOG_WARN("Warning - JSON Request is not a JSON "
+			"object\n");
+		json_decref(root);
+		goto einval_fail;
+	}
+
+	json_t *action = json_object_get(root, "action");
+	if (!action) {
+		TELEMETRY_LOG_WARN("Warning - Request does not have action "
+			"field\n");
+		goto einval_fail;
+	} else if (!json_is_integer(action)) {
+		TELEMETRY_LOG_WARN("Warning - Action value is not an "
+			"integer\n");
+		goto einval_fail;
+	}
+
+	json_t *command = json_object_get(root, "command");
+	if (!command) {
+		TELEMETRY_LOG_WARN("Warning - Request does not have command "
+			"field\n");
+		goto einval_fail;
+	} else if (!json_is_string(command)) {
+		TELEMETRY_LOG_WARN("Warning - Command value is not a string\n");
+		goto einval_fail;
+	}
+
+	int action_int = json_integer_value(action);
+	if (action_int != ACTION_GET && action_int != ACTION_DELETE) {
+		TELEMETRY_LOG_WARN("Warning - Invalid action code\n");
+		goto einval_fail;
+	}
+
+	const char *command_string = json_string_value(command);
+	json_t *data = json_object_get(root, "data");
+	if (!data) {
+		TELEMETRY_LOG_WARN("Warning - Request does not have data "
+			"field\n");
+		goto einval_fail;
+	}
+
+	ret = rte_telemetry_parse_command(telemetry, action_int, command_string,
+		data);
+	if (ret < 0) {
+		TELEMETRY_LOG_WARN("Warning - Could not parse command\n");
+		return -EINVAL;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Error - Could not send error\n");
+		return -EPERM;
+	}
+	return -EINVAL;
+}
diff --git a/lib/librte_telemetry/rte_telemetry_parser.h b/lib/librte_telemetry/rte_telemetry_parser.h
new file mode 100644
index 0000000..63e633d
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_parser.h
@@ -0,0 +1,13 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include "rte_telemetry_internal.h"
+
+#ifndef _RTE_TELEMETRY_PARSER_H_
+#define _RTE_TELEMETRY_PARSER_H_
+
+int32_t
+rte_telemetry_parse(struct telemetry_impl *telemetry, char *socket_rx_data);
+
+#endif