* [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
@ 2018-01-11 13:23 David C Harton
2018-01-11 13:48 ` [dpdk-dev] [PATCH v2] " David C Harton
2018-01-11 14:51 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
0 siblings, 2 replies; 9+ messages in thread
From: David C Harton @ 2018-01-11 13:23 UTC (permalink / raw)
To: jingjing.wu, beilei.xing; +Cc: dev, David C Harton, wei.zhao1
Calling i40evf_dev_xstats_reset can sometimes crash.
Fixed issue by adding a pstats NULL check.
Fixes: da61cd0849766 ("i40evf: add extended stats")
8210e9e0d805e ("net/i40e: fix clear xstats bug in VF")
Cc: wei.zhao1@intel.com
Signed-off-by: David C Harton <dharton@cisco.com>
---
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index b96d77a..9c773fe 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -923,7 +923,8 @@ i40evf_dev_xstats_reset(struct rte_eth_dev *dev)
i40evf_query_stats(dev, &pstats);
/* set stats offset base on current values */
- vf->vsi.eth_stats_offset = *pstats;
+ if (pstats != NULL)
+ vf->vsi.eth_stats_offset = *pstats;
}
static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
--
2.10.3.dirty
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2] net/i40e: fix VF reset stats crash
2018-01-11 13:23 [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash David C Harton
@ 2018-01-11 13:48 ` David C Harton
2018-01-11 15:13 ` [dpdk-dev] [PATCH v3] " David C Harton
2018-01-11 14:51 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
1 sibling, 1 reply; 9+ messages in thread
From: David C Harton @ 2018-01-11 13:48 UTC (permalink / raw)
To: jingjing.wu, beilei.xing; +Cc: dev, David C Harton, wei.zhao1
Calling i40evf_dev_xstats_reset can sometimes crash.
Fixed issue by adding a pstats NULL check.
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.
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index b96d77a..9c773fe 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -923,7 +923,8 @@ i40evf_dev_xstats_reset(struct rte_eth_dev *dev)
i40evf_query_stats(dev, &pstats);
/* set stats offset base on current values */
- vf->vsi.eth_stats_offset = *pstats;
+ if (pstats != NULL)
+ vf->vsi.eth_stats_offset = *pstats;
}
static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
--
2.10.3.dirty
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
2018-01-11 13:23 [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash David C Harton
2018-01-11 13:48 ` [dpdk-dev] [PATCH v2] " David C Harton
@ 2018-01-11 14:51 ` Van Haaren, Harry
2018-01-11 14:59 ` David Harton (dharton)
1 sibling, 1 reply; 9+ messages in thread
From: Van Haaren, Harry @ 2018-01-11 14:51 UTC (permalink / raw)
To: David C Harton, Wu, Jingjing, Xing, Beilei; +Cc: dev, Zhao1, Wei
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David C Harton
> Sent: Thursday, January 11, 2018 1:24 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] net/i40e: fix VF reset stats crash
>
> Calling i40evf_dev_xstats_reset can sometimes crash.
>
> Fixed issue by adding a pstats NULL check.
Perhaps add a note here, that the root cause of (pstats == NULL)
is a valid error return from i40evf_query_stats(), if the vf
command fails to execute.
Hence, we are not hiding symptoms of a different issue here,
this NULL check is required to validate the returned values.
> Fixes: da61cd0849766 ("i40evf: add extended stats")
> 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>
> ---
> drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index b96d77a..9c773fe 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -923,7 +923,8 @@ i40evf_dev_xstats_reset(struct rte_eth_dev *dev)
> i40evf_query_stats(dev, &pstats);
>
> /* set stats offset base on current values */
> - vf->vsi.eth_stats_offset = *pstats;
> + if (pstats != NULL)
> + vf->vsi.eth_stats_offset = *pstats;
> }
>
> static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev
> *dev,
> --
> 2.10.3.dirty
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
2018-01-11 14:51 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
@ 2018-01-11 14:59 ` David Harton (dharton)
2018-01-11 15:02 ` Van Haaren, Harry
0 siblings, 1 reply; 9+ messages in thread
From: David Harton (dharton) @ 2018-01-11 14:59 UTC (permalink / raw)
To: Van Haaren, Harry, Wu, Jingjing, Xing, Beilei; +Cc: dev, Zhao1, Wei
> -----Original Message-----
> From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
>
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David C Harton
> >
> > Calling i40evf_dev_xstats_reset can sometimes crash.
> >
> > Fixed issue by adding a pstats NULL check.
>
> Perhaps add a note here, that the root cause of (pstats == NULL) is a
> valid error return from i40evf_query_stats(), if the vf command fails to
> execute.
>
> Hence, we are not hiding symptoms of a different issue here, this NULL
> check is required to validate the returned values.
>
NULL is returned if the call fails.
If desired I can capture the return code and base the check off the rc instead.
That's the pattern used in a couple other places.
Thanks,
Dave
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
2018-01-11 14:59 ` David Harton (dharton)
@ 2018-01-11 15:02 ` Van Haaren, Harry
0 siblings, 0 replies; 9+ messages in thread
From: Van Haaren, Harry @ 2018-01-11 15:02 UTC (permalink / raw)
To: David Harton (dharton), Wu, Jingjing, Xing, Beilei; +Cc: dev, Zhao1, Wei
> From: David Harton (dharton) [mailto:dharton@cisco.com]
> Sent: Thursday, January 11, 2018 2:59 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
>
>
> > -----Original Message-----
> > From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
> >
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David C Harton
> > >
> > > Calling i40evf_dev_xstats_reset can sometimes crash.
> > >
> > > Fixed issue by adding a pstats NULL check.
> >
> > Perhaps add a note here, that the root cause of (pstats == NULL) is a
> > valid error return from i40evf_query_stats(), if the vf command fails to
> > execute.
> >
> > Hence, we are not hiding symptoms of a different issue here, this NULL
> > check is required to validate the returned values.
> >
>
> NULL is returned if the call fails.
> If desired I can capture the return code and base the check off the rc
> instead.
> That's the pattern used in a couple other places.
If you prefer that implementation of the check, you can keep my Ack, I don't mind much.
I reviewed as in my experience a NULL pointer can often be detected earlier, but that wasn't the case here.
PS: Apologies I didn't reply to v2, was reading "up" and didn't check for newer..
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
2018-01-11 13:48 ` [dpdk-dev] [PATCH v2] " David C Harton
@ 2018-01-11 15:13 ` David C Harton
2018-01-11 17:35 ` Van Haaren, Harry
2018-01-12 2:01 ` Zhao1, Wei
0 siblings, 2 replies; 9+ messages in thread
From: David C Harton @ 2018-01-11 15:13 UTC (permalink / raw)
To: jingjing.wu, beilei.xing; +Cc: dev, David C Harton, wei.zhao1
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
2018-01-11 15:13 ` [dpdk-dev] [PATCH v3] " David C Harton
@ 2018-01-11 17:35 ` Van Haaren, Harry
2018-01-13 6:11 ` Zhang, Helin
2018-01-12 2:01 ` Zhao1, Wei
1 sibling, 1 reply; 9+ messages in thread
From: Van Haaren, Harry @ 2018-01-11 17:35 UTC (permalink / raw)
To: David C Harton, Wu, Jingjing, Xing, Beilei; +Cc: dev, Zhao1, Wei
> 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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
2018-01-11 15:13 ` [dpdk-dev] [PATCH v3] " David C Harton
2018-01-11 17:35 ` Van Haaren, Harry
@ 2018-01-12 2:01 ` Zhao1, Wei
1 sibling, 0 replies; 9+ messages in thread
From: Zhao1, Wei @ 2018-01-12 2:01 UTC (permalink / raw)
To: David C Harton, Wu, Jingjing, Xing, Beilei; +Cc: dev
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: fix VF reset stats crash
2018-01-11 17:35 ` Van Haaren, Harry
@ 2018-01-13 6:11 ` Zhang, Helin
0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Helin @ 2018-01-13 6:11 UTC (permalink / raw)
To: Van Haaren, Harry, David C Harton, Wu, Jingjing, Xing, Beilei
Cc: dev, Zhao1, Wei
> -----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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-01-13 6:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-11 13:23 [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash David C Harton
2018-01-11 13:48 ` [dpdk-dev] [PATCH v2] " David C Harton
2018-01-11 15:13 ` [dpdk-dev] [PATCH v3] " David C Harton
2018-01-11 17:35 ` Van Haaren, Harry
2018-01-13 6:11 ` Zhang, Helin
2018-01-12 2:01 ` Zhao1, Wei
2018-01-11 14:51 ` [dpdk-dev] [PATCH] " Van Haaren, Harry
2018-01-11 14:59 ` David Harton (dharton)
2018-01-11 15:02 ` Van Haaren, Harry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).