Skip to contentSkip to conversation

testpmd: add hairpin-map parameter

PATCH In review Gregory Etelson 2 participants 3 messages Sep 28, 2023 – Oct 30, 2024

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 testpmd: add hairpin-map parameter

Hairpin offloads packet forwarding.
Packet is expected on Rx port <rp>, Rx queue <rq> and is delivered to Tx port <tp> Tx queue <tq>.

Testpmd implements a static hairpin configuration scheme. The scheme implicitly matches next valid port for given <rp> or <tp>. That approach can be used in a single or double port setups only.

The new parameter allows explicit selection of Rx and Tx ports and queues in hairpin configuration.
The new hairpin-map parameter is provided with 5 parameters, separated by :

--hairpin-map=Rx port id:Rx queue:Tx port id:Tx queue:queues number

Testpmd operator can provide several hairpin-map parameters for different hairpin maps.
Example:

dpdk-testpmd <EAL params> -- \
<testpmd params> \
--rxq=2 --txq=2 --hairpinq=2 --hairpin-mode=0x12 \
--hairpin-map=0:2:1:2:1 \ # [1]
--hairpin-map=0:3:2:2:3 # [2]

Hairpin map [1] binds Rx port 0, queue 2 with Tx port 1, queue 2. Hairpin map [2] binds
Rx port 0, queue 3 with Tx port 2, queue 2,
Rx port 0, queue 4 with Tx port 2, queue 3,
Rx port 0, queue 5 with Tx port 2, queue 4.

The new hairpin-map parameter is optional. If omitted, testpmd will create "default" hairpin maps.

---
 app/test-pmd/parameters.c             |  63 ++++++++
 app/test-pmd/testpmd.c                | 212 ++++++++++++++++++--------
 app/test-pmd/testpmd.h                |  18 +++
 doc/guides/testpmd_app_ug/run_app.rst |   3 +
 4 files changed, 230 insertions(+), 66 deletions(-)
app/test-pmd/parameters.c +63 -0
index a9ca58339d..675de81861 100644
@@ -206,6 +206,12 @@ usage(char* progname)
 	printf("  --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n"
 	       "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
 	       "    0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
+	printf("  --hairpin-map=rxpi:rxq:txpi:txq:n: hairpin map.\n"
+	       "    rxpi - Rx port index.\n"
+	       "    rxq  - Rx queue.\n"
+	       "    txpi - Tx port index.\n"
+	       "    txq  - Tx queue.\n"
+	       "    n    - hairpin queues number.\n");
 }
 
 #ifdef RTE_LIB_CMDLINE
@@ -588,6 +594,55 @@ parse_link_speed(int n)
 	return speed;
 }
 
+static __rte_always_inline
+char *parse_hairpin_map_entry(char *input, char **next)
+{
+	char *tail = strchr(input, ':');
+
+	if (!tail)
+		return NULL;
+	tail[0] = '\0';
+	*next = tail + 1;
+	return input;
+}
+
+static int
+parse_hairpin_map(char *hpmap)
+{
+	/*
+	 * Testpmd hairpin map format:
+	 * <Rx port id:First Rx queue:Tx port id:First Tx queue:queues number>
+	 */
+	char *head, *next = hpmap;
+	struct hairpin_map *map = calloc(1, sizeof(*map));
+
+	if (!map)
+		return -ENOMEM;
+
+	head = parse_hairpin_map_entry(next, &next);
+	if (!head)
+		goto err;
+	map->rx_port = atoi(head);
+	head = parse_hairpin_map_entry(next, &next);
+	if (!head)
+		goto err;
+	map->rxq_head = atoi(head);
+	head = parse_hairpin_map_entry(next, &next);
+	if (!head)
+		goto err;
+	map->tx_port = atoi(head);
+	head = parse_hairpin_map_entry(next, &next);
+	if (!head)
+		goto err;
+	map->txq_head = atoi(head);
+	map->qnum = atoi(next);
+	hairpin_add_multiport_map(map);
+	return 0;
+err:
+	free(map);
+	return -EINVAL;
+}
+
 void
 launch_args_parse(int argc, char** argv)
 {
@@ -663,6 +718,7 @@ launch_args_parse(int argc, char** argv)
 		{ "txd",			1, 0, 0 },
 		{ "hairpinq",			1, 0, 0 },
 		{ "hairpin-mode",		1, 0, 0 },
+		{ "hairpin-map",                1, 0, 0 },
 		{ "burst",			1, 0, 0 },
 		{ "flowgen-clones",		1, 0, 0 },
 		{ "flowgen-flows",		1, 0, 0 },
@@ -1111,6 +1167,13 @@ launch_args_parse(int argc, char** argv)
 				else
 					hairpin_mode = (uint32_t)n;
 			}
