DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2] app/testpmd: txonly multiflow port change support
@ 2023-04-11 20:17 Joshua Washington
  2023-04-11 20:24 ` [PATCH v3] " Joshua Washington
  0 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2023-04-11 20:17 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang; +Cc: dev, Joshua Washington, Rushil Gupta

Google cloud routes traffic using IP addresses without the support of MAC
addresses, so changing source IP address for txonly-multi-flow can have
negative performance implications for net/gve when using testpmd. This
change adds a new flag --txonly-alter-port, which allows the alteration
of source port instead of source IP address for txonly multiflow mode.

The new flag can be tested with the following command:
dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
  --txonly-alter-port --txip=<SRC>,<DST>

Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Rushil Gupta <rushilg@google.com>
---
 app/test-pmd/parameters.c             |  4 +++
 app/test-pmd/testpmd.c                |  6 ++++
 app/test-pmd/testpmd.h                |  1 +
 app/test-pmd/txonly.c                 | 51 ++++++++++++++++++---------
 doc/guides/testpmd_app_ug/run_app.rst |  6 ++++
 5 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 3b37809baf..a99f04b4f0 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,6 +158,7 @@ usage(char* progname)
 		" or total packet length.\n");
 	printf("  --multi-rx-mempool: enable multi-rx-mempool support\n");
 	printf("  --txonly-multi-flow: generate multiple flows in txonly mode\n");
+	printf("  --txonly-alter-port: alter source port when generating multiple flows in txonly mode. Source IP address is changed by default\n");
 	printf("  --tx-ip=src,dst: IP addresses in Tx-only mode\n");
 	printf("  --tx-udp=src[,dst]: UDP ports in Tx-only mode\n");
 	printf("  --eth-link-speed: force link speed.\n");
@@ -678,6 +679,7 @@ launch_args_parse(int argc, char** argv)
 		{ "txpkts",			1, 0, 0 },
 		{ "multi-rx-mempool",           0, 0, 0 },
 		{ "txonly-multi-flow",		0, 0, 0 },
+		{ "txonly-alter-port",		0, 0, 0 },
 		{ "rxq-share",			2, 0, 0 },
 		{ "eth-link-speed",		1, 0, 0 },
 		{ "disable-link-check",		0, 0, 0 },
@@ -1307,6 +1309,8 @@ launch_args_parse(int argc, char** argv)
 				multi_rx_mempool = 1;
 			if (!strcmp(lgopts[opt_idx].name, "txonly-multi-flow"))
 				txonly_multi_flow = 1;
+			if (!strcmp(lgopts[opt_idx].name, "txonly-alter-port"))
+				txonly_alter_port = 1;
 			if (!strcmp(lgopts[opt_idx].name, "rxq-share")) {
 				if (optarg == NULL) {
 					rxq_share = UINT32_MAX;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5cb6f92523..b78bc4c26d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -267,6 +267,12 @@ enum tx_pkt_split tx_pkt_split = TX_PKT_SPLIT_OFF;
 uint8_t txonly_multi_flow;
 /**< Whether multiple flows are generated in TXONLY mode. */
 
+uint8_t txonly_alter_port;
+/*
+ * Whether source port should be altered instead of IP address when generating
+ * multiple flows in TXONLY mode.
+ */
+
 uint32_t tx_pkt_times_inter;
 /**< Timings for send scheduling in TXONLY mode, time between bursts. */
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index bdfbfd36d3..8926b51595 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -619,6 +619,7 @@ enum tx_pkt_split {
 extern enum tx_pkt_split tx_pkt_split;
 
 extern uint8_t txonly_multi_flow;
+extern uint8_t txonly_alter_port;
 
 extern uint32_t rxq_share;
 
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index b3d6873104..58b8834f08 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
 #define IP_DEFTTL  64   /* from RFC 1340. */
 
 static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
-RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
+RTE_DEFINE_PER_LCORE(uint16_t, _src_var); /**< Source address/port variation */
 static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
 
 static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
@@ -230,28 +230,47 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
 	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
 			sizeof(struct rte_ether_hdr));
+	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
+			sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr));
 	if (txonly_multi_flow) {
-		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
-		struct rte_ipv4_hdr *ip_hdr;
-		uint32_t addr;
+		uint16_t src_var = RTE_PER_LCORE(_src_var);
 
-		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
-				struct rte_ipv4_hdr *,
-				sizeof(struct rte_ether_hdr));
 		/*
-		 * Generate multiple flows by varying IP src addr. This
-		 * enables packets are well distributed by RSS in
+		 * Generate multiple flows by varying IP source address/port.
+		 * This enables packets are well distributed by RSS in
 		 * receiver side if any and txonly mode can be a decent
 		 * packet generator for developer's quick performance
 		 * regression test.
 		 */
-		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
-		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
-		RTE_PER_LCORE(_ip_var) = ip_var;
+		if (txonly_alter_port) {
+			struct rte_udp_hdr *udp_hdr;
+			uint16_t port;
+
+			udp_hdr = rte_pktmbuf_mtod_offset(pkt,
+					struct rte_udp_hdr *,
+					sizeof(struct rte_ether_hdr) +
+					sizeof(struct rte_ipv4_hdr));
+			/*
+			 *  Vary source port.
+			 */
+			port = src_var++;
+			udp_hdr->src_port = rte_cpu_to_be_16(port);
+		} else {
+			struct rte_ipv4_hdr *ip_hdr;
+			uint32_t addr;
+
+			ip_hdr = rte_pktmbuf_mtod_offset(pkt,
+					struct rte_ipv4_hdr *,
+					sizeof(struct rte_ether_hdr));
+			/*
+			 * Vary IP source address.
+			 */
+			addr = (tx_ip_dst_addr | ((src_var++ & 0xFF) << 8)) + rte_lcore_id();
+			ip_hdr->src_addr = rte_cpu_to_be_32(addr);
+		}
+		RTE_PER_LCORE(_src_var) = src_var;
 	}
-	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
-			sizeof(struct rte_ether_hdr) +
-			sizeof(struct rte_ipv4_hdr));
 
 	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
 		update_pkt_header(pkt, pkt_len);
@@ -393,7 +412,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
 
 	if (txonly_multi_flow)
-		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+		RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
 
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 57b23241cf..89fac9e964 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -373,6 +373,12 @@ The command line options are:
 
     Generate multiple flows in txonly mode.
 
+*   ``--txonly-alter-port``
+
+    Alter source port when generating multiple flows in txonly mode. The default
+    behavior is to alter source IP address. This flag must be used in
+    conjunction with --txonly-multi-flow.
+
 *   ``--rxq-share=[X]``
 
     Create queues in shared Rx queue mode if device supports.
-- 
2.40.0.577.gac1e443424-goog


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

* [PATCH v3] app/testpmd: txonly multiflow port change support
  2023-04-11 20:17 [PATCH v2] app/testpmd: txonly multiflow port change support Joshua Washington
@ 2023-04-11 20:24 ` Joshua Washington
  2023-04-12 18:16   ` [PATCH v4] " Joshua Washington
  0 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2023-04-11 20:24 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang; +Cc: dev, Joshua Washington, Rushil Gupta

Google cloud routes traffic using IP addresses without the support of MAC
addresses, so changing source IP address for txonly-multi-flow can have
negative performance implications for net/gve when using testpmd. This
change adds a new flag --txonly-alter-port, which allows the alteration
of source port instead of source IP address for txonly multiflow mode.

The new flag can be tested with the following command:
dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
  --txonly-alter-port --txip=<SRC>,<DST>

Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Rushil Gupta <rushilg@google.com>
---
 app/test-pmd/parameters.c             |  4 +++
 app/test-pmd/testpmd.c                |  6 ++++
 app/test-pmd/testpmd.h                |  1 +
 app/test-pmd/txonly.c                 | 51 ++++++++++++++++++---------
 doc/guides/testpmd_app_ug/run_app.rst |  6 ++++
 5 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 3b37809baf..a99f04b4f0 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -158,6 +158,7 @@ usage(char* progname)
 		" or total packet length.\n");
 	printf("  --multi-rx-mempool: enable multi-rx-mempool support\n");
 	printf("  --txonly-multi-flow: generate multiple flows in txonly mode\n");
+	printf("  --txonly-alter-port: alter source port when generating multiple flows in txonly mode. Source IP address is changed by default\n");
 	printf("  --tx-ip=src,dst: IP addresses in Tx-only mode\n");
 	printf("  --tx-udp=src[,dst]: UDP ports in Tx-only mode\n");
 	printf("  --eth-link-speed: force link speed.\n");
@@ -678,6 +679,7 @@ launch_args_parse(int argc, char** argv)
 		{ "txpkts",			1, 0, 0 },
 		{ "multi-rx-mempool",           0, 0, 0 },
 		{ "txonly-multi-flow",		0, 0, 0 },
+		{ "txonly-alter-port",		0, 0, 0 },
 		{ "rxq-share",			2, 0, 0 },
 		{ "eth-link-speed",		1, 0, 0 },
 		{ "disable-link-check",		0, 0, 0 },
@@ -1307,6 +1309,8 @@ launch_args_parse(int argc, char** argv)
 				multi_rx_mempool = 1;
 			if (!strcmp(lgopts[opt_idx].name, "txonly-multi-flow"))
 				txonly_multi_flow = 1;
+			if (!strcmp(lgopts[opt_idx].name, "txonly-alter-port"))
+				txonly_alter_port = 1;
 			if (!strcmp(lgopts[opt_idx].name, "rxq-share")) {
 				if (optarg == NULL) {
 					rxq_share = UINT32_MAX;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5cb6f92523..b78bc4c26d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -267,6 +267,12 @@ enum tx_pkt_split tx_pkt_split = TX_PKT_SPLIT_OFF;
 uint8_t txonly_multi_flow;
 /**< Whether multiple flows are generated in TXONLY mode. */
 
+uint8_t txonly_alter_port;
+/*
+ * Whether source port should be altered instead of IP address when generating
+ * multiple flows in TXONLY mode.
+ */
+
 uint32_t tx_pkt_times_inter;
 /**< Timings for send scheduling in TXONLY mode, time between bursts. */
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index bdfbfd36d3..8926b51595 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -619,6 +619,7 @@ enum tx_pkt_split {
 extern enum tx_pkt_split tx_pkt_split;
 
 extern uint8_t txonly_multi_flow;
+extern uint8_t txonly_alter_port;
 
 extern uint32_t rxq_share;
 
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index b3d6873104..58b8834f08 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
 #define IP_DEFTTL  64   /* from RFC 1340. */
 
 static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
-RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
+RTE_DEFINE_PER_LCORE(uint16_t, _src_var); /**< Source address/port variation */
 static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
 
 static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
@@ -230,28 +230,47 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
 	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
 			sizeof(struct rte_ether_hdr));
+	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
+			sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr));
 	if (txonly_multi_flow) {
-		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
-		struct rte_ipv4_hdr *ip_hdr;
-		uint32_t addr;
+		uint16_t src_var = RTE_PER_LCORE(_src_var);
 
-		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
-				struct rte_ipv4_hdr *,
-				sizeof(struct rte_ether_hdr));
 		/*
-		 * Generate multiple flows by varying IP src addr. This
-		 * enables packets are well distributed by RSS in
+		 * Generate multiple flows by varying IP source address/port.
+		 * This enables packets are well distributed by RSS in
 		 * receiver side if any and txonly mode can be a decent
 		 * packet generator for developer's quick performance
 		 * regression test.
 		 */
-		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
-		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
-		RTE_PER_LCORE(_ip_var) = ip_var;
+		if (txonly_alter_port) {
+			struct rte_udp_hdr *udp_hdr;
+			uint16_t port;
+
+			udp_hdr = rte_pktmbuf_mtod_offset(pkt,
+					struct rte_udp_hdr *,
+					sizeof(struct rte_ether_hdr) +
+					sizeof(struct rte_ipv4_hdr));
+			/*
+			 *  Vary source port.
+			 */
+			port = src_var++;
+			udp_hdr->src_port = rte_cpu_to_be_16(port);
+		} else {
+			struct rte_ipv4_hdr *ip_hdr;
+			uint32_t addr;
+
+			ip_hdr = rte_pktmbuf_mtod_offset(pkt,
+					struct rte_ipv4_hdr *,
+					sizeof(struct rte_ether_hdr));
+			/*
+			 * Vary IP source address.
+			 */
+			addr = (tx_ip_dst_addr | ((src_var++ & 0xFF) << 8)) + rte_lcore_id();
+			ip_hdr->src_addr = rte_cpu_to_be_32(addr);
+		}
+		RTE_PER_LCORE(_src_var) = src_var;
 	}
-	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
-			sizeof(struct rte_ether_hdr) +
-			sizeof(struct rte_ipv4_hdr));
 
 	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
 		update_pkt_header(pkt, pkt_len);
@@ -393,7 +412,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
 
 	if (txonly_multi_flow)
-		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+		RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
 
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 57b23241cf..89fac9e964 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -373,6 +373,12 @@ The command line options are:
 
     Generate multiple flows in txonly mode.
 
+*   ``--txonly-alter-port``
+
+    Alter source port when generating multiple flows in txonly mode. The default
+    behavior is to alter source IP address. This flag must be used in
+    conjunction with --txonly-multi-flow.
+
 *   ``--rxq-share=[X]``
 
     Create queues in shared Rx queue mode if device supports.
-- 
2.40.0.577.gac1e443424-goog


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

* [PATCH v4] app/testpmd: txonly multiflow port change support
  2023-04-11 20:24 ` [PATCH v3] " Joshua Washington
