DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] app/testpmd: fix invalid queue ID when start port
@ 2023-07-03 11:02 Jie Hai
  2023-07-03 12:33 ` Ali Alnubani
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Jie Hai @ 2023-07-03 11:02 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3, alialnu

Function update_queue_state updates queue state of all queues
of all ports, using the queue num nb_rxq|nb_txq stored locally
by testpmd. Error on invalid queue ID occurs if we start testpmd
with two ports and detach-attach one of them and start the other
port first. That's because the attached port has zero queues and
that differs from the nb_rxq|nb_txq. The similar error happens
in multi-process senoris if secondary process attaches a port
and starts it.

This patch updates queue state according to the num of queues
reported by driver instead of testpmd.

Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 app/test-pmd/testpmd.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1fc70650e0a4..c8ce67d0de9f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2479,13 +2479,22 @@ update_tx_queue_state(uint16_t port_id, uint16_t queue_id)
 static void
 update_queue_state(void)
 {
+	struct rte_port *port;
+	uint16_t nb_rx_queues;
+	uint16_t nb_tx_queues;
 	portid_t pi;
 	queueid_t qi;
 
 	RTE_ETH_FOREACH_DEV(pi) {
-		for (qi = 0; qi < nb_rxq; qi++)
+		port = &ports[pi];
+		if (eth_dev_info_get_print_err(pi, &port->dev_info) != 0)
+			continue;
+
+		nb_rx_queues = RTE_MIN(nb_rxq, port->dev_info.nb_rx_queues);
+		nb_tx_queues = RTE_MIN(nb_txq, port->dev_info.nb_tx_queues);
+		for (qi = 0; qi < nb_rx_queues; qi++)
 			update_rx_queue_state(pi, qi);
-		for (qi = 0; qi < nb_txq; qi++)
+		for (qi = 0; qi < nb_tx_queues; qi++)
 			update_tx_queue_state(pi, qi);
 	}
 }
-- 
2.33.0


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

* RE: [PATCH] app/testpmd: fix invalid queue ID when start port
  2023-07-03 11:02 [PATCH] app/testpmd: fix invalid queue ID when start port Jie Hai
@ 2023-07-03 12:33 ` Ali Alnubani
  2023-07-04  2:01   ` Jie Hai
  2023-07-04  2:22 ` lihuisong (C)
  2023-07-04  8:45 ` [PATCH v2] " Jie Hai
  2 siblings, 1 reply; 14+ messages in thread
From: Ali Alnubani @ 2023-07-03 12:33 UTC (permalink / raw)
  To: Jie Hai, Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3

> -----Original Message-----
> From: Jie Hai <haijie1@huawei.com>
> Sent: Monday, July 3, 2023 2:03 PM
> To: Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Shiyang He
> <shiyangx.he@intel.com>
> Cc: dev@dpdk.org; liudongdong3@huawei.com; Ali Alnubani
> <alialnu@nvidia.com>
> Subject: [PATCH] app/testpmd: fix invalid queue ID when start port
> 
> Function update_queue_state updates queue state of all queues
> of all ports, using the queue num nb_rxq|nb_txq stored locally
> by testpmd. Error on invalid queue ID occurs if we start testpmd
> with two ports and detach-attach one of them and start the other
> port first. That's because the attached port has zero queues and
> that differs from the nb_rxq|nb_txq. The similar error happens
> in multi-process senoris if secondary process attaches a port

Do you mean scenarios?

> and starts it.
> 
> This patch updates queue state according to the num of queues
> reported by driver instead of testpmd.
> 
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
> queues")
> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
> forwarding")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---

Thanks Jie.

Tested-by: Ali Alnubani <alialnu@nvidia.com>

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

* Re: [PATCH] app/testpmd: fix invalid queue ID when start port
  2023-07-03 12:33 ` Ali Alnubani