+			if (!strcmp(lgopts[opt_idx].name, "hairpin-map")) {
+				hairpin_multiport_mode = true;
+				ret = parse_hairpin_map(optarg);
+				if (ret)
+					rte_exit(EXIT_FAILURE, "invalid hairpin map\n");
+
+			}
 			if (!strcmp(lgopts[opt_idx].name, "burst")) {
 				n = atoi(optarg);
 				if (n == 0) {
app/test-pmd/testpmd.c +146 -66

259 lines not on this page · raw message

app/test-pmd/testpmd.h +18 -0
index f1df6a8faf..208e8e9514 100644
@@ -125,6 +125,24 @@ enum noisy_fwd_mode {
 	NOISY_FWD_MODE_MAX,
 };
 
+struct hairpin_map {
+	LIST_ENTRY(hairpin_map) entry; /**< List entry. */
+	portid_t rx_port; /**< Hairpin Rx port ID. */
+	portid_t tx_port; /**< Hairpin Tx port ID. */
+	uint16_t rxq_head; /**< Hairpin Rx queue head. */
+	uint16_t txq_head; /**< Hairpin Tx queue head. */
+	uint16_t qnum; /**< Hairpin queues number. */
+};
+
+/**
+ * Command line arguments parser sets `hairpin_multiport_mode` to True
+ * if explicit hairpin map configuration mode was used.
+ */
+extern bool hairpin_multiport_mode;
+
+/** Hairpin maps list. */
+extern void hairpin_add_multiport_map(struct hairpin_map *map);
+
 /**
  * The data structure associated with RX and TX packet burst statistics
  * that are recorded for each forwarding stream.
doc/guides/testpmd_app_ug/run_app.rst +3 -0
index 6e9c552e76..a202c98b4c 100644
@@ -566,6 +566,9 @@ The command line options are:
 
     The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
 
+*   ``--hairpin-map=Rx port id:Rx queue:Tx port id:Tx queue:queues number``
+
+    Set explicit hairpin configuration.
 
 Testpmd Multi-Process Command-line Options
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signature · 1 line
2.39.2
#1 lore raw

app/test-pmd/parameters.c parse_link_speed(int n) · line 608

Quoted 12 lines · On Thu, 28 Sep 2023 16:39:06 +0300 Gregory Etelson <[email protected]> wrote:
app/test-pmd/parameters.c +12 -0
+static __rte_always_inline
+char *parse_hairpin_map_entry(char *input, char **next)
+{
+	char *tail = strchr(input, ':');
+
+	if (!tail)
+		return NULL;
+	tail[0] = '\0';
+	*next = tail + 1;
+	return input;
+}
+

There is no reason to mark this as inline. It is not in fast path. Let compiler decide.

app/test-pmd/parameters.c parse_link_speed(int n) · line 625

Quoted 4 lines
app/test-pmd/parameters.c +4 -0
+	head = parse_hairpin_map_entry(next, &next);
+	if (!head)
+		goto err;
+	map->rx_port = atoi(head);

Use strtoul() to allow checking for invalid number. Seems like the parsing here is following the parsing code pattern that could use strtok_r.

app/test-pmd/testpmd.c port_is_started(portid_t port_id) · line 2729

Quoted 10 lines
app/test-pmd/testpmd.c +10 -0
+		/* Fail to setup rx queue, return */
+		if (port->port_status == RTE_PORT_HANDLING)
+			port->port_status = RTE_PORT_STOPPED;
+		else
+			fprintf(stderr,
+				"Port %d can not be set back to stopped\n", pi);
+		fprintf(stderr,
+			"Port %d failed to configure hairpin on rxq %u.\n"
+			"Peer port: %u peer txq: %u\n",
+			pi, qi, peer_tx_port, i);

Minor nit, port should be printed with %u since it is unsigned. Lots of testpmd code gets this wrong, and compiler doesn't care.

app/test-pmd/testpmd.c setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi) · line 2869

Quoted 8 lines
app/test-pmd/testpmd.c +8 -0
+/* Configure the Rx and Tx hairpin queues for the selected port. */
+static int
+setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi)
+{
+	return !hairpin_multiport_mode ?
+	       setup_legacy_hairpin_queus(pi, p_pi, cnt_pi) :
+	       setup_mapped_harpin_queues(pi);
+}

