[PATCH 0/2] app/test-pipeline: cleanup and add ring/help options

[PATCH 0/2] app/test-pipeline: cleanup and add ring/help options

[Doug Foster]

This patchset adds command-line options to configure the size of RX and
TX rings in the test-pipeline application. The current implementation
fixes the ring size at 128, which cannot be changed through the command
line. This default value may not be optimal, as the ideal ring size
depends on the platform and the CPU's ability to process entries before
they are dequeued.

This was confirmed by the observation of 66% failed enqueue attempts to
the RX ring on the Grace CPU when using a ring size of 128, indicating
that the ring was too small for the workload. Increasing the RX ring
size to 256 reduced the number of failed enqueue attempts to 22% and
improved overall throughput by approximately 22%.

In addition to making the ring sizes configurable, this patchset
introduces a help option to make valid parameters easier to understand.
It also includes minor cleanup to improve readability and
maintainability of the code.

Doug Foster (2):
  app/test-pipeline: cleanup and add help
  app/test-pipeline: add ring size options

 app/test-pipeline/config.c                 | 131 +++++++++++++++------
 app/test-pipeline/init.c                   |   4 -
 app/test-pipeline/main.h                   |   9 +-
 doc/guides/sample_app_ug/test_pipeline.rst |  12 +-
 4 files changed, 114 insertions(+), 42 deletions(-)

-- 
2.34.1

[PATCH 1/2] app/test-pipeline: cleanup and add help

[Doug Foster]

Add a help option and usage statement that is printed when the help option
is used or when incorrect options are provided. Remove redundant calls to
app_print_usage() within app_parse_args(), since main() already calls
app_print_usage() on parse failure, avoiding multiple messages. Use an
enum for the long-options return value to improve readability.

Signed-off-by: Doug Foster <doug.foster@arm.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
---
 app/test-pipeline/config.c | 89 +++++++++++++++++++++++---------------
 app/test-pipeline/main.h   |  5 ++-
 2 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/app/test-pipeline/config.c b/app/test-pipeline/config.c
index daf838948b..0d280cf898 100644
--- a/app/test-pipeline/config.c
+++ b/app/test-pipeline/config.c
@@ -41,7 +41,27 @@
 
 #include "main.h"
 
-static const char usage[] = "\n";
+static const char usage[] =
+"Usage:\n"
+"  dpdk-test-pipeline [EAL options] -- -p PORTMASK --TABLE_TYPE\n"
+"\n"
+"EAL requirements:\n"
+"  -l/--lcores must specify exactly 3 lcores (RX core, pipeline core , TX core)\n"
+"\n"
+"Application options:\n"
+"  -p PORTMASK           Hex bitmask of ports; must include 2 or 4 ports\n"
+"  --none | --stub |\n"
+"  --hash-8-ext  | --hash-8-lru  |\n"
+"  --hash-16-ext | --hash-16-lru |\n"
+"  --hash-32-ext | --hash-32-lru |\n"
+"  --hash-spec-8-ext  | --hash-spec-8-lru  |\n"
+"  --hash-spec-16-ext | --hash-spec-16-lru |\n"
+"  --hash-spec-32-ext | --hash-spec-32-lru |\n"
+"  --acl | --lpm | --lpm-ipv6 |\n"
+"  --hash-cuckoo-8  | --hash-cuckoo-16  | --hash-cuckoo-32  |\n"
+"  --hash-cuckoo-48 | --hash-cuckoo-64  | --hash-cuckoo-80  |\n"
+"  --hash-cuckoo-96 | --hash-cuckoo-112 | --hash-cuckoo-128\n"
+"  -h/--help    print help statement and exit\n";
 
 void
 app_print_usage(void)