@ 2023-04-12 18:16   ` Joshua Washington
  2023-04-19 12:21     ` Singh, Aman Deep
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
  0 siblings, 2 replies; 16+ messages in thread
From: Joshua Washington @ 2023-04-12 18:16 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang; +Cc: dev, Joshua Washington, Rushil Gupta

Google cloud routes traffic using IP addresses without the support of MAC
addresses, so changing source IP address for txonly-multi-flow can have
negative performance implications for net/gve when using testpmd. This
patch updates txonly multiflow mode to modify source ports instead of
source IP addresses.

The change can be tested with the following command:
dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
    --txip=<SRC>,<DST>

Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Rushil Gupta <rushilg@google.com>
---
 app/test-pmd/txonly.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index b3d6873104..7fc743b508 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
 #define IP_DEFTTL  64   /* from RFC 1340. */
 
 static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
-RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
+RTE_DEFINE_PER_LCORE(uint16_t, _src_var); /**< Source port variation */
 static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
 
 static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
@@ -230,28 +230,30 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
 	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
 			sizeof(struct rte_ether_hdr));
+	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
+			sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr));
 	if (txonly_multi_flow) {
-		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
-		struct rte_ipv4_hdr *ip_hdr;
-		uint32_t addr;
+		uint16_t src_var = RTE_PER_LCORE(_src_var);
+		struct rte_udp_hdr *udp_hdr;
+		uint16_t port;
 
-		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
-				struct rte_ipv4_hdr *,
-				sizeof(struct rte_ether_hdr));
+		udp_hdr = rte_pktmbuf_mtod_offset(pkt,
+				struct rte_udp_hdr *,
+				sizeof(struct rte_ether_hdr) +
+				sizeof(struct rte_ipv4_hdr));
 		/*
-		 * Generate multiple flows by varying IP src addr. This
-		 * enables packets are well distributed by RSS in
+		 * Generate multiple flows by varying UDP source port.
+		 * This enables packets are well distributed by RSS in
 		 * receiver side if any and txonly mode can be a decent
 		 * packet generator for developer's quick performance
 		 * regression test.
 		 */
