app/testpmd: revert primary process polling all queues fix

Message ID 20230705143206.268958-1-ferruh.yigit@amd.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: revert primary process polling all queues fix |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Ferruh Yigit July 5, 2023, 2:32 p.m. UTC
  For some drivers [1], testpmd forwarding is broken with commit [2].

This is because with [2] testpmd gets queue state from ethdev and
forwarding is done only on queues in started state, but some drivers
don't update queue status properly, and this breaks forwarding for those
drivers.

Drivers should be fixed but more time is required to verify drivers
again, instead reverting [2] for now to not break drivers.
Target is to merge [2] back at the beginning of next release cycle and
fix drivers accordingly.

[1]
Bugzilla ID: 1259

[2]
Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
Cc: haijie1@huawei.com
Cc: songx.jiale@intel.com
Cc: qiming.yang@intel.com
---
 app/test-pmd/testpmd.c                 | 20 ++++----------------
 doc/guides/rel_notes/release_23_07.rst |  9 +++++++++
 2 files changed, 13 insertions(+), 16 deletions(-)
  

Comments

Ferruh Yigit July 5, 2023, 4:50 p.m. UTC | #1
On 7/5/2023 3:32 PM, Ferruh Yigit wrote:
> For some drivers [1], testpmd forwarding is broken with commit [2].
> 
> This is because with [2] testpmd gets queue state from ethdev and
> forwarding is done only on queues in started state, but some drivers
> don't update queue status properly, and this breaks forwarding for those
> drivers.
> 
> Drivers should be fixed but more time is required to verify drivers
> again, instead reverting [2] for now to not break drivers.
> Target is to merge [2] back at the beginning of next release cycle and
> fix drivers accordingly.
> 
> [1]
> Bugzilla ID: 1259
> 
> [2]
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
> Cc: haijie1@huawei.com
> Cc: songx.jiale@intel.com
> Cc: qiming.yang@intel.com

+Ali & Raslan
  
Jiale, SongX July 6, 2023, 9:24 a.m. UTC | #2
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, July 5, 2023 10:32 PM
> To: Singh, Aman Deep <aman.deep.singh@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>;
> Jie Hai <haijie1@huawei.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> Marchand <david.marchand@redhat.com>; stable@dpdk.org; Jiale, SongX
> <songx.jiale@intel.com>; Yang, Qiming <qiming.yang@intel.com>
> Subject: [PATCH] app/testpmd: revert primary process polling all queues fix
> 
> For some drivers [1], testpmd forwarding is broken with commit [2].
> 
> This is because with [2] testpmd gets queue state from ethdev and
> forwarding is done only on queues in started state, but some drivers don't
> update queue status properly, and this breaks forwarding for those drivers.
> 
> Drivers should be fixed but more time is required to verify drivers again,
> instead reverting [2] for now to not break drivers.
> Target is to merge [2] back at the beginning of next release cycle and fix
> drivers accordingly.
> 
> [1]
> Bugzilla ID: 1259
> 
> [2]
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
> queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
Tested-by: Jiale Song <songx.jiale@intel.com>
  
Ali Alnubani July 6, 2023, 10:01 a.m. UTC | #3
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, July 5, 2023 5:32 PM
> To: Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Anatoly Burakov <anatoly.burakov@intel.com>;
> Jie Hai <haijie1@huawei.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; David Marchand <david.marchand@redhat.com>;
> stable@dpdk.org; songx.jiale@intel.com; qiming.yang@intel.com
> Subject: [PATCH] app/testpmd: revert primary process polling all queues fix
> 
> For some drivers [1], testpmd forwarding is broken with commit [2].
> 
> This is because with [2] testpmd gets queue state from ethdev and
> forwarding is done only on queues in started state, but some drivers
> don't update queue status properly, and this breaks forwarding for those
> drivers.
> 
> Drivers should be fixed but more time is required to verify drivers
> again, instead reverting [2] for now to not break drivers.
> Target is to merge [2] back at the beginning of next release cycle and
> fix drivers accordingly.
> 
> [1]
> Bugzilla ID: 1259
> 
> [2]
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
> queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---

