[dpdk-dev] [PATCH] app/testpmd: fix forward port ids setting

[Matan Azrad]

The corrupted code didn't check the port availability when
it was trying to set the forward port IDs array.
However, when it was counting the number of ports, the availability
was checked by RTE_ETH_FOREACH_DEV iterator.

Hence, even when ETH devices ports were not in ATTACHED state,
the testpmd tried to forward traffic by them and got segmentation
fault at queue access time.

For example:
When EAL command line parameters include two devices, the first
is failsafe with two sub devices and the second is any device,
testpmd gets two devices by the iterator and sets for forwarding
both, the failsafe device and the failsafe first sub device
(instead of the second sub device).
After the first failsafe sub device state was changed to DEFERRED,
testpmd tries to forward traffic through the deferred device
because it didn't check the port availability in setting time.

The fix uses the RTE_ETH_FOREACH_DEV iterator for the forward
port IDs default setting.

Fixes: af75078fece3 ("first public release")
Cc: stable at dpdk.org

Signed-off-by: Matan Azrad <matan at mellanox.com>
---
 app/test-pmd/testpmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Hi All
I would like to bring up a discussion to complete this bug
fix.

When user wants to set the list of forwarding ports
by "set portlist" (testpmd command line), the testpmd
application checks the availability of the ports by 
rte_eth_dev_is_valid_port API.
By this way, it gets the DEFERRED port as valid port and 
will try to recieve\send packets via this port.

This scenario will cause the same error as this patch fixes.

Should testpmd allow user to run traffic by DEFERRED port
directly?

If any application wants to check a port availability for device
usage (conf\rxtx), Which API should be used?

According to the patch cb894d99eceb ("ethdev: add deferred intermediate device state"),
DEFERRED ports should be invisible to application,
So maybe the rte_eth_dev_is_valid_port API should be internal
and a new ethdev API should be created for applications.

What do you think?  

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7d40139..f9bdbf8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -463,9 +463,10 @@ static void
 set_default_fwd_ports_config(void)
 {
 	portid_t pt_id;
+	int i = 0;
 
-	for (pt_id = 0; pt_id < nb_ports; pt_id++)
-		fwd_ports_ids[pt_id] = pt_id;
+	RTE_ETH_FOREACH_DEV(pt_id)
+		fwd_ports_ids[i++] = pt_id;
 
 	nb_cfg_ports = nb_ports;
 	nb_fwd_ports = nb_ports;
-- 
2.7.4