-		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
-		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
-		RTE_PER_LCORE(_ip_var) = ip_var;
+
+		port = src_var++;
+		udp_hdr->src_port = rte_cpu_to_be_16(port);
+		RTE_PER_LCORE(_src_var) = src_var;
 	}
-	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
-			sizeof(struct rte_ether_hdr) +
-			sizeof(struct rte_ipv4_hdr));
 
 	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
 		update_pkt_header(pkt, pkt_len);
@@ -393,7 +395,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
 
 	if (txonly_multi_flow)
-		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+		RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
 
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
-- 
2.40.0.577.gac1e443424-goog


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

* Re: [PATCH v4] app/testpmd: txonly multiflow port change support
  2023-04-12 18:16   ` [PATCH v4] " Joshua Washington
@ 2023-04-19 12:21     ` Singh, Aman Deep
  2023-04-19 14:38       ` Stephen Hemminger
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
  1 sibling, 1 reply; 16+ messages in thread
From: Singh, Aman Deep @ 2023-04-19 12:21 UTC (permalink / raw)
  To: Joshua Washington, Yuying Zhang; +Cc: dev, Rushil Gupta


On 4/12/2023 11:46 PM, Joshua Washington wrote:
> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.

Generally routing is based on DST IP addresses, was SRC IP also having an
impact in your case ?

>
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>      --txip=<SRC>,<DST>

Missing "-" in command: --tx-ip=<SRC>,<DST>

>
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>
> ---
>   app/test-pmd/txonly.c | 34 ++++++++++++++++++----------------
>   1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index b3d6873104..7fc743b508 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
>   #define IP_DEFTTL  64   /* from RFC 1340. */
>   
>   static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
> -RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
> +RTE_DEFINE_PER_LCORE(uint16_t, _src_var); /**< Source port variation */
>   static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
>   
>   static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
> @@ -230,28 +230,30 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
>   	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
>   	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
>   			sizeof(struct rte_ether_hdr));
> +	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
> +			sizeof(struct rte_ether_hdr) +
> +			sizeof(struct rte_ipv4_hdr));
>   	if (txonly_multi_flow) {
> -		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
> -		struct rte_ipv4_hdr *ip_hdr;
> -		uint32_t addr;
> +		uint16_t src_var = RTE_PER_LCORE(_src_var);
> +		struct rte_udp_hdr *udp_hdr;
> +		uint16_t port;
>   
> -		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
> -				struct rte_ipv4_hdr *,
> -				sizeof(struct rte_ether_hdr));
> +		udp_hdr = rte_pktmbuf_mtod_offset(pkt,
> +				struct rte_udp_hdr *,
> +				sizeof(struct rte_ether_hdr) +
> +				sizeof(struct rte_ipv4_hdr));
>   		/*
> -		 * Generate multiple flows by varying IP src addr. This
> -		 * enables packets are well distributed by RSS in
> +		 * Generate multiple flows by varying UDP source port.
> +		 * This enables packets are well distributed by RSS in
>   		 * receiver side if any and txonly mode can be a decent
>   		 * packet generator for developer's quick performance
>   		 * regression test.
>   		 */
> -		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();

Good to have feature where last IP octet had lcore_id.
Helped to identify, packet came from which core.

> -		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
> -		RTE_PER_LCORE(_ip_var) = ip_var;
> +
> +		port = src_var++;
> +		udp_hdr->src_port = rte_cpu_to_be_16(port);
> +		RTE_PER_LCORE(_src_var) = src_var;

To be safe, can we use the Ephemeral port range [49152	to 65535]
and adjust lcore_id within it.

>   	}
> -	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
> -			sizeof(struct rte_ether_hdr) +
> -			sizeof(struct rte_ipv4_hdr));
>   
>   	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
>   		update_pkt_header(pkt, pkt_len);
> @@ -393,7 +395,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
>   	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>   
>   	if (txonly_multi_flow)
> -		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
> +		RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
>   
>   	if (unlikely(nb_tx < nb_pkt)) {
>   		if (verbose_level > 0 && fs->fwd_dropped == 0)

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

* Re: [PATCH v4] app/testpmd: txonly multiflow port change support
  2023-04-19 12:21     ` Singh, Aman Deep