Thanks, Ferruh. Tested on top of latest main (df60837ccd65).

Tested-by: Ali Alnubani <alialnu@nvidia.com>
  
Ferruh Yigit July 6, 2023, 10:23 a.m. UTC | #4
On 7/6/2023 10:24 AM, Jiale, SongX wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Wednesday, July 5, 2023 10:32 PM
>> To: Singh, Aman Deep <aman.deep.singh@intel.com>; Zhang, Yuying
>> <yuying.zhang@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>;
>> Jie Hai <haijie1@huawei.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
>> Marchand <david.marchand@redhat.com>; stable@dpdk.org; Jiale, SongX
>> <songx.jiale@intel.com>; Yang, Qiming <qiming.yang@intel.com>
>> Subject: [PATCH] app/testpmd: revert primary process polling all queues fix
>>
>> For some drivers [1], testpmd forwarding is broken with commit [2].
>>
>> This is because with [2] testpmd gets queue state from ethdev and
>> forwarding is done only on queues in started state, but some drivers don't
>> update queue status properly, and this breaks forwarding for those drivers.
>>
>> Drivers should be fixed but more time is required to verify drivers again,
>> instead reverting [2] for now to not break drivers.
>> Target is to merge [2] back at the beginning of next release cycle and fix
>> drivers accordingly.
>>
>> [1]
>> Bugzilla ID: 1259
>>
>> [2]
>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
>> queues")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
>> ---
>
> Tested-by: Jiale Song <songx.jiale@intel.com>
>
> Tested-by: Ali Alnubani <alialnu@nvidia.com>
>

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

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1fc70650e0a4..c6ad9b18bf03 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2424,13 +2424,6 @@  update_rx_queue_state(uint16_t port_id, uint16_t queue_id)
 		ports[port_id].rxq[queue_id].state =
 			rx_qinfo.queue_state;
 	} else if (rc == -ENOTSUP) {
-		/*
-		 * Do not change the rxq state for primary process
-		 * to ensure that the PMDs do not implement
-		 * rte_eth_rx_queue_info_get can forward as before.
-		 */
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-			return;
 		/*
 		 * Set the rxq state to RTE_ETH_QUEUE_STATE_STARTED
 		 * to ensure that the PMDs do not implement
@@ -2456,13 +2449,6 @@  update_tx_queue_state(uint16_t port_id, uint16_t queue_id)
 		ports[port_id].txq[queue_id].state =
 			tx_qinfo.queue_state;
 	} else if (rc == -ENOTSUP) {
-		/*
-		 * Do not change the txq state for primary process
-		 * to ensure that the PMDs do not implement
-		 * rte_eth_tx_queue_info_get can forward as before.
-		 */
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-			return;
 		/*
 		 * Set the txq state to RTE_ETH_QUEUE_STATE_STARTED
 		 * to ensure that the PMDs do not implement
@@ -2530,7 +2516,8 @@  start_packet_forwarding(int with_tx_first)
 		return;
 
 	if (stream_init != NULL) {
-		update_queue_state();
+		if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+			update_queue_state();
 		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
 			stream_init(fwd_streams[i]);
 	}
@@ -3293,7 +3280,8 @@  start_port(portid_t pid)
 		pl[cfg_pi++] = pi;
 	}
 
-	update_queue_state();
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		update_queue_state();
 
 	if (at_least_one_port_successfully_started && !no_link_check)
 		check_all_ports_link_status(RTE_PORT_ALL);
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index 7b1c31469fa5..46b3d915c6e4 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -280,6 +280,15 @@  Known Issues
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* **Testpmd is not forwarding on queues individually stopped.**
+
+  Testpmd forwards packets on started queues.
+  If a queue explicitly stopped, and later port stopped and started again,
+  the status of the previously stopped queue is not updated, so forwarding
+  is not working on those queues.
+
+  As a workaround start queues back explicitly, instead of port stop/start.
+
 
 Tested Platforms
 ----------------