[dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour

[Kevin Traynor]

On 06/23/2018 09:08 AM, Maxime Coquelin wrote:
> From: Jens Freimann <jfreimann at redhat.com>
> 
> This adds a new forwarding mode to testpmd to simulate
> more realistic behavior of a guest machine engaged in receiving
> and sending packets performing Virtual Network Function (VNF).
> 
> The goal is to enable a simple way of measuring performance impact on
> cache and memory footprint utilization from various VNF co-located on
> the same host machine. For this it does:
> 
> * Buffer packets in a FIFO:
> 
> Create a fifo to buffer received packets. Once it flows over put
> those packets into the actual tx queue. The fifo is created per tx
> queue and its size can be set with the --buffersize-before-sending
> commandline parameter.
> 
> A second commandline parameter is used to set a timeout in
> milliseconds after which the fifo is flushed.
> 
> --noisy-buffersize-before-sending [packet numbers]
> Keep the mbuf in a FIFO and forward the over flooding packets from the
> FIFO. This queue is per TX-queue (after all other packet processing).
> 
> --noisy-flush-timeout [delay]
> Flush the packet queue if no packets have been seen during
> [delay]. As long as packets are seen, the timer is reset.
> 
It's just personal preference on naming, so feel free to ignore, but
perhaps grouping the related arguments...

--noisy-tx-sw-buffer-size
--noisy-tx-sw-buffer-flushtime

> Add several options to simulate route lookups (memory reads) in tables
> that can be quite large, as well as route hit statistics update.
> These options simulates the while stack traversal and
> will trash the cache. Memory access is random.
> 
> * simulate route lookups:
> 
> Allocate a buffer and perform reads and writes on it as specified by
> commandline options:
> 
> --noisy-memory-footprint [size]
> Size of the VNF internal memory (MB), in which the random
> read/write will be done, allocated by rte_malloc (hugepages).
> 
> --noisy-nb-rnd-write [num]
> Number of random writes in memory per packet should be
> performed, simulating hit-flags update. 64 bits per write,
> all write in different cache lines.
> 
> --noisy-nb-rnd-read [num]
> Number of random reads in memory per packet should be
> performed, simulating FIB/table lookups. 64 bits per read,
> all write in different cache lines.
> 
> --noisy-nb-rnd-read-write [num]
> Number of random reads and writes in memory per packet should
> be performed, simulating stats update. 64 bits per read-write, all
> reads and writes in different cache lines.
> 

--noisy-lkup-memory [size]
--noisy-lkup-num-writes [num]
--noisy-lkup-num-reads [num]
--noisy-lkup-num-reads-writes [num]

> Signed-off-by: Jens Freimann <jfreimann at redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> ---
>  app/test-pmd/Makefile     |   1 +
>  app/test-pmd/meson.build  |   1 +
>  app/test-pmd/noisy_vnf.c  | 204 ++++++++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/noisy_vnf.h  |  41 ++++++++++
>  app/test-pmd/parameters.c |  54 ++++++++++++
>  app/test-pmd/testpmd.c    |  52 ++++++++++++
>  app/test-pmd/testpmd.h    |  17 ++++
>  7 files changed, 370 insertions(+)
>  create mode 100644 app/test-pmd/noisy_vnf.c
>  create mode 100644 app/test-pmd/noisy_vnf.h
> 
> diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
> index a5a827bbd..2a1030120 100644
> --- a/app/test-pmd/Makefile
> +++ b/app/test-pmd/Makefile
> @@ -32,6 +32,7 @@ SRCS-y += rxonly.c
>  SRCS-y += txonly.c
>  SRCS-y += csumonly.c
>  SRCS-y += icmpecho.c
> +SRCS-y += noisy_vnf.c
>  SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c
>  SRCS-$(CONFIG_RTE_LIBRTE_BPF) += bpf_cmd.c
>  
> diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
> index 4c9fbbfda..b3abc6e24 100644
> --- a/app/test-pmd/meson.build
> +++ b/app/test-pmd/meson.build
> @@ -16,6 +16,7 @@ sources = files('cmdline.c',
>  	'iofwd.c',
>  	'macfwd.c',
>  	'macswap.c',
> +	'noisy_vnf.c',
>  	'parameters.c',
>  	'rxonly.c',
>  	'testpmd.c',
> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c
> new file mode 100644
> index 000000000..ff205495f
> --- /dev/null
> +++ b/app/test-pmd/noisy_vnf.c
> @@ -0,0 +1,204 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Red Hat Corp.
> + */
> +
> +#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_log.h>
> +#include <rte_debug.h>
> +#include <rte_cycles.h>
> +#include <rte_memory.h>
> +#include <rte_launch.h>
> +#include <rte_eal.h>
> +#include <rte_per_lcore.h>
> +#include <rte_lcore.h>
> +#include <rte_memcpy.h>
> +#include <rte_mempool.h>
> +#include <rte_mbuf.h>
> +#include <rte_ethdev.h>
> +#include <rte_flow.h>
> +#include <rte_malloc.h>
> +
> +#include "testpmd.h"
> +#include "noisy_vnf.h"
> +
> +static inline void
> +do_write(char *vnf_mem)
> +{
> +	uint64_t i = rte_rand();
> +	uint64_t w = rte_rand();
> +
> +	vnf_mem[i % ((noisy_vnf_memory_footprint * 1024 * 1024) /
> +			RTE_CACHE_LINE_SIZE)] = w;
> +}
> +
> +	static inline void
> +do_read(char *vnf_mem)
> +{
> +	uint64_t i = rte_rand();
> +	uint64_t r;
> +
> +	r = vnf_mem[i % ((noisy_vnf_memory_footprint * 1024 * 1024) /
> +			RTE_CACHE_LINE_SIZE)];
> +	r++;
> +}
> +
> +	static inline void
> +do_rw(char *vnf_mem)
> +{
> +	do_read(vnf_mem);
> +	do_write(vnf_mem);
> +}
> +
> +/*
> + * Simulate route lookups as defined by commandline parameters
> + */
> +	static void
> +sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts)
> +{
> +	uint16_t i, j;
> +

noisy_vnf_memory_footprint and/or vnf_mem could be 0.

perhaps rte_exit when vnf_mem cannot be allocated, and check
noisy_vnf_memory_footprint > 0 here

> +	for (i = 0; i < nb_pkts; i++) {
> +		for (j = 0; j < noisy_nb_rnd_write; j++)
> +			do_write(ncf->vnf_mem);
> +		for (j = 0; j < noisy_nb_rnd_read; j++)
> +			do_read(ncf->vnf_mem);
> +		for (j = 0; j < noisy_nb_rnd_read_write; j++)
> +			do_rw(ncf->vnf_mem);

May as well just rename do_readwrite()

> +	}
> +}
> +
> +/*
> + * Forwarding of packets in I/O mode.
> + * Forward packets "as-is".
> + * This is the fastest possible forwarding operation, as it does not access
> + * to packets data.
> + */

Comment needs update for noisy

> +static void
> +pkt_burst_noisy_vnf(struct fwd_stream *fs)
> +{
> +	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> +	uint16_t nb_rx;
> +	uint16_t nb_tx = 0;
> +	uint32_t retry;
> +	const uint64_t freq_khz = rte_get_timer_hz() / 1000;
> +	struct noisy_config *ncf = &noisy_cfg[fs->tx_queue];
> +	struct rte_mbuf *tmp_pkts[MAX_PKT_BURST];
> +	uint16_t nb_enqd;
> +	uint16_t nb_deqd = 0;
> +	uint64_t delta_ms;
> +	uint64_t now;
> +
> +	/*
> +	 * Receive a burst of packets and forward them.
> +	 */
> +	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;
> +
> +	if (noisy_bsize_before_send > 0) {
> +		if (rte_ring_free_count(ncf->f) >= nb_rx) {
> +			/* enqueue into fifo */
> +			nb_enqd = fifo_put(ncf->f, pkts_burst, nb_rx);
> +			if (nb_enqd < nb_rx)
> +				nb_rx = nb_enqd;
> +		} else {
> +			/* fifo is full, dequeue first */
> +			nb_deqd = fifo_get(ncf->f, tmp_pkts, nb_rx);
> +			/* enqueue into fifo */
> +			nb_enqd = fifo_put(ncf->f, pkts_burst, nb_deqd);
> +			sim_memory_lookups(ncf, nb_rx);
> +			if (nb_enqd < nb_rx)
> +				nb_rx = nb_enqd;
> +			if (nb_deqd > 0)
> +				nb_tx = rte_eth_tx_burst(fs->tx_port,
> +						fs->tx_queue, tmp_pkts,
> +						nb_deqd);
> +		}
> +	} else {
> +		sim_memory_lookups(ncf, nb_rx);
> +		nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> +				pkts_burst, nb_rx);
> +	}
> +
> +	/*
> +	 * TX burst queue drain
> +	 */

Should this section be also checking "if (noisy_bsize_before_send > 0)
{" or merging with above. I guess unlikely someone would set
flushtimeout and not buffer size, but I don't think there's any checks

> +	if (ncf->prev_time == 0)
> +		now = ncf->prev_time = rte_get_timer_cycles();
> +	else
> +		now = rte_get_timer_cycles();
> +	delta_ms = (now - ncf->prev_time) / freq_khz;
> +	if (unlikely(delta_ms >= noisy_flush_timer) && noisy_flush_timer > 0 &&
> +		     (nb_tx == 0)) {
> +		while (fifo_count(ncf->f) > 0) {
> +			nb_deqd = fifo_get(ncf->f, tmp_pkts, nb_rx);

why no sim_memory_lookup() in this path?

> +			nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> +						 tmp_pkts, nb_deqd);
> +			if (rte_ring_empty(ncf->f))
> +				break;
> +		}
> +		ncf->prev_time = now;
> +	}
> +	if (nb_tx < nb_rx && fs->retry_enabled)
> +		*pkts_burst = *tmp_pkts;
> +
> +	/*
> +	 * Retry if necessary
> +	 */
> +	if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {

The two if's are the same so can be merged

> +		retry = 0;
> +		while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> +			rte_delay_us(burst_tx_delay_time);
> +			sim_memory_lookups(ncf, nb_rx);
> +			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;
> +	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);
> +	}
> +}
> +
> +struct fwd_engine noisy_vnf_engine = {
> +	.fwd_mode_name  = "noisy",
> +	.port_fwd_begin = NULL,
> +	.port_fwd_end   = NULL,
> +	.packet_fwd     = pkt_burst_noisy_vnf,
> +};
> +
> +#define NOISY_STRSIZE 256
> +#define NOISY_RING "noisy_ring_%d:%d\n"
> +struct rte_ring *
> +noisy_init(uint32_t qi, uint32_t pi)
> +{
> +	struct noisy_config *n = &noisy_cfg[qi];
> +	char name[NOISY_STRSIZE];
> +
> +	snprintf(name, NOISY_STRSIZE, NOISY_RING, pi, qi);
> +	n->f = rte_ring_create(name, noisy_bsize_before_send,
> +			rte_socket_id(), 0);
> +	n->vnf_mem = (char *) rte_zmalloc("vnf sim memory",
> +			 noisy_vnf_memory_footprint * 1024 * 1024,
> +			 RTE_CACHE_LINE_SIZE);
> +	if (n->vnf_mem == NULL)
> +		printf("allocating vnf memory failed\n");

TESTPMD_LOG ?

> +
> +	return n->f;
> +}
> diff --git a/app/test-pmd/noisy_vnf.h b/app/test-pmd/noisy_vnf.h
> new file mode 100644
> index 000000000..6c6df7a9e
> --- /dev/null
> +++ b/app/test-pmd/noisy_vnf.h
> @@ -0,0 +1,41 @@
> +#ifndef __NOISY_VNF_H
> +#define __NOISY_VNF_H
> +#include <stdint.h>
> +#include <rte_mbuf.h>
> +#include <rte_ring.h>
> +#include "testpmd.h"
> +
> +/**
> + * Add elements to fifo. Return number of written elements
> + */
> +static inline unsigned int
> +fifo_put(struct rte_ring *r, struct rte_mbuf **data, unsigned num)
> +{
> +
> +	return rte_ring_enqueue_burst(r, (void **)data, num, NULL);
> +}
> +
> +/**
> + * Get elements from fifo. Return number of read elements
> + */
> +static inline unsigned int
> +fifo_get(struct rte_ring *r, struct rte_mbuf **data, unsigned num)
> +{
> +	return rte_ring_dequeue_burst(r, (void **) data, num, NULL);
> +}
> +
> +static inline unsigned int
> +fifo_count(struct rte_ring *r)
> +{
> +	return rte_ring_count(r);
> +}
> +
> +static inline int
> +fifo_full(struct rte_ring *r)
> +{
> +	return rte_ring_full(r);
> +}
> +
> +struct rte_ring *noisy_init(uint32_t qi, uint32_t pi);
> +
> +#endif
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 75807623c..e6d8b3ef9 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -625,6 +625,12 @@ launch_args_parse(int argc, char** argv)
>  		{ "vxlan-gpe-port",		1, 0, 0 },
>  		{ "mlockall",			0, 0, 0 },
>  		{ "no-mlockall",		0, 0, 0 },
> +		{ "noisy-buffersize-before-sending", 1, 0, 0 },
> +		{ "noisy-flush-timeout",        1, 0, 0 },
> +		{ "noisy-memory-footprint",     1, 0, 0 },
> +		{ "noisy-nb-rnd-write",         1, 0, 0 },
> +		{ "noisy-nb-rnd-read",          1, 0, 0 },
> +		{ "noisy-nb-rnd-read-write",    1, 0, 0 },
>  		{ 0, 0, 0, 0 },
>  	};
>  
> @@ -1145,6 +1151,54 @@ launch_args_parse(int argc, char** argv)
>  				do_mlockall = 1;
>  			if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
>  				do_mlockall = 0;
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-buffersize-before-sending")) {
> +				n = atoi(optarg);
> +				if (n > 0)