@ 2023-04-19 14:38       ` Stephen Hemminger
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2023-04-19 14:38 UTC (permalink / raw)
  To: Singh, Aman Deep; +Cc: Joshua Washington, Yuying Zhang, dev, Rushil Gupta

On Wed, 19 Apr 2023 17:51:18 +0530
"Singh, Aman Deep" <aman.deep.singh@intel.com> wrote:

> On 4/12/2023 11:46 PM, Joshua Washington wrote:
> > Google cloud routes traffic using IP addresses without the support of MAC
> > addresses, so changing source IP address for txonly-multi-flow can have
> > negative performance implications for net/gve when using testpmd. This
> > patch updates txonly multiflow mode to modify source ports instead of
> > source IP addresses.  
> 
> Generally routing is based on DST IP addresses, was SRC IP also having an
> impact in your case ?

Most hypervisor infrastructures will not allow a guest to use a source
address that has not been allocated to that guest. Therefore when using
multiple source addresses, either the guest has to be configured to have
all the addresses it will use, or the test needs to only use one source address.

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

* [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-12 18:16   ` [PATCH v4] " Joshua Washington
  2023-04-19 12:21     ` Singh, Aman Deep
@ 2023-04-21 23:20     ` Joshua Washington
  2023-04-24 17:55       ` Joshua Washington
                         ` (4 more replies)
  1 sibling, 5 replies; 16+ messages in thread
From: Joshua Washington @ 2023-04-21 23:20 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang; +Cc: dev, Joshua Washington, Rushil Gupta

Google cloud routes traffic using IP addresses without the support of MAC
addresses, so changing source IP address for txonly-multi-flow can have
negative performance implications for net/gve when using testpmd. This
patch updates txonly multiflow mode to modify source ports instead of
source IP addresses.

The change can be tested with the following command:
dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
    --tx-ip=<SRC>,<DST>

Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Rushil Gupta <rushilg@google.com>
---
 app/test-pmd/txonly.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index b3d6873104..f79e0e5d0b 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
 #define IP_DEFTTL  64   /* from RFC 1340. */
 
 static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
-RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
+RTE_DEFINE_PER_LCORE(uint8_t, _src_var); /**< Source port variation */
 static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
 
 static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
@@ -230,28 +230,35 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
 	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
 			sizeof(struct rte_ether_hdr));
+	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
+			sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr));
 	if (txonly_multi_flow) {
-		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
-		struct rte_ipv4_hdr *ip_hdr;
-		uint32_t addr;
+		uint16_t src_var = RTE_PER_LCORE(_src_var);
+		struct rte_udp_hdr *udp_hdr;
+		uint16_t port;
 
-		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
-				struct rte_ipv4_hdr *,
-				sizeof(struct rte_ether_hdr));
+		udp_hdr = rte_pktmbuf_mtod_offset(pkt,
+				struct rte_udp_hdr *,
+				sizeof(struct rte_ether_hdr) +
+				sizeof(struct rte_ipv4_hdr));
 		/*
-		 * Generate multiple flows by varying IP src addr. This
-		 * enables packets are well distributed by RSS in
+		 * Generate multiple flows by varying UDP source port.
+		 * This enables packets are well distributed by RSS in
 		 * receiver side if any and txonly mode can be a decent
 		 * packet generator for developer's quick performance
 		 * regression test.
+		 *
+		 * Only ports in the range 49152 (0xC000) and 65535 (0xFFFF)
+		 * will be used, with the least significant byte representing
+		 * the lcore ID. As such, the most significant byte will cycle
+		 * through 0xC0 and 0xFF.
 		 */
-		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
-		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
-		RTE_PER_LCORE(_ip_var) = ip_var;
+		port = ((((src_var++) % (0xFF - 0xC0) + 0xC0) & 0xFF) << 8)
+				+ rte_lcore_id();
+		udp_hdr->src_port = rte_cpu_to_be_16(port);
+		RTE_PER_LCORE(_src_var) = src_var;
 	}
-	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
-			sizeof(struct rte_ether_hdr) +
-			sizeof(struct rte_ipv4_hdr));
 
 	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
 		update_pkt_header(pkt, pkt_len);
@@ -393,7 +400,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
 
 	if (txonly_multi_flow)
-		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+		RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
 
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
-- 
2.40.0.634.g4ca3ef3211-goog


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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
@ 2023-04-24 17:55       ` Joshua Washington
  2023-04-25  6:56         ` David Marchand
  2023-04-26  7:24       ` Singh, Aman Deep
                         ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2023-04-24 17:55 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang; +Cc: dev, Rushil Gupta

[-- Attachment #1: Type: text/plain, Size: 5285 bytes --]

After updating the patch, it seems that the `lcores_autotest` unit test now
times out on Windows Server 2019. I looked at the test logs, but they were
identical as far as I could tell, with the timed out test even printing "Test
OK" to stdout. Is this a flake? Or is there any other way to get extra
information about why the test timed out or run the test with extra
debugging information?

Thanks,
Josh

On Fri, Apr 21, 2023 at 4:20 PM Joshua Washington <joshwash@google.com>
wrote:

> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.
>
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>     --tx-ip=<SRC>,<DST>
>
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>
> ---
>  app/test-pmd/txonly.c | 39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index b3d6873104..f79e0e5d0b 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0
> << 8) | 2;
>  #define IP_DEFTTL  64   /* from RFC 1340. */
>
>  static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted
> packets. */
> -RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
> +RTE_DEFINE_PER_LCORE(uint8_t, _src_var); /**< Source port variation */
>  static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
>
>  static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
> @@ -230,28 +230,35 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct
> rte_mempool *mbp,
>         copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
>         copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
>                         sizeof(struct rte_ether_hdr));
> +       copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
> +                       sizeof(struct rte_ether_hdr) +
> +                       sizeof(struct rte_ipv4_hdr));
>         if (txonly_multi_flow) {
> -               uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
> -               struct rte_ipv4_hdr *ip_hdr;
> -               uint32_t addr;
> +               uint16_t src_var = RTE_PER_LCORE(_src_var);
> +               struct rte_udp_hdr *udp_hdr;
> +               uint16_t port;
>
> -               ip_hdr = rte_pktmbuf_mtod_offset(pkt,
> -                               struct rte_ipv4_hdr *,
> -                               sizeof(struct rte_ether_hdr));
> +               udp_hdr = rte_pktmbuf_mtod_offset(pkt,
> +                               struct rte_udp_hdr *,
> +                               sizeof(struct rte_ether_hdr) +
> +                               sizeof(struct rte_ipv4_hdr));
>                 /*
> -                * Generate multiple flows by varying IP src addr. This
> -                * enables packets are well distributed by RSS in
> +                * Generate multiple flows by varying UDP source port.
> +                * This enables packets are well distributed by RSS in
>                  * receiver side if any and txonly mode can be a decent
>                  * packet generator for developer's quick performance
>                  * regression test.
> +                *
> +                * Only ports in the range 49152 (0xC000) and 65535
> (0xFFFF)
> +                * will be used, with the least significant byte
> representing
> +                * the lcore ID. As such, the most significant byte will
> cycle
> +                * through 0xC0 and 0xFF.
>                  */
> -               addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
> -               ip_hdr->src_addr = rte_cpu_to_be_32(addr);
> -               RTE_PER_LCORE(_ip_var) = ip_var;
> +               port = ((((src_var++) % (0xFF - 0xC0) + 0xC0) & 0xFF) << 8)
> +                               + rte_lcore_id();
> +               udp_hdr->src_port = rte_cpu_to_be_16(port);
> +               RTE_PER_LCORE(_src_var) = src_var;
>         }
> -       copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
> -                       sizeof(struct rte_ether_hdr) +
> -                       sizeof(struct rte_ipv4_hdr));
>
>         if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) ||
> txonly_multi_flow)
>                 update_pkt_header(pkt, pkt_len);
> @@ -393,7 +400,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
>         nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>
>         if (txonly_multi_flow)
> -               RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
> +               RTE_PER_LCORE(_src_var) -= nb_pkt - nb_tx;
>
>         if (unlikely(nb_tx < nb_pkt)) {
>                 if (verbose_level > 0 && fs->fwd_dropped == 0)
> --
> 2.40.0.634.g4ca3ef3211-goog
>
>

-- 

Joshua Washington | Software Engineer | joshwash@google.com | (414) 366-4423

[-- Attachment #2: Type: text/html, Size: 7573 bytes --]

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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-24 17:55       ` Joshua Washington
@ 2023-04-25  6:56         ` David Marchand
  0 siblings, 0 replies; 16+ messages in thread
From: David Marchand @ 2023-04-25  6:56 UTC (permalink / raw)
  To: Joshua Washington; +Cc: Aman Singh, Yuying Zhang, dev, Rushil Gupta, dpdklab

Hello Joshua,

On Mon, Apr 24, 2023 at 7:56 PM Joshua Washington <joshwash@google.com> wrote:
>
> After updating the patch, it seems that the `lcores_autotest` unit test now times out on Windows Server 2019. I looked at the test logs, but they were identical as far as I could tell, with the timed out test even printing "Test OK" to stdout. Is this a flake? Or is there any other way to get extra information about why the test timed out or run the test with extra debugging information?

In general, the UNH dashboard provides an archive with logs for each
report, like for example:
https://lab.dpdk.org/results/dashboard/patchsets/26090/
https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/b5a6a2743665426b937603587850aa6d/log_upload_file/2023/4/dpdk_5f34cc454df4_26090_2023-04-22_02-36-52_NA.zip

This timeout is something I did not notice so far, Ccing UNH guys, for info.


Regarding your patch, the CI passes fine on the current main branch.
And there is no relation between testpmd and the EAL unit tests.
So this report is very likely a false positive.

I triggered a retest on your patch.


-- 
David Marchand


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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
  2023-04-24 17:55       ` Joshua Washington
