diff mbox series

[v4,4/9] app/procinfo: add support for show port

Message ID 20181106124912.40700-4-vipin.varghese@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series [v4,1/9] app/procinfo: add usage for new debug | expand

Checks

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

Commit Message

Varghese, Vipin Nov. 6, 2018, 12:49 p.m. UTC
Function show_port is used for displaying the port PMD information under
under primary process. The information shows basic, per queue and security.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---

V3:
 - fix meson build - Reshma Pattan
 - change 100 to MAX_STRING_LEN - Reshma Pattan
 - memset to struct elements - Reshma Pattan
 - printf tab space - Reshma Pattan
 - remove 'drop packet information' - Vipin Varghese

V2:
 - redefine code format - Vipin Varghese
---
 app/proc-info/main.c      | 120 +++++++++++++++++++++++++++++++++++++-
 app/proc-info/meson.build |   2 +-
 2 files changed, 120 insertions(+), 2 deletions(-)

Comments

Pattan, Reshma Nov. 21, 2018, 2:24 p.m. UTC | #1
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Tuesday, November 6, 2018 12:49 PM
> To: dev@dpdk.org; thomas@monjalon.net; Pattan, Reshma
> <reshma.pattan@intel.com>; stephen@networkplumber.org; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: Byrne, Stephen1 <stephen1.byrne@intel.com>; Glynn, Michael J
> <michael.j.glynn@intel.com>; Patel, Amol <amol.patel@intel.com>; Varghese,
> Vipin <vipin.varghese@intel.com>
> Subject: [PATCH v4 4/9] app/procinfo: add support for show port


> +		snprintf(bdr_str, 100, " Port (%u)", i);

%s/100/MAX_STRING_LEN ?

> +			ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
> +			if ((ret) || (rss_conf.rss_key == NULL))
> +				continue;
> +
> +			printf("\t  -- RSS len %u key (hex):",
> +				rss_conf.rss_key_len);
> +			for (k = 0; k < rss_conf.rss_key_len; k++)
> +				printf(" %x", rss_conf.rss_key[k]);
> +			printf("\t  -- hf 0x%"PRIx64"\n",
> +				rss_conf.rss_hf);
> +		}
> +

Should this be out of queues for loop? 

Thanks,
Reshma
Stephen Hemminger Nov. 21, 2018, 8:22 p.m. UTC | #2
On Tue,  6 Nov 2018 18:19:07 +0530
Vipin Varghese <vipin.varghese@intel.com> wrote:

Minor observations which are things that checkpatch etc won't see
but make the code easier to read/maintain.
 
> +/* border variable to hold for show */
> +char bdr_str[MAX_STRING_LEN];

Does this have to be global, could it just be static?

> +		memset(&link, 0, sizeof(link));
> +		memset(&dev_info, 0, sizeof(dev_info));
> +		memset(&queue_info, 0, sizeof(queue_info));
> +		memset(&stats, 0, sizeof(stats));
> +		memset(&rss_conf, 0, sizeof(rss_conf));

These memset's should be unnecessary. For example, dev_info
is always cleared already inside rte_eth_dev_info_get().

> +			if ((ret) || (rss_conf.rss_key == NULL))
> +				continue;

Unnecessary parenthesis hurt readability in this if statement.
Varghese, Vipin Nov. 22, 2018, 2:20 p.m. UTC | #3
> > +		snprintf(bdr_str, 100, " Port (%u)", i);
> 
> %s/100/MAX_STRING_LEN ?	

Done for v5

> 
> > +			ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
> > +			if ((ret) || (rss_conf.rss_key == NULL))
> > +				continue;
> > +
> > +			printf("\t  -- RSS len %u key (hex):",
> > +				rss_conf.rss_key_len);
> > +			for (k = 0; k < rss_conf.rss_key_len; k++)
> > +				printf(" %x", rss_conf.rss_key[k]);
> > +			printf("\t  -- hf 0x%"PRIx64"\n",
> > +				rss_conf.rss_hf);
> > +		}
> > +
> 
> Should this be out of queues for loop?
> 

Done for v5
Varghese, Vipin Nov. 22, 2018, 2:21 p.m. UTC | #4
> 
> Minor observations which are things that checkpatch etc won't see but make
> the code easier to read/maintain.
> 
> > +/* border variable to hold for show */ char bdr_str[MAX_STRING_LEN];

Done for v5

> 
> Does this have to be global, could it just be static?
> 
> > +		memset(&link, 0, sizeof(link));
> > +		memset(&dev_info, 0, sizeof(dev_info));
> > +		memset(&queue_info, 0, sizeof(queue_info));
> > +		memset(&stats, 0, sizeof(stats));
> > +		memset(&rss_conf, 0, sizeof(rss_conf));
> 
> These memset's should be unnecessary. For example, dev_info is always
> cleared already inside rte_eth_dev_info_get().

Done for v5 (link, dev_info, queue_info, stats)

> 
> > +			if ((ret) || (rss_conf.rss_key == NULL))
> > +				continue;
> 
> Unnecessary parenthesis hurt readability in this if statement.

done
diff mbox series

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 8b9cf629d..48477097f 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -29,6 +29,9 @@ 
 #include <rte_branch_prediction.h>
 #include <rte_string_fns.h>
 #include <rte_metrics.h>
