DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v4 0/2] testpmd: simulating noisy host environment
@ 2018-06-23  8:08 Maxime Coquelin
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode Maxime Coquelin
  0 siblings, 2 replies; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-23  8:08 UTC (permalink / raw)
  To: bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson, thomas,
	konstantin.ananyev, ferruh.yigit, Maxime Coquelin

This forth version is a rebase of Jen's v3, fixing meson build
and checkpatch errors.
Some checkpatch warnings remain to keep consistency with other
code.

This series adds a new forwarding mode 'noisy'.  It proposes
enhancements 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 simple of measuring performance impact on cache and
memory footprint utilization from various VNF co-located on the
same host machine.

This series of patches adds the new command line switches to
testpmd:

--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.


Options to simulate route lookups:

--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.

Jens Freimann (2):
  testpmd: add forwarding mode to simulate a noisy neighbour
  testpmd: update testpmd doc to include noisy forwarding mode

 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 +++
 doc/guides/testpmd_app_ug/run_app.rst       |  27 ++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 +-
 9 files changed, 402 insertions(+), 2 deletions(-)
 create mode 100644 app/test-pmd/noisy_vnf.c
 create mode 100644 app/test-pmd/noisy_vnf.h

-- 
2.14.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-23  8:08 [dpdk-dev] [PATCH v4 0/2] testpmd: simulating noisy host environment Maxime Coquelin
@ 2018-06-23  8:08 ` Maxime Coquelin
  2018-06-26 11:09   ` Iremonger, Bernard
                     ` (3 more replies)
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode Maxime Coquelin
  1 sibling, 4 replies; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-23  8:08 UTC (permalink / raw)
  To: bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson, thomas,
	konstantin.ananyev, ferruh.yigit, Jens Freimann, Maxime Coquelin

From: Jens Freimann <jfreimann@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.

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.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@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;
+
+	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);
+	}
+}
+
+/*
+ * 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.
+ */
+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
+	 */
+	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);
+			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) {
+		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");
+
+	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)
+					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;
+
 /*
  * Receive Side Scaling (RSS) configuration.
  */
@@ -1641,12 +1677,26 @@ start_port(portid_t pid)
 				return -1;
 			}
 		}
+		noisy_cfg = (struct noisy_config *) rte_zmalloc(
+				"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)));
+		}
 		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));
 				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);
 	}
 	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;
 
-- 
2.14.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode
  2018-06-23  8:08 [dpdk-dev] [PATCH v4 0/2] testpmd: simulating noisy host environment Maxime Coquelin
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
@ 2018-06-23  8:08 ` Maxime Coquelin
  2018-06-26 11:12   ` Iremonger, Bernard
  1 sibling, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-23  8:08 UTC (permalink / raw)
  To: bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson, thomas,
	konstantin.ananyev, ferruh.yigit, Jens Freimann

From: Jens Freimann <jfreimann@redhat.com>

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 doc/guides/testpmd_app_ug/run_app.rst       | 27 +++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index f301c2b6f..b981f103b 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -340,6 +340,7 @@ The commandline options are:
        icmpecho
        ieee1588
        tm
+       noisy
 
 *   ``--rss-ip``
 
@@ -498,3 +499,29 @@ The commandline options are:
 *   ``--no-mlockall``
 
     Disable locking all memory.
+
+*   ``--noisy-buffersize-before-sending=N``
+
+    Set the number of maximum elements  of the FIFO queue to be created
+    for buffering packets. Only available with the noisy forwarding mode.
+    The default value is 0.
+
+*   ``--noisy-flush-timeout=N``
+
+    Set the size of the FIFO queue to be created for buffering packets.
+    Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--noisy-nb-rnd-read=N``
+
+    Set the number of reads to be done in noisy neighbour simulation memory buffer.
+    Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--noisy-nb-rnd-write=N``
+
+    Set the number of writes to be done in noisy neighbour simulation memory buffer.
+    Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--noisy-nb-rnd-write=N``
+
+    Set the number of r/w access to be done in noisy neighbour simulation memory buffer.
+    Only available with the noisy forwarding mode. The default value is 0.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 0d6fd50ca..fd66afed5 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -289,7 +289,7 @@ set fwd
 Set the packet forwarding mode::
 
    testpmd> set fwd (io|mac|macswap|flowgen| \
-                     rxonly|txonly|csum|icmpecho) (""|retry)
+                     rxonly|txonly|csum|icmpecho|noisy) (""|retry)
 
 ``retry`` can be specified for forwarding engines except ``rx_only``.
 
@@ -327,8 +327,11 @@ The available information categories are:
   also modify the default hierarchy or specify the new hierarchy through CLI for
   implementing QoS scheduler.  Requires ``CONFIG_RTE_LIBRTE_PMD_SOFTNIC=y`` ``CONFIG_RTE_LIBRTE_SCHED=y``.
 
-Example::
+* ``noisy``: Noisy neighbour simulation.
+  Simulate more realistic behavior of a guest machine engaged in receiving
+  and sending packets performing Virtual Network Function (VNF).
 
+Example::
    testpmd> set fwd rxonly
 
    Set rxonly packet forwarding mode
-- 
2.14.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
@ 2018-06-26 11:09   ` Iremonger, Bernard
  2018-06-29 13:38     ` Maxime Coquelin
  2018-06-26 12:17   ` Shahaf Shuler
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Iremonger, Bernard @ 2018-06-26 11:09 UTC (permalink / raw)
  To: Maxime Coquelin, dev
  Cc: ailan, jan.scheurich, vkaplans, Richardson, Bruce, thomas,
	Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Saturday, June 23, 2018 9:09 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org
