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

Message ID 20180103071721.51986-1-wei.zhao1@intel.com (mailing list archive)
State Superseded, 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. 3, 2018, 7:17 a.m. UTC
  This patch will clear all queue region related configuration
when dev stop even if threr is no queue region config,
so this may cause error. So add 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>

---

v2:
-fix patch check warning.

v3:
-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

Qi Zhang Jan. 3, 2018, 8:19 a.m. UTC | #1
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Wednesday, January 3, 2018 3:17 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [PATCH v3] net/i40e: fix port segmentation fault when restart
> 
> This patch will clear all queue region related configuration when dev stop even
> if threr is no queue region config, so this may cause error. So add 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>
> 
> ---
> 
> v2:
> -fix patch check warning.
> 
> v3:
> -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 811cc9f..7a1290b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2154,9 +2154,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 aeb92af..c2e2466 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -2845,22 +2845,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

Acked-by: Qi Zhang <qi.z.zhang@intel.com>
  
Zhang, Helin Jan. 13, 2018, 6:04 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Wednesday, January 3, 2018 4:20 PM
> To: Zhao1, Wei; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: fix port segmentation fault when
> restart
> 
> 
> 
> > -----Original Message-----
> > From: Zhao1, Wei
> > Sent: Wednesday, January 3, 2018 3:17 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei
> > <wei.zhao1@intel.com>
> > Subject: [PATCH v3] net/i40e: fix port segmentation fault when restart
> >
> > This patch will clear all queue region related configuration when dev
> > stop even if threr is no queue region config, so this may cause error.
> > So add 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>
> >
> > ---
> >
> > v2:
> > -fix patch check warning.
> >
> > v3:
> > -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 811cc9f..7a1290b 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2154,9 +2154,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 aeb92af..c2e2466 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.c
> > +++ b/drivers/net/i40e/rte_pmd_i40e.c
> > @@ -2845,22 +2845,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
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel, with minor commit log changes, thanks!

/Helin
>
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 811cc9f..7a1290b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2154,9 +2154,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 aeb92af..c2e2466 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -2845,22 +2845,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;
 }