+#include <rte_cycles.h>
+#include <rte_security.h>
+#include <rte_cryptodev.h>
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -81,6 +84,9 @@  static char *mempool_name;
 static uint32_t nb_xstats_ids;
 static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
 
+/* border variable to hold for show */
+char bdr_str[MAX_STRING_LEN];
+
 /**< display usage */
 static void
 proc_info_usage(const char *prgname)
@@ -631,7 +637,119 @@  metrics_display(int port_id)
 static void
 show_port(void)
 {
-	printf(" port\n");
+	uint16_t i = 0;
+	int ret = 0, j, k;
+
+	snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD %"PRIu64,
+			rte_get_tsc_hz());
+	STATS_BDR_STR(10, bdr_str);
+
+	RTE_ETH_FOREACH_DEV(i) {
+		uint16_t mtu = 0;
+		struct rte_eth_link link;
+		struct rte_eth_dev_info dev_info;
+		struct rte_eth_rxq_info queue_info;
+		struct rte_eth_stats stats;
+		struct rte_eth_rss_conf rss_conf;
+
+		memset(&link, 0, sizeof(link));
+		memset(&dev_info, 0, sizeof(dev_info));
+		memset(&queue_info, 0, sizeof(queue_info));
+		memset(&stats, 0, sizeof(stats));
+		memset(&rss_conf, 0, sizeof(rss_conf));
+
+		snprintf(bdr_str, 100, " Port (%u)", i);
+		STATS_BDR_STR(5, bdr_str);
+		printf("  - generic config\n");
+
+		printf("\t  -- Socket %d\n", rte_eth_dev_socket_id(i));
+		rte_eth_link_get(i, &link);
+		printf("\t  -- link speed %d duplex %d,"
+			" auto neg %d status %d\n",
+			link.link_speed,
+			link.link_duplex,
+			link.link_autoneg,
+			link.link_status);
+		printf("\t  -- promiscuous (%d)\n",
+			rte_eth_promiscuous_get(i));
+		ret = rte_eth_dev_get_mtu(i, &mtu);
+		if (ret == 0)
+			printf("\t  -- mtu (%d)\n", mtu);
+
+		printf("  - queue\n");
+
+		rte_eth_dev_info_get(i, &dev_info);
+
+		for (j = 0; j < dev_info.nb_rx_queues; j++) {
+			ret = rte_eth_rx_queue_info_get(i, j, &queue_info);
+			if (ret == 0) {
+				printf("\t  -- queue %d rx scatter %d"
+					" descriptors %d offloads 0x%"PRIx64
+					" mempool socket %d\n",
+					j,
+					queue_info.scattered_rx,
+					queue_info.nb_desc,
+					queue_info.conf.offloads,
+					queue_info.mp->socket_id);
+
+				ret = rte_eth_stats_get(i, &stats);
+				if (ret == 0) {
+					printf("\t  -- packet input %"PRIu64
+						" output %"PRIu64""
+						" error %"PRIu64"\n",
+						stats.q_ipackets[j],
+						stats.q_opackets[j],
+						stats.q_errors[j]);
+				}
+			}
+
+			ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
+			if ((ret) || (rss_conf.rss_key == NULL))
+				continue;
+
+			printf("\t  -- RSS len %u key (hex):",
+				rss_conf.rss_key_len);
+			for (k = 0; k < rss_conf.rss_key_len; k++)
+				printf(" %x", rss_conf.rss_key[k]);
+			printf("\t  -- hf 0x%"PRIx64"\n",
+				rss_conf.rss_hf);
+		}
+
+		ret = rte_eth_stats_get(i, &stats);
+		if (ret == 0) {
+			printf("\t  -- packet input %"PRIu64
+				" output %"PRIu64"\n",
+				stats.ipackets,
+				stats.opackets);
+			printf("\t  -- packet error input %"PRIu64
+				" output %"PRIu64"\n",
+				stats.ierrors,
+				stats.oerrors);
+			printf("\t  -- RX no mbuf %"PRIu64"\n",
+				stats.rx_nombuf);
+		}
+
+		printf("  - cyrpto context\n");
+		void *ptr_ctx = rte_eth_dev_get_sec_ctx(i);
+		printf("\t  -- security context - %p\n", ptr_ctx);
+
+		if (ptr_ctx) {
+			printf("\t  -- size %u\n",
+				rte_security_session_get_size(ptr_ctx));
+			const struct rte_security_capability *ptr_sec_cap =
+				rte_security_capabilities_get(ptr_ctx);
+			if (ptr_sec_cap) {
+				printf("\t  -- action (0x%x), protocol (0x%x),"
+					" offload flags (0x%x)\n",
+					ptr_sec_cap->action,
+					ptr_sec_cap->protocol,
+					ptr_sec_cap->ol_flags);
+				printf("\t  -- capabilities - oper type %x\n",
+					ptr_sec_cap->crypto_capabilities->op);
+			}
+		}
+	}
+	STATS_BDR_STR(50, "");
 }
 
 static void
diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
index a52b2ee4a..866b390d6 100644
--- a/app/proc-info/meson.build
+++ b/app/proc-info/meson.build
@@ -3,4 +3,4 @@ 
 
 sources = files('main.c')
 allow_experimental_apis = true
-deps += ['ethdev', 'metrics']
+deps += ['ethdev', 'metrics', 'security']