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.laatz@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); +} + +/* + * 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(); + + if (TAILQ_EMPTY(link_list)) { + snprintf(out, out_size, "No links - nothing to show\n"); + return; + } + + 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; + } + + print_link_info(link, o, os); + i++; + } + } else { + link_name = tokens[2]; + link = link_find(link_name); + 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; +} + 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
> -----Original Message----- > From: Laatz, Kevin > Sent: Friday, June 8, 2018 5:31 PM > To: dev@dpdk.org > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Singh, Jasvinder > <jasvinder.singh@intel.com>; Laatz, Kevin <kevin.laatz@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.laatz@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
Hi Cristian, > > + > > + link_list = links_get(); > > We don't need this function, the lined list of links is a global public object, > please access it directly. link_list is declared as a static struct in link.c so it is not accessible from cli.c I can either leave this function in to pass the pointer to the cli.c or move the declaration to link.h. Do you have a preference? > > > > +struct link_list * > > +links_get(void) > > +{ > > + return &link_list; > > +} > > + > > Function to be removed, used linked list of links directly (see comment > above). See above comment Thanks, Kevin
Hi Kevin,
If you only need access to this linked list for iterating through it, you can add a new function: struct link *link_next(struct link *) that gets you the next element in the list (for first element, invoke with link = NULL; at and of list, it returns NULL). Makes sense?
Are there other usages of this linked list that you need to cover?
Regards,
Cristian
> -----Original Message-----
> From: Laatz, Kevin
> Sent: Friday, June 15, 2018 3:57 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org
> Cc: Singh, Jasvinder <jasvinder.singh@intel.com>
> Subject: RE: [PATCH] examples/ip_pipeline: add link show to the cli
>
> Hi Cristian,
>
> > > +
> > > + link_list = links_get();
> >
> > We don't need this function, the lined list of links is a global public object,
> > please access it directly.
>
> link_list is declared as a static struct in link.c so it is not accessible from cli.c
>
> I can either leave this function in to pass the pointer to the cli.c or move the
> declaration to link.h.
> Do you have a preference?
>
> > >
> > > +struct link_list *
> > > +links_get(void)
> > > +{
> > > + return &link_list;
> > > +}
> > > +
> >
> > Function to be removed, used linked list of links directly (see comment
> > above).
>
> See above comment
>
> Thanks,
> Kevin
Add the functionality to track links in the application. This enables the user to print the name, mac address and statistics for each link in the application. Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> --- v2: - Implemeted API to iterate through the linked list - Printing tidy up - Removed unnecessary checks --- examples/ip_pipeline/cli.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ examples/ip_pipeline/link.c | 6 +++ examples/ip_pipeline/link.h | 3 ++ 3 files changed, 100 insertions(+) diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c index c9587f5..27c9844 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" @@ -237,6 +238,91 @@ cmd_link(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; + uint16_t mtu; + + 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); + rte_eth_dev_get_mtu(link->port_id, &mtu); + + snprintf(out, out_size, + "\n" + "%s: flags=<%s> mtu %u\n" + "\tether %02X:%02X:%02X:%02X:%02X:%02X rxqueues %u txqueues %u\n" + "\tport# %u speed %u Mbps\n" + "\tRX packets %" PRIu64" bytes %" PRIu64"\n" + "\tRX errors %" PRIu64" missed %" PRIu64" no-mbuf %" PRIu64"\n" + "\tTX packets %" PRIu64" bytes %" PRIu64"\n" + "\tTX errors %" PRIu64"\n", + link->name, + eth_link.link_status == 0 ? "DOWN" : "UP", + mtu, + 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], + link->n_rxq, + link->n_txq, + link->port_id, + eth_link.link_speed, + stats.ipackets, + stats.ibytes, + stats.ierrors, + stats.imissed, + stats.rx_nombuf, + stats.opackets, + stats.obytes, + stats.oerrors); +} + +/* + * link show [<link_name>] + */ +static void +cmd_link_show(char **tokens, uint32_t n_tokens, char *out, size_t out_size) +{ + struct link *link; + char *link_name; + + if (n_tokens != 2 && n_tokens != 3) { + snprintf(out, out_size, MSG_ARG_MISMATCH, tokens[0]); + return; + } + + if (n_tokens == 2) { + link = link_next(NULL); + + while (link != NULL) { + out_size = out_size - strlen(out); + out = &out[strlen(out)]; + + print_link_info(link, out, out_size); + link = link_next(link); + } + } else { + out_size = out_size - strlen(out); + out = &out[strlen(out)]; + + link_name = tokens[2]; + link = link_find(link_name); + + if (link == NULL) { + snprintf(out, out_size, MSG_ARG_INVALID, + "Link does not exist"); + return; + } + print_link_info(link, out, out_size); + } +} + /** * swq <swq_name> * size <size> @@ -4380,6 +4466,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..28a46d2 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 * +link_next(struct link *link) +{ + return (link == NULL) ? TAILQ_FIRST(&link_list) : TAILQ_NEXT(link, node); +} + 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..34ff114 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 * +link_next(struct link *link); + struct link_params_rss { uint32_t queue_id[LINK_RXQ_RSS_MAX]; uint32_t n_queues; -- 2.9.5