[dpdk-dev,v3] net/i40e: fix VF reset stats crash

Message ID 20180111151358.20102-1-dharton@cisco.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

David Harton Jan. 11, 2018, 3:13 p.m. UTC
  Calling i40evf_dev_xstats_reset can sometimes crash.

Fixed issue by checking return code before using pstats.

Fixes: 8210e9e0d805e ("net/i40e: fix clear xstats bug in VF")
Cc: wei.zhao1@intel.com

Signed-off-by: David C Harton <dharton@cisco.com>
---

v2:
* Fixed fixes line in commit message.

v3:
* Modified conditional to check ret code instead of pointer.


 drivers/net/i40e/i40e_ethdev_vf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

Comments

Van Haaren, Harry Jan. 11, 2018, 5:35 p.m. UTC | #1
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David C Harton
> Sent: Thursday, January 11, 2018 3:14 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; David C Harton <dharton@cisco.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
> 
> Calling i40evf_dev_xstats_reset can sometimes crash.
> 
> Fixed issue by checking return code before using pstats.
> 
> Fixes: 8210e9e0d805e ("net/i40e: fix clear xstats bug in VF")
> Cc: wei.zhao1@intel.com
> 
> Signed-off-by: David C Harton <dharton@cisco.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
  
Zhao1, Wei Jan. 12, 2018, 2:01 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: David C Harton [mailto:dharton@cisco.com]
> Sent: Thursday, January 11, 2018 11:14 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; David C Harton <dharton@cisco.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: [PATCH v3] net/i40e: fix VF reset stats crash
> 
> Calling i40evf_dev_xstats_reset can sometimes crash.
> 
> Fixed issue by checking return code before using pstats.
> 
> Fixes: 8210e9e0d805e ("net/i40e: fix clear xstats bug in VF")
> Cc: wei.zhao1@intel.com
> 
> Signed-off-by: David C Harton <dharton@cisco.com>
> ---
> 
> v2:
> * Fixed fixes line in commit message.
> 
> v3:
> * Modified conditional to check ret code instead of pointer.
> 
> 
>  drivers/net/i40e/i40e_ethdev_vf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index b96d77a..d5a9edb 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -916,14 +916,16 @@ i40evf_update_stats(struct i40e_vsi *vsi,  static
> void  i40evf_dev_xstats_reset(struct rte_eth_dev *dev)  {
> +	int ret;
>  	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
>  	struct i40e_eth_stats *pstats = NULL;
> 
>  	/* read stat values to clear hardware registers */
> -	i40evf_query_stats(dev, &pstats);
> +	ret = i40evf_query_stats(dev, &pstats);
> 
>  	/* set stats offset base on current values */
> -	vf->vsi.eth_stats_offset = *pstats;
> +	if (ret == 0)
> +		vf->vsi.eth_stats_offset = *pstats;
>  }
> 
>  static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev
> *dev,
> --
> 2.10.3.dirty

Acked-by: Wei Zhao <wei.zhao1@intel.com>
  
Zhang, Helin Jan. 13, 2018, 6:11 a.m. UTC | #3
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Van Haaren, Harry
> Sent: Friday, January 12, 2018 1:36 AM
> To: David C Harton; Wu, Jingjing; Xing, Beilei
> Cc: dev@dpdk.org; Zhao1, Wei
> Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
> 
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David C Harton
> > Sent: Thursday, January 11, 2018 3:14 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; David C Harton <dharton@cisco.com>; Zhao1, Wei
> > <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
> >
> > Calling i40evf_dev_xstats_reset can sometimes crash.
> >
> > Fixed issue by checking return code before using pstats.
> >
> > Fixes: 8210e9e0d805e ("net/i40e: fix clear xstats bug in VF")
> > Cc: wei.zhao1@intel.com
> >
> > Signed-off-by: David C Harton <dharton@cisco.com>
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Applied to dpdk-next-net-intel, with minor commit log changes, thanks!

/Helin
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index b96d77a..d5a9edb 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -916,14 +916,16 @@  i40evf_update_stats(struct i40e_vsi *vsi,
 static void
 i40evf_dev_xstats_reset(struct rte_eth_dev *dev)
 {
+	int ret;
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct i40e_eth_stats *pstats = NULL;
 
 	/* read stat values to clear hardware registers */
-	i40evf_query_stats(dev, &pstats);
+	ret = i40evf_query_stats(dev, &pstats);
 
 	/* set stats offset base on current values */
-	vf->vsi.eth_stats_offset = *pstats;
+	if (ret == 0)
+		vf->vsi.eth_stats_offset = *pstats;
 }
 
 static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,