> -----Original Message-----
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Wednesday, October 22, 2014 10:03 PM
> To: Liang, Cunming
> Cc: dev at 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 at tuxdriver.com]
> > > Sent: Tuesday, October 21, 2014 6:33 PM
> > > To: Liang, Cunming
> > > Cc: dev at 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 at tuxdriver.com]
> > > > > Sent: Saturday, October 11, 2014 1:52 AM
> > > > > To: Liang, Cunming
> > > > > Cc: dev at 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 at intel.com>
> > > > > > Acked-by: Bruce Richardson <bruce.richardson at 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
