[dpdk-dev,v2] net/i40e: fix port segmentation fault when restart
Checks
Commit Message
It 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 when flush queue
region config and delete it when device stop.
Fixes: 7cbecc2f742 ("net/i40e: support queue region set and flush")
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
v2:
-fix patch check warning.
---
drivers/net/i40e/i40e_ethdev.c | 3 ---
drivers/net/i40e/rte_pmd_i40e.c | 27 ++++++++++++++-------------
2 files changed, 14 insertions(+), 16 deletions(-)
Comments
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Wednesday, November 15, 2017 1:55 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix port segmentation fault when
> restart
>
> It will clear all queue region related configuration when dev stop even if threr
s/threr/there
> is no queue region config, so this may cause error. So add check when flush
> queue region config and delete it when device stop.
>
> Fixes: 7cbecc2f742 ("net/i40e: support queue region set and flush")
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>
> ---
>
> v2:
> -fix patch check warning.
> ---
> 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);
> -
From the commit log, the reason you remove above line is because the function can't handle the situation when no queue region config,
but what about the case that queue region config does exist? Could you add more message to explain why this not be impacted.
> 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.");
WARNING: line over 80 characters
Regards
Qi
>
> - 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.7.4
Hi, zhangqi
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, December 22, 2017 11:31 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix port segmentation fault
> when restart
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Wednesday, November 15, 2017 1:55 PM
> > To: dev@dpdk.org
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix port segmentation fault
> > when restart
> >
> > It will clear all queue region related configuration when dev stop
> > even if threr
>
> s/threr/there
>
> > is no queue region config, so this may cause error. So add check when
> > flush queue region config and delete it when device stop.
> >
> > Fixes: 7cbecc2f742 ("net/i40e: support queue region set and flush")
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >
> > ---
> >
> > v2:
> > -fix patch check warning.
> > ---
> > 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);
> > -
> From the commit log, the reason you remove above line is because the
> function can't handle the situation when no queue region config, but what
> about the case that queue region config does exist? Could you add more
> message to explain why this not be impacted.
Ok， I will add some more info about when queue region config does exist.
In this case, it treat no queue region config as queue region config does exist.
>
> > 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.");
>
> WARNING: line over 80 characters
There is a new rule that I has been told by Ferruh Yigit, log message in double quotation marks can over 80 characters.
> Regards
> Qi
>
> >
> > - 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.7.4
@@ -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;
}
@@ -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;
}