> Cc: ailan@redhat.com; jan.scheurich@ericsson.com; vkaplans@redhat.com;
> Richardson, Bruce <bruce.richardson@intel.com>; thomas@monjalon.net;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Jens Freimann <jfreimann@redhat.com>; Maxime
> Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy
> neighbor

testpmd should be app/testpmd  in patch title.

> 
> From: Jens Freimann <jfreimann@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.
> 
> 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.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@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;
> +
> +	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);
> +	}
> +}
> +
> +/*
> + * 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.
> + */
> +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
> +	 */
> +	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);
> +			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) {
> +		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");
> +
> +	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 @@

License is missing from the noisy_vnf.h file.

> +#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)
> +					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;
> +
>  /*
>   * Receive Side Scaling (RSS) configuration.
>   */
> @@ -1641,12 +1677,26 @@ start_port(portid_t pid)
>  				return -1;
>  			}
>  		}
> +		noisy_cfg = (struct noisy_config *) rte_zmalloc(
> +				"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)));
> +		}
>  		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));
>  				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);
>  	}
>  	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;
> +};

Should  struct noisy_config{} be moved to noisy_vnf.h ?

> +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;
> 
> --
> 2.14.4

checkpatch.pl  is showing the following warnings:

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#122: FILE: app/test-pmd/noisy_vnf.c:1:
+/* SPDX-License-Identifier: BSD-3-Clause

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#332: FILE: app/test-pmd/noisy_vnf.h:1:
+#ifndef __NOISY_VNF_H

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#343: FILE: app/test-pmd/noisy_vnf.h:12:
+fifo_put(struct rte_ring *r, struct rte_mbuf **data, unsigned num)

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#353: FILE: app/test-pmd/noisy_vnf.h:22:
+fifo_get(struct rte_ring *r, struct rte_mbuf **data, unsigned num)

WARNING: line over 80 characters
#394: FILE: app/test-pmd/parameters.c:1154:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-buffersize-before-sending")) {

WARNING: line over 80 characters
#402: FILE: app/test-pmd/parameters.c:1162:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-flush-timeout")) {

WARNING: line over 80 characters
#410: FILE: app/test-pmd/parameters.c:1170:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-memory-footprint")) {

WARNING: line over 80 characters
#413: FILE: app/test-pmd/parameters.c:1173:
+                                       noisy_vnf_memory_footprint = (uint16_t) n;

WARNING: line over 80 characters
#418: FILE: app/test-pmd/parameters.c:1178:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-write")) {

WARNING: line over 80 characters
#426: FILE: app/test-pmd/parameters.c:1186:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read")) {

WARNING: line over 80 characters
#434: FILE: app/test-pmd/parameters.c:1194:
+                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read-write")) {

total: 0 errors, 12 warnings, 454 lines checked

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode Maxime Coquelin
@ 2018-06-26 11:12   ` Iremonger, Bernard
  0 siblings, 0 replies; 16+ messages in thread
From: Iremonger, Bernard @ 2018-06-26 11:12 UTC (permalink / raw)
  To: Maxime Coquelin, dev
  Cc: ailan, jan.scheurich, vkaplans, Richardson, Bruce, thomas,
	Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Saturday, June 23, 2018 9:09 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org
> Cc: ailan@redhat.com; jan.scheurich@ericsson.com; vkaplans@redhat.com;
> Richardson, Bruce <bruce.richardson@intel.com>; thomas@monjalon.net;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Jens Freimann <jfreimann@redhat.com>
> Subject: [PATCH v4 2/2] testpmd: update testpmd doc to include noisy
> forwarding mode
> 

The patch title should app/testpmd instead of testpmd.

> From: Jens Freimann <jfreimann@redhat.com>
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---

<snip>

This patch could be squashed into the first patch.

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
  2018-06-26 11:09   ` Iremonger, Bernard
@ 2018-06-26 12:17   ` Shahaf Shuler
  2018-06-26 13:38     ` Ferruh Yigit
  2018-06-29 14:23     ` Maxime Coquelin
  2018-06-27 11:43   ` Kevin Traynor
  2018-06-27 14:12   ` Adrien Mazarguil
  3 siblings, 2 replies; 16+ messages in thread
From: Shahaf Shuler @ 2018-06-26 12:17 UTC (permalink / raw)
  To: Maxime Coquelin, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson,
	Thomas Monjalon, konstantin.ananyev, ferruh.yigit, Jens Freimann

HI Maxime, Jens

Saturday, June 23, 2018 11:09 AM, Maxime Coquelin:
> Subject: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
> simulate a noisy neighbour
> 
> From: Jens Freimann <jfreimann@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).

I really like the idea of adding application w/ more realistic behavior to the DPDK tree in order to use them in the non-regression cycle. 

I am just wondering if putting it as another forward engine in testpmd is the right place for it.
>From one side it reuses testpmd command line options which is good.
On the other hand it tends to have a lot of options specific only for the VNF forward use case. And I am quite sure that w/ the time goes by the number of configuration options will increase. 

I think this code needs to be as a dedicated example under examples/. 
Moreover I would refine from directly associate it with VNF. VNF can be one example but for sure there are more. 
We can call the example noisy_neigbour or any other name you like.

One more comment below. 

> 
> 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.
> 
> 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.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@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;
> +
> +	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);
> +	}
> +}
> +
> +/*
> + * 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.
> + */
> +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;