@ 2023-07-04  2:01   ` Jie Hai
  0 siblings, 0 replies; 14+ messages in thread
From: Jie Hai @ 2023-07-04  2:01 UTC (permalink / raw)
  To: Ali Alnubani, Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3

On 2023/7/3 20:33, Ali Alnubani wrote:
>> -----Original Message-----
>> From: Jie Hai <haijie1@huawei.com>
>> Sent: Monday, July 3, 2023 2:03 PM
>> To: Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
>> <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Shiyang He
>> <shiyangx.he@intel.com>
>> Cc: dev@dpdk.org; liudongdong3@huawei.com; Ali Alnubani
>> <alialnu@nvidia.com>
>> Subject: [PATCH] app/testpmd: fix invalid queue ID when start port
>>
>> Function update_queue_state updates queue state of all queues
>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>> by testpmd. Error on invalid queue ID occurs if we start testpmd
>> with two ports and detach-attach one of them and start the other
>> port first. That's because the attached port has zero queues and
>> that differs from the nb_rxq|nb_txq. The similar error happens
>> in multi-process senoris if secondary process attaches a port
> 
> Do you mean scenarios?
Yes, sorry for the spelling mistake. I'll correct it.
> 
>> and starts it.
>>
>> This patch updates queue state according to the num of queues
>> reported by driver instead of testpmd.
>>
>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
>> queues")
>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>> forwarding")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
> 
> Thanks Jie.
> 
> Tested-by: Ali Alnubani <alialnu@nvidia.com>

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

* Re: [PATCH] app/testpmd: fix invalid queue ID when start port
  2023-07-03 11:02 [PATCH] app/testpmd: fix invalid queue ID when start port Jie Hai
  2023-07-03 12:33 ` Ali Alnubani
@ 2023-07-04  2:22 ` lihuisong (C)
  2023-07-04  8:45 ` [PATCH v2] " Jie Hai
  2 siblings, 0 replies; 14+ messages in thread
From: lihuisong (C) @ 2023-07-04  2:22 UTC (permalink / raw)
  To: Jie Hai, Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3, alialnu


在 2023/7/3 19:02, Jie Hai 写道:
> Function update_queue_state updates queue state of all queues
> of all ports, using the queue num nb_rxq|nb_txq stored locally
> by testpmd. Error on invalid queue ID occurs if we start testpmd
> with two ports and detach-attach one of them and start the other
How do start the first port? start packet forwarding?
> port first. That's because the attached port has zero queues and
> that differs from the nb_rxq|nb_txq. The similar error happens
> in multi-process senoris if secondary process attaches a port
> and starts it.
>
> This patch updates queue state according to the num of queues
> reported by driver instead of testpmd.
>
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
> Cc: stable@dpdk.org
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>   app/test-pmd/testpmd.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 1fc70650e0a4..c8ce67d0de9f 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2479,13 +2479,22 @@ update_tx_queue_state(uint16_t port_id, uint16_t queue_id)
>   static void
>   update_queue_state(void)
>   {
> +	struct rte_port *port;
> +	uint16_t nb_rx_queues;
> +	uint16_t nb_tx_queues;
>   	portid_t pi;
>   	queueid_t qi;
>   
>   	RTE_ETH_FOREACH_DEV(pi) {
> -		for (qi = 0; qi < nb_rxq; qi++)
> +		port = &ports[pi];
> +		if (eth_dev_info_get_print_err(pi, &port->dev_info) != 0)
> +			continue;
Is this something to do for the dettach-attached port?
If so I don't think this is the root method.
Because the global ports[attached_port_id] isn't initialized, and the 
attached port isn't started.
And the first port cannot be start to forward(start_packet_forwarding 
will do check this).
Another possible reason is that the attached port status is incorrect.

Please take a look at the following patchset.
http://patches.dpdk.org/project/dpdk/list/?series=&submitter=2085&state=&q=&archive=&delegate=
> +
> +		nb_rx_queues = RTE_MIN(nb_rxq, port->dev_info.nb_rx_queues);
> +		nb_tx_queues = RTE_MIN(nb_txq, port->dev_info.nb_tx_queues);
> +		for (qi = 0; qi < nb_rx_queues; qi++)
>   			update_rx_queue_state(pi, qi);
> -		for (qi = 0; qi < nb_txq; qi++)
> +		for (qi = 0; qi < nb_tx_queues; qi++)
>   			update_tx_queue_state(pi, qi);
>   	}
>   }

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

