[v4,4/9] app/procinfo: add support for show port
Checks
Commit Message
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
> -----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
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.
> > + 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
>
> 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
@@ -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
@@ -3,4 +3,4 @@
sources = files('main.c')
allow_experimental_apis = true
-deps += ['ethdev', 'metrics']
+deps += ['ethdev', 'metrics', 'security']