Am not sure I understand this part.
The simulation of the memory lookups should happen for every packet **before** deciding on the output queue. Isn't it more realistic to simulate the memory access after the rx_burst?

> +		} 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
> +	 */
> +	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);
> +			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) {
> +		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");
> +
> +	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)
> +					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;
> +
>  /*
>   * Receive Side Scaling (RSS) configuration.
>   */
> @@ -1641,12 +1677,26 @@ start_port(portid_t pid)
>  				return -1;
>  			}
>  		}
> +		noisy_cfg = (struct noisy_config *) rte_zmalloc(
> +				"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)));
> +		}
>  		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));
>  				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);
>  	}
>  	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;
> 
> --
> 2.14.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-26 12:17   ` Shahaf Shuler
@ 2018-06-26 13:38     ` Ferruh Yigit
  2018-06-27 13:51       ` Shahaf Shuler
  2018-06-29 14:23     ` Maxime Coquelin
  1 sibling, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2018-06-26 13:38 UTC (permalink / raw)
  To: Shahaf Shuler, Maxime Coquelin, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson,
	Thomas Monjalon, konstantin.ananyev, Jens Freimann

On 6/26/2018 1:17 PM, Shahaf Shuler wrote:
> HI Maxime, Jens
> 
> Saturday, June 23, 2018 11:09 AM, Maxime Coquelin:
>> Subject: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
>> simulate a noisy neighbour
>>
>> From: Jens Freimann <jfreimann@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).
> 
> I really like the idea of adding application w/ more realistic behavior to the DPDK tree in order to use them in the non-regression cycle. 
> 
> I am just wondering if putting it as another forward engine in testpmd is the right place for it.

Hi Shahaf,

