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

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

On 3/28/2022 3:36 PM, Singh, Aman Deep wrote:
>
>
> 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]);
>>           }
>>       }
>