From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: "David Harton (dharton)" <dharton@cisco.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"Xing, Beilei" <beilei.xing@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Zhao1, Wei" <wei.zhao1@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix VF reset stats crash
Date: Thu, 11 Jan 2018 15:02:10 +0000 [thread overview]
Message-ID: <E923DB57A917B54B9182A2E928D00FA650FEDA5A@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <b1872b8455f2439a8e66f3a71b17f094@XCH-RCD-016.cisco.com>
> 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..
prev parent reply other threads:[~2018-01-11 15:02 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-11 13:23 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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E923DB57A917B54B9182A2E928D00FA650FEDA5A@IRSMSX102.ger.corp.intel.com \
--to=harry.van.haaren@intel.com \
--cc=beilei.xing@intel.com \
--cc=dev@dpdk.org \
--cc=dharton@cisco.com \
--cc=jingjing.wu@intel.com \
--cc=wei.zhao1@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).