This was the request in old review comments. As you said it is good to have more
realistic behavior but also we would like to test/use io forwarding, so new
forwarding engine is way of having both two.


> From one side it reuses testpmd command line options which is good.
> On the other hand it tends to have a lot of options specific only for the VNF forward use case. And I am quite sure that w/ the time goes by the number of configuration options will increase. 
> 
> I think this code needs to be as a dedicated example under examples/. 
> Moreover I would refine from directly associate it with VNF. VNF can be one example but for sure there are more. 
> We can call the example noisy_neigbour or any other name you like.
> 
> One more comment below. 
> 
>>
>> 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.
>>
>> 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.
>>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@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;
>> +
>> +	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);
>> +	}
>> +}
>> +
>> +/*
>> + * 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.
>> + */
>> +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;
> 
> Am not sure I understand this part.
> The simulation of the memory lookups should happen for every packet **before** deciding on the output queue. Isn't it more realistic to simulate the memory access after the rx_burst?
> 
>> +		} 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
>> +	 */
>> +	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);
>> +			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) {
>> +		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");
>> +
>> +	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)
>> +					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;
>> +
>>  /*
>>   * Receive Side Scaling (RSS) configuration.
>>   */
>> @@ -1641,12 +1677,26 @@ start_port(portid_t pid)
>>  				return -1;
>>  			}
>>  		}
>> +		noisy_cfg = (struct noisy_config *) rte_zmalloc(
>> +				"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)));
>> +		}
>>  		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));
>>  				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);
>>  	}
>>  	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;
>>
>> --
>> 2.14.4
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
  2018-06-26 11:09   ` Iremonger, Bernard
  2018-06-26 12:17   ` Shahaf Shuler
@ 2018-06-27 11:43   ` Kevin Traynor
  2018-06-27 14:12   ` Adrien Mazarguil
  3 siblings, 0 replies; 16+ messages in thread
From: Kevin Traynor @ 2018-06-27 11:43 UTC (permalink / raw)
  To: Maxime Coquelin, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson, thomas,
	konstantin.ananyev, ferruh.yigit, Jens Freimann

On 06/23/2018 09:08 AM, Maxime Coquelin wrote:
> From: Jens Freimann <jfreimann@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@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@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;
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-26 13:38     ` Ferruh Yigit
@ 2018-06-27 13:51       ` Shahaf Shuler
  2018-06-27 14:09         ` Maxime Coquelin
  0 siblings, 1 reply; 16+ messages in thread
From: Shahaf Shuler @ 2018-06-27 13:51 UTC (permalink / raw)
  To: Ferruh Yigit, Maxime Coquelin, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson,
	Thomas Monjalon, konstantin.ananyev, Jens Freimann

Tuesday, June 26, 2018 4:39 PM, Ferruh Yigit:
> Subject: Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
> simulate a noisy neighbour
> 
> On 6/26/2018 1:17 PM, Shahaf Shuler wrote:
> > HI Maxime, Jens
> >
> > Saturday, June 23, 2018 11:09 AM, Maxime Coquelin:
> >> Subject: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
> >> simulate a noisy neighbour
> >>
> >> From: Jens Freimann <jfreimann@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).
> >
> > I really like the idea of adding application w/ more realistic behavior to the
> DPDK tree in order to use them in the non-regression cycle.
> >
> > I am just wondering if putting it as another forward engine in testpmd is
> the right place for it.
> 
> Hi Shahaf,
> 
> This was the request in old review comments. As you said it is good to have
> more realistic behavior but also we would like to test/use io forwarding, so
> new forwarding engine is way of having both two.

Sorry, I missed reading the previous comments. 
As I said, there are upsides and downsides to it. I still think it deserve a separate example but we can start inside testpmd and move it to other location when needed. 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-27 13:51       ` Shahaf Shuler
@ 2018-06-27 14:09         ` Maxime Coquelin
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-27 14:09 UTC (permalink / raw)
  To: Shahaf Shuler, Ferruh Yigit, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson,
	Thomas Monjalon, konstantin.ananyev, Jens Freimann

Hi Sharaf,

On 06/27/2018 03:51 PM, Shahaf Shuler wrote:
> Tuesday, June 26, 2018 4:39 PM, Ferruh Yigit:
>> Subject: Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
>> simulate a noisy neighbour
>>
>> On 6/26/2018 1:17 PM, Shahaf Shuler wrote:
>>> HI Maxime, Jens
>>>
>>> Saturday, June 23, 2018 11:09 AM, Maxime Coquelin:
>>>> Subject: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to
>>>> simulate a noisy neighbour
>>>>
>>>> From: Jens Freimann <jfreimann@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).
>>>
>>> I really like the idea of adding application w/ more realistic behavior to the
>> DPDK tree in order to use them in the non-regression cycle.
>>>
>>> I am just wondering if putting it as another forward engine in testpmd is
>> the right place for it.
>>
>> Hi Shahaf,
>>
>> This was the request in old review comments. As you said it is good to have
>> more realistic behavior but also we would like to test/use io forwarding, so
>> new forwarding engine is way of having both two.
> 
> Sorry, I missed reading the previous comments.
> As I said, there are upsides and downsides to it. I still think it deserve a separate example but we can start inside testpmd and move it to other location when needed.

I get your point, but I think it is better to have it in testpmd as we
intend to use it to gather some benchmarks.

And yes, if this is turn out to be problematic, we can move it outside
testpmd at some points.

Thanks for your insights,
Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
                     ` (2 preceding siblings ...)
  2018-06-27 11:43   ` Kevin Traynor
@ 2018-06-27 14:12   ` Adrien Mazarguil
  3 siblings, 0 replies; 16+ messages in thread
