From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id DBED35F22 for ; Fri, 8 Jun 2018 18:51:14 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jun 2018 09:51:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,490,1520924400"; d="scan'208";a="235843311" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by fmsmga006.fm.intel.com with ESMTP; 08 Jun 2018 09:51:13 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.139]) by IRSMSX107.ger.corp.intel.com ([169.254.10.238]) with mapi id 14.03.0319.002; Fri, 8 Jun 2018 17:51:12 +0100 From: "Dumitrescu, Cristian" To: "Laatz, Kevin" , "dev@dpdk.org" CC: "Singh, Jasvinder" Thread-Topic: [PATCH] examples/ip_pipeline: add link show to the cli Thread-Index: AQHT/0YppBug4mZuIkqcEWNgOpUgOKRWkDPg Date: Fri, 8 Jun 2018 16:51:11 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891268E757604@IRSMSX108.ger.corp.intel.com> References: <20180608163053.37891-1-kevin.laatz@intel.com> In-Reply-To: <20180608163053.37891-1-kevin.laatz@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] examples/ip_pipeline: add link show to the cli X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jun 2018 16:51:15 -0000 > -----Original Message----- > From: Laatz, Kevin > Sent: Friday, June 8, 2018 5:31 PM > To: dev@dpdk.org > Cc: Dumitrescu, Cristian ; Singh, Jasvinde= r > ; Laatz, Kevin > Subject: [PATCH] examples/ip_pipeline: add link show to the cli >=20 > 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. >=20 > Signed-off-by: Kevin Laatz > --- > examples/ip_pipeline/cli.c | 103 > ++++++++++++++++++++++++++++++++++++++++++++ > examples/ip_pipeline/link.c | 6 +++ > examples/ip_pipeline/link.h | 3 ++ > 3 files changed, 112 insertions(+) >=20 > 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 @@ >=20 > #include > #include > +#include >=20 > #include "cli.h" > #include "kni.h" > @@ -4355,6 +4356,103 @@ cmd_thread_pipeline_disable(char **tokens, > } > } >=20 > +/* 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 =3D=3D 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 [] > + */ > +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 =3D 0; > + > + if (n_tokens !=3D 2 && n_tokens !=3D 3) { > + snprintf(out, out_size, MSG_ARG_MISMATCH, tokens[0]); > + return; > + } > + > + link_list =3D links_get(); We don't need this function, the lined list of links is a global public obj= ect, 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 =3D=3D 2) { > + TAILQ_FOREACH(link, link_list, node) { > + size_t os =3D out_size - (i * strlen(out)); > + char *o =3D &out[i * strlen(out)]; > + > + if (os =3D=3D 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 =3D tokens[2]; > + link =3D 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) > } >=20 > if (strcmp(tokens[0], "link") =3D=3D 0) { > + if (strcmp(tokens[1], "show") =3D=3D 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; > } >=20 > +struct link_list * > +links_get(void) > +{ > + return &link_list; > +} > + Function to be removed, used linked list of links directly (see comment abo= ve). > static struct rte_eth_conf port_conf_default =3D { > .link_speeds =3D 0, > .rxmode =3D { > 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); >=20 > +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