diff mbox series

app/testpmd: fix fail to set queue number when attach device

Message ID 20250218113156.7174-1-lihuisong@huawei.com (mailing list archive)
State Accepted
Delegated to: Stephen Hemminger
Headers
Series app/testpmd: fix fail to set queue number when attach device |

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/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

lihuisong (C) Feb. 18, 2025, 11:31 a.m. UTC 
Testpmd doesn't modify the Rx/Tx queue number of all ports after attach a
new device. The root cause is that "port config all rxq <value>" would
check if all ports are stopped and the status of this port is still in
'RTE_PORT_HANDLING'. This status is changed to 'RTE_PORT_STOPPED' in
setup_attached_port() when start port.

In addition, this new port needs to be initialized in advance because these
information are depanded on many commands. Otherwise other commands like
"port config xxx" may also encounter similar issue. But the initialization
can't be in new event callback as discussed in following commit. So this
patch moves setup_attached_port() to the time point 100ms after received
the new event.

Bugzilla ID: 1663
Fixes: 994635edb2c0 ("app/testpmd: add port attach/detach for multiple process")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/testpmd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Stephen Hemminger Feb. 20, 2025, 8:56 p.m. UTC | #1 
On Tue, 18 Feb 2025 19:31:56 +0800
Huisong Li <lihuisong@huawei.com> wrote:

> Testpmd doesn't modify the Rx/Tx queue number of all ports after attach a
> new device. The root cause is that "port config all rxq <value>" would
> check if all ports are stopped and the status of this port is still in
> 'RTE_PORT_HANDLING'. This status is changed to 'RTE_PORT_STOPPED' in
> setup_attached_port() when start port.
> 
> In addition, this new port needs to be initialized in advance because these
> information are depanded on many commands. Otherwise other commands like
> "port config xxx" may also encounter similar issue. But the initialization
> can't be in new event callback as discussed in following commit. So this
> patch moves setup_attached_port() to the time point 100ms after received
> the new event.
> 
> Bugzilla ID: 1663
> Fixes: 994635edb2c0 ("app/testpmd: add port attach/detach for multiple process")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---

Applied to next-net with some rewording of commit message to improve grammar.
Should this patch be cc to stable since it has a Fixes tag?
lihuisong (C) Feb. 21, 2025, 1:16 a.m. UTC | #2 
在 2025/2/21 4:56, Stephen Hemminger 写道:
> On Tue, 18 Feb 2025 19:31:56 +0800
> Huisong Li <lihuisong@huawei.com> wrote:
>
>> Testpmd doesn't modify the Rx/Tx queue number of all ports after attach a
>> new device. The root cause is that "port config all rxq <value>" would
>> check if all ports are stopped and the status of this port is still in
>> 'RTE_PORT_HANDLING'. This status is changed to 'RTE_PORT_STOPPED' in
>> setup_attached_port() when start port.
>>
>> In addition, this new port needs to be initialized in advance because these
>> information are depanded on many commands. Otherwise other commands like
>> "port config xxx" may also encounter similar issue. But the initialization
>> can't be in new event callback as discussed in following commit. So this
>> patch moves setup_attached_port() to the time point 100ms after received
>> the new event.
>>
>> Bugzilla ID: 1663
>> Fixes: 994635edb2c0 ("app/testpmd: add port attach/detach for multiple process")
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
> Applied to next-net with some rewording of commit message to improve grammar.
> Should this patch be cc to stable since it has a Fixes tag?
No need to cc to stable because above Fixes is just in this 25.03 version.
> .
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 19e2e37c11..b0ee39db52 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -705,7 +705,7 @@  eth_dev_set_mtu_mp(uint16_t port_id, uint16_t mtu)
 }
 
 /* Forward function declarations */
-static void setup_attached_port(portid_t pi);
+static void setup_attached_port(void *arg);
 static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
@@ -2906,8 +2906,6 @@  start_port(portid_t pid)
 		at_least_one_port_exist = true;
 
 		port = &ports[pi];
-		if (port->need_setup)
-			setup_attached_port(pi);
 
 		if (port->port_status == RTE_PORT_STOPPED) {
 			port->port_status = RTE_PORT_HANDLING;
@@ -3441,7 +3439,7 @@  attach_port(char *identifier)
 		/* setup ports matching the devargs used for probing */
 		if (port_is_forwarding(pi))
 			continue; /* port was already attached before */
-		setup_attached_port(pi);
+		setup_attached_port((void *)(intptr_t)pi);
 	}
 out:
 	printf("Port %s is attached.\n", identifier);
@@ -3449,8 +3447,9 @@  attach_port(char *identifier)
 }
 
 static void
-setup_attached_port(portid_t pi)
+setup_attached_port(void *arg)
 {
+	portid_t pi = (intptr_t)arg;
 	unsigned int socket_id;
 	int ret;
 
@@ -3786,6 +3785,10 @@  eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 			ports[port_id].need_setup = 1;
 			ports[port_id].port_status = RTE_PORT_HANDLING;
 		}
+		/* Can't initialize port directly in new event. */
+		if (rte_eal_alarm_set(100000, setup_attached_port,
+				      (void *)(intptr_t)port_id))
+			fprintf(stderr, "Could not set up deferred task to setup this attached port.\n");
 		break;
 	case RTE_ETH_EVENT_INTR_RMV:
 		if (port_id_is_invalid(port_id, DISABLED_WARN))