From: Adrien Mazarguil @ 2018-06-27 14:12 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev

On Sat, Jun 23, 2018 at 10:08:39AM +0200, Maxime Coquelin wrote:
> From: Jens Freimann <jfreimann@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.
> 
> 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.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

I'm personally more interested in a forwarding mode for nosy neighbors :)

Sorry, couldn't resist. I know where the door is.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-26 11:09   ` Iremonger, Bernard
@ 2018-06-29 13:38     ` Maxime Coquelin
  2018-06-29 14:05       ` Iremonger, Bernard
  0 siblings, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-29 13:38 UTC (permalink / raw)
  To: Iremonger, Bernard, dev
  Cc: ailan, jan.scheurich, vkaplans, Richardson, Bruce, thomas,
	Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann

Hi Bernard,

On 06/26/2018 01:09 PM, Iremonger, Bernard wrote:
> checkpatch.pl  is showing the following warnings:
> 
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> #122: FILE: app/test-pmd/noisy_vnf.c:1:
> +/* SPDX-License-Identifier: BSD-3-Clause

I'm not sure to get what is wrong here, any thoughts?

<snip/>

> WARNING: line over 80 characters
> #394: FILE: app/test-pmd/parameters.c:1154:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-buffersize-before-sending")) {
> 
> WARNING: line over 80 characters
> #402: FILE: app/test-pmd/parameters.c:1162:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-flush-timeout")) {
> 
> WARNING: line over 80 characters
> #410: FILE: app/test-pmd/parameters.c:1170:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-memory-footprint")) {
> 
> WARNING: line over 80 characters
> #413: FILE: app/test-pmd/parameters.c:1173:
> +                                       noisy_vnf_memory_footprint = (uint16_t) n;
> 
> WARNING: line over 80 characters
> #418: FILE: app/test-pmd/parameters.c:1178:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-write")) {
> 
> WARNING: line over 80 characters
> #426: FILE: app/test-pmd/parameters.c:1186:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read")) {
> 
> WARNING: line over 80 characters
> #434: FILE: app/test-pmd/parameters.c:1194:
> +                       if (!strcmp(lgopts[opt_idx].name, "noisy-nb-rnd-read-write")) {

The above ones were left intentionally for consistency with code around
them.

Should I fix them? (I'm fine doing it if you prefer)

Thanks,
Maxime

> total: 0 errors, 12 warnings, 454 lines checked
> 
> Regards,
> 
> Bernard.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-29 13:38     ` Maxime Coquelin
@ 2018-06-29 14:05       ` Iremonger, Bernard
  2018-06-29 14:24         ` Maxime Coquelin
  2018-06-29 14:28         ` Thomas Monjalon
  0 siblings, 2 replies; 16+ messages in thread
From: Iremonger, Bernard @ 2018-06-29 14:05 UTC (permalink / raw)
  To: Maxime Coquelin, dev
  Cc: ailan, jan.scheurich, vkaplans, Richardson, Bruce, thomas,
	Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann

Hi Maxime, Thomas,

<snip>
> Subject: Re: [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy
> neighbour
> 
> Hi Bernard,
> 
> On 06/26/2018 01:09 PM, Iremonger, Bernard wrote:
> > checkpatch.pl  is showing the following warnings:
> >
> > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> > #122: FILE: app/test-pmd/noisy_vnf.c:1:
> > +/* SPDX-License-Identifier: BSD-3-Clause
> 
> I'm not sure to get what is wrong here, any thoughts?
> 

I have looked at code and compared it with other license headers and it looks ok to me.

Hi Thomas,

Could you take a look please to see if you can spot something.

> <snip/>
> 
> > WARNING: line over 80 characters
> > #394: FILE: app/test-pmd/parameters.c:1154:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-buffersize-before-sending")) {
> >
> > WARNING: line over 80 characters
> > #402: FILE: app/test-pmd/parameters.c:1162:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-flush-timeout")) {
> >
> > WARNING: line over 80 characters
> > #410: FILE: app/test-pmd/parameters.c:1170:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-memory-footprint")) {
> >
> > WARNING: line over 80 characters
> > #413: FILE: app/test-pmd/parameters.c:1173:
> > +                                       noisy_vnf_memory_footprint =
> > + (uint16_t) n;
> >
> > WARNING: line over 80 characters
> > #418: FILE: app/test-pmd/parameters.c:1178:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-nb-rnd-write")) {
> >
> > WARNING: line over 80 characters
> > #426: FILE: app/test-pmd/parameters.c:1186:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-nb-rnd-read")) {
> >
> > WARNING: line over 80 characters
> > #434: FILE: app/test-pmd/parameters.c:1194:
> > +                       if (!strcmp(lgopts[opt_idx].name,
> > + "noisy-nb-rnd-read-write")) {
> 
> The above ones were left intentionally for consistency with code around them.
> 
> Should I fix them? (I'm fine doing it if you prefer)

I have looked at the code and I think it would be better to fix them.
The existing code has much shorter strings.
There is some flexibility around the line length, but it depends on the tree maintainer.

Regards,

Bernard.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-26 12:17   ` Shahaf Shuler
  2018-06-26 13:38     ` Ferruh Yigit
@ 2018-06-29 14:23     ` Maxime Coquelin
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-29 14:23 UTC (permalink / raw)
  To: Shahaf Shuler, bernard.iremonger, dev
  Cc: ailan, jan.scheurich, vkaplans, bruce.richardson,
	Thomas Monjalon, konstantin.ananyev, ferruh.yigit, Jens Freimann

Hi Sharaf,

On 06/26/2018 02:17 PM, Shahaf Shuler wrote:
>> +/*
>> + * 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.
>> + */
>> +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;
> Am not sure I understand this part.
> The simulation of the memory lookups should happen for every packet **before** deciding on the output queue. Isn't it more realistic to simulate the memory access after the rx_burst?
> 

