[dpdk-dev,v4] net/i40e: fix port segmentation fault when restart

Message ID 20180112065919.64982-1-wei.zhao1@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Zhao1, Wei Jan. 12, 2018, 6:59 a.m. UTC
  This patch will go into the process of clear all queue region
related configuration when dev stop even if there is no queue region
command before, so this is a bug, it may cause error. So add code
to check if there is queue configuration exist when flush queue
region config and remove this process when device stop. Queue region
clear only do when device initialization or PMD get flush command.

Fixes: 7cbecc2f742 ("net/i40e: support queue region set and flush")

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>

---

v2:
-fix patch check warning.

v3:
-add more log message.

v4:
-add more log message.
---
 drivers/net/i40e/i40e_ethdev.c  |  3 ---
 drivers/net/i40e/rte_pmd_i40e.c | 27 ++++++++++++++-------------
 2 files changed, 14 insertions(+), 16 deletions(-)
  

Comments

Zhang, Helin Jan. 13, 2018, 3:29 p.m. UTC | #1
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, January 12, 2018 2:59 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Zhao1, Wei
> Subject: [PATCH v4] net/i40e: fix port segmentation fault when restart
> 
> This patch will go into the process of clear all queue region related
> configuration when dev stop even if there is no queue region command before,
> so this is a bug, it may cause error. So add code to check if there is queue
> configuration exist when flush queue region config and remove this process
> when device stop. Queue region clear only do when device initialization or
> PMD get flush command.
> 
> Fixes: 7cbecc2f742 ("net/i40e: support queue region set and flush")
> 
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel, with minor commit log changes. Thanks!
BTW, I replied v3 email with a mistake, though v4 just modifed the commit log a bit.
Sorry for any confusing!

/Helin
> 
> ---
> 
> v2:
> -fix patch check warning.
> 
> v3:
> -add more log message.
> 
> v4:
> -add more log message.
> ---
>  drivers/net/i40e/i40e_ethdev.c  |  3 ---  drivers/net/i40e/rte_pmd_i40e.c | 27
> ++++++++++++++-------------
>  2 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index d80671a..5d1b916 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2144,9 +2144,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
>  	/* reset hierarchy commit */
>  	pf->tm_conf.committed = false;
> 
> -	/* Remove all the queue region configuration */
> -	i40e_flush_queue_region_all_conf(dev, hw, pf, 0);
> -
>  	hw->adapter_stopped = 1;
>  }
> 
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> b/drivers/net/i40e/rte_pmd_i40e.c index 55ae2fe..2cb22d4 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -2816,22 +2816,23 @@ i40e_flush_queue_region_all_conf(struct
> rte_eth_dev *dev,
>  		return 0;
>  	}
> 
> -	info->queue_region_number = 1;
> -	info->region[0].queue_num = main_vsi->nb_used_qps;
> -	info->region[0].queue_start_index = 0;
> +	if (info->queue_region_number) {
> +		info->queue_region_number = 1;
> +		info->region[0].queue_num = main_vsi->nb_used_qps;
> +		info->region[0].queue_start_index = 0;
> 
> -	ret = i40e_vsi_update_queue_region_mapping(hw, pf);
> -	if (ret != I40E_SUCCESS)
> -		PMD_DRV_LOG(INFO, "Failed to flush queue region mapping.");
> -
> -	ret = i40e_dcb_init_configure(dev, TRUE);
> -	if (ret != I40E_SUCCESS) {
> -		PMD_DRV_LOG(INFO, "Failed to flush dcb.");
> -		pf->flags &= ~I40E_FLAG_DCB;
> -	}
> +		ret = i40e_vsi_update_queue_region_mapping(hw, pf);
> +		if (ret != I40E_SUCCESS)
> +			PMD_DRV_LOG(INFO, "Failed to flush queue region
> mapping.");
> 
> -	i40e_init_queue_region_conf(dev);
> +		ret = i40e_dcb_init_configure(dev, TRUE);
> +		if (ret != I40E_SUCCESS) {
> +			PMD_DRV_LOG(INFO, "Failed to flush dcb.");
> +			pf->flags &= ~I40E_FLAG_DCB;
> +		}
> 
> +		i40e_init_queue_region_conf(dev);
> +	}
>  	return 0;
>  }
> 
> --
> 2.9.3
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d80671a..5d1b916 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2144,9 +2144,6 @@  i40e_dev_stop(struct rte_eth_dev *dev)
 	/* reset hierarchy commit */
 	pf->tm_conf.committed = false;
 
-	/* Remove all the queue region configuration */
-	i40e_flush_queue_region_all_conf(dev, hw, pf, 0);
-
 	hw->adapter_stopped = 1;
 }
 
diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index 55ae2fe..2cb22d4 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -2816,22 +2816,23 @@  i40e_flush_queue_region_all_conf(struct rte_eth_dev *dev,
 		return 0;
 	}
 
-	info->queue_region_number = 1;
-	info->region[0].queue_num = main_vsi->nb_used_qps;
-	info->region[0].queue_start_index = 0;
+	if (info->queue_region_number) {
+		info->queue_region_number = 1;
+		info->region[0].queue_num = main_vsi->nb_used_qps;
+		info->region[0].queue_start_index = 0;
 
-	ret = i40e_vsi_update_queue_region_mapping(hw, pf);
-	if (ret != I40E_SUCCESS)
-		PMD_DRV_LOG(INFO, "Failed to flush queue region mapping.");
-
-	ret = i40e_dcb_init_configure(dev, TRUE);
-	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(INFO, "Failed to flush dcb.");
-		pf->flags &= ~I40E_FLAG_DCB;
-	}
+		ret = i40e_vsi_update_queue_region_mapping(hw, pf);
+		if (ret != I40E_SUCCESS)
+			PMD_DRV_LOG(INFO, "Failed to flush queue region mapping.");
 
-	i40e_init_queue_region_conf(dev);
+		ret = i40e_dcb_init_configure(dev, TRUE);
+		if (ret != I40E_SUCCESS) {
+			PMD_DRV_LOG(INFO, "Failed to flush dcb.");
+			pf->flags &= ~I40E_FLAG_DCB;
+		}
 
+		i40e_init_queue_region_conf(dev);
+	}
 	return 0;
 }