Re: [dpdk-dev] [PATCH v5 1/6] app/testpmd: fix ports list after removing several at once

["Iremonger, Bernard" <bernard.iremonger@intel.com>]

Hi Wisam,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, October 18, 2018 2:24 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; arybchenko@solarflare.com
> Cc: dev@dpdk.org; ophirmu@mellanox.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; rahul.lakkireddy@chelsio.com; Wisam Jaddo
> <wisamm@mellanox.com>
> Subject: [PATCH v5 1/6] app/testpmd: fix ports list after removing several at
> once
> 
> From: Wisam Jaddo <wisamm@mellanox.com>
> 
> When detaching a port, the full rte_device is removed.
> If the rte_device was hosting several ports, the testpmd list of ports must be
> updated for multiple removals.

./devtools/check-git-log.sh -1
Missing 'Fixes' tag:
        app/testpmd: fix ports list after removing several at once
 
> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
> ---
>  app/test-pmd/testpmd.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 5dbbf783f..c4109417a 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2186,6 +2186,30 @@ stop_port(portid_t pid)
>  	printf("Done\n");
>  }
> 
> +static void
> +remove_unused_fwd_ports(void)
> +{
> +	int i;
> +	int last_port_idx = nb_ports - 1;
> +
> +	for (i = 0; i < last_port_idx + 1; i++) { /* iterate in ports_ids */

Why not use  " i <= last_port_index" instead of adding back 1

> +		if (rte_eth_devices[ports_ids[i]].state ==
> RTE_ETH_DEV_UNUSED) {
> +			/* skip unused ports at the end */
> +			while (rte_eth_devices[ports_ids[last_port_idx]].state
> +					== RTE_ETH_DEV_UNUSED && i <=
> last_port_idx)

WARNING:LONG_LINE: line over 80 characters
#48: FILE: app/test-pmd/testpmd.c:2199:
+                                       == RTE_ETH_DEV_UNUSED && i <= last_port_idx)

> +				last_port_idx--;
> +			if (last_port_idx < i)
> +				break;
> +			/* overwrite unused port with last valid port */
> +			ports_ids[i] = ports_ids[last_port_idx];
> +			/* decrease ports count */
> +			last_port_idx--;
> +		}
> +	}
> +	nb_ports = rte_eth_dev_count_avail();
> +	update_fwd_ports(RTE_MAX_ETHPORTS);
> +}
> +
>  void
>  close_port(portid_t pid)
>  {
> @@ -2315,7 +2339,6 @@ void
>  detach_port(portid_t port_id)
>  {
>  	char name[RTE_ETH_NAME_MAX_LEN];
> -	uint16_t i;
> 
>  	printf("Detaching a port...\n");
> 
> @@ -2332,16 +2355,7 @@ detach_port(portid_t port_id)
>  		return;
>  	}
> 
> -	for (i = 0; i < nb_ports; i++) {
> -		if (ports_ids[i] == port_id) {
> -			ports_ids[i] = ports_ids[nb_ports-1];
> -			ports_ids[nb_ports-1] = 0;
> -			break;
> -		}
> -	}
> -	nb_ports = rte_eth_dev_count_avail();
> -
> -	update_fwd_ports(RTE_MAX_ETHPORTS);
> +	remove_unused_fwd_ports();
> 
>  	printf("Port %u is detached. Now total ports is %d\n",
>  			port_id, nb_ports);
> --
> 2.19.0

Regards,

Bernard.