The idea with the series is to simulate the noise caused by other VNFs
running on the same cores (it is not simulating memory accesses on the
burst of packets being handled).

Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-29 14:05       ` Iremonger, Bernard
@ 2018-06-29 14:24         ` Maxime Coquelin
  2018-06-29 14:28         ` Thomas Monjalon
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2018-06-29 14:24 UTC (permalink / raw)
  To: Iremonger, Bernard, dev
  Cc: ailan, jan.scheurich, vkaplans, Richardson, Bruce, thomas,
	Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann



On 06/29/2018 04:05 PM, Iremonger, Bernard wrote:
> Hi Maxime, Thomas,
> 
> <snip>
>> Subject: Re: [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy
>> neighbour
>>
>> Hi Bernard,
>>
>> On 06/26/2018 01:09 PM, Iremonger, Bernard wrote:
>>> checkpatch.pl  is showing the following warnings:
>>>
>>> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
>>> #122: FILE: app/test-pmd/noisy_vnf.c:1:
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>
>> I'm not sure to get what is wrong here, any thoughts?
>>
> 
> I have looked at code and compared it with other license headers and it looks ok to me.
> 
> Hi Thomas,
> 
> Could you take a look please to see if you can spot something.
> 
>> <snip/>
>>
>>> WARNING: line over 80 characters
>>> #394: FILE: app/test-pmd/parameters.c:1154:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-buffersize-before-sending")) {
>>>
>>> WARNING: line over 80 characters
>>> #402: FILE: app/test-pmd/parameters.c:1162:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-flush-timeout")) {
>>>
>>> WARNING: line over 80 characters
>>> #410: FILE: app/test-pmd/parameters.c:1170:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-memory-footprint")) {
>>>
>>> WARNING: line over 80 characters
>>> #413: FILE: app/test-pmd/parameters.c:1173:
>>> +                                       noisy_vnf_memory_footprint =
>>> + (uint16_t) n;
>>>
>>> WARNING: line over 80 characters
>>> #418: FILE: app/test-pmd/parameters.c:1178:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-nb-rnd-write")) {
>>>
>>> WARNING: line over 80 characters
>>> #426: FILE: app/test-pmd/parameters.c:1186:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-nb-rnd-read")) {
>>>
>>> WARNING: line over 80 characters
>>> #434: FILE: app/test-pmd/parameters.c:1194:
>>> +                       if (!strcmp(lgopts[opt_idx].name,
>>> + "noisy-nb-rnd-read-write")) {
>>
>> The above ones were left intentionally for consistency with code around them.
>>
>> Should I fix them? (I'm fine doing it if you prefer)
> 
> I have looked at the code and I think it would be better to fix them.
> The existing code has much shorter strings.
> There is some flexibility around the line length, but it depends on the tree maintainer.

Great, thanks for the quick feedback.
Maxime

> Regards,
> 
> Bernard.
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour
  2018-06-29 14:05       ` Iremonger, Bernard
  2018-06-29 14:24         ` Maxime Coquelin
@ 2018-06-29 14:28         ` Thomas Monjalon
  1 sibling, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2018-06-29 14:28 UTC (permalink / raw)
  To: Iremonger, Bernard
  Cc: Maxime Coquelin, dev, ailan, jan.scheurich, vkaplans, Richardson,
	Bruce, Ananyev, Konstantin, Yigit, Ferruh, Jens Freimann

