[dpdk-dev,1/6] testpmd: add io_retry forwarding
Commit Message
This patch adds an io_retry-fwd in testpmd to prevent most packet
losses. It can be enabled by "set fwd io_retry".
io-fwd is the fastest possible forwarding engine, good for basic
performance test. Adding retry mechanism expands test case coverage
to support scenarios where packet loss affects test results.
Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
app/test-pmd/Makefile | 1 +
app/test-pmd/iofwd-retry.c | 139 +++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.c | 1 +
app/test-pmd/testpmd.h | 1 +
4 files changed, 142 insertions(+)
create mode 100644 app/test-pmd/iofwd-retry.c
Comments
2016-05-05 18:46, Zhihong Wang:
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> extern struct fwd_engine io_fwd_engine;
> +extern struct fwd_engine io_retry_fwd_engine;
> extern struct fwd_engine mac_fwd_engine;
> extern struct fwd_engine mac_retry_fwd_engine;
> extern struct fwd_engine mac_swap_engine;
We now have 2 engines with "retry" behaviour.
It is maybe the way to go, but I want to ask the question:
Would it be possible to have "retry" as an engine parameter?
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, May 25, 2016 5:32 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [PATCH 1/6] testpmd: add io_retry forwarding
>
> 2016-05-05 18:46, Zhihong Wang:
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > extern struct fwd_engine io_fwd_engine;
> > +extern struct fwd_engine io_retry_fwd_engine;
> > extern struct fwd_engine mac_fwd_engine;
> > extern struct fwd_engine mac_retry_fwd_engine;
> > extern struct fwd_engine mac_swap_engine;
>
> We now have 2 engines with "retry" behaviour.
> It is maybe the way to go, but I want to ask the question:
> Would it be possible to have "retry" as an engine parameter?
>
If it's just about the way to write commands there isn't much difference,
like "set fwd io_rety" and "set fwd io retry".
Do you mean to add the "retry" for all engines, and also implement this
as a parameter in each original engine? So for example, no iofwd-retry.c,
just add this feature inside iofwd.c?
2016-05-26 02:40, Wang, Zhihong:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-05-05 18:46, Zhihong Wang:
> > > --- a/app/test-pmd/testpmd.h
> > > +++ b/app/test-pmd/testpmd.h
> > > extern struct fwd_engine io_fwd_engine;
> > > +extern struct fwd_engine io_retry_fwd_engine;
> > > extern struct fwd_engine mac_fwd_engine;
> > > extern struct fwd_engine mac_retry_fwd_engine;
> > > extern struct fwd_engine mac_swap_engine;
> >
> > We now have 2 engines with "retry" behaviour.
> > It is maybe the way to go, but I want to ask the question:
> > Would it be possible to have "retry" as an engine parameter?
> >
>
> If it's just about the way to write commands there isn't much difference,
> like "set fwd io_rety" and "set fwd io retry".
>
> Do you mean to add the "retry" for all engines, and also implement this
> as a parameter in each original engine? So for example, no iofwd-retry.c,
> just add this feature inside iofwd.c?
Yes, if it makes sense. For engines other than io_fwd and mac_fwd, the retry
option can be unsupported (return an error) as a first step.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, May 26, 2016 2:27 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [PATCH 1/6] testpmd: add io_retry forwarding
>
> 2016-05-26 02:40, Wang, Zhihong:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-05-05 18:46, Zhihong Wang:
> > > > --- a/app/test-pmd/testpmd.h
> > > > +++ b/app/test-pmd/testpmd.h
> > > > extern struct fwd_engine io_fwd_engine;
> > > > +extern struct fwd_engine io_retry_fwd_engine;
> > > > extern struct fwd_engine mac_fwd_engine;
> > > > extern struct fwd_engine mac_retry_fwd_engine;
> > > > extern struct fwd_engine mac_swap_engine;
> > >
> > > We now have 2 engines with "retry" behaviour.
> > > It is maybe the way to go, but I want to ask the question:
> > > Would it be possible to have "retry" as an engine parameter?
> > >
> >
> > If it's just about the way to write commands there isn't much difference,
> > like "set fwd io_rety" and "set fwd io retry".
> >
> > Do you mean to add the "retry" for all engines, and also implement this
> > as a parameter in each original engine? So for example, no iofwd-retry.c,
> > just add this feature inside iofwd.c?
>
> Yes, if it makes sense. For engines other than io_fwd and mac_fwd, the retry
> option can be unsupported (return an error) as a first step.
I think it makes sense in terms of making code more clear and manageable.
@@ -49,6 +49,7 @@ SRCS-y += parameters.c
SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline.c
SRCS-y += config.c
SRCS-y += iofwd.c
+SRCS-y += iofwd-retry.c
SRCS-y += macfwd.c
SRCS-y += macfwd-retry.c
SRCS-y += macswap.c
new file mode 100644
@@ -0,0 +1,139 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <inttypes.h>
+
+#include <sys/queue.h>
+#include <sys/stat.h>
+
+#include <rte_common.h>
+#include <rte_byteorder.h>
+#include <rte_log.h>
+#include <rte_debug.h>
+#include <rte_cycles.h>
+#include <rte_memory.h>
+#include <rte_memzone.h>
+#include <rte_launch.h>
+#include <rte_eal.h>
+#include <rte_per_lcore.h>
+#include <rte_lcore.h>
+#include <rte_atomic.h>
+#include <rte_branch_prediction.h>
+#include <rte_ring.h>
+#include <rte_memory.h>
+#include <rte_memcpy.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_interrupts.h>
+#include <rte_pci.h>
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_string_fns.h>
+
+#include "testpmd.h"
+
+#define TX_RETRY_TIMES 128
+#define TX_RETRY_WAIT_US 1
+
+/*
+ * I/O retry forwarding mode.
+ * Forward packets "as-is" without access to packet data.
+ */
+static void
+pkt_burst_io_retry_forward(struct fwd_stream *fs)
+{
+ struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
+ uint16_t nb_rx;
+ uint16_t nb_tx;
+ uint32_t retry;
+
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+ uint64_t start_tsc;
+ uint64_t end_tsc;
+ uint64_t core_cycles;
+#endif
+
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+ start_tsc = rte_rdtsc();
+#endif
+
+ nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
+ pkts_burst, nb_pkt_per_burst);
+ if (unlikely(nb_rx == 0))
+ return;
+ fs->rx_packets += nb_rx;
+
+#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
+ fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
+#endif
+
+ retry = 0;
+ nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
+ pkts_burst, nb_rx);
+ while (unlikely(nb_tx < nb_rx) && (retry++ < TX_RETRY_TIMES)) {
+ rte_delay_us(TX_RETRY_WAIT_US);
+ nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
+ &pkts_burst[nb_tx], nb_rx - nb_tx);
+ }
+ fs->tx_packets += nb_tx;
+
+#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
+ fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
+#endif
+
+ if (unlikely(nb_tx < nb_rx)) {
+ fs->fwd_dropped += (nb_rx - nb_tx);
+ do {
+ rte_pktmbuf_free(pkts_burst[nb_tx]);
+ } while (++nb_tx < nb_rx);
+ }
+
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+ end_tsc = rte_rdtsc();
+ core_cycles = (end_tsc - start_tsc);
+ fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
+#endif
+}
+
+struct fwd_engine io_retry_fwd_engine = {
+ .fwd_mode_name = "io_retry",
+ .port_fwd_begin = NULL,
+ .port_fwd_end = NULL,
+ .packet_fwd = pkt_burst_io_retry_forward,
+};
@@ -143,6 +143,7 @@ streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
*/
struct fwd_engine * fwd_engines[] = {
&io_fwd_engine,
+ &io_retry_fwd_engine,
&mac_fwd_engine,
&mac_retry_fwd_engine,
&mac_swap_engine,
@@ -221,6 +221,7 @@ struct fwd_engine {
};
extern struct fwd_engine io_fwd_engine;
+extern struct fwd_engine io_retry_fwd_engine;
extern struct fwd_engine mac_fwd_engine;
extern struct fwd_engine mac_retry_fwd_engine;
extern struct fwd_engine mac_swap_engine;