DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options.
@ 2017-02-24 16:52 Roman Korynkevych
  2017-02-27 14:16 ` [dpdk-dev] [PATCH v2] " Roman Korynkevych
  2017-02-27 15:12 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
  0 siblings, 2 replies; 8+ messages in thread
From: Roman Korynkevych @ 2017-02-24 16:52 UTC (permalink / raw)
  To: dev; +Cc: Roman Korynkevych

Extended proc-info application to send DPDK port statistics to
STDOUT in the format expected by collectd exec plugin. Added
HOST ID option to identify the host DPDK process is running on
when multiple instance of DPDK are running in parallel. This is
needed for the barometer project in OPNFV.

Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
---
 app/proc_info/main.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 119 insertions(+), 5 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..fe1ff97 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -40,6 +40,7 @@
 #include <sys/queue.h>
 #include <stdlib.h>
 #include <getopt.h>
+#include <unistd.h>
 
 #include <rte_eal.h>
 #include <rte_common.h>
@@ -62,12 +63,20 @@
 #define MAX_LONG_OPT_SZ 64
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
 
+#define MAX_STRING_LEN 256
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
 static uint32_t enable_stats;
 /**< Enable xstats. */
 static uint32_t enable_xstats;
+/**< Enable collectd format*/
+static uint32_t enable_collectd_format;
+/**< FD to send collectd format messages to STDOUT*/
+static int stdout_fd;
+/**< Host id process is running on */
+static char host_id[MAX_LONG_OPT_SZ];
 /**< Enable stats reset. */
 static uint32_t reset_stats;
 /**< Enable xstats reset. */
@@ -86,7 +95,9 @@ proc_info_usage(const char *prgname)
 		"  --xstats: to display extended port statistics, disabled by "
 			"default\n"
 		"  --stats-reset: to reset port statistics\n"
-		"  --xstats-reset: to reset port extended statistics\n",
+		"  --xstats-reset: to reset port extended statistics\n"
+		"  --collectd-format: to print statistics to STDOUT in expected by collectd format\n"
+		"  --host-id STRING: host id used to identify the system process is running on\n",
 		prgname);
 }
 
@@ -116,6 +127,35 @@ parse_portmask(const char *portmask)
 
 }
 
+static int
+proc_info_preparse_args(int argc, char **argv)
+{
+	char *prgname = argv[0];
+	int i;
+
+	for (i = 0; i < argc; i++) {
+		/* Print stats or xstats to STDOUT in collectd format */
+		if (!strncmp(argv[i], "--collectd-format", MAX_LONG_OPT_SZ)) {
+			enable_collectd_format = 1;
+			stdout_fd = dup(STDOUT_FILENO);
+			close(STDOUT_FILENO);
+		}
+		if (!strncmp(argv[i], "--host-id", MAX_LONG_OPT_SZ)) {
+			if ((i + 1) == argc) {
+				printf("Invalid host id or not specified\n");
+                                proc_info_usage(prgname);
+                                return -1;
+			}
+			strncpy(host_id, argv[i+1], sizeof(host_id));
+		}
+	}
+
+	if (!strlen(host_id))
+		strcpy(host_id, "unknown");
+
+	return 0;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 proc_info_parse_args(int argc, char **argv)
@@ -128,6 +168,8 @@ proc_info_parse_args(int argc, char **argv)
 		{"stats-reset", 0, NULL, 0},
 		{"xstats", 0, NULL, 0},
 		{"xstats-reset", 0, NULL, 0},
+		{"collectd-format", 0, NULL, 0},
+		{"host-id", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -240,6 +282,59 @@ nic_stats_clear(uint8_t port_id)
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
 }
 
+static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
+                                      const char *cnt_name) {
+	char *type_end;
+	type_end = strrchr(cnt_name, '_');
+
+	if ((type_end != NULL) && (strncmp(cnt_name, "rx_", strlen("rx_")) == 0)) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_rx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_rx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_rx_packets", cnt_type_len);
+		else if (strncmp(type_end, "_placement", strlen("_placement")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_buff", strlen("_buff")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		(strncmp(cnt_name, "tx_", strlen("tx_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_tx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_tx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_tx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_tx_packets", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		(strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
+		if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "operations", cnt_type_len);
+		else if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+		else if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "filter_result", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		(strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+	} else {
+		/* Does not fit obvious type, or strrchr error:
+		 *   use a more generic type */
+		strncpy(cnt_type, "derive", cnt_type_len);
+	}
+}
+
 static void
 nic_xstats_display(uint8_t port_id)
 {
@@ -281,10 +376,22 @@ nic_xstats_display(uint8_t port_id)
 		goto err;
 	}
 
-	for (i = 0; i < len; i++)
-		printf("%s: %"PRIu64"\n",
-			xstats_names[i].name,
-			xstats[i].value);
+	for (i = 0; i < len; i++) {
+		if (enable_collectd_format) {
+			char counter_type[MAX_STRING_LEN];
+			collectd_resolve_cnt_type(counter_type, sizeof(counter_type), xstats_names[i].name);
+
+			char buf[MAX_STRING_LEN];
+			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"PRIu64"\n",
+				host_id, port_id, counter_type, xstats_names[i].name,
+				xstats[i].value);
+			write(stdout_fd, buf, strlen(buf));
+                } else {
+			printf("%s: %"PRIu64"\n",
+				xstats_names[i].name,
+				xstats[i].value);
+		}
+	}
 
 	printf("%s############################\n",
 			   nic_stats_border);
@@ -312,6 +419,13 @@ main(int argc, char **argv)
 	char *argp[argc + 3];
 	uint8_t nb_ports;
 
+	/* preparse app arguments */
+	ret = proc_info_preparse_args(argc, argv);
+	if (ret < 0) {
+		printf("Failed to parse arguments\n");
+		return -1;
+	}
+
 	argp[0] = argv[0];
 	argp[1] = c_flag;
 	argp[2] = n_flag;
-- 
2.1.0

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

* [dpdk-dev] [PATCH v2] proc-info: added collectd-format and host-id options.
  2017-02-24 16:52 [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options Roman Korynkevych
@ 2017-02-27 14:16 ` Roman Korynkevych
  2017-02-27 15:18   ` Van Haaren, Harry
  2017-03-01 16:27   ` [dpdk-dev] [PATCH v3] " Roman Korynkevych
  2017-02-27 15:12 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
  1 sibling, 2 replies; 8+ messages in thread
