From: "Liang, Cunming" <cunming.liang@intel.com> To: Neil Horman <nhorman@tuxdriver.com> Cc: "dev@dpdk.org" <dev@dpdk.org> Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet Date: Wed, 22 Oct 2014 14:48:55 +0000 Message-ID: <D0158A423229094DA7ABF71CF2FA0DA3118513C5@shsmsx102.ccr.corp.intel.com> (raw) In-Reply-To: <20141022140316.GA13913@localhost.localdomain> > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Wednesday, October 22, 2014 10:03 PM > 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 > > On Tue, Oct 21, 2014 at 01:17:01PM +0000, Liang, Cunming wrote: > > > > > > > -----Original Message----- > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > Sent: Tuesday, October 21, 2014 6:33 PM > > > 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 > > > > > > On Sun, Oct 12, 2014 at 11:10:39AM +0000, Liang, Cunming wrote: > > > > 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 > > > > > > > > > > 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 > > > equipment. > > > > > > 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' > without > > > > > INC_VEC=y in config > > > > > > > > > > > > Signed-off-by: Cunming Liang <cunming.liang@intel.com> > > > > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > > > > > > > > > > Notes inline > > > > > > > > > > > --- > > > > > > 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 += test_memzone.c > > > > > > > > > > > > SRCS-y += test_ring.c > > > > > > SRCS-y += test_ring_perf.c > > > > > > +SRCS-y += test_pmd_perf.c > > > > > > > > > > > > ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) > > > > > > SRCS-y += 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 = { > > > > > > > > > > > > +#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 > > > sending > > > > > (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. > > > > > > > > That presumes a non-memory constrained system, doesn't it? I suppose in > the > > > end > > > as long as you have consistency, its not overly relevant, but it seems like > > > you'll want to add data sizing as a feature to this eventually (i.e. the ability > > > to test performance for larger frames sizes), at which point you'll need to > make > > > this non-static anyway. > > [Liang, Cunming] For a normal Ethernet packet(w/o jumbo frame), packet size is > 1518B. > > As in really network, there won't have huge number of jumbo frames. > > The mbuf size 2048 is a reasonable value to cover most of the packet size. > > It's also be chosen by lots of NIC as the default receiving buffer size in DMA > register. > > In case larger than the size, it need do scatter and gather but lose some > performance. > > The unit test won't measure size from 64 to 9600, won't plan to measure > scatter-gather rx/tx. > > It focus on 64B packet size and taking the mbuf size being used the most often. > Fine. > > > > > > > > > > +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 > > > common > > > > > function in rte_ether.h > > > > [Liang, Cunming] Agree with you, some of samples now use it with the > same > > > copy. > > > > I'll rework it. Adding 'ether_format_addr' in rte_ether.h only for format the > > > 48bits address output. > > > > And leaving other prints for application customization. > > > > > > > > Sounds good. > > > > > > > > > > > > > > +} > > > > > > + > > > > > > +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 > the > > > > > program > > > > [Liang, Cunming] Thanks, it's a typo. > > > > > > > > > > > + if (signum == SIGUSR1) { > > > > > SIGINT instead. Thats the common practice. > > > > [Liang, Cunming] I understood your opinion. > > > > 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. > > > > Only when exception timeout, I leave this backdoor for automation test > > > control. > > > > For manual test, we can easily force kill the process. > > > > > > > Hmm, ok, that sounds reasonable. > > > > > > > > > > > > > > + printf("Force Stop!\n"); > > > > > > + stop = 1; > > > > > > + } > > > > > > + if (signum == 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 = 0, nb_tx = 0; > > > > > > + struct lcore_conf *conf; > > > > > > + uint64_t prev_tsc, cur_tsc; > > > > > > + int pkt_per_port; > > > > > > + uint64_t packets_per_second, total_packets; > > > > > > + > > > > > > + lcore_id = rte_lcore_id(); > > > > > > + conf = &lcore_conf[lcore_id]; > > > > > > + if (conf->status != LCORE_USED) > > > > > > + return 0; > > > > > > + > > > > > > + pkt_per_port = MAX_TRAFIC_BURST / conf->nb_ports; > > > > > > + > > > > > > + int idx = 0; > > > > > > + for (i = 0; i < conf->nb_ports; i++) { > > > > > > + int num = pkt_per_port; > > > > > > + portid = conf->portlist[i]; > > > > > > + printf("inject %d packet to port %d\n", num, portid); > > > > > > + while (num) { > > > > > > + nb_tx = RTE_MIN(MAX_PKT_BURST, num); > > > > > > + nb_tx = rte_eth_tx_burst(portid, 0, > > > > > > + &tx_burst[idx], nb_tx); > > > > > > + num -= nb_tx; > > > > > > + idx += nb_tx; > > > > > > + } > > > > > > + } > > > > > > + printf("Total packets inject to prime ports = %u\n", idx); > > > > > > + > > > > > > + packets_per_second = (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 = RTE_TEST_DURATION * conf->nb_ports * > > > > > packets_per_second; > > > > > > + printf("Test will stop after at least %"PRIu64" packets received\n", > > > > > > + + total_packets); > > > > > > + > > > > > > + prev_tsc = rte_rdtsc(); > > > > > > + > > > > > > + while (likely(!stop)) { > > > > > > + for (i = 0; i < conf->nb_ports; i++) { > > > > > > + portid = conf->portlist[i]; > > > > > > + nb_rx = rte_eth_rx_burst((uint8_t) portid, 0, > > > > > > + pkts_burst, MAX_PKT_BURST); > > > > > > + if (unlikely(nb_rx == 0)) { > > > > > > + idle++; > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + count += nb_rx; > > > > > Doesn't take into consideration error conditions. rte_eth_rx_burst can > > > return > > > > > -ENOTSUP > > > > [Liang, Cunming] It returns -ENOTSUP when turning on ETHDEV_DEBUG > > > CONFIG. > > > > The error is used to avoid no function call register. > > > > When ETHDEV_DEBUG turn off, the NULL function call cause segfault > directly. > > > > So I think it's a library internal error. > > > No, look at FUNC_PTR_OR_ERR_RET. If the passed in function pointer is null, > > > -ENOTSUPP will be returned to the application, you need to handle the error > > > condition. > > [Liang, Cunming] The runtime rte_eth_rx_burst is a inline function in > rte_ethdev.h. > > The one you're talking about is the one defined in rte_ethdev.c which is a > extern non-inline function. > > It works only when RTE_LIBRTE_ETHDEV_DEBUG turns on. > > If we always turns such library internal checking on, it lose performance. > > So I insist it's a library internal error checking, doesn't need to take care in > application level. > I'm really flored that you would respond this way. > > What you have is two versions of a function, one returns errors and one doesn't, > and the version used is based on compile time configuration. You've written > your application such that it can only gracefully handle one of the two > versions, and you're happy to allow the other version, the one thats supposed to > be in use when you are debugging applications, to create silent accounting > failures in your application. And it completely ignores the fact that the > non-debug version might one day return errors as well (even if it doesn't > currently). Your assertion above that we can ignore it because its debug code > is the most short sighted thing I've read in a long time. > [Liang, Cunming] If as you said, two version of function, one returns errors and one doesn't. I'll agree with your point. But in fact in either one, will cause error. Non-debug version will just segment fault as calling NULL function pointer. Debug version will print the error root cause. If you searching the rte_eth_rx_burst calling in DPDK, it's used in every sample, no one checks it. This function call is in fast path, I don't think it's a good idea pay additional cycles on such branch condition. > > > > > > > In such library exceptional case, I prefer not expecting sample/application to > > > condition check library functional error. > > > But you're assertion about the library handling the exception is wrong. > > > > > > > > > > > > > > + nb_tx = rte_eth_tx_burst(portid, 0, pkts_burst, nb_rx); > > > > > Ditto with -ENOTSUP > > > > > > > > > > > + 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 receive > 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 > packets 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 > size, > > > 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 cannot > > > archive line rate. > > > > When it happens, the cycles/packets result make no sense, as the bottle > neck is > > > NIC. > > > > The drop counter can record it. > > > Ok. > > > > > > > > > > > > > > + drop += (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 > > > times even cost more than free it. > > > > The cycles/packets is useful when there's no packet drop, otherwise it gives > the > > > hint where the problem comes from (by idle or drop). > > > I'm not sure what you're asserting here. Are you suggesting that you want > to > > > include the time it takes to free memory buffers in your testing? That seems > > > dubious at best to me. If you want to measure I/O, thats great, but memory > > > management of packet buffers is a separate operation from that I/O > > [Liang, Cunming] Agree with you. I means it doesn't need to take care of the > mbuf free cost. > > As I said in previous reply, it rarely happens in line rate. > > The cycle measurement doesn't make much sense if happens. > > On that time the unit test just notify it happens, and keep safe free all the > unsent mbuf. > > > > > > > > > > > > > > > > + } while (++nb_tx < nb_rx); > > > > > > + } > > > > > > + } > > > > > > + if (unlikely(count >= 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 printed. > > > > [Liang, Cunming] The loop exits when the pre-calculated total_packets are > > > received. > > > > As the nb_rx is unpredictable, the packet counter may large equal than > > > total_packets the last time. > > > > The reason unlikely used here is because the condition becomes true only > the > > > last time. > > > ok > > > > > > > > > > > > > > + } > > > > > > + > > > > > > + cur_tsc = rte_rdtsc(); > > > > > > + > > > > > > + for (i = 0; i < conf->nb_ports; i++) { > > > > > > + portid = conf->portlist[i]; > > > > > > + int nb_free = pkt_per_port; > > > > > > + do { /* dry out */ > > > > > > + nb_rx = rte_eth_rx_burst((uint8_t) portid, 0, > > > > > > + pkts_burst, MAX_PKT_BURST); > > > > > > + nb_tx = 0; > > > > > > + while (nb_tx < nb_rx) > > > > > > + rte_pktmbuf_free(pkts_burst[nb_tx++]); > > > > > > + nb_free -= nb_rx; > > > > > > + } while (nb_free != 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 > warming > > > up or for getting average number. > > > > So stopping device is not enough, we have to release the flying packets. > > > > > > > Ok > > > > > > > > > > > > > > + if (count == 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) / > count); > > > > > > + > > > > > Bad math here. Theres no guarantee that the tsc hasn't wrapped > > > (potentially > > > > > 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. > > > > 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. > > > But theres no guarantee that the tsc starts at zero when you begin your test. > > > The system may have been up for a long time and near wrapping already. > > > Regardless, you need to account for the possibility that cur_tsc is smaller > > > than prev_tsc, or this breaks. > > [Liang, Cunming] In case prev_tsc near wrapping, and cur_tsc get wrapped. > > As they are unsigned, it still ok. > > e.g. cur_tsc=0x2, prev_tsc=0xFFFFFFFFFFFFFFFC > > Delta=cur_tsc-prev_tsc is 6, which is still correct. > Ah, you're right, my fault. > > And for uint64_t, we need to start computer for hundreds of years. > That only assumes that the tsc has been kept at its initial state from boot, its > entirely possible that the tsc is writen to a different value. IIRC, in several > debug configurations, the linux kernel inits the tsc to a large value so that it > rolls over shortly after boot to catch wrapping errors. Though as you note, > your used of uint64 makes it irrelevant. [Liang, Cunming] Ok, you're right. I think we've already got agreement on delta of tsc. So we can close this one. > > > > > > > > > > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static int > > > > > > +test_pmd_perf(void) > > > > > > +{ > > > > > > + uint16_t nb_ports, num, nb_lcores, slave_id = (uint16_t)-1; > > > > > > + uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; > > > > > > + uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; > > > > > > + uint16_t portid; > > > > > > + uint16_t nb_rx_queue = 1, nb_tx_queue = 1; > > > > > > + int socketid = -1; > > > > > > + int ret; > > > > > > + > > > > > > + printf("Start PMD RXTX cycles cost test.\n"); > > > > > > + > > > > > > + signal(SIGUSR1, signal_handler); > > > > > Again SIGINT here. > > > > > > > > > > > + signal(SIGUSR2, signal_handler); > > > > > > + > > > > > > + nb_ports = 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 = RTE_MAX_ETHPORTS; > > > > > > + > > > > > > + nb_lcores = rte_lcore_count(); > > > > > > + > > > > > > + memset(lcore_conf, 0, sizeof(lcore_conf)); > > > > > > + init_lcores(); > > > > > > + > > > > > > + init_mbufpool(NB_MBUF); > > > > > > + > > > > > > + reset_count(); > > > > > > + num = 0; > > > > > > + for (portid = 0; portid < nb_ports; portid++) { > > > > > > + if (socketid == -1) { > > > > > > + socketid = rte_eth_dev_socket_id(portid); > > > > > > + slave_id = alloc_lcore(socketid); > > > > > > + if (slave_id == (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 != rte_eth_dev_socket_id(portid)) { > > > > > > + printf("Skip port %d\n", portid); > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + /* port configure */ > > > > > > + ret = rte_eth_dev_configure(portid, nb_rx_queue, > > > > > > + nb_tx_queue, &port_conf); > > > > > > + if (ret < 0) > > > > > > + rte_exit(EXIT_FAILURE, > > > > > > + "Cannot configure device: err=%d, port=%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 = 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=%d, " > > > > > > + "port=%d\n", ret, portid); > > > > > > + > > > > > > + /* rx queue steup */ > > > > > > + ret = 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=%d," > > > > > > + "port=%d\n", ret, portid); > > > > > > + > > > > > > + /* Start device */ > > > > > > + stop = 0; > > > > > > + ret = rte_eth_dev_start(portid); > > > > > > + if (ret < 0) > > > > > > + rte_exit(EXIT_FAILURE, > > > > > > + "rte_eth_dev_start: err=%d, port=%d\n", > > > > > > + ret, portid); > > > > > > + > > > > > > + /* always eanble promiscuous */ > > > > > > + rte_eth_promiscuous_enable(portid); > > > > > > + > > > > > > + lcore_conf[slave_id].portlist[num++] = 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 = 0; portid < nb_ports; portid++) { > > > > > > + if (socketid != 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. > > > Its a janitorial issue. Before the program exits, you need to free any > > > resources that you've allocated. > > [Liang, Cunming] Yes, I do but only not for the mbuf pool. > > Firstly the case exit is not equal to program exit. I expect the re-enter will use > the same mempool, so that can do cache warm up. > > Secondly, DPDK doesn't supply really mempool release. The program exit, the > memzone then free. > I continue to find the fact that freeing mempools isn't supported is rediculous, > but I suppose thats neither here nor there, you need to be able to clean up > resources that you allocate, weather you are exiting a test case, a program, or > simply a function, its just not sane to not be able to do that. [Liang, Cunming] Got it, agree with you on the principle. > > Given the return code check above, all thats left to say I think is NAK to this > code. If Thomas wants to ignore me, I suppose thats his perogative, but I can't > see how you can ignore that case. > > Neil
next prev parent reply other threads:[~2014-10-22 14:40 UTC|newest] Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-08-25 6:12 [dpdk-dev] [PATCH 0/5] app/test: unit test to measure cycles per packet Cunming Liang 2014-08-25 6:12 ` [dpdk-dev] [PATCH 1/5] app/test: unit test for rx and tx cycles/packet Cunming Liang 2014-08-25 6:12 ` [dpdk-dev] [PATCH 2/5] app/test: measure standalone rx or " Cunming Liang 2014-08-25 6:12 ` [dpdk-dev] [PATCH 3/5] ixgbe/vpmd: add fix to store unaligned mbuf point array Cunming Liang 2014-08-25 6:12 ` [dpdk-dev] [PATCH 4/5] app/test: add unit test to measure RX burst cycles Cunming Liang 2014-08-25 6:12 ` [dpdk-dev] [PATCH 5/5] app/test: allow to create packets in different sizes Cunming Liang 2014-09-03 1:46 ` [dpdk-dev] [PATCH 0/5] app/test: unit test to measure cycles per packet Zhan, Zhaochen 2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 0/4] " Cunming Liang 2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet Cunming Liang 2014-10-10 17:52 ` Neil Horman 2014-10-12 11:10 ` Liang, Cunming [not found] ` <E115CCD9D858EF4F90C690B0DCB4D8972262B720@IRSMSX108.ger.corp.intel.com> 2014-10-14 0:54 ` Liang, Cunming 2014-10-21 10:33 ` Neil Horman 2014-10-21 10:43 ` Richardson, Bruce 2014-10-21 13:37 ` Neil Horman 2014-10-21 22:43 ` Ananyev, Konstantin 2014-10-21 13:17 ` Liang, Cunming 2014-10-22 14:03 ` Neil Horman 2014-10-22 14:48 ` Liang, Cunming [this message] 2014-10-22 14:53 ` Ananyev, Konstantin 2014-10-22 15:09 ` Richardson, Bruce 2014-10-24 3:06 ` Liang, Cunming 2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 2/4] app/test: measure standalone rx or " Cunming Liang 2014-10-10 12:30 ` [dpdk-dev] [PATCH v2 3/4] app/test: add unit test to measure RX burst cycles Cunming Liang 2014-10-10 12:30 ` [dpdk-dev] [PATCH v2 4/4] app/test: allow to create packets in different sizes Cunming Liang 2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 0/2] app/test: unit test to measure cycles per packet Cunming Liang 2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 1/2] app/test: allow to create packets in different sizes Cunming Liang 2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 2/2] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang 2014-10-21 2:40 ` [dpdk-dev] [PATCH v3 0/2] app/test: unit test to measure cycles per packet Liu, Yong 2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 0/3] " Cunming Liang 2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 1/3] app/test: allow to create packets in different sizes Cunming Liang 2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang 2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 3/3] ethdev: fix wrong error return refer to API definition Cunming Liang 2014-10-24 5:57 ` [dpdk-dev] [PATCH v5 0/3] app/test: unit test to measure cycles per packet Cunming Liang 2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 1/3] app/test: allow to create packets in different sizes Cunming Liang 2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang 2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 3/3] ethdev: fix wrong error return refere to API definition Cunming Liang 2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 0/3] app/test: unit test to measure cycles per packet Cunming Liang 2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 1/3] app/test: allow to create packets in different sizes Cunming Liang 2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang 2014-11-11 23:28 ` Thomas Monjalon 2014-11-12 6:32 ` Liang, Cunming 2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 3/3] ethdev: fix wrong error return refere to API definition Cunming Liang 2014-10-27 16:03 ` Ananyev, Konstantin 2014-10-27 1:45 ` [dpdk-dev] [PATCH v6 0/3] app/test: unit test to measure cycles per packet Liu, Yong 2014-10-29 5:06 ` Liang, Cunming 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 0/7] " Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 1/7] app/test: allow to create packets in different sizes Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 2/7] ixgbe:clean scattered_rx configure in dev_stop Cunming Liang 2014-11-12 7:53 ` Thomas Monjalon 2014-11-12 8:21 ` Liang, Cunming 2014-11-12 9:24 ` Thomas Monjalon 2014-11-12 10:29 ` Liang, Cunming 2014-11-12 10:32 ` Thomas Monjalon 2014-11-12 10:42 ` Liang, Cunming 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 3/7] ether: new API to format eth_addr in string Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 4/7] app/testpmd: cleanup eth_addr print Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 5/7] examples: " Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 6/7] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang 2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 7/7] ethdev: fix wrong error return refere to API definition Cunming Liang 2014-11-12 23:50 ` [dpdk-dev] [PATCH v7 0/7] app/test: unit test to measure cycles per packet Thomas Monjalon 2014-10-24 5:59 ` [dpdk-dev] [PATCH v4 0/3] " Liang, Cunming [not found] ` <1414130090-17910-1-git-send-email-y> [not found] ` <1414130090-17910-4-git-send-email-y> 2014-10-24 11:04 ` [dpdk-dev] [PATCH v5 3/3] ethdev: fix wrong error return refere to API definition Ananyev, Konstantin 2014-10-27 0:58 ` Liang, Cunming 2014-10-28 12:21 ` [dpdk-dev] [PATCH v2 0/4] app/test: unit test to measure cycles per packet Neil Horman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=D0158A423229094DA7ABF71CF2FA0DA3118513C5@shsmsx102.ccr.corp.intel.com \ --to=cunming.liang@intel.com \ --cc=dev@dpdk.org \ --cc=nhorman@tuxdriver.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git