[dpdk-dev] [PATCH 0/6] Extensions to test-pmd

[dpdk-dev] [PATCH 0/6] Extensions to test-pmd

[Cyril Chemparathy]

Hi,

This patch series contains a few minor extensions to test-pmd.  These changes
have been added primarily for convenience while testing out various scenarios
with DPDK.

Thanks
-- Cyril.

Cyril Chemparathy (6):
  test-pmd: add support for single port loopback topology
  test-pmd: add support for auto-start when interactive
  test-pmd: allow command line selection of forwarding mode
  test-pmd: allow txpkts to be setup via command line
  test-pmd: add mac swap forwarding mode
  test-pmd: add flowgen forwarding engine

 app/test-pmd/Makefile     |   2 +
 app/test-pmd/config.c     |  16 ++-
 app/test-pmd/flowgen.c    | 247 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/macswap.c    | 148 +++++++++++++++++++++++++++
 app/test-pmd/parameters.c |  83 +++++++++++++++-
 app/test-pmd/testpmd.c    |  11 ++-
 app/test-pmd/testpmd.h    |   6 +-
 7 files changed, 504 insertions(+), 9 deletions(-)
 create mode 100644 app/test-pmd/flowgen.c
 create mode 100644 app/test-pmd/macswap.c

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/6] test-pmd: add support for single port loopback topology

[Cyril Chemparathy]

This commits adds a new "loop" option to the --port-topology argument.  With
the loop option specified, ingress traffic is simply transmitted back on the
same interface.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/config.c     | 16 ++++++++++++++--
 app/test-pmd/parameters.c |  2 ++
 app/test-pmd/testpmd.h    |  3 ++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 0816227..1d3cdba 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -733,7 +733,8 @@ simple_fwd_config_setup(void)
 	portid_t j;
 	portid_t inc = 2;
 