* [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-03 11:02 [PATCH] app/testpmd: fix invalid queue ID when start port Jie Hai
  2023-07-03 12:33 ` Ali Alnubani
  2023-07-04  2:22 ` lihuisong (C)
@ 2023-07-04  8:45 ` Jie Hai
  2023-07-04  9:16   ` Ali Alnubani
                     ` (2 more replies)
  2 siblings, 3 replies; 14+ messages in thread
From: Jie Hai @ 2023-07-04  8:45 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3, alialnu

Function update_queue_state updates queue state of all queues
of all ports, using the queue num nb_rxq|nb_txq stored locally
by testpmd. An error on the invalid queue ID occurs if we run
testpmd with two ports and detach-attach one of them and start
the other port first. This is because the attached port has not
been configured and has no queues, which differs from nb_rxq|nb_txq.
The similar error happens in multi-process senoris if secondary
process attaches a port and starts it.

This patch updates queue state of the specified port, which has
been configured by primary process. As the secondary process
cannot configure the ports, make sure that the secondary process
starts the port only after the primary process has done so.

Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 app/test-pmd/testpmd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--
v1->v2:
1. Fix spelling mistakes.
2. Update queue state of a specified port insead of all port in start_port()

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1fc70650e0a4..904184d0836b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2477,12 +2477,15 @@ update_tx_queue_state(uint16_t port_id, uint16_t queue_id)
 }
 
 static void
-update_queue_state(void)
+update_queue_state(portid_t pid)
 {
 	portid_t pi;
 	queueid_t qi;
 
 	RTE_ETH_FOREACH_DEV(pi) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
 		for (qi = 0; qi < nb_rxq; qi++)
 			update_rx_queue_state(pi, qi);
 		for (qi = 0; qi < nb_txq; qi++)
@@ -2530,7 +2533,7 @@ start_packet_forwarding(int with_tx_first)
 		return;
 
 	if (stream_init != NULL) {
-		update_queue_state();
+		update_queue_state(RTE_PORT_ALL);
 		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
 			stream_init(fwd_streams[i]);
 	}
@@ -3293,7 +3296,7 @@ start_port(portid_t pid)
 		pl[cfg_pi++] = pi;
 	}
 
-	update_queue_state();
+	update_queue_state(pid);
 
 	if (at_least_one_port_successfully_started && !no_link_check)
 		check_all_ports_link_status(RTE_PORT_ALL);
-- 
2.33.0


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

* RE: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-04  8:45 ` [PATCH v2] " Jie Hai
@ 2023-07-04  9:16   ` Ali Alnubani
  2023-07-04  9:42   ` fengchengwen
  2023-07-04 10:59   ` Ferruh Yigit
  2 siblings, 0 replies; 14+ messages in thread
From: Ali Alnubani @ 2023-07-04  9:16 UTC (permalink / raw)
  To: Jie Hai, Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3

> -----Original Message-----
> From: Jie Hai <haijie1@huawei.com>
> Sent: Tuesday, July 4, 2023 11:45 AM
> To: Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Shiyang He
> <shiyangx.he@intel.com>
> Cc: dev@dpdk.org; liudongdong3@huawei.com; Ali Alnubani
> <alialnu@nvidia.com>
> Subject: [PATCH v2] app/testpmd: fix invalid queue ID when start port
> 
> Function update_queue_state updates queue state of all queues
> of all ports, using the queue num nb_rxq|nb_txq stored locally
> by testpmd. An error on the invalid queue ID occurs if we run
> testpmd with two ports and detach-attach one of them and start
> the other port first. This is because the attached port has not
> been configured and has no queues, which differs from nb_rxq|nb_txq.
> The similar error happens in multi-process senoris if secondary
> process attaches a port and starts it.
> 
> This patch updates queue state of the specified port, which has
> been configured by primary process. As the secondary process
> cannot configure the ports, make sure that the secondary process
> starts the port only after the primary process has done so.
> 
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all
> queues")
> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
> forwarding")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>  app/test-pmd/testpmd.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> --

Thanks Jie.

Tested-by: Ali Alnubani <alialnu@nvidia.com>

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-04  8:45 ` [PATCH v2] " Jie Hai
  2023-07-04  9:16   ` Ali Alnubani
@ 2023-07-04  9:42   ` fengchengwen
  2023-07-04 10:59   ` Ferruh Yigit
  2 siblings, 0 replies; 14+ messages in thread
From: fengchengwen @ 2023-07-04  9:42 UTC (permalink / raw)
  To: Jie Hai, Aman Singh, Yuying Zhang, Ferruh Yigit, Shiyang He
  Cc: dev, liudongdong3, alialnu

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/7/4 16:45, Jie Hai wrote:
> Function update_queue_state updates queue state of all queues
> of all ports, using the queue num nb_rxq|nb_txq stored locally
> by testpmd. An error on the invalid queue ID occurs if we run
> testpmd with two ports and detach-attach one of them and start
> the other port first. This is because the attached port has not
> been configured and has no queues, which differs from nb_rxq|nb_txq.
> The similar error happens in multi-process senoris if secondary
> process attaches a port and starts it.
> 
> This patch updates queue state of the specified port, which has
> been configured by primary process. As the secondary process
> cannot configure the ports, make sure that the secondary process
> starts the port only after the primary process has done so.
> 
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
...

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-04  8:45 ` [PATCH v2] " Jie Hai
  2023-07-04  9:16   ` Ali Alnubani
  2023-07-04  9:42   ` fengchengwen
@ 2023-07-04 10:59   ` Ferruh Yigit
  2023-07-05  3:16     ` lihuisong (C)
  2 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2023-07-04 10:59 UTC (permalink / raw)
  To: Jie Hai, Aman Singh, Yuying Zhang, Shiyang He; +Cc: dev, liudongdong3, alialnu

On 7/4/2023 9:45 AM, Jie Hai wrote:
> Function update_queue_state updates queue state of all queues
> of all ports, using the queue num nb_rxq|nb_txq stored locally
> by testpmd. An error on the invalid queue ID occurs if we run
> testpmd with two ports and detach-attach one of them and start
> the other port first. This is because the attached port has not
> been configured and has no queues, which differs from nb_rxq|nb_txq.
> The similar error happens in multi-process senoris if secondary
> process attaches a port and starts it.
> 
> This patch updates queue state of the specified port, which has
> been configured by primary process. As the secondary process
> cannot configure the ports, make sure that the secondary process
> starts the port only after the primary process has done so.
> 
> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
>

The problem description and solution looks reasonable to me, but Intel
testing still reporting the issue.

There is a chance that the issue Intel side observing is different,
waiting for more information from Intel test team.


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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-04 10:59   ` Ferruh Yigit
@ 2023-07-05  3:16     ` lihuisong (C)
  2023-07-05  8:02       ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2023-07-05  3:16 UTC (permalink / raw)
  To: Ferruh Yigit, Jie Hai, Aman Singh, Yuying Zhang, Shiyang He
  Cc: dev, liudongdong3, alialnu


在 2023/7/4 18:59, Ferruh Yigit 写道:
> On 7/4/2023 9:45 AM, Jie Hai wrote:
>> Function update_queue_state updates queue state of all queues
>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>> by testpmd. An error on the invalid queue ID occurs if we run
>> testpmd with two ports and detach-attach one of them and start
>> the other port first. This is because the attached port has not
>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>> The similar error happens in multi-process senoris if secondary
>> process attaches a port and starts it.
>>
>> This patch updates queue state of the specified port, which has
>> been configured by primary process. As the secondary process
>> cannot configure the ports, make sure that the secondary process
>> starts the port only after the primary process has done so.
Now look good to me.
Acked-by: Huisong Li <lihuisong@huawei.com>
>>
>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>
> The problem description and solution looks reasonable to me, but Intel
> testing still reporting the issue.
>
> There is a chance that the issue Intel side observing is different,
> waiting for more information from Intel test team.
>
> .

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-05  3:16     ` lihuisong (C)
@ 2023-07-05  8:02       ` Ferruh Yigit
  2023-07-05  8:07         ` Ferruh Yigit
  2023-07-05  9:40         ` lihuisong (C)
  0 siblings, 2 replies; 14+ messages in thread
From: Ferruh Yigit @ 2023-07-05  8:02 UTC (permalink / raw)
  To: lihuisong (C),
	Jie Hai, Aman Singh, Yuying Zhang, Shiyang He, Jiale, SongX
  Cc: dev, liudongdong3, alialnu, Mcnamara, John, Qi Z Zhang,
	Thomas Monjalon, David Marchand

On 7/5/2023 4:16 AM, lihuisong (C) wrote:
> 
> 在 2023/7/4 18:59, Ferruh Yigit 写道:
>> On 7/4/2023 9:45 AM, Jie Hai wrote:
>>> Function update_queue_state updates queue state of all queues
>>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>>> by testpmd. An error on the invalid queue ID occurs if we run
>>> testpmd with two ports and detach-attach one of them and start
>>> the other port first. This is because the attached port has not
>>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>>> The similar error happens in multi-process senoris if secondary
>>> process attaches a port and starts it.
>>>
>>> This patch updates queue state of the specified port, which has
>>> been configured by primary process. As the secondary process
>>> cannot configure the ports, make sure that the secondary process
>>> starts the port only after the primary process has done so.
> Now look good to me.
> Acked-by: Huisong Li <lihuisong@huawei.com>
>>>
>>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling
>>> all queues")
>>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>>> forwarding")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>>
>> The problem description and solution looks reasonable to me, but Intel
>> testing still reporting the issue.
>>
>> There is a chance that the issue Intel side observing is different,
>> waiting for more information from Intel test team.
>>
>> .

Hi Song,

As far as I understand this patch works with an update from ixgbevf
driver, can you please confirm?
And can we have the ixgbevf fix soon, to not block the -rc3?

Thanks,
ferruh

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-05  8:02       ` Ferruh Yigit
@ 2023-07-05  8:07         ` Ferruh Yigit
  2023-07-05  9:40         ` lihuisong (C)
  1 sibling, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2023-07-05  8:07 UTC (permalink / raw)
  To: lihuisong (C),
	Jie Hai, Aman Singh, Yuying Zhang, Shiyang He, Jiale, SongX
  Cc: dev, liudongdong3, alialnu, Mcnamara, John, Qi Z Zhang,
	Thomas Monjalon, David Marchand

On 7/5/2023 9:02 AM, Ferruh Yigit wrote:
> On 7/5/2023 4:16 AM, lihuisong (C) wrote:
>>
>> 在 2023/7/4 18:59, Ferruh Yigit 写道:
>>> On 7/4/2023 9:45 AM, Jie Hai wrote:
>>>> Function update_queue_state updates queue state of all queues
>>>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>>>> by testpmd. An error on the invalid queue ID occurs if we run
>>>> testpmd with two ports and detach-attach one of them and start
>>>> the other port first. This is because the attached port has not
>>>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>>>> The similar error happens in multi-process senoris if secondary
>>>> process attaches a port and starts it.
>>>>
>>>> This patch updates queue state of the specified port, which has
>>>> been configured by primary process. As the secondary process
>>>> cannot configure the ports, make sure that the secondary process
>>>> starts the port only after the primary process has done so.
>> Now look good to me.
>> Acked-by: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling
>>>> all queues")
>>>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>>>> forwarding")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>>>
>>> The problem description and solution looks reasonable to me, but Intel
>>> testing still reporting the issue.
>>>
>>> There is a chance that the issue Intel side observing is different,
>>> waiting for more information from Intel test team.
>>>
>>> .
> 
> Hi Song,
> 
> As far as I understand this patch works with an update from ixgbevf
> driver, can you please confirm?
> And can we have the ixgbevf fix soon, to not block the -rc3?
> 
> 

To record, the problem is testpmd change was to get queue status from
ethdev but ixgbe is not setting queue status correctly.


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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-05  8:02       ` Ferruh Yigit
  2023-07-05  8:07         ` Ferruh Yigit
@ 2023-07-05  9:40         ` lihuisong (C)
  2023-07-05 11:41           ` Ferruh Yigit
  1 sibling, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2023-07-05  9:40 UTC (permalink / raw)
  To: Ferruh Yigit, Jie Hai
  Cc: dev, liudongdong3, alialnu, Mcnamara, John, Qi Z Zhang,
	Yuying Zhang, Thomas Monjalon, Shiyang He, Aman Singh, Jiale,
	SongX, David Marchand


在 2023/7/5 16:02, Ferruh Yigit 写道:
> On 7/5/2023 4:16 AM, lihuisong (C) wrote:
>> 在 2023/7/4 18:59, Ferruh Yigit 写道:
>>> On 7/4/2023 9:45 AM, Jie Hai wrote:
>>>> Function update_queue_state updates queue state of all queues
>>>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>>>> by testpmd. An error on the invalid queue ID occurs if we run
>>>> testpmd with two ports and detach-attach one of them and start
>>>> the other port first. This is because the attached port has not
>>>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>>>> The similar error happens in multi-process senoris if secondary
>>>> process attaches a port and starts it.
>>>>
>>>> This patch updates queue state of the specified port, which has
>>>> been configured by primary process. As the secondary process
>>>> cannot configure the ports, make sure that the secondary process
>>>> starts the port only after the primary process has done so.
>> Now look good to me.
>> Acked-by: Huisong Li <lihuisong@huawei.com>
>>>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling
>>>> all queues")
>>>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>>>> forwarding")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>>>
>>> The problem description and solution looks reasonable to me, but Intel
>>> testing still reporting the issue.
>>>
>>> There is a chance that the issue Intel side observing is different,
>>> waiting for more information from Intel test team.
>>>
>>> .
> Hi Song,
>
> As far as I understand this patch works with an update from ixgbevf
> driver, can you please confirm?
> And can we have the ixgbevf fix soon, to not block the -rc3?
>
Hi Ferruh,

Yes, ixgbe is not setting queue status correctly.
Whether tesptmd polls the queue depends on the queue status 
(dev->data->rx_queue_state[queue_id]).
But some drivers (not just ixgbe) do not set the status correctly.
Jie is doing this. will be sent ASAP.

/Huisong
> .

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-05  9:40         ` lihuisong (C)
@ 2023-07-05 11:41           ` Ferruh Yigit
  2023-07-06  2:48             ` lihuisong (C)
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2023-07-05 11:41 UTC (permalink / raw)
  To: lihuisong (C), Jie Hai
  Cc: dev, liudongdong3, alialnu, Mcnamara, John, Qi Z Zhang,
	Yuying Zhang, Thomas Monjalon, Shiyang He, Aman Singh, Jiale,
	SongX, David Marchand

On 7/5/2023 10:40 AM, lihuisong (C) wrote:
> 
> 在 2023/7/5 16:02, Ferruh Yigit 写道:
>> On 7/5/2023 4:16 AM, lihuisong (C) wrote:
>>> 在 2023/7/4 18:59, Ferruh Yigit 写道:
>>>> On 7/4/2023 9:45 AM, Jie Hai wrote:
>>>>> Function update_queue_state updates queue state of all queues
>>>>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>>>>> by testpmd. An error on the invalid queue ID occurs if we run
>>>>> testpmd with two ports and detach-attach one of them and start
>>>>> the other port first. This is because the attached port has not
>>>>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>>>>> The similar error happens in multi-process senoris if secondary
>>>>> process attaches a port and starts it.
>>>>>
>>>>> This patch updates queue state of the specified port, which has
>>>>> been configured by primary process. As the secondary process
>>>>> cannot configure the ports, make sure that the secondary process
>>>>> starts the port only after the primary process has done so.
>>> Now look good to me.
>>> Acked-by: Huisong Li <lihuisong@huawei.com>
>>>>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling
>>>>> all queues")
>>>>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>>>>> forwarding")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>>>>
>>>> The problem description and solution looks reasonable to me, but Intel
>>>> testing still reporting the issue.
>>>>
>>>> There is a chance that the issue Intel side observing is different,
>>>> waiting for more information from Intel test team.
>>>>
>>>> .
>> Hi Song,
>>
>> As far as I understand this patch works with an update from ixgbevf
>> driver, can you please confirm?
>> And can we have the ixgbevf fix soon, to not block the -rc3?
>>
> Hi Ferruh,
> 
> Yes, ixgbe is not setting queue status correctly.
> Whether tesptmd polls the queue depends on the queue status
> (dev->data->rx_queue_state[queue_id]).
> But some drivers (not just ixgbe) do not set the status correctly.
> Jie is doing this. will be sent ASAP.
> 

Hi Huisong, Jie

As you said some drivers don't do it right, and my concern is:
a) if we have a side effect from last minute changes from drivers,
b) if there are more drivers impacted but not recognized yet