@ 2023-04-26  7:24       ` Singh, Aman Deep
  2023-05-15 18:11         ` Joshua Washington
  2023-05-15 22:26       ` Ferruh Yigit
                         ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Singh, Aman Deep @ 2023-04-26  7:24 UTC (permalink / raw)
  To: Joshua Washington, Yuying Zhang; +Cc: dev, Rushil Gupta


On 4/22/2023 4:50 AM, Joshua Washington wrote:
> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.
>
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>      --tx-ip=<SRC>,<DST>
>
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>

Acked-by: Aman Singh <aman.deep.singh@intel.com>

<snip>


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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-26  7:24       ` Singh, Aman Deep
@ 2023-05-15 18:11         ` Joshua Washington
  0 siblings, 0 replies; 16+ messages in thread
From: Joshua Washington @ 2023-05-15 18:11 UTC (permalink / raw)
  To: Singh, Aman Deep; +Cc: Yuying Zhang, dev, Rushil Gupta

[-- Attachment #1: Type: text/plain, Size: 152 bytes --]

Does this patch need anything else to be done before it can be merged? I'm
hoping to get this patch merged as part of the 23.07 release.

Thanks,
 Josh

[-- Attachment #2: Type: text/html, Size: 234 bytes --]

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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
  2023-04-24 17:55       ` Joshua Washington
  2023-04-26  7:24       ` Singh, Aman Deep
@ 2023-05-15 22:26       ` Ferruh Yigit
  2023-05-16 17:32         ` Joshua Washington
  2023-05-17 10:34       ` Ferruh Yigit
  2023-05-19  0:22       ` [PATCH v6] " Joshua Washington
  4 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2023-05-15 22:26 UTC (permalink / raw)
  To: Joshua Washington, Aman Singh, Yuying Zhang
  Cc: dev, Rushil Gupta, Stephen Hemminger

On 4/22/2023 12:20 AM, Joshua Washington wrote:
> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.
> 
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>     --tx-ip=<SRC>,<DST>
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>

Hi Joshua, Aman,

The reason to support multi-flow in Tx-only mode is, to test RSS in the
Rx side.
When used in guest, in a hypervisor infrastructure what is the usecase
for multi-flow, why not use default single IP?

And if there is a need to support multi-flow without updating source IP,
what about to support various modes as "--txonly-multi-flow=XXX", where
XXX can be src_ip, dst_ip, src_port, dst_port?
If possible by keeping no param '--txonly-multi-flow' same as current
usage (src_ip) for backward capability..

This extends testing capability and lets different platforms select
different config based on their needs.

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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-05-15 22:26       ` Ferruh Yigit
@ 2023-05-16 17:32         ` Joshua Washington
  2023-05-17 10:10           ` Ferruh Yigit
  0 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2023-05-16 17:32 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Aman Singh, Yuying Zhang, dev, Rushil Gupta, Stephen Hemminger

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

Hello,

I believe my original solution was something closer to what you are
suggesting for backward compatibility. I originally had a flag that enabled
changing source port instead of source IP addresses, but I received
feedback that adding an extra flag was complicating things too much from
Stephen.