-	if (port_topology == PORT_TOPOLOGY_CHAINED) {
+	if (port_topology == PORT_TOPOLOGY_CHAINED ||
+	    port_topology == PORT_TOPOLOGY_LOOP) {
 		inc = 1;
 	} else if (nb_fwd_ports % 2) {
 		printf("\nWarning! Cannot handle an odd number of ports "
@@ -761,7 +762,10 @@ simple_fwd_config_setup(void)
 	setup_fwd_config_of_each_lcore(&cur_fwd_config);
 
 	for (i = 0; i < cur_fwd_config.nb_fwd_ports; i = (portid_t) (i + inc)) {
-		j = (portid_t) ((i + 1) % cur_fwd_config.nb_fwd_ports);
+		if (port_topology != PORT_TOPOLOGY_LOOP)
+			j = (portid_t) ((i + 1) % cur_fwd_config.nb_fwd_ports);
+		else
+			j = i;
 		fwd_streams[i]->rx_port   = fwd_ports_ids[i];
 		fwd_streams[i]->rx_queue  = 0;
 		fwd_streams[i]->tx_port   = fwd_ports_ids[j];
@@ -825,10 +829,18 @@ rss_fwd_config_setup(void)
 		struct fwd_stream *fs;
 
 		fs = fwd_streams[lc_id];
+
 		if ((rxp & 0x1) == 0)
 			txp = (portid_t) (rxp + 1);
 		else
 			txp = (portid_t) (rxp - 1);
+		/*
+		 * if we are in loopback, simply send stuff out through the
+		 * ingress port
+		 */
+		if (port_topology == PORT_TOPOLOGY_LOOP)
+			txp = rxp;
+
 		fs->rx_port = fwd_ports_ids[rxp];
 		fs->rx_queue = rxq;
 		fs->tx_port = fwd_ports_ids[txp];
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index f537e49..d47d020 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -756,6 +756,8 @@ launch_args_parse(int argc, char** argv)
 					port_topology = PORT_TOPOLOGY_PAIRED;
 				else if (!strcmp(optarg, "chained"))
 					port_topology = PORT_TOPOLOGY_CHAINED;
+				else if (!strcmp(optarg, "loop"))
+					port_topology = PORT_TOPOLOGY_LOOP;
 				else
 					rte_exit(EXIT_FAILURE, "port-topology %s invalid -"
 						 " must be: paired or chained \n",
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5b4ee6f..a4842ec 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -80,7 +80,8 @@ typedef uint16_t streamid_t;
 
 enum {
 	PORT_TOPOLOGY_PAIRED,
-	PORT_TOPOLOGY_CHAINED
+	PORT_TOPOLOGY_CHAINED,
+	PORT_TOPOLOGY_LOOP,
 };
 
 #ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/6] test-pmd: add support for auto-start when interactive

[Cyril Chemparathy]

This patch adds support for a command-line argument --auto-start (-a).  When
running in interactive mode, this allows us to start traffic without user
intervention before dropping to the prompt.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/parameters.c | 20 ++++++++++++++++----
 app/test-pmd/testpmd.c    |  9 +++++++--
 app/test-pmd/testpmd.h    |  1 +
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index d47d020..18b52cc 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -85,7 +85,7 @@ usage(char* progname)
 #ifdef RTE_LIBRTE_CMDLINE
 	       "[--interactive|-i] "
 #endif
-	       "[--help|-h] | ["
+	       "[--help|-h] | [--auto-start|-a] | ["
 	       "--coremask=COREMASK --portmask=PORTMASK --numa "
 	       "--mbuf-size= | --total-num-mbufs= | "
 	       "--nb-cores= | --nb-ports= | "
@@ -102,6 +102,8 @@ usage(char* progname)
 #ifdef RTE_LIBRTE_CMDLINE
 	printf("  --interactive: run in interactive mode.\n");
 #endif
+	printf("  --auto-start: start forwarding on init "
+	       "[always when non-interactive].\n");
 	printf("  --help: display this message and quit.\n");
 	printf("  --nb-cores=N: set the number of forwarding cores "
 	       "(1 <= N <= %d).\n", nb_lcores);
@@ -485,6 +487,7 @@ launch_args_parse(int argc, char** argv)
 		{ "help",			0, 0, 0 },
 #ifdef RTE_LIBRTE_CMDLINE
 		{ "interactive",		0, 0, 0 },
+		{ "auto-start",			0, 0, 0 },
 		{ "eth-peers-configfile",	1, 0, 0 },
 		{ "eth-peer",			1, 0, 0 },
 #endif
@@ -539,11 +542,11 @@ launch_args_parse(int argc, char** argv)
 	argvopt = argv;
 
 #ifdef RTE_LIBRTE_CMDLINE
-#define SHORTOPTS "ih"
+#define SHORTOPTS "i"
 #else
-#define SHORTOPTS "h"
+#define SHORTOPTS ""
 #endif
-	while ((opt = getopt_long(argc, argvopt, SHORTOPTS,
+	while ((opt = getopt_long(argc, argvopt, SHORTOPTS "ah",
 				 lgopts, &opt_idx)) != EOF) {
 		switch (opt) {
 #ifdef RTE_LIBRTE_CMDLINE
@@ -552,6 +555,11 @@ launch_args_parse(int argc, char** argv)
 			interactive = 1;
 			break;
 #endif
+		case 'a':
+			printf("Auto-start selected\n");
+			auto_start = 1;
+			break;
+
 		case 0: /*long options */
 			if (!strcmp(lgopts[opt_idx].name, "help")) {
 				usage(argv[0]);
@@ -562,6 +570,10 @@ launch_args_parse(int argc, char** argv)
 				printf("Interactive-mode selected\n");
 				interactive = 1;
 			}
+			if (!strcmp(lgopts[opt_idx].name, "auto-start")) {
+				printf("Auto-start selected\n");
+				auto_start = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name,
 				    "eth-peers-configfile")) {
 				if (init_peer_eth_addrs(optarg) != 0)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 04dca57..cde9c2b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -83,6 +83,7 @@ uint16_t verbose_level = 0; /**< Silent by default. */
 
 /* use master core for command line ? */
 uint8_t interactive = 0;
+uint8_t auto_start = 0;
 
 /*
  * NUMA support configuration.
@@ -1794,9 +1795,13 @@ main(int argc, char** argv)
 		rte_eth_promiscuous_enable(port_id);
 
 #ifdef RTE_LIBRTE_CMDLINE
-	if (interactive == 1)
+	if (interactive == 1) {
+		if (auto_start) {
+			printf("Start automatic packet forwarding\n");
+			start_packet_forwarding(0);
+		}
 		prompt();
-	else
+	} else
 #endif
 	{
 		char c;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a4842ec..52d3543 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -270,6 +270,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
 /* globals used for configuration */
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern uint8_t  interactive;
+extern uint8_t  auto_start;
 extern uint8_t  numa_support; /**< set by "--numa" parameter */
 extern uint16_t port_topology; /**< set by "--port-topology" parameter */
 extern uint8_t no_flush_rx; /**<set by "--no-flush-rx" parameter */
-- 
1.8.3.1

[dpdk-dev] [PATCH 3/6] test-pmd: allow command line selection of forwarding mode

[Cyril Chemparathy]

This commit allows testpmd forwarding mode selection through command line
parameters instead of having to set this up in interactive mode.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/parameters.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 18b52cc..67081d7 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,6 +158,7 @@ usage(char* progname)
 	printf("  --disable-rss: disable rss.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
+	printf("  --forward-mode=N: set forwarding mode.\n");
 	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
 	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
 	printf("  --rxq=N: set the number of RX queues per port to N.\n");
@@ -515,6 +516,7 @@ launch_args_parse(int argc, char** argv)
 		{ "enable-drop-en",            0, 0, 0 },
 		{ "disable-rss",                0, 0, 0 },
 		{ "port-topology",              1, 0, 0 },
+		{ "forward-mode",               1, 0, 0 },
 		{ "rss-ip",			0, 0, 0 },
 		{ "rss-udp",			0, 0, 0 },
 		{ "rxq",			1, 0, 0 },
@@ -775,6 +777,8 @@ launch_args_parse(int argc, char** argv)
 						 " must be: paired or chained \n",
 						 optarg);
 			}
+			if (!strcmp(lgopts[opt_idx].name, "forward-mode"))
+				set_pkt_forwarding_mode(optarg);
 			if (!strcmp(lgopts[opt_idx].name, "rss-ip"))
 				rss_hf = ETH_RSS_IPV4 | ETH_RSS_IPV6;
 			if (!strcmp(lgopts[opt_idx].name, "rss-udp"))
-- 
1.8.3.1

[dpdk-dev] [PATCH 4/6] test-pmd: allow txpkts to be setup via command line

[Cyril Chemparathy]

We allow a new --txpkts command-line parameter to configure segment sizes when
in txonly or flowgen forwarding modes.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/parameters.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 67081d7..ce90399 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -476,6 +476,52 @@ parse_ringnuma_config(const char *q_arg)
 	return 0;
 }
 
+static unsigned int
+parse_item_list(char* str, unsigned int max_items, unsigned int *parsed_items)
+{
+	unsigned int nb_item;
+	unsigned int value;
+	unsigned int i;
+	int value_ok;
+	char c;
+
+	/*
+	 * First parse all items in the list and store their value.
+	 */
+	value = 0;
+	nb_item = 0;
+	value_ok = 0;
+	for (i = 0; i < strlen(str); i++) {
+		c = str[i];
+		if ((c >= '0') && (c <= '9')) {
+			value = (unsigned int) (value * 10 + (c - '0'));
+			value_ok = 1;
+			continue;
+		}
+		if (c != ',') {
+			printf("character %c is not a decimal digit\n", c);
+			return (0);
+		}
+		if (! value_ok) {
+			printf("No valid value before comma\n");
+			return (0);
+		}
+		if (nb_item < max_items) {
+			parsed_items[nb_item] = value;
+			value_ok = 0;
+			value = 0;
+		}
+		nb_item++;
+	}
+
+	if (nb_item >= max_items)
+		rte_exit(EXIT_FAILURE, "too many txpkt segments!\n");
+
+	parsed_items[nb_item++] = value;
+
+	return (nb_item);
+}
+
 void
 launch_args_parse(int argc, char** argv)
 {
@@ -538,6 +584,7 @@ launch_args_parse(int argc, char** argv)
 		{ "tx-queue-stats-mapping",	1, 0, 0 },
 		{ "rx-queue-stats-mapping",	1, 0, 0 },
 		{ "no-flush-rx",	0, 0, 0 },
+		{ "txpkts",			1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -983,6 +1030,16 @@ launch_args_parse(int argc, char** argv)
 						 "invalid RX queue statistics mapping config entered\n");
 				}
 			}
+			if (!strcmp(lgopts[opt_idx].name, "txpkts")) {
+				unsigned seg_lengths[RTE_MAX_SEGS_PER_PKT];
+				unsigned int nb_segs;
+
+				nb_segs = parse_item_list(optarg, RTE_MAX_SEGS_PER_PKT, seg_lengths);
+				if (nb_segs > 0)
+					set_tx_pkt_segments(seg_lengths, nb_segs);
+				else
+					rte_exit(EXIT_FAILURE, "bad txpkts\n");
+			}
 			if (!strcmp(lgopts[opt_idx].name, "no-flush-rx"))
 				no_flush_rx = 1;
 
-- 
1.8.3.1

[dpdk-dev] [PATCH 5/6] test-pmd: add mac swap forwarding mode

[Cyril Chemparathy]

This commit adds a new forwarding mode, in which the source and destination
MAC addresses of packets are swapped before forwarding according to the
port-topology in effect.  This has been designed for operation primarily with
--port-topology=loopback.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/Makefile  |   1 +
 app/test-pmd/macswap.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.c |   1 +
 app/test-pmd/testpmd.h |   1 +
 4 files changed, 151 insertions(+)
 create mode 100644 app/test-pmd/macswap.c

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index f0d4b4d..f0d0262 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -51,6 +51,7 @@ SRCS-$(CONFIG_RTE_TEST_PMD) += config.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += iofwd.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += macfwd.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += macfwd-retry.c
+SRCS-$(CONFIG_RTE_TEST_PMD) += macswap.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += rxonly.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += txonly.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += csumonly.c
diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
new file mode 100644
index 0000000..4ed6096
--- /dev/null
+++ b/app/test-pmd/macswap.c
@@ -0,0 +1,148 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2014 Tilera Corporation. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Tilera Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <stdarg.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <inttypes.h>
+
+#include <sys/queue.h>
+#include <sys/stat.h>
+
+#include <rte_common.h>
+#include <rte_byteorder.h>
+#include <rte_log.h>
+#include <rte_debug.h>
+#include <rte_cycles.h>
+#include <rte_memory.h>
+#include <rte_memcpy.h>
+#include <rte_memzone.h>
+#include <rte_launch.h>
+#include <rte_tailq.h>
+#include <rte_eal.h>
+#include <rte_per_lcore.h>
+#include <rte_lcore.h>
+#include <rte_atomic.h>
+#include <rte_branch_prediction.h>
+#include <rte_ring.h>
+#include <rte_memory.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_interrupts.h>
+#include <rte_pci.h>
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_ip.h>
+#include <rte_string_fns.h>
+
+#include "testpmd.h"
+
+/*
+ * MAC swap forwarding mode: Swap the source and the destination Ethernet
+ * addresses of packets before forwarding them.
+ */
+static void
+pkt_burst_mac_swap(struct fwd_stream *fs)
+{
+	struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
+	struct rte_port  *txp;
+	struct rte_mbuf  *mb;
+	struct ether_hdr *eth_hdr;
+	struct ether_addr addr;
+	uint16_t nb_rx;
+	uint16_t nb_tx;
+	uint16_t i;
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	uint64_t start_tsc;
+	uint64_t end_tsc;
+	uint64_t core_cycles;
+#endif
+
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	start_tsc = rte_rdtsc();
+#endif
+
+	/*
+	 * 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;
+
+#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
+	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
+#endif
+	fs->rx_packets += nb_rx;
+	txp = &ports[fs->tx_port];
+	for (i = 0; i < nb_rx; i++) {
+		mb = pkts_burst[i];
+		eth_hdr = (struct ether_hdr *) mb->pkt.data;
+
+		/* Swap dest and src mac addresses. */
+		ether_addr_copy(&eth_hdr->d_addr, &addr);
+		ether_addr_copy(&eth_hdr->s_addr, &eth_hdr->d_addr);
+		ether_addr_copy(&addr, &eth_hdr->s_addr);
+
+		mb->ol_flags = txp->tx_ol_flags;
+		mb->pkt.vlan_macip.f.l2_len = sizeof(struct ether_hdr);
+		mb->pkt.vlan_macip.f.l3_len = sizeof(struct ipv4_hdr);
+		mb->pkt.vlan_macip.f.vlan_tci = txp->tx_vlan_id;
+	}
+	nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
+	fs->tx_packets += nb_tx;
+#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
+	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
+#endif
+	if (unlikely(nb_tx < nb_rx)) {
+		fs->fwd_dropped += (nb_rx - nb_tx);
+		do {
+			rte_pktmbuf_free(pkts_burst[nb_tx]);
+		} while (++nb_tx < nb_rx);
+	}
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	end_tsc = rte_rdtsc();
+	core_cycles = (end_tsc - start_tsc);
+	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
+#endif
+}
+
+struct fwd_engine mac_swap_engine = {
+	.fwd_mode_name  = "macswap",
+	.port_fwd_begin = NULL,
+	.port_fwd_end   = NULL,
+	.packet_fwd     = pkt_burst_mac_swap,
+};
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index cde9c2b..c75ae8e 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -145,6 +145,7 @@ struct fwd_engine * fwd_engines[] = {
 	&io_fwd_engine,
 	&mac_fwd_engine,
 	&mac_retry_fwd_engine,
+	&mac_swap_engine,
 	&rx_only_engine,
 	&tx_only_engine,
 	&csum_fwd_engine,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 52d3543..1b9ca65 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -200,6 +200,7 @@ struct fwd_engine {
 extern struct fwd_engine io_fwd_engine;
 extern struct fwd_engine mac_fwd_engine;
 extern struct fwd_engine mac_retry_fwd_engine;
+extern struct fwd_engine mac_swap_engine;
 extern struct fwd_engine rx_only_engine;
 extern struct fwd_engine tx_only_engine;
 extern struct fwd_engine csum_fwd_engine;
-- 
1.8.3.1

[dpdk-dev] [PATCH 6/6] test-pmd: add flowgen forwarding engine

[Cyril Chemparathy]

This commit adds a multi-flow traffic generator forwarding engine.  In effect,
this forwarding mode functions very similar to the txonly mode, with the
difference that it generates multiple L4 flows.

Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
---
 app/test-pmd/Makefile  |   1 +
 app/test-pmd/flowgen.c | 247 +++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.c |   1 +
 app/test-pmd/testpmd.h |   1 +
 4 files changed, 250 insertions(+)
 create mode 100644 app/test-pmd/flowgen.c

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index f0d0262..6ed2634 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -52,6 +52,7 @@ SRCS-$(CONFIG_RTE_TEST_PMD) += iofwd.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += macfwd.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += macfwd-retry.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += macswap.c
+SRCS-$(CONFIG_RTE_TEST_PMD) += flowgen.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += rxonly.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += txonly.c
 SRCS-$(CONFIG_RTE_TEST_PMD) += csumonly.c
diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
new file mode 100644
index 0000000..04911ff
--- /dev/null
+++ b/app/test-pmd/flowgen.c
@@ -0,0 +1,247 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2013 Tilera Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Tilera Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <stdarg.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <inttypes.h>
+
+#include <sys/queue.h>
+#include <sys/stat.h>
+
+#include <rte_common.h>
+#include <rte_byteorder.h>
+#include <rte_log.h>
+#include <rte_debug.h>
+#include <rte_cycles.h>
+#include <rte_memory.h>
+#include <rte_memcpy.h>
+#include <rte_memzone.h>
+#include <rte_launch.h>
+#include <rte_tailq.h>
+#include <rte_eal.h>
+#include <rte_per_lcore.h>
+#include <rte_lcore.h>
+#include <rte_atomic.h>
+#include <rte_branch_prediction.h>
+#include <rte_ring.h>
+#include <rte_memory.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_interrupts.h>
+#include <rte_pci.h>
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_ip.h>
+#include <rte_tcp.h>
+#include <rte_udp.h>
+#include <rte_string_fns.h>
+
+#include "testpmd.h"
+
+/* hardcoded configuration (for now) */
+static unsigned cfg_n_flows	= 1024;
+static unsigned cfg_pkt_size	= 300;
+static uint32_t cfg_ip_src	= IPv4(10, 254, 0, 0);
+static uint32_t cfg_ip_dst	= IPv4(10, 253, 0, 0);
+static uint16_t cfg_udp_src	= 1000;
+static uint16_t cfg_udp_dst	= 1001;
+static struct ether_addr cfg_ether_src	=
+	{{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x00 }};
+static struct ether_addr cfg_ether_dst	=
+	{{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x01 }};
+
+#define IP_DEFTTL  64   /* from RFC 1340. */
+#define IP_VERSION 0x40
+#define IP_HDRLEN  0x05 /* default IP header length == five 32-bits words. */
+#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
+
+static inline struct rte_mbuf *
+tx_mbuf_alloc(struct rte_mempool *mp)
+{
+	struct rte_mbuf *m;
+
+	m = __rte_mbuf_raw_alloc(mp);
+	__rte_mbuf_sanity_check_raw(m, RTE_MBUF_PKT, 0);
+	return (m);
+}
+
+
+static inline uint16_t
+ip_sum(const uint16_t *hdr, int hdr_len)
+{
+	uint32_t sum = 0;
+
+	while (hdr_len > 1)
+	{
+		sum += *hdr++;
+		if (sum & 0x80000000)
+			sum = (sum & 0xFFFF) + (sum >> 16);
+		hdr_len -= 2;
+	}
+
+	while (sum >> 16)
+		sum = (sum & 0xFFFF) + (sum >> 16);
+
+	return ~sum;
+}
+
+/*
+ * Multi-flow generation mode.
+ *
+ * We originate a bunch of flows (varying destination IP addresses), and
+ * terminate receive traffic.  Received traffic is simply discarded, but we
+ * still do so in order to maintain traffic statistics.
+ */
+static void
+pkt_burst_flow_gen(struct fwd_stream *fs)
+{
+	unsigned pkt_size = cfg_pkt_size - 4;	/* Adjust FCS */
+	struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
+	struct rte_mempool *mbp;
+	struct rte_mbuf  *pkt;
+	struct ether_hdr *eth_hdr;
+	struct ipv4_hdr *ip_hdr;
+	struct udp_hdr *udp_hdr;
+	uint16_t vlan_tci;
+	uint16_t ol_flags;
+	uint16_t nb_rx;
+	uint16_t nb_tx;
+	uint16_t nb_pkt;
+	uint16_t i;
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	uint64_t start_tsc;
+	uint64_t end_tsc;
+	uint64_t core_cycles;
+#endif
+	static int next_flow = 0;
+
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	start_tsc = rte_rdtsc();
+#endif
+
+	/* Receive a burst of packets and discard them. */
+	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
+				 nb_pkt_per_burst);
+	fs->rx_packets += nb_rx;
+
+	for (i = 0; i < nb_rx; i++)
+		rte_pktmbuf_free(pkts_burst[i]);
+
+	mbp = current_fwd_lcore()->mbp;
+	vlan_tci = ports[fs->tx_port].tx_vlan_id;
+	ol_flags = ports[fs->tx_port].tx_ol_flags;
+
+	for (nb_pkt = 0; nb_pkt < nb_pkt_per_burst; nb_pkt++) {
+		pkt = tx_mbuf_alloc(mbp);
+		if (!pkt)
+			break;
+
+		pkt->pkt.data_len = pkt_size;
+		pkt->pkt.next = NULL;
+
+		/* Initialize Ethernet header. */
+		eth_hdr = (struct ether_hdr *)pkt->pkt.data;
+		ether_addr_copy(&cfg_ether_dst, &eth_hdr->d_addr);
+		ether_addr_copy(&cfg_ether_src, &eth_hdr->s_addr);
+		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+
+		/* Initialize IP header. */
+		ip_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
+		memset(ip_hdr, 0, sizeof(*ip_hdr));
+		ip_hdr->version_ihl	= IP_VHL_DEF;
+		ip_hdr->type_of_service	= 0;
+		ip_hdr->fragment_offset	= 0;
+		ip_hdr->time_to_live	= IP_DEFTTL;
+		ip_hdr->next_proto_id	= IPPROTO_UDP;
+		ip_hdr->packet_id	= 0;
+		ip_hdr->src_addr	= rte_cpu_to_be_32(cfg_ip_src);
+		ip_hdr->dst_addr	= rte_cpu_to_be_32(cfg_ip_dst +
+							   next_flow);
+		ip_hdr->total_length	= RTE_CPU_TO_BE_16(pkt_size -
+							   sizeof(*eth_hdr));
+		ip_hdr->hdr_checksum	= ip_sum((uint16_t *)ip_hdr,
+						 sizeof(*ip_hdr));
+
+		/* Initialize UDP header. */
+		udp_hdr = (struct udp_hdr *)(ip_hdr + 1);
+		udp_hdr->src_port	= rte_cpu_to_be_16(cfg_udp_src);
+		udp_hdr->dst_port	= rte_cpu_to_be_16(cfg_udp_dst);
+		udp_hdr->dgram_cksum	= 0; /* No UDP checksum. */
+		udp_hdr->dgram_len	= RTE_CPU_TO_BE_16(pkt_size -
+							   sizeof(*eth_hdr) -
+							   sizeof(*ip_hdr));
+		pkt->pkt.nb_segs		= 1;
+		pkt->pkt.pkt_len		= pkt_size;
+		pkt->ol_flags			= ol_flags;
+		pkt->pkt.vlan_macip.f.vlan_tci	= vlan_tci;
+		pkt->pkt.vlan_macip.f.l2_len	= sizeof(struct ether_hdr);
+		pkt->pkt.vlan_macip.f.l3_len	= sizeof(struct ipv4_hdr);
+		pkts_burst[nb_pkt]		= pkt;
+
+		next_flow = (next_flow + 1) % cfg_n_flows;
+	}
+
+	nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
+	fs->tx_packets += nb_tx;
+
+#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
+	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
+#endif
+	if (unlikely(nb_tx < nb_pkt)) {
+		/* Back out the flow counter. */
+		next_flow -= (nb_pkt - nb_tx);
+		while (next_flow < 0)
+			next_flow += cfg_n_flows;
+
+		do {
+			rte_pktmbuf_free(pkts_burst[nb_tx]);
+		} while (++nb_tx < nb_pkt);
+	}
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	end_tsc = rte_rdtsc();
+	core_cycles = (end_tsc - start_tsc);
+	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
+#endif
+}
+
+struct fwd_engine flow_gen_engine = {
+	.fwd_mode_name  = "flowgen",
+	.port_fwd_begin = NULL,
+	.port_fwd_end   = NULL,
+	.packet_fwd     = pkt_burst_flow_gen,
+};
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c75ae8e..0aa0afa 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -146,6 +146,7 @@ struct fwd_engine * fwd_engines[] = {
 	&mac_fwd_engine,
 	&mac_retry_fwd_engine,
 	&mac_swap_engine,
+	&flow_gen_engine,
 	&rx_only_engine,
 	&tx_only_engine,
 	&csum_fwd_engine,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 1b9ca65..3f0ecfc 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -201,6 +201,7 @@ extern struct fwd_engine io_fwd_engine;
 extern struct fwd_engine mac_fwd_engine;
 extern struct fwd_engine mac_retry_fwd_engine;
 extern struct fwd_engine mac_swap_engine;
+extern struct fwd_engine flow_gen_engine;
 extern struct fwd_engine rx_only_engine;
 extern struct fwd_engine tx_only_engine;
 extern struct fwd_engine csum_fwd_engine;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 3/6] test-pmd: allow command line selection of forwarding mode

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cyril Chemparathy
> Sent: Thursday, April 03, 2014 6:30 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 3/6] test-pmd: allow command line selection of forwarding mode
>
> This commit allows testpmd forwarding mode selection through command line parameters instead of having to set this up in 
> interactive mode.
>
> Signed-off-by: Cyril Chemparathy <cchemparathy@tilera.com>
> ---
> app/test-pmd/parameters.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index 18b52cc..67081d7 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -158,6 +158,7 @@ usage(char* progname)
> 	printf("  --disable-rss: disable rss.\n");
>  	printf("  --port-topology=N: set port topology (N: paired (default) or "
> 	       "chained).\n");
> +	printf("  --forward-mode=N: set forwarding mode.\n");

The help text should probably include a list of what the modes are and what values need to be provided for each one.

[dpdk-dev] [PATCH] app/testpmd: list forwarding modes

[Thomas Monjalon]

Having a function to list forwarding modes helps to show them
in cli help and in parameters usage witout duplicating code.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test-pmd/cmdline.c    | 13 +++----------
 app/test-pmd/config.c     | 20 ++++++++++++++++++++
 app/test-pmd/parameters.c |  3 ++-
 app/test-pmd/testpmd.h    |  1 +
 4 files changed, 26 insertions(+), 11 deletions(-)

This patch is based on top of "test-pmd: allow command line selection of forwarding mode".
It fixes issue raised by Bruce:
	http://dpdk.org/ml/archives/dev/2014-April/001809.html

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7f59333..c81d69a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -294,17 +294,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"        bit 3 - insert sctp checksum offload if set\n"
 			"    Please check the NIC datasheet for HW limits.\n\n"
 
-#ifdef RTE_LIBRTE_IEEE1588
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum|ieee1588)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY or ieee1588"
-			" packet forwarding mode.\n\n"
-
-#else
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY packet"
-			" forwarding mode.\n\n"
+			"set fwd (%s)\n"
+			"    Set packet forwarding mode.\n\n"
 
-#endif
 			"mac_addr add (port_id) (XX:XX:XX:XX:XX:XX)\n"
 			"    Add a MAC address on port_id.\n\n"
 
@@ -398,6 +390,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" using the lowest port on the NIC.\n\n"
 #endif
 
+			, list_pkt_forwarding_modes()
 		);
 	}
 
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index d0e21b7..8f08cc2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1231,6 +1231,26 @@ set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
 	tx_pkt_nb_segs = (uint8_t) nb_segs;
 }
 
+char*
+list_pkt_forwarding_modes(void)
+{
+	static char modes[128] = "";
+	struct fwd_engine *fwd_eng;
+	unsigned i;
+
+	if (strlen (modes) == 0) {
+		i = 0;
+		while ((fwd_eng = fwd_engines[i]) != NULL) {
+			strcat(modes, fwd_eng->fwd_mode_name);
+			strcat(modes, "|");
+			i++;
+		}
+		modes[strlen(modes) - 1] = '\0';
+	}
+
+	return modes;
+}
+
 void
 set_pkt_forwarding_mode(const char *fwd_mode_name)
 {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 67081d7..8a3ce2c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,7 +158,8 @@ usage(char* progname)
 	printf("  --disable-rss: disable rss.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
-	printf("  --forward-mode=N: set forwarding mode.\n");
+	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
+	       list_pkt_forwarding_modes());
 	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
 	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
 	printf("  --rxq=N: set the number of RX queues per port to N.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 52d3543..4afa793 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -491,6 +491,7 @@ void tx_cksum_set(portid_t port_id, uint8_t cksum_mask);
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void set_nb_pkt_per_burst(uint16_t pkt_burst);
+char* list_pkt_forwarding_modes(void);
 void set_pkt_forwarding_mode(const char *fwd_mode);
 void start_packet_forwarding(int with_tx_first);
 void stop_packet_forwarding(void);
-- 
1.9.2

[dpdk-dev] [PATCH v2] app/testpmd: list forwarding modes

[Thomas Monjalon]

Having a function to list forwarding modes helps to show them
in cli help and in parameters usage witout duplicating code.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test-pmd/cmdline.c    | 55 ++++++++++++++++++++++++++++-------------------
 app/test-pmd/config.c     | 19 ++++++++++++++++
 app/test-pmd/parameters.c |  3 ++-
 app/test-pmd/testpmd.h    |  1 +
 4 files changed, 55 insertions(+), 23 deletions(-)

changes in v2:
- factorize also contextual help

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7f59333..12a09d0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -294,17 +294,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"        bit 3 - insert sctp checksum offload if set\n"
 			"    Please check the NIC datasheet for HW limits.\n\n"
 
-#ifdef RTE_LIBRTE_IEEE1588
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum|ieee1588)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY or ieee1588"
-			" packet forwarding mode.\n\n"
+			"set fwd (%s)\n"
+			"    Set packet forwarding mode.\n\n"
 
-#else
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY packet"
-			" forwarding mode.\n\n"
-
-#endif
 			"mac_addr add (port_id) (XX:XX:XX:XX:XX:XX)\n"
 			"    Add a MAC address on port_id.\n\n"
 
@@ -398,6 +390,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" using the lowest port on the NIC.\n\n"
 #endif
 
+			, list_pkt_forwarding_modes()
 		);
 	}
 
@@ -2688,22 +2681,12 @@ cmdline_parse_token_string_t cmd_setfwd_fwd =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, fwd, "fwd");
 cmdline_parse_token_string_t cmd_setfwd_mode =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, mode,