From: Roman Korynkevych @ 2017-02-27 14:16 UTC (permalink / raw)
  To: dev; +Cc: Roman Korynkevych

Extended proc-info application to send DPDK port statistics to
STDOUT in the format expected by collectd exec plugin. Added
HOST ID option to identify the host DPDK process is running on
when multiple instance of DPDK are running in parallel. This is
needed for the barometer project in OPNFV.

Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
---
 app/proc_info/main.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 5 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..2bf280d 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -40,6 +40,7 @@
 #include <sys/queue.h>
 #include <stdlib.h>
 #include <getopt.h>
+#include <unistd.h>
 
 #include <rte_eal.h>
 #include <rte_common.h>
@@ -62,12 +63,20 @@
 #define MAX_LONG_OPT_SZ 64
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
 
+#define MAX_STRING_LEN 256
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
 static uint32_t enable_stats;
 /**< Enable xstats. */
 static uint32_t enable_xstats;
+/**< Enable collectd format*/
+static uint32_t enable_collectd_format;
+/**< FD to send collectd format messages to STDOUT*/
+static int stdout_fd;
+/**< Host id process is running on */
+static char host_id[MAX_LONG_OPT_SZ];
 /**< Enable stats reset. */
 static uint32_t reset_stats;
 /**< Enable xstats reset. */
@@ -86,7 +95,9 @@ proc_info_usage(const char *prgname)
 		"  --xstats: to display extended port statistics, disabled by "
 			"default\n"
 		"  --stats-reset: to reset port statistics\n"
-		"  --xstats-reset: to reset port extended statistics\n",
+		"  --xstats-reset: to reset port extended statistics\n"
+		"  --collectd-format: to print statistics to STDOUT in expected by collectd format\n"
+		"  --host-id STRING: host id used to identify the system process is running on\n",
 		prgname);
 }
 
@@ -116,6 +127,35 @@ parse_portmask(const char *portmask)
 
 }
 
