> -----Original Message----- > From: Pattan, Reshma > Sent: Tuesday, September 30, 2014 9:03 AM > To: Ananyev, Konstantin; De Lara Guarch, Pablo; dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > Hi Konstantin, > > Any comments on below Pablos comment? If so please provide. > > Thanks, > Reshma >
Hi Reshma, No really. I would just change printf to what I suggested. For your app, I believe that is more than enough. But if you'd like to introduce some sort of rate limiting for logging - sure go ahead would be interesting to see that patch. Konstantin > > > -----Original Message----- > From: De Lara Guarch, Pablo > Sent: Monday, September 29, 2014 2:35 PM > To: Ananyev, Konstantin; Pattan, Reshma; dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Monday, September 29, 2014 2:07 PM > > To: Pattan, Reshma; De Lara Guarch, Pablo; dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > > > > > -----Original Message----- > > > From: Pattan, Reshma > > > Sent: Monday, September 29, 2014 1:40 PM > > > To: Ananyev, Konstantin; De Lara Guarch, Pablo; dev at dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > > > > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Friday, September 26, 2014 4:52 PM > > > To: De Lara Guarch, Pablo; Pattan, Reshma; dev at dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > > > > > > > > > -----Original Message----- > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of De Lara > > > > Guarch, Pablo > > > > Sent: Friday, September 26, 2014 4:12 PM > > > > To: Pattan, Reshma; dev at dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > > > > Hi, > > > > > > > > > -----Original Message----- > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of reshmapa > > > > > Sent: Wednesday, September 24, 2014 3:17 PM > > > > > To: dev at dpdk.org > > > > > Subject: [dpdk-dev] [PATCH v2] distributor_app: new sample app > > > > > > > > > > From: Reshma Pattan <reshma.pattan at intel.com> > > > > > > > > > > A new sample app that shows the usage of the distributor library. > > > > > This app works as follows: > > > > > > > > > > * An RX thread runs which pulls packets from each ethernet port in > > > > > turn > > > > > and passes those packets to worker using a distributor component. > > > > > * The workers take the packets in turn, and determine the output port > > > > > for those packets using basic l2forwarding doing an xor on the > > > > > source > > > > > port id. > > > > > * The RX thread takes the returned packets from the workers and > > enqueue > > > > > those packets into an rte_ring structure. > > > > > * A TX thread pulls the packets off the rte_ring structure and then > > > > > sends each packet out the output port specified previously by > > > > > the worker > > > > > * Command-line option support provided only for portmask. > > > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson at intel.com> > > > > > Signed-off-by: Reshma Pattan <reshma.pattan at intel.com> > > > > > --- > > > > > examples/Makefile | 1 + > > > > > examples/distributor_app/Makefile | 57 ++++ > > > > > examples/distributor_app/main.c | 585 > > > > > ++++++++++++++++++++++++++++++++++++++ > > > > > examples/distributor_app/main.h | 46 +++ > > > > > 4 files changed, 689 insertions(+) create mode 100644 > > > > > examples/distributor_app/Makefile create mode > > > > > 100644 examples/distributor_app/main.c create mode 100644 > > > > > examples/distributor_app/main.h > > > > > > > > > > diff --git a/examples/Makefile b/examples/Makefile index > > > > > 6245f83..2ba82b0 100644 > > > > > --- a/examples/Makefile > > > > > +++ b/examples/Makefile > > > > > @@ -66,5 +66,6 @@ DIRS-y += vhost > > > > > DIRS-$(CONFIG_RTE_LIBRTE_XEN_DOM0) += vhost_xen DIRS-y += > > vmdq > > > > > DIRS-y += vmdq_dcb > > > > > +DIRS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += distributor_app > > > > > > > > > > include $(RTE_SDK)/mk/rte.extsubdir.mk diff --git > > > > > a/examples/distributor_app/Makefile > > > > > b/examples/distributor_app/Makefile > > > > > new file mode 100644 > > > > > index 0000000..394785d > > > > > --- /dev/null > > > > > +++ b/examples/distributor_app/Makefile > > > > > @@ -0,0 +1,57 @@ > > > > > +# 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. > > > > > + > > > > > +ifeq ($(RTE_SDK),) > > > > > +$(error "Please define RTE_SDK environment variable") endif > > > > > + > > > > > +# Default target, can be overriden by command line or > > > > > +environment RTE_TARGET ?= x86_64-default-linuxapp-gcc > > > > > > > > This target is not present anymore. Change it to > > > > x86_64-native-linuxapp- > > gcc. > > > > > > > > > + > > > > > +include $(RTE_SDK)/mk/rte.vars.mk > > > > > + > > > > > +# binary name > > > > > +APP = distributor_app > > > > > + > > > > > +# all source are stored in SRCS-y SRCS-y := main.c > > > > > + > > > > > +CFLAGS += $(WERROR_FLAGS) > > > > > + > > > > > +# workaround for a gcc bug with noreturn attribute # > > > > > +http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 > > > > > +ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y) CFLAGS_main.o += > > > > > +-Wno-return-type endif > > > > > + > > > > > +EXTRA_CFLAGS += -O3 -Wfatal-errors > > > > > + > > > > > +include $(RTE_SDK)/mk/rte.extapp.mk > > > > > diff --git a/examples/distributor_app/main.c > > > > > b/examples/distributor_app/main.c new file mode 100644 index > > > > > 0000000..628810a > > > > > --- /dev/null > > > > > +++ b/examples/distributor_app/main.c > > > > > @@ -0,0 +1,585 @@ > > > > > +/*- > > > > > + * 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 <stdint.h> > > > > > +#include <inttypes.h> > > > > > +#include <unistd.h> > > > > > +#include <signal.h> > > > > > +#include <getopt.h> > > > > > + > > > > > +#include <rte_eal.h> > > > > > +#include <rte_ethdev.h> > > > > > +#include <rte_cycles.h> > > > > > +#include <rte_malloc.h> > > > > > +#include <rte_debug.h> > > > > > +#include <rte_distributor.h> > > > > > + > > > > > +#include "main.h" > > > > > + > > > > > +#define RX_RING_SIZE 256 > > > > > +#define RX_FREE_THRESH 32 > > > > > +#define RX_PTHRESH 8 > > > > > +#define RX_HTHRESH 8 > > > > > +#define RX_WTHRESH 0 > > > > > + > > > > > +#define TX_RING_SIZE 512 > > > > > +#define TX_FREE_THRESH 32 > > > > > +#define TX_PTHRESH 32 > > > > > +#define TX_HTHRESH 0 > > > > > +#define TX_WTHRESH 0 > > > > > +#define TX_RSBIT_THRESH 32 > > > > > +#define TX_Q_FLAGS (ETH_TXQ_FLAGS_NOMULTSEGS | > > > > > ETH_TXQ_FLAGS_NOVLANOFFL |\ > > > > > + ETH_TXQ_FLAGS_NOXSUMSCTP | ETH_TXQ_FLAGS_NOXSUMUDP | > > > > > \ > > > > > + ETH_TXQ_FLAGS_NOXSUMTCP) > > > > > + > > > > > +#define NUM_MBUFS ((64*1024)-1) #define MBUF_SIZE (2048 + > > > > > +sizeof(struct rte_mbuf) + > > > > > RTE_PKTMBUF_HEADROOM) > > > > > +#define MBUF_CACHE_SIZE 250 > > > > > +#define BURST_SIZE 32 > > > > > +#define RTE_RING_SZ 1024 > > > > > + > > > > > +/* mask of enabled ports */ > > > > > +static uint32_t enabled_port_mask = 0; > > > > > + > > > > > +static volatile struct app_stats { > > > > > + struct { > > > > > + uint64_t rx_pkts; > > > > > + uint64_t returned_pkts; > > > > > + uint64_t enqueued_pkts; > > > > > + } rx __rte_cache_aligned; > > > > > + > > > > > + struct { > > > > > + uint64_t dequeue_pkts; > > > > > + uint64_t tx_pkts; > > > > > + } tx __rte_cache_aligned; > > > > > +} app_stats; > > > > > + > > > > > +static const struct rte_eth_conf port_conf_default = { > > > > > + .rxmode = { > > > > > + .mq_mode = ETH_MQ_RX_RSS, > > > > > + .max_rx_pkt_len = ETHER_MAX_LEN, > > > > > + .split_hdr_size = 0, > > > > > + .header_split = 0, /**< Header Split disabled */ > > > > > + .hw_ip_checksum = 0, /**< IP checksum offload enabled */ > > > > > + .hw_vlan_filter = 0, /**< VLAN filtering disabled */ > > > > > + .jumbo_frame = 0, /**< Jumbo Frame Support disabled > > > > > */ > > > > > + .hw_strip_crc = 0, /**< CRC stripped by hardware */ > > > > > + }, > > > > > + .txmode = { > > > > > + .mq_mode = ETH_MQ_TX_NONE, > > > > > + }, > > > > > + .lpbk_mode = 0, > > > > > + .rx_adv_conf = { > > > > > + .rss_conf = { > > > > > + .rss_hf = ETH_RSS_IPV4 | ETH_RSS_IPV6 | > > > > > + ETH_RSS_IPV4_TCP | > > > > > ETH_RSS_IPV4_UDP | > > > > > + ETH_RSS_IPV6_TCP | > > > > > ETH_RSS_IPV6_UDP, > > > > > + } > > > > > + }, > > > > > +}; > > > > > + > > > > > +static const struct rte_eth_rxconf rx_conf_default = { > > > > > + .rx_thresh = { > > > > > + .pthresh = RX_PTHRESH, > > > > > + .hthresh = RX_HTHRESH, > > > > > + .wthresh = RX_WTHRESH, > > > > > + }, > > > > > + .rx_free_thresh = RX_FREE_THRESH, > > > > > + .rx_drop_en = 0, > > > > > +}; > > > > > + > > > > > +static const struct rte_eth_txconf tx_conf_default = { > > > > > + .tx_thresh = { > > > > > + .pthresh = TX_PTHRESH, > > > > > + .hthresh = TX_HTHRESH, > > > > > + .wthresh = TX_WTHRESH, > > > > > + }, > > > > > + .tx_free_thresh = TX_FREE_THRESH, > > > > > + .tx_rs_thresh = TX_RSBIT_THRESH, > > > > > + .txq_flags = TX_Q_FLAGS > > > > > + > > > > > +}; > > > > > + > > > > > +struct output_buffer { > > > > > + unsigned count; > > > > > + struct rte_mbuf *mbufs[BURST_SIZE]; }; > > > > > + > > > > > +/* > > > > > + * Initialises a given port using global settings and with the > > > > > +rx buffers > > > > > + * coming from the mbuf_pool passed as parameter */ static > > > > > +inline int port_init(uint8_t port, struct rte_mempool *mbuf_pool) { > > > > > + struct rte_eth_conf port_conf = port_conf_default; > > > > > + const uint16_t rxRings = 1, txRings = rte_lcore_count() - 1; > > > > > + int retval; > > > > > + uint16_t q; > > > > > + > > > > > + if (port >= rte_eth_dev_count()) > > > > > + return -1; > > > > > + > > > > > + retval = rte_eth_dev_configure(port, rxRings, txRings, > > > > > &port_conf); > > > > > + if (retval != 0) > > > > > + return retval; > > > > > + > > > > > + for (q = 0; q < rxRings; q++) { > > > > > + retval = rte_eth_rx_queue_setup(port, q, RX_RING_SIZE, > > > > > + > > > > > rte_eth_dev_socket_id(port), > > > > > + &rx_conf_default, > > > > > mbuf_pool); > > > > > + if (retval < 0) > > > > > + return retval; > > > > > + } > > > > > + > > > > > + for (q = 0; q < txRings; q++) { > > > > > + retval = rte_eth_tx_queue_setup(port, q, TX_RING_SIZE, > > > > > + > > > > > rte_eth_dev_socket_id(port), > > > > > + &tx_conf_default); > > > > > + if (retval < 0) > > > > > + return retval; > > > > > + } > > > > > + > > > > > + retval = rte_eth_dev_start(port); > > > > > + if (retval < 0) > > > > > + return retval; > > > > > + > > > > > + struct rte_eth_link link; > > > > > + rte_eth_link_get_nowait(port, &link); > > > > > + if (!link.link_status) { > > > > > + sleep(1); > > > > > + rte_eth_link_get_nowait(port, &link); > > > > > + } > > > > > + > > > > > + if (!link.link_status) { > > > > > + printf("Link down on port %"PRIu8"\n", port); > > > > > + return 0; > > > > > + } > > > > > + > > > > > + struct ether_addr addr; > > > > > + rte_eth_macaddr_get(port, &addr); > > > > > + printf("Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8 > > > > > + " %02"PRIx8" %02"PRIx8" %02"PRIx8"\n", > > > > > + (unsigned)port, > > > > > + addr.addr_bytes[0], addr.addr_bytes[1], > > > > > + addr.addr_bytes[2], addr.addr_bytes[3], > > > > > + addr.addr_bytes[4], addr.addr_bytes[5]); > > > > > + > > > > > + rte_eth_promiscuous_enable(port); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +struct lcore_params { > > > > > + unsigned worker_id; > > > > > + struct rte_distributor *d; > > > > > + struct rte_ring *r; > > > > > +}; > > > > > + > > > > > +static __attribute__((noreturn)) void lcore_rx(struct > > > > > +lcore_params > > > > > +*p) { > > > > > + struct rte_distributor *d = p->d; > > > > > + struct rte_ring *r = p->r; > > > > > + const uint8_t nb_ports = rte_eth_dev_count(); > > > > > + const int socket_id = rte_socket_id(); > > > > > + uint8_t port; > > > > > + > > > > > + for (port = 0; port < nb_ports; port++) { > > > > > + /* skip ports that are not enabled */ > > > > > + if ((enabled_port_mask & (1 << port)) == 0) > > > > > + continue; > > > > > + > > > > > + if (rte_eth_dev_socket_id(port) > 0 && > > > > > + rte_eth_dev_socket_id(port) != > > > > > socket_id) > > > > > + printf("WARNING, port %u is on remote NUMA node > > > > > to " > > > > > + "RX thread.\n\tPerformance will > > > > > not > > > > > " > > > > > + "be optimal.\n", port); > > > > > + } > > > > > + > > > > > + printf("\nCore %u doing packet RX.\n", rte_lcore_id()); > > > > > + port = 0; > > > > > + for (;;) { > > > > > + /* skip ports that are not enabled */ > > > > > + if ((enabled_port_mask & (1 << port)) == 0) { > > > > > + if (++port == nb_ports) > > > > > + port = 0; > > > > > + continue; > > > > > + } > > > > > + struct rte_mbuf *bufs[BURST_SIZE*2]; > > > > > + const uint16_t nb_rx = rte_eth_rx_burst(port, 0, bufs, > > > > > + BURST_SIZE); > > > > > + app_stats.rx.rx_pkts += nb_rx; > > > > > + > > > > > + rte_distributor_process(d, bufs, nb_rx); > > > > > + const uint16_t nb_ret = rte_distributor_returned_pkts(d, > > > > > + bufs, BURST_SIZE*2); > > > > > + app_stats.rx.returned_pkts += nb_ret; > > > > > + if (unlikely(nb_ret == 0)) > > > > > + continue; > > > > > + > > > > > + uint16_t sent = rte_ring_enqueue_burst(r, (void *)bufs, > > > > > nb_ret); > > > > > + app_stats.rx.enqueued_pkts += sent; > > > > > + if (unlikely(sent < nb_ret)) { > > > > > + printf("Packet loss due to full ring\n"); > > > > > > > > I would remove this printf. If core is too busy to process > > > > packets, this may > > make performance worse. > > > > Statistics should reflect this packet loss. > > > > > > >Probably use RTE_LOG(DEBUG, ...). > > > >That's way you can switch it on/off at compile time. > > > > > > By default LOG_LEVEL is 8 in config/common_linuxapp, hence I see > > RTE_LOG(DEBUG,...) always works. > > > So until unless change the LOG-LEVEL to lesser value, > > > RTE_LOG(DEBUG,..) > > will be logged .. > > > Please let me know if I can go ahead and remove the printf as of now > > > until > > the LOG_LEVEL is fixed. (or) I have to add some logic to log > > > RTE_LOG(DEBUG,..) only once. > > > > Sorry, I probably wasn't clear enough. > > What I am saying - create something like that inside your app code: > > > > #ifdef XXX_DEBUG > > #define XXX_LOG(level, fmt, args...) \ > > RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args) > > #else #define XXX_LOG(level, fmt, args...) do { } while(0) #endif > > > > And replace your pritnf()s with it. > > I think we should make some changes in some logs in DPDK, and change the > default log level to be INFO or less, so we could use the > DEBUG level for things like this, but anyway, that would be another patch > that I am thinking to submit some other day. > Anyway, for this one, this option could work, but still that message may be > shown hundreds of thousands of times per second, > considering that CPU is overloaded. So, should we include something like a > flag here, so message is shown just once? > I know that this will only be used when DEBUG is enabled, but still that > could flood the screen, as it is in an infinite loop. > > > > > > > > > > > > > > > + while (sent < nb_ret) > > > > > + rte_pktmbuf_free(bufs[sent++]); > > > > > + } > > > > > + if (++port == nb_ports) > > > > > + port = 0; > > > > > + } > > > > > +} > > > > > + > > > > > +static inline void > > > > > +flush_one_port(struct output_buffer *outbuf, uint8_t outp) { > > > > > + unsigned nb_tx = rte_eth_tx_burst(outp, 0, outbuf->mbufs, > > > > > + outbuf->count); > > > > > + app_stats.tx.tx_pkts += nb_tx; > > > > > + > > > > > + if (unlikely(nb_tx < outbuf->count)) { > > > > > + printf("Packet loss with tx_burst\n"); > > > > > > > > Same here. This is fine for debugging, but statistics are updated > > > > as well, > > so we do not need this printf. > > > > > > > > > + do { > > > > > + rte_pktmbuf_free(outbuf->mbufs[nb_tx]); > > > > > + } while (++nb_tx < outbuf->count); > > > > > + } > > > > > + outbuf->count = 0; > > > > > +} > > > > > +
