Re: [PATCH v2 2/2] app/testpmd:fix testpmd quit failure

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/3/2022 1:22 PM, Wu, WenxuanX wrote:

moved down, please don't top post

>> -----Original Message-----
>> From: Wu, WenxuanX <wenxuanx.wu@intel.com>
>> Sent: 2022年2月23日 19:33
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; dev@dpdk.org
>> Cc: Wu, WenxuanX <wenxuanx.wu@intel.com>; stable@dpdk.org
>> Subject: [PATCH v2 2/2] app/testpmd:fix testpmd quit failure
>>
>> From: wenxuan wu <wenxuanx.wu@intel.com>
>>
>> When testpmd start ed with 1 pf and 2 vfs, testpmd quited while vfs were
>> still alive would result in failure. Root cause is that pf had been released
>> already but vfs were still accessing by func rte_eth_dev_info_get, which
>> would result in heap-free-after-use error.
>>
>> By quitting our ports in reverse order to avoid this.And the order is
>> guaranteed that vf are created after pfs.
>>
>> Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: wenxuan wu <wenxuanx.wu@intel.com>
>> ---
>>   app/test-pmd/testpmd.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>> e1da961311..698b6d8cc4 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3384,12 +3384,12 @@ pmd_test_exit(void)  #endif
>>   	if (ports != NULL) {
>>   		no_link_check = 1;
>> -		RTE_ETH_FOREACH_DEV(pt_id) {
>> +		RTE_ETH_FOREACH_DEV_REVERSE(pt_id) {
>>   			printf("\nStopping port %d...\n", pt_id);
>>   			fflush(stdout);
>>   			stop_port(pt_id);
>>   		}
>> -		RTE_ETH_FOREACH_DEV(pt_id) {
>> +		RTE_ETH_FOREACH_DEV_REVERSE(pt_id) {
>>   			printf("\nShutting down port %d...\n", pt_id);
>>   			fflush(stdout);
>>   			close_port(pt_id);
>> --
>> 2.25.1
> 
> 
> I found this meaning in DPDK testplan.
> Note that currently hot-plugging of representor ports is not supported so all the required representors must be specified on the creation of the PF or the trusted VF.
> When testpmd is started with pf and vf representors, the order of representor is determined on creation. So it is guaranteed that ,pf is beneath the vf representors, we implemented in a reverse way is acceptable just at present,  depends on when the hot-plugging of representor is supported.
> 

The patch mentions from PF and VFs, and now you are referring
to port representor.
Is the problem related to VF or port representor.

For both, VF and port reporesentor should be closed before
PF, that part is OK. My comment is if reversing port id
traverse will fix the issue or do we need more complex
solution.
Like have APIs to get VF and representor ports from a given
port id, and free them first.