+static int
+proc_info_preparse_args(int argc, char **argv)
+{
+	char *prgname = argv[0];
+	int i;
+
+	for (i = 0; i < argc; i++) {
+		/* Print stats or xstats to STDOUT in collectd format */
+		if (!strncmp(argv[i], "--collectd-format", MAX_LONG_OPT_SZ)) {
+			enable_collectd_format = 1;
+			stdout_fd = dup(STDOUT_FILENO);
+			close(STDOUT_FILENO);
+		}
+		if (!strncmp(argv[i], "--host-id", MAX_LONG_OPT_SZ)) {
+			if ((i + 1) == argc) {
+				printf("Invalid host id or not specified\n");
+				proc_info_usage(prgname);
+				return -1;
+			}
+			strncpy(host_id, argv[i+1], sizeof(host_id));
+		}
+	}
+
+	if (!strlen(host_id))
+		strcpy(host_id, "unknown");
+
+	return 0;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 proc_info_parse_args(int argc, char **argv)
@@ -128,6 +168,8 @@ proc_info_parse_args(int argc, char **argv)
 		{"stats-reset", 0, NULL, 0},
 		{"xstats", 0, NULL, 0},
 		{"xstats-reset", 0, NULL, 0},
+		{"collectd-format", 0, NULL, 0},
+		{"host-id", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -240,6 +282,60 @@ nic_stats_clear(uint8_t port_id)
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
 }
 
+static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
+				      const char *cnt_name) {
+	char *type_end = strrchr(cnt_name, '_');
+
+	if ((type_end != NULL) &&
+	    (strncmp(cnt_name, "rx_", strlen("rx_")) == 0)) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_rx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_rx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_rx_packets", cnt_type_len);
+		else if (strncmp(type_end, "_placement",
+				 strlen("_placement")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_buff", strlen("_buff")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		(strncmp(cnt_name, "tx_", strlen("tx_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_tx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_tx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_tx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_tx_packets", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		   (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
+		if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "operations", cnt_type_len);
+		else if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+		else if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "filter_result", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		   (strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+	} else {
+		/* Does not fit obvious type, or strrchr error: */
+		/* use a more generic type */
+		strncpy(cnt_type, "derive", cnt_type_len);
+	}
+}
+
 static void
 nic_xstats_display(uint8_t port_id)
 {
@@ -281,10 +377,23 @@ nic_xstats_display(uint8_t port_id)
 		goto err;
 	}
 
-	for (i = 0; i < len; i++)
-		printf("%s: %"PRIu64"\n",
-			xstats_names[i].name,
-			xstats[i].value);
+	for (i = 0; i < len; i++) {
+		if (enable_collectd_format) {
+			char counter_type[MAX_STRING_LEN];
+			char buf[MAX_STRING_LEN];
+
+			collectd_resolve_cnt_type(counter_type,
+						  sizeof(counter_type),
+						  xstats_names[i].name);
+			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
+				PRIu64"\n", host_id, port_id, counter_type,
+				xstats_names[i].name, xstats[i].value);
+			write(stdout_fd, buf, strlen(buf));
+		} else {
+			printf("%s: %"PRIu64"\n", xstats_names[i].name,
+			       xstats[i].value);
+		}
+	}
 
 	printf("%s############################\n",
 			   nic_stats_border);
@@ -312,6 +421,13 @@ main(int argc, char **argv)
 	char *argp[argc + 3];
 	uint8_t nb_ports;
 
+	/* preparse app arguments */
+	ret = proc_info_preparse_args(argc, argv);
+	if (ret < 0) {
+		printf("Failed to parse arguments\n");
+		return -1;
+	}
+
 	argp[0] = argv[0];
 	argp[1] = c_flag;
 	argp[2] = n_flag;
-- 
2.1.0

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

