> -----Original Message-----
> From: Laatz, Kevin
> Sent: Friday, June 8, 2018 5:31 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitre...@intel.com>; Singh, Jasvinder
> <jasvinder.si...@intel.com>; Laatz, Kevin <kevin.la...@intel.com>
> Subject: [PATCH] examples/ip_pipeline: add link show to the cli
>
> Add the functionality to track links in the application. This enables the
> user to print the name, mac address and per-port statistics for each link
> in the application.
>
> Signed-off-by: Kevin Laatz <kevin.la...@intel.com>
> ---
> examples/ip_pipeline/cli.c | 103
> ++++++++++++++++++++++++++++++++++++++++++++
> examples/ip_pipeline/link.c | 6 +++
> examples/ip_pipeline/link.h | 3 ++
> 3 files changed, 112 insertions(+)
>
> diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c
> index c9587f5..b7fba3b 100644
> --- a/examples/ip_pipeline/cli.c
> +++ b/examples/ip_pipeline/cli.c
> @@ -9,6 +9,7 @@
>
> #include <rte_common.h>
> #include <rte_cycles.h>
> +#include <rte_ethdev.h>
>
> #include "cli.h"
> #include "kni.h"
> @@ -4355,6 +4356,103 @@ cmd_thread_pipeline_disable(char **tokens,
> }
> }
>
> +/* Print the link stats and info */
> +static void
> +print_link_info(struct link *link, char *out, size_t out_size)
> +{
> + struct rte_eth_stats stats;
> + struct ether_addr mac_addr;
> + struct rte_eth_link eth_link;
> +
> + memset(&stats, 0, sizeof(stats));
> + rte_eth_stats_get(link->port_id, &stats);
> +
> + rte_eth_macaddr_get(link->port_id, &mac_addr);
> + rte_eth_link_get(link->port_id, ð_link);
> +
> + snprintf(out, out_size,
> + "\n"
> + "Link name: %s\n"
> + "\tPort %u MAC: %02"PRIx8":%02"PRIx8":%02"PRIx8
> + ":%02"PRIx8":%02"PRIx8":%02"PRIx8"\n"
> + "\tLink Status: %s\n"
> + "\tLink Speed: %u\n"
> + "\n"
> + "\tRX info:\n"
> + "\t\tQueues: %u\n"
> + "\t\tPackets: %" PRIu64
> + "\t\tBytes: %" PRIu64"\n"
> + "\t\tErrors: %" PRIu64
> + "\t\tMissed: %" PRIu64
> + "\t\tNo-mbuf: %" PRIu64"\n"
> + "\tTX info:\n"
> + "\t\tQueues: %u\n"
> + "\t\tPackets: %" PRIu64
> + "\t\tBytes: %" PRIu64"\n"
> + "\t\tErrors: %" PRIu64"\n",
> + link->name,
> + link->port_id,
> + mac_addr.addr_bytes[0], mac_addr.addr_bytes[1],
> + mac_addr.addr_bytes[2], mac_addr.addr_bytes[3],
> + mac_addr.addr_bytes[4], mac_addr.addr_bytes[5],
> + eth_link.link_status == 1 ? "UP" : "DOWN",
> + eth_link.link_speed,
> + link->n_rxq,
> + stats.ipackets,
> + stats.ibytes,
> + stats.ierrors,
> + stats.imissed,
> + stats.rx_nombuf,
> + link->n_txq,
> + stats.opackets,
> + stats.obytes,
> + stats.oerrors);
> +}
Can we organize this as table, so for each link we print exactly one line?
If one link does not fit into a single line, we can have a
print_link_info_short() that is used when all links are printed, and a
print_link_info_long() that is used only when a single link is printed.
Don't forget about a function to print the table header. :)
> +
> +/*
> + * link show [<link_name>]
> + */
> +static void
> +cmd_link_show(char **tokens, uint32_t n_tokens, char *out, size_t
> out_size)
> +{
> + struct link *link;
> + struct link_list *link_list;
> + char *link_name;
> + uint32_t i = 0;
> +
> + if (n_tokens != 2 && n_tokens != 3) {
> + snprintf(out, out_size, MSG_ARG_MISMATCH, tokens[0]);
> + return;
> + }
> +
> + link_list = links_get();
We don't need this function, the lined list of links is a global public object,
please access it directly.
> +
> + if (TAILQ_EMPTY(link_list)) {
> + snprintf(out, out_size, "No links - nothing to show\n");
> + return;
> + }
No need for this, could be detected by empty table (only table header gets
printed).
> +
> + if (n_tokens == 2) {
> + TAILQ_FOREACH(link, link_list, node) {
> + size_t os = out_size - (i * strlen(out));
> + char *o = &out[i * strlen(out)];
> +
> + if (os == 0) {
> + snprintf(out, out_size, MSG_CMD_FAIL,
> + "Output buffer too small");
> + return;
> + }
> +
This check can be removed, no need to worry about output buffer overrun.
> + print_link_info(link, o, os);
> + i++;
> + }
> + } else {
> + link_name = tokens[2];
> + link = link_find(link_name);
What if link with this name does not exist? Checking that link is not NULL
might avoid a crash as well :)
> + print_link_info(link, out, out_size);
> + }
> +}
> +
> void
> cli_process(char *in, char *out, size_t out_size)
> {
> @@ -4380,6 +4478,11 @@ cli_process(char *in, char *out, size_t out_size)
> }
>
> if (strcmp(tokens[0], "link") == 0) {
> + if (strcmp(tokens[1], "show") == 0) {
> + cmd_link_show(tokens, n_tokens, out, out_size);
> + return;
> + }
> +
> cmd_link(tokens, n_tokens, out, out_size);
> return;
> }
> diff --git a/examples/ip_pipeline/link.c b/examples/ip_pipeline/link.c
> index b8a431f..b11f0c1 100644
> --- a/examples/ip_pipeline/link.c
> +++ b/examples/ip_pipeline/link.c
> @@ -36,6 +36,12 @@ link_find(const char *name)
> return NULL;
> }
>
> +struct link_list *
> +links_get(void)
> +{
> + return &link_list;
> +}
> +
Function to be removed, used linked list of links directly (see comment above).
> static struct rte_eth_conf port_conf_default = {
> .link_speeds = 0,
> .rxmode = {
> diff --git a/examples/ip_pipeline/link.h b/examples/ip_pipeline/link.h
> index 37d3dc4..10f9d51 100644
> --- a/examples/ip_pipeline/link.h
> +++ b/examples/ip_pipeline/link.h
> @@ -30,6 +30,9 @@ link_init(void);
> struct link *
> link_find(const char *name);
>
> +struct link_list *
> +links_get(void);
> +
> struct link_params_rss {
> uint32_t queue_id[LINK_RXQ_RSS_MAX];
> uint32_t n_queues;
> --
> 2.9.5