On a VM, the purpose of using multi-flow is similar to that of bare metal:
to test RSS in the RX side. However, generating traffic by changing source
IP address can cause inconsistencies in performance due to protections in
cloud infrastructure from sending packets from a different source IP
address than is provisioned for the VM. Changing source UDP port to test
RSS should be functionally equivalent while allowing VMs to send traffic
from a single source IP address.

If everyone agrees that adding --txonly-multi-flow as an option as well as
keeping the flag is an acceptable way of moving forward, I can do that.

Thanks,
Josh

[-- Attachment #2: Type: text/html, Size: 1234 bytes --]

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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-05-16 17:32         ` Joshua Washington
@ 2023-05-17 10:10           ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2023-05-17 10:10 UTC (permalink / raw)
  To: Joshua Washington
  Cc: Aman Singh, Yuying Zhang, dev, Rushil Gupta, Stephen Hemminger

On 5/16/2023 6:32 PM, Joshua Washington wrote:
> Hello,
> 
> I believe my original solution was something closer to what you are
> suggesting for backward compatibility. I originally had a flag that
> enabled changing source port instead of source IP addresses, but I
> received feedback that adding an extra flag was complicating things too
> much from Stephen.
> 
> On a VM, the purpose of using multi-flow is similar to that of bare
> metal: to test RSS in the RX side. However, generating traffic by
> changing source IP address can cause inconsistencies in performance due
> to protections in cloud infrastructure from sending packets from a
> different source IP address than is provisioned for the VM. Changing
> source UDP port to test RSS should be functionally equivalent while
> allowing VMs to send traffic from a single source IP address.
> 
> If everyone agrees that adding --txonly-multi-flow as an option as well
> as keeping the flag is an acceptable way of moving forward, I can do that.
> 

Hi Joshua,

I missed the first version of the patch and the comment, thinking twice
comment has a point, I was thinking some users doing RSS based on IP but
probably all supports RSS based on UDP too. Better to keep simple.

Lets proceed with latest version, I will put some comments on it.

Thanks,
Ferruh


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

* Re: [PATCH v5] app/testpmd: txonly multiflow port change support
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
                         ` (2 preceding siblings ...)
  2023-05-15 22:26       ` Ferruh Yigit
@ 2023-05-17 10:34       ` Ferruh Yigit
  2023-05-19  0:22       ` [PATCH v6] " Joshua Washington
  4 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2023-05-17 10:34 UTC (permalink / raw)
  To: Joshua Washington, Aman Singh, Yuying Zhang; +Cc: dev, Rushil Gupta

On 4/22/2023 12:20 AM, Joshua Washington wrote:
> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.
> 
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>     --tx-ip=<SRC>,<DST>
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>
> ---
>  app/test-pmd/txonly.c | 39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index b3d6873104..f79e0e5d0b 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
>  #define IP_DEFTTL  64   /* from RFC 1340. */
>  
>  static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
> -RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
> +RTE_DEFINE_PER_LCORE(uint8_t, _src_var); /**< Source port variation */

What about '_src_port_var' as variable name?

>  static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
>  
>  static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
> @@ -230,28 +230,35 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
>  	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
>  	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
>  			sizeof(struct rte_ether_hdr));
> +	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
> +			sizeof(struct rte_ether_hdr) +
> +			sizeof(struct rte_ipv4_hdr));
>  	if (txonly_multi_flow) {
> -		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
> -		struct rte_ipv4_hdr *ip_hdr;
> -		uint32_t addr;
> +		uint16_t src_var = RTE_PER_LCORE(_src_var);
> +		struct rte_udp_hdr *udp_hdr;
> +		uint16_t port;

'port' is used in testpmd for port_id in many places, what do you think
to rename variable as 'src_port'?

>  
> -		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
> -				struct rte_ipv4_hdr *,
> -				sizeof(struct rte_ether_hdr));
> +		udp_hdr = rte_pktmbuf_mtod_offset(pkt,
> +				struct rte_udp_hdr *,
> +				sizeof(struct rte_ether_hdr) +
> +				sizeof(struct rte_ipv4_hdr));
>  		/*
> -		 * Generate multiple flows by varying IP src addr. This
> -		 * enables packets are well distributed by RSS in
> +		 * Generate multiple flows by varying UDP source port.
> +		 * This enables packets are well distributed by RSS in
>  		 * receiver side if any and txonly mode can be a decent
>  		 * packet generator for developer's quick performance
>  		 * regression test.
> +		 *
> +		 * Only ports in the range 49152 (0xC000) and 65535 (0xFFFF)
> +		 * will be used, with the least significant byte representing
> +		 * the lcore ID. As such, the most significant byte will cycle
> +		 * through 0xC0 and 0xFF.
>  		 */
> -		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
> -		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
> -		RTE_PER_LCORE(_ip_var) = ip_var;
> +		port = ((((src_var++) % (0xFF - 0xC0) + 0xC0) & 0xFF) << 8)
> +				+ rte_lcore_id();

To prevent '%' in the datapath, I think following does the same thing:
port = ((src_var++ | 0xC0) << 8) + rte_lcore_id();

Although it is not big deal, in original version you never got 0xFFXX
values:
3E % 3F + C0 = FE
3F % 3F + C0 = C0


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

* [PATCH v6] app/testpmd: txonly multiflow port change support
  2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
                         ` (3 preceding siblings ...)
  2023-05-17 10:34       ` Ferruh Yigit
@ 2023-05-19  0:22       ` Joshua Washington
  2023-05-19 11:23         ` Ferruh Yigit
  4 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2023-05-19  0:22 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang
  Cc: dev, Ferruh Yigit, Joshua Washington, Rushil Gupta

Google cloud routes traffic using IP addresses without the support of MAC
addresses, so changing source IP address for txonly-multi-flow can have
negative performance implications for net/gve when using testpmd. This
patch updates txonly multiflow mode to modify source ports instead of
source IP addresses.

The change can be tested with the following command:
dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
    --tx-ip=<SRC>,<DST>

Signed-off-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Rushil Gupta <rushilg@google.com>
---
 app/test-pmd/txonly.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index b3d6873104..c2b88764be 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -56,7 +56,7 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << 16) | (0 << 8) | 2;
 #define IP_DEFTTL  64   /* from RFC 1340. */
 
 static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of transmitted packets. */
-RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */
+RTE_DEFINE_PER_LCORE(uint8_t, _src_port_var); /**< Source port variation */
 static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets. */
 
 static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */
@@ -230,28 +230,34 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0);
 	copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt,
 			sizeof(struct rte_ether_hdr));
+	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
+			sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr));
 	if (txonly_multi_flow) {
-		uint8_t  ip_var = RTE_PER_LCORE(_ip_var);
-		struct rte_ipv4_hdr *ip_hdr;
-		uint32_t addr;
+		uint16_t src_var = RTE_PER_LCORE(_src_port_var);
+		struct rte_udp_hdr *udp_hdr;
+		uint16_t src_port;
 
-		ip_hdr = rte_pktmbuf_mtod_offset(pkt,
-				struct rte_ipv4_hdr *,
-				sizeof(struct rte_ether_hdr));
+		udp_hdr = rte_pktmbuf_mtod_offset(pkt,
+				struct rte_udp_hdr *,
+				sizeof(struct rte_ether_hdr) +
+				sizeof(struct rte_ipv4_hdr));
 		/*
-		 * Generate multiple flows by varying IP src addr. This
-		 * enables packets are well distributed by RSS in
+		 * Generate multiple flows by varying UDP source port.
+		 * This enables packets are well distributed by RSS in
 		 * receiver side if any and txonly mode can be a decent
 		 * packet generator for developer's quick performance
 		 * regression test.
+		 *
+		 * Only ports in the range 49152 (0xC000) and 65535 (0xFFFF)
+		 * will be used, with the least significant byte representing
+		 * the lcore ID. As such, the most significant byte will cycle
+		 * through 0xC0 and 0xFF.
 		 */
-		addr = (tx_ip_dst_addr | (ip_var++ << 8)) + rte_lcore_id();
-		ip_hdr->src_addr = rte_cpu_to_be_32(addr);
-		RTE_PER_LCORE(_ip_var) = ip_var;
+		src_port = ((src_var++ | 0xC0) << 8) + rte_lcore_id();
+		udp_hdr->src_port = rte_cpu_to_be_16(src_port);
+		RTE_PER_LCORE(_src_port_var) = src_var;
 	}
-	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
-			sizeof(struct rte_ether_hdr) +
-			sizeof(struct rte_ipv4_hdr));
 
 	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
 		update_pkt_header(pkt, pkt_len);
@@ -393,7 +399,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
 
 	if (txonly_multi_flow)
-		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+		RTE_PER_LCORE(_src_port_var) -= nb_pkt - nb_tx;
 
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
-- 
2.40.1.698.g37aff9b760-goog


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

* Re: [PATCH v6] app/testpmd: txonly multiflow port change support
  2023-05-19  0:22       ` [PATCH v6] " Joshua Washington
@ 2023-05-19 11:23         ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2023-05-19 11:23 UTC (permalink / raw)
  To: Joshua Washington, Aman Singh, Yuying Zhang; +Cc: dev, Rushil Gupta

On 5/19/2023 1:22 AM, Joshua Washington wrote:
> Google cloud routes traffic using IP addresses without the support of MAC
> addresses, so changing source IP address for txonly-multi-flow can have
> negative performance implications for net/gve when using testpmd. This
> patch updates txonly multiflow mode to modify source ports instead of
> source IP addresses.
> 
> The change can be tested with the following command:
> dpdk-testpmd -- --forward-mode=txonly --txonly-multi-flow \
>     --tx-ip=<SRC>,<DST>
> 
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Rushil Gupta <rushilg@google.com>
>

Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.


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

end of thread, other threads:[~2023-05-19 11:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 20:17 [PATCH v2] app/testpmd: txonly multiflow port change support Joshua Washington
2023-04-11 20:24 ` [PATCH v3] " Joshua Washington
2023-04-12 18:16   ` [PATCH v4] " Joshua Washington
2023-04-19 12:21     ` Singh, Aman Deep
2023-04-19 14:38       ` Stephen Hemminger
2023-04-21 23:20     ` [PATCH v5] " Joshua Washington
2023-04-24 17:55       ` Joshua Washington
2023-04-25  6:56         ` David Marchand
2023-04-26  7:24       ` Singh, Aman Deep
2023-05-15 18:11         ` Joshua Washington
2023-05-15 22:26       ` Ferruh Yigit
2023-05-16 17:32         ` Joshua Washington
2023-05-17 10:10           ` Ferruh Yigit
2023-05-17 10:34       ` Ferruh Yigit
2023-05-19  0:22       ` [PATCH v6] " Joshua Washington
2023-05-19 11:23         ` Ferruh Yigit

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