It is clearer to use an if statement in this case. Also, better to write positive rather than negative logic.

	if (hairpin_multiport_mode)
		return setup_mapped_hairpin_queues(pi);
	else
		return setup_legacy_hairpin_queus(pi, p_pi, cnt_pi);
#2 lore raw

Hello Stephen,

Quoted 7 lines
Quoted 7 lines
app/test-pmd/parameters.c +7 -0
+static __rte_always_inline
+char *parse_hairpin_map_entry(char *input, char **next)
+{
+     char *tail = strchr(input, ':');
+
+     if (!tail)
+             return NULL;
Quoted 9 lines

app/test-pmd/parameters.c parse_link_speed(int n) · line 608

Quoted 5 lines
app/test-pmd/parameters.c +5 -0
+     tail[0] = '\0';
+     *next = tail + 1;
+     return input;
+}
+

There is no reason to mark this as inline. It is not in fast path. Let compiler decide.

There will be no parse_hairpin_map_entry() in the next patch version.

Quoted 9 lines

app/test-pmd/parameters.c parse_link_speed(int n) · line 625

Quoted 5 lines
app/test-pmd/parameters.c +4 -0
+     head = parse_hairpin_map_entry(next, &next);
+     if (!head)
+             goto err;
+     map->rx_port = atoi(head);

Use strtoul() to allow checking for invalid number. Seems like the parsing here is following the parsing code pattern that could use strtok_r.

I switched to sscanf().

Quoted 12 lines

app/test-pmd/testpmd.c port_is_started(portid_t port_id) · line 2729

Quoted 11 lines
app/test-pmd/testpmd.c +10 -0
+             /* Fail to setup rx queue, return */
+             if (port->port_status == RTE_PORT_HANDLING)
+                     port->port_status = RTE_PORT_STOPPED;
+             else
+                     fprintf(stderr,
+                             "Port %d can not be set back to stopped\n", pi);
+             fprintf(stderr,
+                     "Port %d failed to configure hairpin on rxq %u.\n"
+                     "Peer port: %u peer txq: %u\n",
+                     pi, qi, peer_tx_port, i);
Quoted 3 lines

Minor nit, port should be printed with %u since it is unsigned. Lots of testpmd code gets this wrong, and compiler doesn't care.

fixed.

Quoted 13 lines

app/test-pmd/testpmd.c setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi) · line 2869

Quoted 9 lines
app/test-pmd/testpmd.c +8 -0
+/* Configure the Rx and Tx hairpin queues for the selected port. */
+static int
+setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi)
+{
+     return !hairpin_multiport_mode ?
+            setup_legacy_hairpin_queus(pi, p_pi, cnt_pi) :
+            setup_mapped_harpin_queues(pi);
+}

It is clearer to use an if statement in this case. Also, better to write positive rather than negative logic.

Quoted 4 lines
       if (hairpin_multiport_mode)
               return setup_mapped_hairpin_queues(pi);
       else
               return setup_legacy_hairpin_queus(pi, p_pi, cnt_pi);

fixed.

I'll post fixes in the next patch version.

Regards,
Gregory