DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jie Hai <haijie1@huawei.com>
To: Aman Singh <aman.deep.singh@intel.com>,
	Yuying Zhang <yuying.zhang@intel.com>,
	Anatoly Burakov <anatoly.burakov@intel.com>,
	Matan Azrad <matan@nvidia.com>,
	Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Cc: <dev@dpdk.org>, <liudongdong3@huawei.com>,
	<shiyangx.he@intel.com>, <ferruh.yigit@amd.com>
Subject: [PATCH v4] app/testpmd: fix primary process not polling all queues
Date: Fri, 9 Jun 2023 17:03:40 +0800	[thread overview]
Message-ID: <20230609090340.3942-1-haijie1@huawei.com> (raw)
In-Reply-To: <20230529022649.51425-1-haijie1@huawei.com>

Here's how the problem arises.
step1: Start the app.
    dpdk-testpmd -a 0000:35:00.0 -l 0-3 -- -i --rxq=10 --txq=10

step2: Perform the following steps and send traffic. As expected,
queue 7 does not send or receive packets, and other queues do.
    port 0 rxq 7 stop
    port 0 txq 7 stop
    set fwd mac
    start

step3: Perform the following steps and send traffic. All queues
are expected to send and receive packets normally, but that's not
the case for queue 7.
    stop
    port stop all
    port start all
    start
    show port xstats all

In fact, only the value of rx_q7_packets for queue 7 is not zero,
which means queue 7 is enabled for the driver but is not involved
in packet receiving and forwarding by software. If we check queue
state by command 'show rxq info 0 7' and 'show txq info 0 7',
we see queue 7 is started as other queues are.
    Rx queue state: started
    Tx queue state: started
The queue 7 is started but cannot forward. That's the problem.

We know that each stream has a read-only "disabled" field that
control if this stream should be used to forward. This field
depends on testpmd local queue state, please see
commit 3c4426db54fc ("app/testpmd: do not poll stopped queues").
DPDK framework maintains ethdev queue state that drivers reported,
which indicates the real state of queues.

There are commands that update these two kind queue state such as
'port X rxq|txq start|stop'. But these operations take effect only
in one stop-start round. In the following stop-start round, the
preceding operations do not take effect anymore. However, only
the ethdev queue state is updated, causing the testpmd and ethdev
state information to diverge and causing unexpected side effects
as above problem.

There was a similar problem for the secondary process, please see
commit 5028f207a4fa ("app/testpmd: fix secondary process packet
forwarding").

This patch applies its workaround with some difference to the
primary process. Not all PMDs implement rte_eth_rx_queue_info_get and
rte_eth_tx_queue_info_get, however they may support deferred_start
with primary process. To not break their behavior, retain the original
testpmd local queue state for those PMDs.

Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
v1->v2:
1. Fix misspelled word 'deferred'.
2. Fix incorrect format of reference to commits.

v2->v3:
1. Fix incorrect format of reference to commits.

v3->v4:
1. Remove deferred_start change.
2. Modify commit log.
---
 app/test-pmd/testpmd.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c6ad9b18bf03..1fc70650e0a4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2424,6 +2424,13 @@ 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
@@ -2449,6 +2456,13 @@ 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
@@ -2516,8 +2530,7 @@ start_packet_forwarding(int with_tx_first)
 		return;
 
 	if (stream_init != NULL) {
-		if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-			update_queue_state();
+		update_queue_state();
 		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
 			stream_init(fwd_streams[i]);
 	}
@@ -3280,8 +3293,7 @@ start_port(portid_t pid)
 		pl[cfg_pi++] = pi;
 	}
 
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-		update_queue_state();
+	update_queue_state();
 
 	if (at_least_one_port_successfully_started && !no_link_check)
 		check_all_ports_link_status(RTE_PORT_ALL);
-- 
2.33.0


  parent reply	other threads:[~2023-06-09  9:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-06  9:26 [PATCH] app/test-pmd: fix not polling all queues without defered starting Jie Hai
2023-05-08  3:10 ` [PATCH v2] app/test-pmd: fix not polling all queues without deferred starting Jie Hai
2023-05-29  2:26   ` [PATCH v3] " Jie Hai
2023-06-06 14:45     ` Ferruh Yigit
2023-06-07  7:04       ` Jie Hai
2023-06-07 17:38         ` Ferruh Yigit
2023-06-07 18:12     ` Ferruh Yigit
2023-06-09  8:54       ` Jie Hai
2023-06-09  9:03     ` Jie Hai [this message]
2023-06-09 11:10       ` [PATCH v4] app/testpmd: fix primary process not polling all queues Ferruh Yigit
2023-06-20 10:07         ` Jie Hai
2023-06-20 10:57           ` Ferruh Yigit
2023-06-20 17:05           ` Ferruh Yigit
2023-06-22 16:40       ` Ali Alnubani
2023-06-26  9:30         ` Jie Hai
2023-06-27 11:05           ` Ferruh Yigit
2023-07-03 13:40             ` Ali Alnubani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230609090340.3942-1-haijie1@huawei.com \
    --to=haijie1@huawei.com \
    --cc=aman.deep.singh@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=ferruh.yigit@amd.com \
    --cc=liudongdong3@huawei.com \
    --cc=matan@nvidia.com \
    --cc=shiyangx.he@intel.com \
    --cc=yuying.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).