That is why my proposal is to revert Jie's fix for this release, record
known issue, and merge it back early next release to give more time to
drivers adjust. What do you think?

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

* Re: [PATCH v2] app/testpmd: fix invalid queue ID when start port
  2023-07-05 11:41           ` Ferruh Yigit
@ 2023-07-06  2:48             ` lihuisong (C)
  0 siblings, 0 replies; 14+ messages in thread
From: lihuisong (C) @ 2023-07-06  2:48 UTC (permalink / raw)
  To: Ferruh Yigit, Jie Hai
  Cc: dev, liudongdong3, alialnu, Mcnamara, John, Qi Z Zhang,
	Yuying Zhang, Thomas Monjalon, Shiyang He, Aman Singh, Jiale,
	SongX, David Marchand, lihuisong


在 2023/7/5 19:41, Ferruh Yigit 写道:
> On 7/5/2023 10:40 AM, lihuisong (C) wrote:
>> 在 2023/7/5 16:02, Ferruh Yigit 写道:
>>> On 7/5/2023 4:16 AM, lihuisong (C) wrote:
>>>> 在 2023/7/4 18:59, Ferruh Yigit 写道:
>>>>> On 7/4/2023 9:45 AM, Jie Hai wrote:
>>>>>> Function update_queue_state updates queue state of all queues
>>>>>> of all ports, using the queue num nb_rxq|nb_txq stored locally
>>>>>> by testpmd. An error on the invalid queue ID occurs if we run
>>>>>> testpmd with two ports and detach-attach one of them and start
>>>>>> the other port first. This is because the attached port has not
>>>>>> been configured and has no queues, which differs from nb_rxq|nb_txq.
>>>>>> The similar error happens in multi-process senoris if secondary
>>>>>> process attaches a port and starts it.
>>>>>>
>>>>>> This patch updates queue state of the specified port, which has
>>>>>> been configured by primary process. As the secondary process
>>>>>> cannot configure the ports, make sure that the secondary process
>>>>>> starts the port only after the primary process has done so.
>>>> Now look good to me.
>>>> Acked-by: Huisong Li <lihuisong@huawei.com>
>>>>>> Fixes: 141a520b35f7 ("app/testpmd: fix primary process not polling
>>>>>> all queues")
>>>>>> Fixes: 5028f207a4fa ("app/testpmd: fix secondary process packet
>>>>>> forwarding")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>>>>>
>>>>> The problem description and solution looks reasonable to me, but Intel
>>>>> testing still reporting the issue.
>>>>>
>>>>> There is a chance that the issue Intel side observing is different,
>>>>> waiting for more information from Intel test team.
>>>>>
>>>>> .
>>> Hi Song,
>>>
>>> As far as I understand this patch works with an update from ixgbevf
>>> driver, can you please confirm?
>>> And can we have the ixgbevf fix soon, to not block the -rc3?
>>>
>> Hi Ferruh,
>>
>> Yes, ixgbe is not setting queue status correctly.
>> Whether tesptmd polls the queue depends on the queue status
>> (dev->data->rx_queue_state[queue_id]).
>> But some drivers (not just ixgbe) do not set the status correctly.
>> Jie is doing this. will be sent ASAP.
>>
> Hi Huisong, Jie
>
> As you said some drivers don't do it right, and my concern is:
> a) if we have a side effect from last minute changes from drivers,
> b) if there are more drivers impacted but not recognized yet
>
> That is why my proposal is to revert Jie's fix for this release, record
> known issue, and merge it back early next release to give more time to
> drivers adjust. What do you think?
Agreed.
And we can also take a look at Jie's fix for other drivers later.

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

end of thread, other threads:[~2023-07-06  2:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-03 11:02 [PATCH] app/testpmd: fix invalid queue ID when start port Jie Hai
2023-07-03 12:33 ` Ali Alnubani
2023-07-04  2:01   ` Jie Hai
2023-07-04  2:22 ` lihuisong (C)
2023-07-04  8:45 ` [PATCH v2] " Jie Hai
2023-07-04  9:16   ` Ali Alnubani
2023-07-04  9:42   ` fengchengwen
2023-07-04 10:59   ` Ferruh Yigit
2023-07-05  3:16     ` lihuisong (C)
2023-07-05  8:02       ` Ferruh Yigit
2023-07-05  8:07         ` Ferruh Yigit
2023-07-05  9:40         ` lihuisong (C)
2023-07-05 11:41           ` Ferruh Yigit
2023-07-06  2:48             ` lihuisong (C)

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