@@ -124,32 +144,33 @@ app_parse_args(int argc, char **argv)
 	int option_index;
 	char *prgname = argv[0];
 	static struct option lgopts[] = {
-		{"none", 0, 0, 0},
-		{"stub", 0, 0, 0},
-		{"hash-8-ext", 0, 0, 0},
-		{"hash-8-lru", 0, 0, 0},
-		{"hash-16-ext", 0, 0, 0},
-		{"hash-16-lru", 0, 0, 0},
-		{"hash-32-ext", 0, 0, 0},
-		{"hash-32-lru", 0, 0, 0},
-		{"hash-spec-8-ext", 0, 0, 0},
-		{"hash-spec-8-lru", 0, 0, 0},
-		{"hash-spec-16-ext", 0, 0, 0},
-		{"hash-spec-16-lru", 0, 0, 0},
-		{"hash-spec-32-ext", 0, 0, 0},
-		{"hash-spec-32-lru", 0, 0, 0},
-		{"acl", 0, 0, 0},
-		{"lpm", 0, 0, 0},
-		{"lpm-ipv6", 0, 0, 0},
-		{"hash-cuckoo-8", 0, 0, 0},
-		{"hash-cuckoo-16", 0, 0, 0},
-		{"hash-cuckoo-32", 0, 0, 0},
-		{"hash-cuckoo-48", 0, 0, 0},
-		{"hash-cuckoo-64", 0, 0, 0},
-		{"hash-cuckoo-80", 0, 0, 0},
-		{"hash-cuckoo-96", 0, 0, 0},
-		{"hash-cuckoo-112", 0, 0, 0},
-		{"hash-cuckoo-128", 0, 0, 0},
+		{"none", 0, 0, e_APP_PIPELINES},
+		{"stub", 0, 0, e_APP_PIPELINES},
+		{"hash-8-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-8-lru", 0, 0, e_APP_PIPELINES},
+		{"hash-16-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-16-lru", 0, 0, e_APP_PIPELINES},
+		{"hash-32-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-32-lru", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-8-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-8-lru", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-16-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-16-lru", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-32-ext", 0, 0, e_APP_PIPELINES},
+		{"hash-spec-32-lru", 0, 0, e_APP_PIPELINES},
+		{"acl", 0, 0, e_APP_PIPELINES},
+		{"lpm", 0, 0, e_APP_PIPELINES},
+		{"lpm-ipv6", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-8", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-16", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-32", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-48", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-64", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-80", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-96", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-112", 0, 0, e_APP_PIPELINES},
+		{"hash-cuckoo-128", 0, 0, e_APP_PIPELINES},
+		{"help", 0, 0, e_APP_HELP},
 		{NULL, 0, 0, 0}
 	};
 	uint32_t lcores[3], n_lcores, lcore_id, pipeline_type_provided;
@@ -162,7 +183,6 @@ app_parse_args(int argc, char **argv)
 
 		if (n_lcores >= 3) {
 			RTE_LOG(ERR, USER1, "Number of cores must be 3\n");
-			app_print_usage();
 			return -1;
 		}
 
@@ -172,7 +192,6 @@ app_parse_args(int argc, char **argv)
 
 	if (n_lcores != 3) {
 		RTE_LOG(ERR, USER1, "Number of cores must be 3\n");
-		app_print_usage();
 		return -1;
 	}
 
@@ -186,17 +205,16 @@ app_parse_args(int argc, char **argv)
 	app.pipeline_type = e_APP_PIPELINE_HASH_KEY16_LRU;
 	pipeline_type_provided = 0;
 
-	while ((opt = getopt_long(argc, argvopt, "p:",
+	while ((opt = getopt_long(argc, argvopt, "p:h",
 			lgopts, &option_index)) != EOF) {
 		switch (opt) {
 		case 'p':
-			if (app_parse_port_mask(optarg) < 0) {
-				app_print_usage();
+			if (app_parse_port_mask(optarg) < 0)
 				return -1;
-			}
+
 			break;
 
-		case 0: /* long options */
+		case e_APP_PIPELINES: /* long options */
 			if (!pipeline_type_provided) {
 				uint32_t i;
 
@@ -213,9 +231,10 @@ app_parse_args(int argc, char **argv)
 				break;
 			}
 
-			app_print_usage();
 			return -1;
 
+		case e_APP_HELP:
+		case 'h':
 		default:
 			return -1;
 		}
diff --git a/app/test-pipeline/main.h b/app/test-pipeline/main.h
index ee9c58ac4c..1f93dc3964 100644
--- a/app/test-pipeline/main.h
+++ b/app/test-pipeline/main.h
@@ -67,7 +67,7 @@ void app_print_usage(void);
 void app_init(void);
 int app_lcore_main_loop(void *arg);
 
-/* Pipeline */
+/* Pipeline and help*/
 enum {
 	e_APP_PIPELINE_NONE = 0,
 	e_APP_PIPELINE_STUB,
@@ -99,7 +99,8 @@ enum {
 	e_APP_PIPELINE_HASH_CUCKOO_KEY96,
 	e_APP_PIPELINE_HASH_CUCKOO_KEY112,
 	e_APP_PIPELINE_HASH_CUCKOO_KEY128,
-	e_APP_PIPELINES
+	e_APP_PIPELINES,
+	e_APP_HELP
 };
 
 void app_main_loop_rx(void);
-- 
2.34.1

[PATCH 2/2] app/test-pipeline: add ring size options

[Doug Foster]

Add command-line options to configure the size of the RX and TX rings.
The ring sizes should provide enough capacity to accommodate the number
of new entries that accumulate during the time it takes for existing
entries to be dequeued from the rings. This depends on CPU processing
rate, which varies across platforms, so configurable options are
provided. Use of these options is optional, and the default size
remains 128 for both the TX and RX rings. When setting the size, the
value must be a power of two, as required by the ring library.

Signed-off-by: Doug Foster <doug.foster@arm.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
---
 app/test-pipeline/config.c                 | 48 ++++++++++++++++++++--
 app/test-pipeline/init.c                   |  4 --
 app/test-pipeline/main.h                   |  8 +++-
 doc/guides/sample_app_ug/test_pipeline.rst | 12 +++++-
 4 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/app/test-pipeline/config.c b/app/test-pipeline/config.c
index 0d280cf898..ef0438c06c 100644
--- a/app/test-pipeline/config.c
+++ b/app/test-pipeline/config.c
@@ -43,7 +43,7 @@
 
 static const char usage[] =
 "Usage:\n"
-"  dpdk-test-pipeline [EAL options] -- -p PORTMASK --TABLE_TYPE\n"
+"  dpdk-test-pipeline [EAL options] -- -p PORTMASK --PIPELINE_ARGS\n"
 "\n"
 "EAL requirements:\n"
 "  -l/--lcores must specify exactly 3 lcores (RX core, pipeline core , TX core)\n"
@@ -61,7 +61,10 @@ static const char usage[] =
 "  --hash-cuckoo-8  | --hash-cuckoo-16  | --hash-cuckoo-32  |\n"
 "  --hash-cuckoo-48 | --hash-cuckoo-64  | --hash-cuckoo-80  |\n"
 "  --hash-cuckoo-96 | --hash-cuckoo-112 | --hash-cuckoo-128\n"
-"  -h/--help    print help statement and exit\n";
+"  -h/--help    print help statement and exit\n"
+"  --rx-ring-size=N    Optional, size of RX ring (power-of-two)\n"
+"  --tx-ring-size=N    Optional, size of TX ring (power-of-two)\n";
+
 
 void
 app_print_usage(void)
@@ -104,6 +107,25 @@ app_parse_port_mask(const char *arg)
 	return 0;
 }
 
+static int
+app_parse_ring_size(const char *ring_size_arg, uint32_t *size)
+{
+	char *end = NULL;
+	unsigned long value;
+
+	value = strtoul(ring_size_arg, &end, 10);
+
+	/* Check for conversion of invalid string */
+	if (!(*ring_size_arg != '\0' && *end == '\0'))
+		return -1;
+
+	if (!rte_is_power_of_2(value) || value > UINT32_MAX)
+		return -1;
+
+	*size = value;
+	return 0;
+}
+
 struct {
 	const char *name;
 	uint32_t value;
@@ -142,6 +164,7 @@ app_parse_args(int argc, char **argv)
 	int opt, ret;
 	char **argvopt;
 	int option_index;
+	uint32_t rx_ring_size, tx_ring_size;
 	char *prgname = argv[0];
 	static struct option lgopts[] = {
 		{"none", 0, 0, e_APP_PIPELINES},
@@ -171,6 +194,8 @@ app_parse_args(int argc, char **argv)
 		{"hash-cuckoo-112", 0, 0, e_APP_PIPELINES},
 		{"hash-cuckoo-128", 0, 0, e_APP_PIPELINES},
 		{"help", 0, 0, e_APP_HELP},
+		{"rx-ring-size", 1, 0, e_APP_RX_RING_SIZE},
+		{"tx-ring-size", 1, 0, e_APP_TX_RING_SIZE},
 		{NULL, 0, 0, 0}
 	};
 	uint32_t lcores[3], n_lcores, lcore_id, pipeline_type_provided;
@@ -205,6 +230,9 @@ app_parse_args(int argc, char **argv)
 	app.pipeline_type = e_APP_PIPELINE_HASH_KEY16_LRU;
 	pipeline_type_provided = 0;
 
+	app.ring_rx_size = APP_RING_SIZE_DEFAULT;
+	app.ring_tx_size = APP_RING_SIZE_DEFAULT;
+
 	while ((opt = getopt_long(argc, argvopt, "p:h",
 			lgopts, &option_index)) != EOF) {
 		switch (opt) {
@@ -214,7 +242,7 @@ app_parse_args(int argc, char **argv)
 
 			break;
 
-		case e_APP_PIPELINES: /* long options */
+		case e_APP_PIPELINES:
 			if (!pipeline_type_provided) {
 				uint32_t i;
 
@@ -233,6 +261,20 @@ app_parse_args(int argc, char **argv)
 
 			return -1;
 
+		case e_APP_RX_RING_SIZE:
+			if (app_parse_ring_size(optarg, &rx_ring_size) < 0)
+				return -1;
+
+			app.ring_rx_size = rx_ring_size;
+			break;
+
+		case e_APP_TX_RING_SIZE:
+			if (app_parse_ring_size(optarg, &tx_ring_size) < 0)
+				return -1;
+
+			app.ring_tx_size = tx_ring_size;
+			break;
+
 		case e_APP_HELP:
 		case 'h':
 		default:
diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
index 558f0e428d..f596337751 100644
--- a/app/test-pipeline/init.c
+++ b/app/test-pipeline/init.c
@@ -48,10 +48,6 @@ struct app_params app = {
 	.port_rx_ring_size = 128,
 	.port_tx_ring_size = 512,
 
-	/* Rings */
-	.ring_rx_size = 128,
-	.ring_tx_size = 128,
-
 	/* Buffer pool */
 	.pool_buffer_size = 2048 + RTE_PKTMBUF_HEADROOM,
 	.pool_size = 32 * 1024,
diff --git a/app/test-pipeline/main.h b/app/test-pipeline/main.h
index 1f93dc3964..6f9ef6e2e2 100644
--- a/app/test-pipeline/main.h
+++ b/app/test-pipeline/main.h
@@ -18,6 +18,8 @@ struct app_mbuf_array {
 #define APP_MAX_PORTS 4
 #endif
 
+#define APP_RING_SIZE_DEFAULT 128
+
 struct __rte_cache_aligned app_params {
 	/* CPU cores */
 	uint32_t core_rx;
@@ -67,7 +69,7 @@ void app_print_usage(void);
 void app_init(void);
 int app_lcore_main_loop(void *arg);
 
-/* Pipeline and help*/
+/* Pipelines, help, ring size */
 enum {
 	e_APP_PIPELINE_NONE = 0,
 	e_APP_PIPELINE_STUB,
@@ -100,7 +102,9 @@ enum {
 	e_APP_PIPELINE_HASH_CUCKOO_KEY112,
 	e_APP_PIPELINE_HASH_CUCKOO_KEY128,
 	e_APP_PIPELINES,
-	e_APP_HELP
+	e_APP_HELP,
+	e_APP_RX_RING_SIZE,
+	e_APP_TX_RING_SIZE
 };
 
 void app_main_loop_rx(void);
diff --git a/doc/guides/sample_app_ug/test_pipeline.rst b/doc/guides/sample_app_ug/test_pipeline.rst
index 818be93cd6..46a5ecc780 100644
--- a/doc/guides/sample_app_ug/test_pipeline.rst
+++ b/doc/guides/sample_app_ug/test_pipeline.rst
@@ -45,13 +45,23 @@ The application execution command line is:
 
 .. code-block:: console
 
-    ./dpdk-test-pipeline [EAL options] -- -p PORTMASK --TABLE_TYPE
+    ./dpdk-test-pipeline [EAL options] -- -p PORTMASK --PIPELINE_ARGS
 
 The ``-l/--lcores`` EAL CPU corelist option has to contain exactly 3 CPU cores.
 The first CPU core in the core mask is assigned for core A, the second for core B and the third for core C.
 
 The PORTMASK parameter must contain 2 or 4 ports.
 
+PIPELINE_ARGS represents TABLE_TYPE and pipeline options, such as ``--rx-ring-size`` and ``--tx-ring-size``, described below.
+The ``--rx-ring-size`` and ``--tx-ring-size`` options control the sizes of the rings used between cores.
+Both values **must be powers of two**. If not used, the default size is 128.
+
+* ``--rx-ring-size=N``
+  Optional, sets size of the RX ring between **Core A (RX)** and **Core B (Pipeline)**.
+
+* ``--tx-ring-size=N``
+  Optional, sets size of the TX ring between **Core B (Pipeline)** and **Core C (TX)**.
+
 Table Types and Behavior
 ~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.34.1