From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id EF5FD8B19 for ; Sun, 12 Oct 2014 13:03:08 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 12 Oct 2014 04:04:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,703,1406617200"; d="scan'208";a="587539917" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga001.jf.intel.com with ESMTP; 12 Oct 2014 04:10:41 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.195.1; Sun, 12 Oct 2014 04:10:41 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.195.1; Sun, 12 Oct 2014 04:10:41 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.192]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.203]) with mapi id 14.03.0195.001; Sun, 12 Oct 2014 19:10:39 +0800 From: "Liang, Cunming" To: Neil Horman Thread-Topic: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet Thread-Index: AQHP5IYD5aCkJuEDxEurfcum8uYUPpwpFr4AgALSmBA= Date: Sun, 12 Oct 2014 11:10:39 +0000 Message-ID: References: <1408947174-11323-1-git-send-email-cunming.liang@intel.com> <1412944201-30703-1-git-send-email-cunming.liang@intel.com> <1412944201-30703-2-git-send-email-cunming.liang@intel.com> <20141010175226.GG19499@hmsreliant.think-freely.org> In-Reply-To: <20141010175226.GG19499@hmsreliant.think-freely.org> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Oct 2014 11:03:10 -0000 Hi Neil, Very appreciate your comments. I add inline reply, will send v3 asap when we get alignment. BRs, Liang Cunming > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Saturday, October 11, 2014 1:52 AM > To: Liang, Cunming > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx = cycles/packet >=20 > On Fri, Oct 10, 2014 at 08:29:58PM +0800, Cunming Liang wrote: > > It provides unit test to measure cycles/packet in NIC loopback mode. > > It simply gives the average cycles of IO used per packet without test e= quipment. > > When doing the test, make sure the link is UP. > > > > Usage Example: > > 1. Run unit test app in interactive mode > > app/test -c f -n 4 -- -i > > 2. Run and wait for the result > > pmd_perf_autotest > > > > There's option to choose rx/tx pair, default is vector. > > set_rxtx_mode [vector|scalar|full|hybrid] > > Note: To get acurate scalar fast, please choose 'vector' or 'hybrid' wi= thout > INC_VEC=3Dy in config > > > > Signed-off-by: Cunming Liang > > Acked-by: Bruce Richardson >=20 > Notes inline >=20 > > --- > > app/test/Makefile | 1 + > > app/test/commands.c | 38 +++ > > app/test/packet_burst_generator.c | 4 +- > > app/test/test.h | 4 + > > app/test/test_pmd_perf.c | 626 > +++++++++++++++++++++++++++++++++++ > > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 6 + > > 6 files changed, 677 insertions(+), 2 deletions(-) > > create mode 100644 app/test/test_pmd_perf.c > > > > diff --git a/app/test/Makefile b/app/test/Makefile > > index 6af6d76..ebfa0ba 100644 > > --- a/app/test/Makefile > > +++ b/app/test/Makefile > > @@ -56,6 +56,7 @@ SRCS-y +=3D test_memzone.c > > > > SRCS-y +=3D test_ring.c > > SRCS-y +=3D test_ring_perf.c > > +SRCS-y +=3D test_pmd_perf.c > > > > ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) > > SRCS-y +=3D test_table.c > > diff --git a/app/test/commands.c b/app/test/commands.c > > index a9e36b1..f1e746e 100644 > > --- a/app/test/commands.c > > +++ b/app/test/commands.c > > @@ -310,12 +310,50 @@ cmdline_parse_inst_t cmd_quit =3D { > > > > +#define NB_ETHPORTS_USED (1) > > +#define NB_SOCKETS (2) > > +#define MEMPOOL_CACHE_SIZE 250 > > +#define MBUF_SIZE (2048 + sizeof(struct rte_mbuf) + > RTE_PKTMBUF_HEADROOM) > Don't you want to size this in accordance with the amount of data your se= nding > (64 Bytes as noted above)? [Liang, Cunming] The case is designed to measure small packet IO cost with = normal mbuf size. Even if decreasing the size, it won't gain significant cycles. >=20 > > +static void > > +print_ethaddr(const char *name, const struct ether_addr *eth_addr) > > +{ > > + printf("%s%02X:%02X:%02X:%02X:%02X:%02X", name, > > + eth_addr->addr_bytes[0], > > + eth_addr->addr_bytes[1], > > + eth_addr->addr_bytes[2], > > + eth_addr->addr_bytes[3], > > + eth_addr->addr_bytes[4], > > + eth_addr->addr_bytes[5]); > > +} > > + > This was copieed from print_ethaddr. Seems like a good candidate for a c= ommon > function in rte_ether.h [Liang, Cunming] Agree with you, some of samples now use it with the same c= opy. I'll rework it. Adding 'ether_format_addr' in rte_ether.h only for format t= he 48bits address output. And leaving other prints for application customization. >=20 >=20 > > +} > > + > > +static void > > +signal_handler(int signum) > > +{ > > + /* When we receive a USR1 signal, print stats */ > I think you mean SIGUSR2, below, SIGUSR1 tears the test down and exits th= e > program [Liang, Cunming] Thanks, it's a typo. >=20 > > + if (signum =3D=3D SIGUSR1) { > SIGINT instead. Thats the common practice. [Liang, Cunming] I understood your opinion.=20 The considerations I'm not using SIGINT instead are: 1. We unset ISIG in c_lflag of term. CRTL+C won't trigger SIGINT in command= interactive. It always has to explicitly send signal. No matter SIGUSR1 or SIGINT. 2. By SIGINT semantic, expect to terminate the process. Here I expect to force stop this case, but still alive in command line. After it stopped, it can run again or start to run other test cases. So I keep SIGINT, SIGUSR1 in different behavior. 3. It should be rarely used.=20 Only when exception timeout, I leave this backdoor for automation test co= ntrol. For manual test, we can easily force kill the process. >=20 > > + printf("Force Stop!\n"); > > + stop =3D 1; > > + } > > + if (signum =3D=3D SIGUSR2) > > + stats_display(0); > > +} > > +/* main processing loop */ > > +static int > > +main_loop(__rte_unused void *args) > > +{ > > +#define PACKET_SIZE 64 > > +#define FRAME_GAP 12 > > +#define MAC_PREAMBLE 8 > > + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; > > + unsigned lcore_id; > > + unsigned i, portid, nb_rx =3D 0, nb_tx =3D 0; > > + struct lcore_conf *conf; > > + uint64_t prev_tsc, cur_tsc; > > + int pkt_per_port; > > + uint64_t packets_per_second, total_packets; > > + > > + lcore_id =3D rte_lcore_id(); > > + conf =3D &lcore_conf[lcore_id]; > > + if (conf->status !=3D LCORE_USED) > > + return 0; > > + > > + pkt_per_port =3D MAX_TRAFIC_BURST / conf->nb_ports; > > + > > + int idx =3D 0; > > + for (i =3D 0; i < conf->nb_ports; i++) { > > + int num =3D pkt_per_port; > > + portid =3D conf->portlist[i]; > > + printf("inject %d packet to port %d\n", num, portid); > > + while (num) { > > + nb_tx =3D RTE_MIN(MAX_PKT_BURST, num); > > + nb_tx =3D rte_eth_tx_burst(portid, 0, > > + &tx_burst[idx], nb_tx); > > + num -=3D nb_tx; > > + idx +=3D nb_tx; > > + } > > + } > > + printf("Total packets inject to prime ports =3D %u\n", idx); > > + > > + packets_per_second =3D (link_mbps * 1000 * 1000) / > > + +((PACKET_SIZE + FRAME_GAP + MAC_PREAMBLE) * CHAR_BIT); > > + printf("Each port will do %"PRIu64" packets per second\n", > > + +packets_per_second); > > + > > + total_packets =3D RTE_TEST_DURATION * conf->nb_ports * > packets_per_second; > > + printf("Test will stop after at least %"PRIu64" packets received\n", > > + + total_packets); > > + > > + prev_tsc =3D rte_rdtsc(); > > + > > + while (likely(!stop)) { > > + for (i =3D 0; i < conf->nb_ports; i++) { > > + portid =3D conf->portlist[i]; > > + nb_rx =3D rte_eth_rx_burst((uint8_t) portid, 0, > > + pkts_burst, MAX_PKT_BURST); > > + if (unlikely(nb_rx =3D=3D 0)) { > > + idle++; > > + continue; > > + } > > + > > + count +=3D nb_rx; > Doesn't take into consideration error conditions. rte_eth_rx_burst can r= eturn > -ENOTSUP [Liang, Cunming] It returns -ENOTSUP when turning on ETHDEV_DEBUG CONFIG. The error is used to avoid no function call register.=20 When ETHDEV_DEBUG turn off, the NULL function call cause segfault directly. So I think it's a library internal error. In such library exceptional case, I prefer not expecting sample/application= to condition check library functional error. >=20 > > + nb_tx =3D rte_eth_tx_burst(portid, 0, pkts_burst, nb_rx); > Ditto with -ENOTSUP >=20 > > + if (unlikely(nb_tx < nb_rx)) { > What makes this unlikely? Seems like a perfectly reasonable condition to= happen > to me. If the network is busy, its completely likely that you will recei= ve more > frames than you send, if you elect to receive all frames. [Liang, Cunming] For this case, NIC works in MAC loopback mode. It firstly injects numbers of packets to NIC. Then NIC will loopback all pa= ckets to ingress side. The code here will receive the packets, and send them back to NIC Packets looping inside all come from initial injection. As the total number of injected packets is much less than in-chip queue siz= e, the tx egress queue shouldn't block desc. ring update. So when receiving packets in line rate and nb_tx < nb_rx, it means tx canno= t archive line rate.=20 When it happens, the cycles/packets result make no sense, as the bottle nec= k is NIC. The drop counter can record it. >=20 > > + drop +=3D (nb_rx - nb_tx); > > + do { > > + rte_pktmbuf_free(pkts_burst[nb_tx]); > Defer this, it skews your timing [Liang, Cunming] Agree with you, I ever thought about it. This test cases is designed to measure pure IO RX/TX routine. When doing tx burst with result nb_tx < nb_rx, we either repeat re-send or = drop it. Each way introduces noise(adding additional control code), resending much t= imes even cost more than free it. The cycles/packets is useful when there's no packet drop, otherwise it give= s the hint where the problem comes from (by idle or drop). >=20 > > + } while (++nb_tx < nb_rx); > > + } > > + } > > + if (unlikely(count >=3D total_packets)) > > + break; > Whats the reasoning here? Do you only ever expect to receive frames that= you > send? If so, seems like this should call for a big warning to get printe= d. [Liang, Cunming] The loop exits when the pre-calculated total_packets are r= eceived. As the nb_rx is unpredictable, the packet counter may large equal than tota= l_packets the last time. The reason unlikely used here is because the condition becomes true only th= e last time.=20 >=20 > > + } > > + > > + cur_tsc =3D rte_rdtsc(); > > + > > + for (i =3D 0; i < conf->nb_ports; i++) { > > + portid =3D conf->portlist[i]; > > + int nb_free =3D pkt_per_port; > > + do { /* dry out */ > > + nb_rx =3D rte_eth_rx_burst((uint8_t) portid, 0, > > + pkts_burst, MAX_PKT_BURST); > > + nb_tx =3D 0; > > + while (nb_tx < nb_rx) > > + rte_pktmbuf_free(pkts_burst[nb_tx++]); > > + nb_free -=3D nb_rx; > > + } while (nb_free !=3D 0); > > + printf("free %d mbuf left in port %u\n", pkt_per_port, portid); > > + } > > + > Whats the purpose of this? Are you trying to flush the device? Wouldn't= it be > enough just to stop the interface? [Liang, Cunming] If we only run the cases once and exit, it doesn't matter. But it's designed to run multi-times without exit, for the purpose of warmi= ng up or for getting average number. So stopping device is not enough, we have to release the flying packets. >=20 > > + if (count =3D=3D 0) > > + return -1; > > + > > + printf("%lu packet, %lu drop, %lu idle\n", count, drop, idle); > > + printf("Result: %ld cycles per packet\n", (cur_tsc - prev_tsc) / coun= t); > > + > Bad math here. Theres no guarantee that the tsc hasn't wrapped (potentia= lly > more than once) depending on your test length. you need to check the tsc= before > and after each burst and record an average of deltas instead, accounting = in each > instance for the possibility of wrap. [Liang, Cunming] I'm not sure catch your point correctly. I think both cur_tsc and prev_tsc are 64 bits width.=20 For 3GHz, I think it won't wrapped so quick. As it's uint64_t, so even get wrapped, the delta should still be correct. >=20 > > + return 0; > > +} > > + > > +static int > > +test_pmd_perf(void) > > +{ > > + uint16_t nb_ports, num, nb_lcores, slave_id =3D (uint16_t)-1; > > + uint16_t nb_rxd =3D RTE_TEST_RX_DESC_DEFAULT; > > + uint16_t nb_txd =3D RTE_TEST_TX_DESC_DEFAULT; > > + uint16_t portid; > > + uint16_t nb_rx_queue =3D 1, nb_tx_queue =3D 1; > > + int socketid =3D -1; > > + int ret; > > + > > + printf("Start PMD RXTX cycles cost test.\n"); > > + > > + signal(SIGUSR1, signal_handler); > Again SIGINT here. >=20 > > + signal(SIGUSR2, signal_handler); > > + > > + nb_ports =3D rte_eth_dev_count(); > > + if (nb_ports < NB_ETHPORTS_USED) { > > + printf("At least %u port(s) used for perf. test\n", > > + NB_ETHPORTS_USED); > > + return -1; > > + } > > + > > + if (nb_ports > RTE_MAX_ETHPORTS) > > + nb_ports =3D RTE_MAX_ETHPORTS; > > + > > + nb_lcores =3D rte_lcore_count(); > > + > > + memset(lcore_conf, 0, sizeof(lcore_conf)); > > + init_lcores(); > > + > > + init_mbufpool(NB_MBUF); > > + > > + reset_count(); > > + num =3D 0; > > + for (portid =3D 0; portid < nb_ports; portid++) { > > + if (socketid =3D=3D -1) { > > + socketid =3D rte_eth_dev_socket_id(portid); > > + slave_id =3D alloc_lcore(socketid); > > + if (slave_id =3D=3D (uint16_t)-1) { > > + printf("No avail lcore to run test\n"); > > + return -1; > > + } > > + printf("Performance test runs on lcore %u socket %u\n", > > + slave_id, socketid); > > + } > > + > > + if (socketid !=3D rte_eth_dev_socket_id(portid)) { > > + printf("Skip port %d\n", portid); > > + continue; > > + } > > + > > + /* port configure */ > > + ret =3D rte_eth_dev_configure(portid, nb_rx_queue, > > + nb_tx_queue, &port_conf); > > + if (ret < 0) > > + rte_exit(EXIT_FAILURE, > > + "Cannot configure device: err=3D%d, port=3D%d\n", > > + ret, portid); > > + > > + rte_eth_macaddr_get(portid, &ports_eth_addr[portid]); > > + printf("Port %u ", portid); > > + print_ethaddr("Address:", &ports_eth_addr[portid]); > > + printf("\n"); > > + > > + /* tx queue setup */ > > + ret =3D rte_eth_tx_queue_setup(portid, 0, nb_txd, > > + socketid, &tx_conf); > > + if (ret < 0) > > + rte_exit(EXIT_FAILURE, > > + "rte_eth_tx_queue_setup: err=3D%d, " > > + "port=3D%d\n", ret, portid); > > + > > + /* rx queue steup */ > > + ret =3D rte_eth_rx_queue_setup(portid, 0, nb_rxd, > > + socketid, &rx_conf, > > + mbufpool[socketid]); > > + if (ret < 0) > > + rte_exit(EXIT_FAILURE, "rte_eth_rx_queue_setup: err=3D%d," > > + "port=3D%d\n", ret, portid); > > + > > + /* Start device */ > > + stop =3D 0; > > + ret =3D rte_eth_dev_start(portid); > > + if (ret < 0) > > + rte_exit(EXIT_FAILURE, > > + "rte_eth_dev_start: err=3D%d, port=3D%d\n", > > + ret, portid); > > + > > + /* always eanble promiscuous */ > > + rte_eth_promiscuous_enable(portid); > > + > > + lcore_conf[slave_id].portlist[num++] =3D portid; > > + lcore_conf[slave_id].nb_ports++; > > + } > > + check_all_ports_link_status(nb_ports, RTE_PORT_ALL); > > + > > + init_traffic(mbufpool[socketid], tx_burst, MAX_TRAFIC_BURST); > > + > > + rte_eal_remote_launch(main_loop, NULL, slave_id); > > + if (rte_eal_wait_lcore(slave_id) < 0) > > + return -1; > > + > > + /* port tear down */ > > + for (portid =3D 0; portid < nb_ports; portid++) { > > + if (socketid !=3D rte_eth_dev_socket_id(portid)) > > + continue; > > + > > + rte_eth_dev_stop(portid); > > + } > > + > Clean up your allocated memory/lcores/etc? [Liang, Cunming] It do cleanup on the beginning of case. "Init_lcores","init_mbufpool","reset_count" guarantee the data clean before= each testing. And mbufpool only allocated once even if we run multiple times. >=20 > Neil