[1/2] app/testpmd: fix quit testpmd with vfs and pf

Message ID 20220217101428.385751-2-wenxuanx.wu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix quit testpmd with vfs and pf |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wu, WenxuanX Feb. 17, 2022, 10:14 a.m. UTC
  From: wenxuan wu <wenxuanx.wu@intel.com>

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 that release our allocated ports in reverse
order,add two macros RTE_ETH_FOREACH_DEV_REVERSE_OWNED_BY and
RTE_ETH_FOREACH_DEV_REVERSE,which would be used in quit procedure of
testpmd, error is fixed.

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(-)
  

Comments

Zhang, Yuying Feb. 18, 2022, 8:40 a.m. UTC | #1
Hi Wenxuan,

> -----Original Message-----
> From: Wu, WenxuanX <wenxuanx.wu@intel.com>
> Sent: Thursday, February 17, 2022 6:14 PM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH 1/2] app/testpmd: fix quit testpmd with vfs and pf
> 
> From: wenxuan wu <wenxuanx.wu@intel.com>
> 
> 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 that release our allocated ports in reverse order,add two macros
> RTE_ETH_FOREACH_DEV_REVERSE_OWNED_BY and
> RTE_ETH_FOREACH_DEV_REVERSE,which would be used in quit procedure of
> testpmd, error is fixed.
> 

Please correct grammatical errors and refine your commit log.

> 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
  
Ferruh Yigit Feb. 18, 2022, 4:58 p.m. UTC | #2
On 2/17/2022 10:14 AM, wenxuanx.wu@intel.com wrote:
> From: wenxuan wu <wenxuanx.wu@intel.com>
> 
> 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 that release our allocated ports in reverse
> order,add two macros RTE_ETH_FOREACH_DEV_REVERSE_OWNED_BY and
> RTE_ETH_FOREACH_DEV_REVERSE,which would be used in quit procedure of
> testpmd, error is fixed.
> 
> 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) {

This fix assumes PF is always probed before VFs (so PF has smaller
port id than VF).

I am not sure if this is guaranteed, ports ported based on scan
in the sysfs folder.

Instead, what do you think PF port stop/close fail when there
are still VFs probed? I think PF driver can know if there are
VFs ported.

>   			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);
  

Patch

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);