(n >= 0) might be better because someone testing different configs can
just change the number to 0 rather than removing (and re-adding)
arguments. Also, it looks a bit odd if the documented default cannot be
set by the user.

> +					noisy_bsize_before_send = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-buffersize-before-sending must be > 0\n");
> +			}
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-flush-timeout")) {
> +				n = atoi(optarg);
> +				if (n >= 0)
> +					noisy_flush_timer = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-flush-timeout must be > 0\n");
> +			}
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-memory-footprint")) {
> +				n = atoi(optarg);
> +				if (n > 0)
> +					noisy_vnf_memory_footprint = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-memory-footprint must be > 0\n");
> +			}
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-write")) {
> +				n = atoi(optarg);
> +				if (n > 0)
> +					noisy_nb_rnd_write = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-nb-rnd-write must be > 0\n");
> +			}
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read")) {
> +				n = atoi(optarg);
> +				if (n > 0)
> +					noisy_nb_rnd_read = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-nb-rnd-read must be > 0\n");
> +			}
> +			if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read-write")) {
> +				n = atoi(optarg);
> +				if (n > 0)
> +					noisy_nb_rnd_read_write = (uint16_t) n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "noisy-nb-rnd-read-write must be > 0\n");
> +			}
>  			break;
>  		case 'h':
>  			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 24c199844..7f3dc6f1c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -61,6 +61,7 @@
>  #include <rte_latencystats.h>
>  #endif
>  
> +#include "noisy_vnf.h"
>  #include "testpmd.h"
>  
>  uint16_t verbose_level = 0; /**< Silent by default. */
> @@ -155,6 +156,7 @@ struct fwd_engine * fwd_engines[] = {
>  	&tx_only_engine,
>  	&csum_fwd_engine,
>  	&icmp_echo_engine,
> +	&noisy_vnf_engine,
>  #if defined RTE_LIBRTE_PMD_SOFTNIC && defined RTE_LIBRTE_SCHED
>  	&softnic_tm_engine,
>  	&softnic_tm_bypass_engine,
> @@ -251,6 +253,40 @@ int16_t tx_free_thresh = RTE_PMD_PARAM_UNSET;
>   */
>  int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
>  
> +/*
> + * Configurable value of buffered packed before sending.
> + */
> +uint16_t noisy_bsize_before_send;
> +
> +/*
> + * Configurable value of packet buffer timeout.
> + */
> +uint16_t noisy_flush_timer;
> +
> +/*
> + * Configurable value for size of VNF internal memory area
> + * used for simulating noisy neighbour behaviour
> + */
> +uint64_t noisy_vnf_memory_footprint;
> +
> +/*
> + * Configurable value of number of random writes done in
> + * VNF simulation memory area.
> + */
> +uint64_t noisy_nb_rnd_write;
> +
> +/*
> + * Configurable value of number of random reads done in
> + * VNF simulation memory area.
> + */
> +uint64_t noisy_nb_rnd_read;
> +
> +/*
> + * Configurable value of number of random reads/wirtes done in
> + * VNF simulation memory area.
> + */
> +uint64_t noisy_nb_rnd_read_write;
> +

These are cast to uint16_t in the only place they set in parameters.c

>  /*
>   * Receive Side Scaling (RSS) configuration.
>   */
> @@ -1641,12 +1677,26 @@ start_port(portid_t pid)
>  				return -1;
>  			}
>  		}
> +		noisy_cfg = (struct noisy_config *) rte_zmalloc(

Is the cast needed?

> +				"testpmd noisy fifo and timers",
> +				nb_txq * sizeof(struct noisy_config),
> +				RTE_CACHE_LINE_SIZE);
> +		if (noisy_cfg == NULL) {
> +			rte_exit(EXIT_FAILURE,
> +					"rte_zmalloc(%d) struct noisy_config) failed\n",
> +					(int)(nb_txq *
> +					sizeof(struct noisy_config)));

There's an unmatched ")" and also it looks to be inconsistent with the
rest of the errors like this, where they print equivalent to the nb, not
nb * sizeof(struct ...)

> +		}
>  		if (port->need_reconfig_queues > 0) {
>  			port->need_reconfig_queues = 0;
>  			/* setup tx queues */
>  			for (qi = 0; qi < nb_txq; qi++) {
>  				port->tx_conf[qi].txq_flags =
>  					ETH_TXQ_FLAGS_IGNORE;
> +				if (!noisy_init(qi, pi) &&
> +						noisy_bsize_before_send > 0)
> +					rte_exit(EXIT_FAILURE, "%s\n",
> +						 rte_strerror(rte_errno));

noisy_init is being called once to init buffers and/or lookups. One is
getting checked for success inside the fn and one outside, one exits and
the other doesn't. Maybe there's a good reason, but it seems inconsistent

>  				if ((numa_support) &&
>  					(txring_numa[pi] != NUMA_NO_CONFIG))
>  					diag = rte_eth_tx_queue_setup(pi, qi,
> @@ -1817,6 +1867,8 @@ stop_port(portid_t pid)
>  			RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
>  			printf("Port %d can not be set into stopped\n", pi);
>  		need_check_link_status = 1;
> +
> +		rte_free(noisy_cfg);

What about the vnf_mem and the ring

>  	}
>  	if (need_check_link_status && !no_link_check)
>  		check_all_ports_link_status(RTE_PORT_ALL);
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index f51cd9dd9..75fb18a25 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -9,6 +9,7 @@
>  #include <rte_bus_pci.h>
>  #include <rte_gro.h>
>  #include <rte_gso.h>
> +#include "noisy_vnf.h"
>  
>  #define RTE_PORT_ALL            (~(portid_t)0x0)
>  
> @@ -122,6 +123,14 @@ struct fwd_stream {
>  #endif
>  };
>  
> +struct noisy_config {
> +	struct rte_ring *f;
> +	uint64_t prev_time;
> +	char *vnf_mem;
> +};
> +struct noisy_config *noisy_cfg;
> +
> +
>  /** Descriptor for a single flow. */
>  struct port_flow {
>  	size_t size; /**< Allocated space including data[]. */
> @@ -266,6 +275,7 @@ extern struct fwd_engine rx_only_engine;
>  extern struct fwd_engine tx_only_engine;
>  extern struct fwd_engine csum_fwd_engine;
>  extern struct fwd_engine icmp_echo_engine;
> +extern struct fwd_engine noisy_vnf_engine;
>  #ifdef TM_MODE
>  extern struct fwd_engine softnic_tm_engine;
>  extern struct fwd_engine softnic_tm_bypass_engine;
> @@ -399,6 +409,13 @@ extern int8_t rx_drop_en;
>  extern int16_t tx_free_thresh;
>  extern int16_t tx_rs_thresh;
>  
> +extern uint16_t noisy_bsize_before_send;
> +extern uint16_t noisy_flush_timer;
> +extern uint64_t noisy_vnf_memory_footprint;
> +extern uint64_t noisy_nb_rnd_write;
> +extern uint64_t noisy_nb_rnd_read;
> +extern uint64_t noisy_nb_rnd_read_write;
> +
>  extern uint8_t dcb_config;
>  extern uint8_t dcb_test;
>  
>