29/06/2018 16:05, Iremonger, Bernard:
> Hi Maxime, Thomas,
> 
> <snip>
> > Subject: Re: [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy
> > neighbour
> > 
> > Hi Bernard,
> > 
> > On 06/26/2018 01:09 PM, Iremonger, Bernard wrote:
> > > checkpatch.pl  is showing the following warnings:
> > >
> > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> > > #122: FILE: app/test-pmd/noisy_vnf.c:1:
> > > +/* SPDX-License-Identifier: BSD-3-Clause
> > 
> > I'm not sure to get what is wrong here, any thoughts?
> > 
> 
> I have looked at code and compared it with other license headers and it looks ok to me.
> 
> Hi Thomas,
> 
> Could you take a look please to see if you can spot something.

You should use devtools/checkpatches.sh, it is configured to ignore SPDX warnings
among other things.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-06-29 14:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-23  8:08 [dpdk-dev] [PATCH v4 0/2] testpmd: simulating noisy host environment Maxime Coquelin
2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 1/2] testpmd: add forwarding mode to simulate a noisy neighbour Maxime Coquelin
2018-06-26 11:09   ` Iremonger, Bernard
2018-06-29 13:38     ` Maxime Coquelin
2018-06-29 14:05       ` Iremonger, Bernard
2018-06-29 14:24         ` Maxime Coquelin
2018-06-29 14:28         ` Thomas Monjalon
2018-06-26 12:17   ` Shahaf Shuler
2018-06-26 13:38     ` Ferruh Yigit
2018-06-27 13:51       ` Shahaf Shuler
2018-06-27 14:09         ` Maxime Coquelin
2018-06-29 14:23     ` Maxime Coquelin
2018-06-27 11:43   ` Kevin Traynor
2018-06-27 14:12   ` Adrien Mazarguil
2018-06-23  8:08 ` [dpdk-dev] [PATCH v4 2/2] testpmd: update testpmd doc to include noisy forwarding mode Maxime Coquelin
2018-06-26 11:12   ` Iremonger, Bernard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).