Re: [PATCH] app/testpmd: fix quit testpmd with vfs and pf

["Singh, Aman Deep" <aman.deep.singh@intel.com>]

On 3/22/2022 12:48 PM, Ke Zhang wrote:
> When testpmd startups with pf and vfs,this error occurs when quitting,
> results in pf is released before vfs ,so the vf would access an
> freed heap memory.
>
> The solution is two steps:
> 1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
> 2. free the port in reverse order.
>
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
This fix looks similar to series: 21823. Please check Ferruh's comments 
on it.

There is a scenario, where this fix may not work.
"if there is an earlier port id which is released, after that new
vf is created taking that port id. In such case port id of vf < pf port id"

This patch covers most of the cases and that way its an improvement.
So I am ok with it.

Acked-by: Aman Singh <aman.deep.singh@intel.com>
>   app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
>   1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index f7e18aee25..ca6c77b14b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
>   void
>   pmd_test_exit(void)
>   {
> +	unsigned int ports[RTE_MAX_ETHPORTS];
> +	unsigned int count = 0;
>   	portid_t pt_id;
>   	unsigned int i;
> +	int index;
>   	int ret;
>   
>   	if (test_done == 0)
> @@ -3396,15 +3399,30 @@ pmd_test_exit(void)
>   #endif
>   	if (ports != NULL) {
>   		no_link_check = 1;
> +		i = 0;
> +
> +		/* Fetch the valid port id from port list*/
>   		RTE_ETH_FOREACH_DEV(pt_id) {
> -			printf("\nStopping port %d...\n", pt_id);
> +			ports[i] = pt_id;
> +			i++;
> +		}
> +
> +		count = i;
> +		/*
> +		 * Free the port from Reverse order, as general,
> +		 * PF port < VF port， VF should be free before PF
> +		 * be free.
> +		 */
> +		for (index = count - 1 ; index >= 0 ; index--) {
> +			printf("\nStopping port %d...\n", ports[index]);
>   			fflush(stdout);
> -			stop_port(pt_id);
> +			stop_port(ports[index]);
>   		}
> -		RTE_ETH_FOREACH_DEV(pt_id) {
> -			printf("\nShutting down port %d...\n", pt_id);
> +
> +		for (index = count - 1 ; index >= 0 ; index--) {
> +			printf("\nShutting down port %d...\n", ports[index]);
>   			fflush(stdout);
> -			close_port(pt_id);
> +			close_port(ports[index]);
>   		}
>   	}
>