-#ifdef RTE_LIBRTE_IEEE1588
-				 "io#mac#mac_retry#rxonly#txonly#csum#ieee1588");
-#else
-				 "io#mac#mac_retry#rxonly#txonly#csum");
-#endif
+		"" /* defined at init */);
 
 cmdline_parse_inst_t cmd_set_fwd_mode = {
 	.f = cmd_set_fwd_mode_parsed,
 	.data = NULL,
-#ifdef RTE_LIBRTE_IEEE1588
-	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum|ieee1588 - set IO, MAC,"
-	" MAC_RETRY, RXONLY, TXONLY, CSUM or IEEE1588 packet forwarding mode",
-#else
-	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum - set IO, MAC,"
-	" MAC_RETRY, RXONLY, CSUM or TXONLY packet forwarding mode",
-#endif
+	.help_str = NULL, /* defined at init */
 	.tokens = {
 		(void *)&cmd_setfwd_set,
 		(void *)&cmd_setfwd_fwd,
@@ -2712,6 +2695,26 @@ cmdline_parse_inst_t cmd_set_fwd_mode = {
 	},
 };
 
+static void cmd_set_fwd_mode_init(cmdline_parse_inst_t *cmd)
+{
+	char *modes, *c;
+	static char token[128];
+	static char help[256];
+
+	modes = list_pkt_forwarding_modes();
+	rte_snprintf(help, sizeof help, "set fwd %s - "
+		"set packet forwarding mode", modes);
+	cmd->help_str = help;
+
+	/* string token separator is # */
+	for (c = token; *modes != '\0'; modes++)
+		if (*modes == '|')
+			*c++ = '#';
+		else
+			*c++ = *modes;
+	((cmdline_parse_token_string_t *)cmd->tokens[2])->string_data.str = token;
+}
+
 /* *** SET BURST TX DELAY TIME RETRY NUMBER *** */
 struct cmd_set_burst_tx_retry_result {
 	cmdline_fixed_string_t set;
@@ -5197,6 +5200,14 @@ void
 prompt(void)
 {
 	struct cmdline *cl;
+	unsigned i = 0;
+	cmdline_parse_inst_t *cmd;
+
+	/* initialize non-constant commands */
+	while ((cmd = main_ctx[i++]) != NULL) {
+		if (cmd->f == cmd_set_fwd_mode_parsed)
+			cmd_set_fwd_mode_init(cmd);
+	}
 
 	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
 	if (cl == NULL) {
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index d0e21b7..2e4b7a5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1231,6 +1231,25 @@ set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
 	tx_pkt_nb_segs = (uint8_t) nb_segs;
 }
 
+char*
+list_pkt_forwarding_modes(void)
+{
+	static char modes[128] = "";
+	const char *separator = "|";
+	struct fwd_engine *fwd_eng;
+	unsigned i = 0;
+
+	if (strlen (modes) == 0) {
+		while ((fwd_eng = fwd_engines[i++]) != NULL) {
+			strcat(modes, fwd_eng->fwd_mode_name);
+			strcat(modes, separator);
+		}
+		modes[strlen(modes) - strlen(separator)] = '\0';
+	}
+
+	return modes;
+}
+
 void
 set_pkt_forwarding_mode(const char *fwd_mode_name)
 {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 67081d7..8a3ce2c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,7 +158,8 @@ usage(char* progname)
 	printf("  --disable-rss: disable rss.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
-	printf("  --forward-mode=N: set forwarding mode.\n");
+	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
+	       list_pkt_forwarding_modes());
 	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
 	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
 	printf("  --rxq=N: set the number of RX queues per port to N.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 52d3543..0e4a35e 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -491,6 +491,7 @@ void tx_cksum_set(portid_t port_id, uint8_t cksum_mask);
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void set_nb_pkt_per_burst(uint16_t pkt_burst);
+char *list_pkt_forwarding_modes(void);
 void set_pkt_forwarding_mode(const char *fwd_mode);
 void start_packet_forwarding(int with_tx_first);
 void stop_packet_forwarding(void);
-- 
1.9.2

[dpdk-dev] [PATCH v3] app/testpmd: list forwarding engines

[Thomas Monjalon]

Having a function to list forwarding engines helps to show them
in cli help and in parameters usage witout duplicating code.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test-pmd/cmdline.c    | 52 +++++++++++++++++++++++++++--------------------
 app/test-pmd/config.c     | 19 +++++++++++++++++
 app/test-pmd/parameters.c |  3 ++-
 app/test-pmd/testpmd.h    |  1 +
 4 files changed, 52 insertions(+), 23 deletions(-)

changes in v2:
- factorize also contextual help
changes in v3:
- remove useless loop

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7becedc..c6a417a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -294,17 +294,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"        bit 3 - insert sctp checksum offload if set\n"
 			"    Please check the NIC datasheet for HW limits.\n\n"
 
-#ifdef RTE_LIBRTE_IEEE1588
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum|ieee1588)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY or ieee1588"
-			" packet forwarding mode.\n\n"
+			"set fwd (%s)\n"
+			"    Set packet forwarding mode.\n\n"
 
-#else
-			"set fwd (io|mac|mac_retry|rxonly|txonly|csum)\n"
-			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY packet"
-			" forwarding mode.\n\n"
-
-#endif
 			"mac_addr add (port_id) (XX:XX:XX:XX:XX:XX)\n"
 			"    Add a MAC address on port_id.\n\n"
 
@@ -398,6 +390,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" using the lowest port on the NIC.\n\n"
 #endif
 
+			, list_pkt_forwarding_modes()
 		);
 	}
 
@@ -2688,22 +2681,12 @@ cmdline_parse_token_string_t cmd_setfwd_fwd =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, fwd, "fwd");
 cmdline_parse_token_string_t cmd_setfwd_mode =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, mode,
-#ifdef RTE_LIBRTE_IEEE1588
-				 "io#mac#mac_retry#rxonly#txonly#csum#ieee1588");
-#else
-				 "io#mac#mac_retry#rxonly#txonly#csum");
-#endif
+		"" /* defined at init */);
 
 cmdline_parse_inst_t cmd_set_fwd_mode = {
 	.f = cmd_set_fwd_mode_parsed,
 	.data = NULL,
-#ifdef RTE_LIBRTE_IEEE1588
-	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum|ieee1588 - set IO, MAC,"
-	" MAC_RETRY, RXONLY, TXONLY, CSUM or IEEE1588 packet forwarding mode",
-#else
-	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum - set IO, MAC,"
-	" MAC_RETRY, RXONLY, CSUM or TXONLY packet forwarding mode",
-#endif
+	.help_str = NULL, /* defined at init */
 	.tokens = {
 		(void *)&cmd_setfwd_set,
 		(void *)&cmd_setfwd_fwd,
@@ -2712,6 +2695,28 @@ cmdline_parse_inst_t cmd_set_fwd_mode = {
 	},
 };
 
+static void cmd_set_fwd_mode_init(void)
+{
+	char *modes, *c;
+	static char token[128];
+	static char help[256];
+	cmdline_parse_token_string_t *token_struct;
+
+	modes = list_pkt_forwarding_modes();
+	rte_snprintf(help, sizeof help, "set fwd %s - "
+		"set packet forwarding mode", modes);
+	cmd_set_fwd_mode.help_str = help;
+
+	/* string token separator is # */
+	for (c = token; *modes != '\0'; modes++)
+		if (*modes == '|')
+			*c++ = '#';
+		else
+			*c++ = *modes;
+	token_struct = (cmdline_parse_token_string_t*)cmd_set_fwd_mode.tokens[2];
+	token_struct->string_data.str = token;
+}
+
 /* *** SET BURST TX DELAY TIME RETRY NUMBER *** */
 struct cmd_set_burst_tx_retry_result {
 	cmdline_fixed_string_t set;
@@ -5198,6 +5203,9 @@ prompt(void)
 {
 	struct cmdline *cl;
 
+	/* initialize non-constant commands */
+	cmd_set_fwd_mode_init();
+
 	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
 	if (cl == NULL) {
 		return;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index d0e21b7..41326fe 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1231,6 +1231,25 @@ set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
 	tx_pkt_nb_segs = (uint8_t) nb_segs;
 }
 
+char*
+list_pkt_forwarding_modes(void)
+{
+	static char fwd_modes[128] = "";
+	const char *separator = "|";
+	struct fwd_engine *fwd_eng;
+	unsigned i = 0;
+
+	if (strlen (fwd_modes) == 0) {
+		while ((fwd_eng = fwd_engines[i++]) != NULL) {
+			strcat(fwd_modes, fwd_eng->fwd_mode_name);
+			strcat(fwd_modes, separator);
+		}
+		fwd_modes[strlen(fwd_modes) - strlen(separator)] = '\0';
+	}
+
+	return fwd_modes;
+}
+
 void
 set_pkt_forwarding_mode(const char *fwd_mode_name)
 {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 67081d7..8a3ce2c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,7 +158,8 @@ usage(char* progname)
 	printf("  --disable-rss: disable rss.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
-	printf("  --forward-mode=N: set forwarding mode.\n");
+	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
+	       list_pkt_forwarding_modes());
 	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
 	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
 	printf("  --rxq=N: set the number of RX queues per port to N.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 52d3543..0e4a35e 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -491,6 +491,7 @@ void tx_cksum_set(portid_t port_id, uint8_t cksum_mask);
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void set_nb_pkt_per_burst(uint16_t pkt_burst);
+char *list_pkt_forwarding_modes(void);
 void set_pkt_forwarding_mode(const char *fwd_mode);
 void start_packet_forwarding(int with_tx_first);
 void stop_packet_forwarding(void);
-- 
1.9.2

Re: [dpdk-dev] [PATCH v3] app/testpmd: list forwarding engines

[Ivan Boule]

On 05/15/2014 06:08 PM, Thomas Monjalon wrote:
> Having a function to list forwarding engines helps to show them
> in cli help and in parameters usage witout duplicating code.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>   app/test-pmd/cmdline.c    | 52 +++++++++++++++++++++++++++--------------------
>   app/test-pmd/config.c     | 19 +++++++++++++++++
>   app/test-pmd/parameters.c |  3 ++-
>   app/test-pmd/testpmd.h    |  1 +
>   4 files changed, 52 insertions(+), 23 deletions(-)
>
> changes in v2:
> - factorize also contextual help
> changes in v3:
> - remove useless loop
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 7becedc..c6a417a 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -294,17 +294,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>   			"        bit 3 - insert sctp checksum offload if set\n"
>   			"    Please check the NIC datasheet for HW limits.\n\n"
>   
> -#ifdef RTE_LIBRTE_IEEE1588
> -			"set fwd (io|mac|mac_retry|rxonly|txonly|csum|ieee1588)\n"
> -			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY or ieee1588"
> -			" packet forwarding mode.\n\n"
> +			"set fwd (%s)\n"
> +			"    Set packet forwarding mode.\n\n"
>   
> -#else
> -			"set fwd (io|mac|mac_retry|rxonly|txonly|csum)\n"
> -			"    Set IO, MAC, MAC_RETRY, RXONLY, CSUM or TXONLY packet"
> -			" forwarding mode.\n\n"
> -
> -#endif
>   			"mac_addr add (port_id) (XX:XX:XX:XX:XX:XX)\n"
>   			"    Add a MAC address on port_id.\n\n"
>   
> @@ -398,6 +390,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>   			" using the lowest port on the NIC.\n\n"
>   #endif
>   
> +			, list_pkt_forwarding_modes()
>   		);
>   	}
>   
> @@ -2688,22 +2681,12 @@ cmdline_parse_token_string_t cmd_setfwd_fwd =
>   	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, fwd, "fwd");
>   cmdline_parse_token_string_t cmd_setfwd_mode =
>   	TOKEN_STRING_INITIALIZER(struct cmd_set_fwd_mode_result, mode,
> -#ifdef RTE_LIBRTE_IEEE1588
> -				 "io#mac#mac_retry#rxonly#txonly#csum#ieee1588");
> -#else
> -				 "io#mac#mac_retry#rxonly#txonly#csum");
> -#endif
> +		"" /* defined at init */);
>   
>   cmdline_parse_inst_t cmd_set_fwd_mode = {
>   	.f = cmd_set_fwd_mode_parsed,
>   	.data = NULL,
> -#ifdef RTE_LIBRTE_IEEE1588
> -	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum|ieee1588 - set IO, MAC,"
> -	" MAC_RETRY, RXONLY, TXONLY, CSUM or IEEE1588 packet forwarding mode",
> -#else
> -	.help_str = "set fwd io|mac|mac_retry|rxonly|txonly|csum - set IO, MAC,"
> -	" MAC_RETRY, RXONLY, CSUM or TXONLY packet forwarding mode",
> -#endif
> +	.help_str = NULL, /* defined at init */
>   	.tokens = {
>   		(void *)&cmd_setfwd_set,
>   		(void *)&cmd_setfwd_fwd,
> @@ -2712,6 +2695,28 @@ cmdline_parse_inst_t cmd_set_fwd_mode = {
>   	},
>   };
>   
> +static void cmd_set_fwd_mode_init(void)
> +{
> +	char *modes, *c;
> +	static char token[128];
> +	static char help[256];
> +	cmdline_parse_token_string_t *token_struct;
> +
> +	modes = list_pkt_forwarding_modes();
> +	rte_snprintf(help, sizeof help, "set fwd %s - "
> +		"set packet forwarding mode", modes);
> +	cmd_set_fwd_mode.help_str = help;
> +
> +	/* string token separator is # */
> +	for (c = token; *modes != '\0'; modes++)
> +		if (*modes == '|')
> +			*c++ = '#';
> +		else
> +			*c++ = *modes;
> +	token_struct = (cmdline_parse_token_string_t*)cmd_set_fwd_mode.tokens[2];
> +	token_struct->string_data.str = token;
> +}
> +
>   /* *** SET BURST TX DELAY TIME RETRY NUMBER *** */
>   struct cmd_set_burst_tx_retry_result {
>   	cmdline_fixed_string_t set;
> @@ -5198,6 +5203,9 @@ prompt(void)
>   {
>   	struct cmdline *cl;
>   
> +	/* initialize non-constant commands */
> +	cmd_set_fwd_mode_init();
> +
>   	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
>   	if (cl == NULL) {
>   		return;
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index d0e21b7..41326fe 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1231,6 +1231,25 @@ set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
>   	tx_pkt_nb_segs = (uint8_t) nb_segs;
>   }
>   
> +char*
> +list_pkt_forwarding_modes(void)
> +{
> +	static char fwd_modes[128] = "";
> +	const char *separator = "|";
> +	struct fwd_engine *fwd_eng;
> +	unsigned i = 0;
> +
> +	if (strlen (fwd_modes) == 0) {
> +		while ((fwd_eng = fwd_engines[i++]) != NULL) {
> +			strcat(fwd_modes, fwd_eng->fwd_mode_name);
> +			strcat(fwd_modes, separator);
> +		}
> +		fwd_modes[strlen(fwd_modes) - strlen(separator)] = '\0';
> +	}
> +
> +	return fwd_modes;
> +}
> +
>   void
>   set_pkt_forwarding_mode(const char *fwd_mode_name)
>   {
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 67081d7..8a3ce2c 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -158,7 +158,8 @@ usage(char* progname)
>   	printf("  --disable-rss: disable rss.\n");
>   	printf("  --port-topology=N: set port topology (N: paired (default) or "
>   	       "chained).\n");
> -	printf("  --forward-mode=N: set forwarding mode.\n");
> +	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
> +	       list_pkt_forwarding_modes());
>   	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
>   	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
>   	printf("  --rxq=N: set the number of RX queues per port to N.\n");
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 52d3543..0e4a35e 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -491,6 +491,7 @@ void tx_cksum_set(portid_t port_id, uint8_t cksum_mask);
>   void set_verbose_level(uint16_t vb_level);
>   void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
>   void set_nb_pkt_per_burst(uint16_t pkt_burst);
> +char *list_pkt_forwarding_modes(void);
>   void set_pkt_forwarding_mode(const char *fwd_mode);
>   void start_packet_forwarding(int with_tx_first);
>   void stop_packet_forwarding(void);

Acked by: Ivan Boule<ivan.boule@6wind.com>

-- 
Ivan Boule
6WIND Development Engineer

Re: [dpdk-dev] [PATCH 0/6] Extensions to test-pmd

[Thomas Monjalon]

Hi Cyril,

2014-04-03 10:30, Cyril Chemparathy:
> This patch series contains a few minor extensions to test-pmd.  These
> changes have been added primarily for convenience while testing out various
> scenarios with DPDK.
> 
> Cyril Chemparathy (6):
>   test-pmd: add support for single port loopback topology
>   test-pmd: add support for auto-start when interactive
>   test-pmd: allow command line selection of forwarding mode
>   test-pmd: allow txpkts to be setup via command line
>   test-pmd: add mac swap forwarding mode
>   test-pmd: add flowgen forwarding engine

Thanks for these new features.

This is not the first time a new engine is added by copy/pasting the most part 
of an existing engine. For instance, the "mac-retry" engine was added by Intel 
as a copy/paste of the original "mac" one.
This is acceptable but not the perfect way to implement engines.
To address this issue, a new engine function could be introduced to setup some 
parameters to be used by "packet_fwd" function. This way, similar engines 
could be removed.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied for version 1.7.0.
-- 
Thomas

Re: [dpdk-dev] [PATCH v3] app/testpmd: list forwarding engines

[Thomas Monjalon]

> > Having a function to list forwarding engines helps to show them
> > in cli help and in parameters usage witout duplicating code.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Acked by: Ivan Boule<ivan.boule@6wind.com>

Applied for version 1.7.0.
-- 
Thomas

Re: [dpdk-dev] [PATCH 0/6] Extensions to test-pmd

[Cyril Chemparathy]

On 5/16/2014 7:22 AM, Thomas Monjalon wrote:
> Hi Cyril,
>
> 2014-04-03 10:30, Cyril Chemparathy:
>> This patch series contains a few minor extensions to test-pmd.  These
>> changes have been added primarily for convenience while testing out various
>> scenarios with DPDK.
>>
>> Cyril Chemparathy (6):
>>    test-pmd: add support for single port loopback topology
>>    test-pmd: add support for auto-start when interactive
>>    test-pmd: allow command line selection of forwarding mode
>>    test-pmd: allow txpkts to be setup via command line
>>    test-pmd: add mac swap forwarding mode
>>    test-pmd: add flowgen forwarding engine
> Thanks for these new features.
>
> This is not the first time a new engine is added by copy/pasting the most part
> of an existing engine. For instance, the "mac-retry" engine was added by Intel
> as a copy/paste of the original "mac" one.
> This is acceptable but not the perfect way to implement engines.
> To address this issue, a new engine function could be introduced to setup some
> parameters to be used by "packet_fwd" function. This way, similar engines
> could be removed.

Agreed that it sucks to incessantly replicate code.  Maybe some of the 
packet_fwd code is common enough to bump into run_pkt_fwd_on_lcore()?  
Most of these forwarding modes have similar looking code to 
receive/transmit bursts and free the failed remnants of the burst.  
Could this common code be bumped up into run_pkt_fwd_on_lcore() maybe?

> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>
> Applied for version 1.7.0.

Thanks!  Much appreciated.

-- Cyril.

Re: [dpdk-dev] [PATCH 0/6] Extensions to test-pmd

[Thomas Monjalon]

2014-05-16 09:55, Cyril Chemparathy:
> On 5/16/2014 7:22 AM, Thomas Monjalon wrote:
> > This is not the first time a new engine is added by copy/pasting the most
> > part of an existing engine. For instance, the "mac-retry" engine was
> > added by Intel as a copy/paste of the original "mac" one.
> > This is acceptable but not the perfect way to implement engines.
> > To address this issue, a new engine function could be introduced to setup
> > some parameters to be used by "packet_fwd" function. This way, similar
> > engines could be removed.
> 
> Agreed that it sucks to incessantly replicate code.  Maybe some of the
> packet_fwd code is common enough to bump into run_pkt_fwd_on_lcore()?
> Most of these forwarding modes have similar looking code to
> receive/transmit bursts and free the failed remnants of the burst.
> Could this common code be bumped up into run_pkt_fwd_on_lcore() maybe?

Not sure it fits into run_pkt_fwd_on_lcore() for all engines.
Feel free to suggest patches :)

-- 
Thomas