* Re: [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options.
  2017-02-24 16:52 [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options Roman Korynkevych
  2017-02-27 14:16 ` [dpdk-dev] [PATCH v2] " Roman Korynkevych
@ 2017-02-27 15:12 ` Van Haaren, Harry
  1 sibling, 0 replies; 8+ messages in thread
From: Van Haaren, Harry @ 2017-02-27 15:12 UTC (permalink / raw)
  To: Korynkevych, RomanX, dev; +Cc: Korynkevych, RomanX

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Roman Korynkevych
> Sent: Friday, February 24, 2017 4:52 PM
> To: dev@dpdk.org
> Cc: Korynkevych, RomanX <romanx.korynkevych@intel.com>
> Subject: [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options.
> 
> Extended proc-info application to send DPDK port statistics to
> STDOUT in the format expected by collectd exec plugin. Added
> HOST ID option to identify the host DPDK process is running on
> when multiple instance of DPDK are running in parallel. This is
> needed for the barometer project in OPNFV.
> 
> Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
> ---

One comment on using hostname retrieval below, but with that fixed in a v2;

Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>


>  app/proc_info/main.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 119 insertions(+), 5 deletions(-)

<snip>

> +static int
> +proc_info_preparse_args(int argc, char **argv)
> +{
> +	char *prgname = argv[0];
> +	int i;
> +
> +	for (i = 0; i < argc; i++) {
> +		/* Print stats or xstats to STDOUT in collectd format */
> +		if (!strncmp(argv[i], "--collectd-format", MAX_LONG_OPT_SZ)) {
> +			enable_collectd_format = 1;
> +			stdout_fd = dup(STDOUT_FILENO);
> +			close(STDOUT_FILENO);
> +		}
> +		if (!strncmp(argv[i], "--host-id", MAX_LONG_OPT_SZ)) {
> +			if ((i + 1) == argc) {
> +				printf("Invalid host id or not specified\n");
> +                                proc_info_usage(prgname);
> +                                return -1;
> +			}
> +			strncpy(host_id, argv[i+1], sizeof(host_id));
> +		}
> +	}
> +
> +	if (!strlen(host_id))
> +		strcpy(host_id, "unknown");



We should get the machine hostname as a default, IMO better than "unknown". We can fallback to that if the hostname isn't set at all:

if (!strlen(host_id))
     int err = gethostname(host_id, MAX_LONG_OPT_SZ-1);
     if(err)
          strcpy(host_id, "unknown");


> +
> +	return 0;
> +}
> +

<snip>

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

* Re: [dpdk-dev] [PATCH v2] proc-info: added collectd-format and host-id options.
  2017-02-27 14:16 ` [dpdk-dev] [PATCH v2] " Roman Korynkevych
@ 2017-02-27 15:18   ` Van Haaren, Harry
  2017-03-01 16:27   ` [dpdk-dev] [PATCH v3] " Roman Korynkevych
  1 sibling, 0 replies; 8+ messages in thread
From: Van Haaren, Harry @ 2017-02-27 15:18 UTC (permalink / raw)
  To: Korynkevych, RomanX, dev; +Cc: Korynkevych, RomanX

Apologies, I reviewed patch v2, but replied to v1 on the mailing list.


For reference, the original reply on v1:
http://dpdk.org/ml/archives/dev/2017-February/058518.html


The same feedback provided here:


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Roman Korynkevych
> Sent: Monday, February 27, 2017 2:16 PM
> To: dev@dpdk.org
> Cc: Korynkevych, RomanX <romanx.korynkevych@intel.com>
> Subject: [dpdk-dev] [PATCH v2] proc-info: added collectd-format and host-id options.
> 
> Extended proc-info application to send DPDK port statistics to
> STDOUT in the format expected by collectd exec plugin. Added
> HOST ID option to identify the host DPDK process is running on
> when multiple instance of DPDK are running in parallel. This is
> needed for the barometer project in OPNFV.
> 
> Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>


One comment on using hostname retrieval below, but with that fixed in a v3;

Reviewed-by: Harry van Haaren <harry.van.haaren at intel.com>


> ---
>  app/proc_info/main.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 121 insertions(+), 5 deletions(-)
> 
> diff --git a/app/proc_info/main.c b/app/proc_info/main.c

<snip>

> 
> +static int
> +proc_info_preparse_args(int argc, char **argv)
> +{
> +	char *prgname = argv[0];
> +	int i;
> +
> +	for (i = 0; i < argc; i++) {
> +		/* Print stats or xstats to STDOUT in collectd format */
> +		if (!strncmp(argv[i], "--collectd-format", MAX_LONG_OPT_SZ)) {
> +			enable_collectd_format = 1;
> +			stdout_fd = dup(STDOUT_FILENO);
> +			close(STDOUT_FILENO);
> +		}
> +		if (!strncmp(argv[i], "--host-id", MAX_LONG_OPT_SZ)) {
> +			if ((i + 1) == argc) {
> +				printf("Invalid host id or not specified\n");
> +				proc_info_usage(prgname);
> +				return -1;
> +			}
> +			strncpy(host_id, argv[i+1], sizeof(host_id));
> +		}
> +	}
> +
> +	if (!strlen(host_id))
> +		strcpy(host_id, "unknown");
> +
> +	return 0;
> +}
> +


We should get the machine hostname as a default, IMO better than "unknown". We can fallback to that if the hostname isn't set at all:

if (!strlen(host_id))
     int err = gethostname(host_id, MAX_LONG_OPT_SZ-1);
     if(err)
          strcpy(host_id, "unknown");

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

* [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id options.
  2017-02-27 14:16 ` [dpdk-dev] [PATCH v2] " Roman Korynkevych
  2017-02-27 15:18   ` Van Haaren, Harry
@ 2017-03-01 16:27   ` Roman Korynkevych
  2017-03-03 13:31     ` Tahhan, Maryam
  1 sibling, 1 reply; 8+ messages in thread
From: Roman Korynkevych @ 2017-03-01 16:27 UTC (permalink / raw)
  To: dev; +Cc: Roman Korynkevych

Extended proc-info application to send DPDK port statistics to
STDOUT in the format expected by collectd exec plugin. Added
HOST ID option to identify the host DPDK process is running on
when multiple instance of DPDK are running in parallel. This is
needed for the barometer project in OPNFV.

Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
---
 app/proc_info/main.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 125 insertions(+), 5 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..ef2098d 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -40,6 +40,7 @@
 #include <sys/queue.h>
 #include <stdlib.h>
 #include <getopt.h>
+#include <unistd.h>
 
 #include <rte_eal.h>
 #include <rte_common.h>
@@ -62,12 +63,20 @@
 #define MAX_LONG_OPT_SZ 64
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
 
+#define MAX_STRING_LEN 256
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
 static uint32_t enable_stats;
 /**< Enable xstats. */
 static uint32_t enable_xstats;
+/**< Enable collectd format*/
+static uint32_t enable_collectd_format;
+/**< FD to send collectd format messages to STDOUT*/
+static int stdout_fd;
+/**< Host id process is running on */
+static char host_id[MAX_LONG_OPT_SZ];
 /**< Enable stats reset. */
 static uint32_t reset_stats;
 /**< Enable xstats reset. */
@@ -86,7 +95,9 @@ proc_info_usage(const char *prgname)
 		"  --xstats: to display extended port statistics, disabled by "
 			"default\n"
 		"  --stats-reset: to reset port statistics\n"
-		"  --xstats-reset: to reset port extended statistics\n",
+		"  --xstats-reset: to reset port extended statistics\n"
+		"  --collectd-format: to print statistics to STDOUT in expected by collectd format\n"
+		"  --host-id STRING: host id used to identify the system process is running on\n",
 		prgname);
 }
 
@@ -116,6 +127,39 @@ parse_portmask(const char *portmask)
 
 }
 
+static int
+proc_info_preparse_args(int argc, char **argv)
+{
+	char *prgname = argv[0];
+	int i;
+
+	for (i = 0; i < argc; i++) {
+		/* Print stats or xstats to STDOUT in collectd format */
+		if (!strncmp(argv[i], "--collectd-format", MAX_LONG_OPT_SZ)) {
+			enable_collectd_format = 1;
+			stdout_fd = dup(STDOUT_FILENO);
+			close(STDOUT_FILENO);
+		}
+		if (!strncmp(argv[i], "--host-id", MAX_LONG_OPT_SZ)) {
+			if ((i + 1) == argc) {
+				printf("Invalid host id or not specified\n");
+				proc_info_usage(prgname);
+				return -1;
+			}
+			strncpy(host_id, argv[i+1], sizeof(host_id));
+		}
+	}
+
+	if (!strlen(host_id)) {
+		int err = gethostname(host_id, MAX_LONG_OPT_SZ-1);
+
+		if (err)
+			strcpy(host_id, "unknown");
+	}
+
+	return 0;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 proc_info_parse_args(int argc, char **argv)
@@ -128,6 +172,8 @@ proc_info_parse_args(int argc, char **argv)
 		{"stats-reset", 0, NULL, 0},
 		{"xstats", 0, NULL, 0},
 		{"xstats-reset", 0, NULL, 0},
+		{"collectd-format", 0, NULL, 0},
+		{"host-id", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -240,6 +286,60 @@ nic_stats_clear(uint8_t port_id)
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
 }
 
+static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
+				      const char *cnt_name) {
+	char *type_end = strrchr(cnt_name, '_');
+
+	if ((type_end != NULL) &&
+	    (strncmp(cnt_name, "rx_", strlen("rx_")) == 0)) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_rx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_rx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_rx_packets", cnt_type_len);
+		else if (strncmp(type_end, "_placement",
+				 strlen("_placement")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_buff", strlen("_buff")) == 0)
+			strncpy(cnt_type, "if_rx_errors", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		(strncmp(cnt_name, "tx_", strlen("tx_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "if_tx_errors", cnt_type_len);
+		else if (strncmp(type_end, "_dropped", strlen("_dropped")) == 0)
+			strncpy(cnt_type, "if_tx_dropped", cnt_type_len);
+		else if (strncmp(type_end, "_bytes", strlen("_bytes")) == 0)
+			strncpy(cnt_type, "if_tx_octets", cnt_type_len);
+		else if (strncmp(type_end, "_packets", strlen("_packets")) == 0)
+			strncpy(cnt_type, "if_tx_packets", cnt_type_len);
+		else
+			/* Does not fit obvious type: use a more generic one */
+			strncpy(cnt_type, "derive", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		   (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
+		if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "operations", cnt_type_len);
+		else if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+		else if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
+			strncpy(cnt_type, "filter_result", cnt_type_len);
+	} else if ((type_end != NULL) &&
+		   (strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) {
+		if (strncmp(type_end, "_errors", strlen("_errors")) == 0)
+			strncpy(cnt_type, "errors", cnt_type_len);
+	} else {
+		/* Does not fit obvious type, or strrchr error: */
+		/* use a more generic type */
+		strncpy(cnt_type, "derive", cnt_type_len);
+	}
+}
+
 static void
 nic_xstats_display(uint8_t port_id)
 {
@@ -281,10 +381,23 @@ nic_xstats_display(uint8_t port_id)
 		goto err;
 	}
 
-	for (i = 0; i < len; i++)
-		printf("%s: %"PRIu64"\n",
-			xstats_names[i].name,
-			xstats[i].value);
+	for (i = 0; i < len; i++) {
+		if (enable_collectd_format) {
+			char counter_type[MAX_STRING_LEN];
+			char buf[MAX_STRING_LEN];
+
+			collectd_resolve_cnt_type(counter_type,
+						  sizeof(counter_type),
+						  xstats_names[i].name);
+			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
+				PRIu64"\n", host_id, port_id, counter_type,
+				xstats_names[i].name, xstats[i].value);
+			write(stdout_fd, buf, strlen(buf));
+		} else {
+			printf("%s: %"PRIu64"\n", xstats_names[i].name,
+			       xstats[i].value);
+		}
+	}
 
 	printf("%s############################\n",
 			   nic_stats_border);
@@ -312,6 +425,13 @@ main(int argc, char **argv)
 	char *argp[argc + 3];
 	uint8_t nb_ports;
 
+	/* preparse app arguments */
+	ret = proc_info_preparse_args(argc, argv);
+	if (ret < 0) {
+		printf("Failed to parse arguments\n");
+		return -1;
+	}
+
 	argp[0] = argv[0];
 	argp[1] = c_flag;
 	argp[2] = n_flag;
-- 
2.1.0

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

* Re: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id options.
  2017-03-01 16:27   ` [dpdk-dev] [PATCH v3] " Roman Korynkevych
@ 2017-03-03 13:31     ` Tahhan, Maryam
  2017-03-03 14:23       ` Van Haaren, Harry
  0 siblings, 1 reply; 8+ messages in thread
From: Tahhan, Maryam @ 2017-03-03 13:31 UTC (permalink / raw)
  To: Korynkevych, RomanX, dev; +Cc: Korynkevych, RomanX

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Roman Korynkevych
> Sent: Wednesday, March 1, 2017 4:27 PM
> To: dev@dpdk.org
> Cc: Korynkevych, RomanX <romanx.korynkevych@intel.com>
> Subject: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id
> options.
> 
> Extended proc-info application to send DPDK port statistics to STDOUT in the
> format expected by collectd exec plugin. Added HOST ID option to identify the
> host DPDK process is running on when multiple instance of DPDK are running in
> parallel. This is needed for the barometer project in OPNFV.
> 
> Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>

Reviewed-by: Maryam Tahhan <maryam.tahhan@intel.com>

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

* Re: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id options.
  2017-03-03 13:31     ` Tahhan, Maryam
@ 2017-03-03 14:23       ` Van Haaren, Harry
  2017-03-10 14:32         ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Van Haaren, Harry @ 2017-03-03 14:23 UTC (permalink / raw)
  To: Tahhan, Maryam, dev; +Cc: Korynkevych, RomanX

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tahhan, Maryam
> Sent: Friday, March 3, 2017 1:32 PM
> To: Korynkevych, RomanX <romanx.korynkevych@intel.com>; dev@dpdk.org
> Cc: Korynkevych, RomanX <romanx.korynkevych@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id options.
> 
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Roman Korynkevych
> > Sent: Wednesday, March 1, 2017 4:27 PM
> > To: dev@dpdk.org
> > Cc: Korynkevych, RomanX <romanx.korynkevych@intel.com>
> > Subject: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id
> > options.
> >
> > Extended proc-info application to send DPDK port statistics to STDOUT in the
> > format expected by collectd exec plugin. Added HOST ID option to identify the
> > host DPDK process is running on when multiple instance of DPDK are running in
> > parallel. This is needed for the barometer project in OPNFV.
> >
> > Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
> 
> Reviewed-by: Maryam Tahhan <maryam.tahhan@intel.com>


Hi Roman,

Just a few pointers on DPDK patch rework / review practice;

- CC people involved in discussion / review of previous versions
- If somebody review / acks your patch, with a request for simple-error rework,
  it is allowed to include their Reviewed-by / acked by. Note the changes must
  be minor, or keeping the Review/Ack suggested by the person[1]
- When updating a patch, it is good practice to summarize the
  changes made under the --- line, so it is easier for reviewers
  to see changes made.

That said, no problem, just something to keep in mind for next patches :)
Nice work,

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>


[1] http://dpdk.org/dev/patchwork/patch/20898/

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

* Re: [dpdk-dev] [PATCH v3] proc-info: added collectd-format and host-id options.
  2017-03-03 14:23       ` Van Haaren, Harry
@ 2017-03-10 14:32         ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2017-03-10 14:32 UTC (permalink / raw)
  To: Korynkevych, RomanX; +Cc: dev, Van Haaren, Harry, Tahhan, Maryam

> > > Extended proc-info application to send DPDK port statistics to STDOUT in the
> > > format expected by collectd exec plugin. Added HOST ID option to identify the
> > > host DPDK process is running on when multiple instance of DPDK are running in
> > > parallel. This is needed for the barometer project in OPNFV.
> > >
> > > Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
> > 
> > Reviewed-by: Maryam Tahhan <maryam.tahhan@intel.com>
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied, thanks

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

end of thread, other threads:[~2017-03-10 14:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-24 16:52 [dpdk-dev] [PATCH] proc-info: added collectd-format and host-id options Roman Korynkevych
2017-02-27 14:16 ` [dpdk-dev] [PATCH v2] " Roman Korynkevych
2017-02-27 15:18   ` Van Haaren, Harry
2017-03-01 16:27   ` [dpdk-dev] [PATCH v3] " Roman Korynkevych
2017-03-03 13:31     ` Tahhan, Maryam
2017-03-03 14:23       ` Van Haaren, Harry
2017-03-10 14:32         ` Thomas Monjalon
2017-02-27 15:12 ` [dpdk-dev] [PATCH] " Van Haaren, Harry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).