Hi, Paul
> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell...@intel.com>
> Sent: Thursday, November 7, 2019 12:10 AM
> To: Su, Simei <simei...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; Ye,
> Xiaolong <xiaolong...@intel.com>; Yang, Qiming <qiming.y...@intel.com>
> Cc: dev@dpdk.org; Su, Simei <simei...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] net/ice: fix segmentation fault for RSS
> init
>
> Disregard my previous email, I didn't see this one. See my comments inline.
>
> Paul
>
> > -----Original Message-----
> > From: dev <dev-boun...@dpdk.org> On Behalf Of Simei Su
> > Sent: Wednesday, November 6, 2019 12:26 AM
> > To: Zhang, Qi Z <qi.z.zh...@intel.com>; Ye, Xiaolong
> > <xiaolong...@intel.com>; Yang, Qiming <qiming.y...@intel.com>
> > Cc: dev@dpdk.org; Su, Simei <simei...@intel.com>
> > Subject: [dpdk-dev] [PATCH v2] net/ice: fix segmentation fault for RSS
> > init
> >
> > This patch fixes core dump issue when entering safe mode with a wrong
> > package file. This patch also fixes build failure issue.
> >
> > Fixes: 5ad3db8d4bdd ("net/ice: enable advanced RSS")
> >
> > Signed-off-by: Simei Su <simei...@intel.com>
> > ---
> > drivers/net/ice/ice_hash.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
> > index
> > 3381b45..08b2502 100644
> > --- a/drivers/net/ice/ice_hash.c
> > +++ b/drivers/net/ice/ice_hash.c
> > @@ -236,12 +236,12 @@ struct ice_hash_match_type
> ice_hash_type_list[]
> > = { static int ice_hash_init(struct ice_adapter *ad) { -struct
> > ice_flow_parser *parser = NULL;
> > +struct ice_flow_parser *parser;
> >
> > -if (ad->active_pkg_type == ICE_PKG_TYPE_OS_DEFAULT) -parser =
> > &ice_hash_parser_os; -else if (ad->active_pkg_type ==
> > ICE_PKG_TYPE_COMMS)
> > +if (ad->active_pkg_type == ICE_PKG_TYPE_COMMS)
> > parser = &ice_hash_parser_comms;
> > +else
> > +parser = &ice_hash_parser_os;
>
> This doesn't make sense to me. Why set up a parser at all if we are in safe
> mode? Safe mode means 1 queue so there isn't anything to RSS to. It seems
> like if we are in safe mode we should just return an error here with a message
> that we are in safe mode.
>
When application starts with a wrong ice.pkg, the output shows:
ice_load_pkg(): failed to allocate buf of size 0 for package
ice_dev_init(): Failed to load the DDP package,Entering Safe Mode
Segmentation fault (core dumped)
I used gdb to track this issue and found it cored dump at ice_hash_init().
It is caused by the pointer *parser is set NULL. If I don't set NULL, the
output shows normally:
ice_load_pkg(): failed to allocate buf of size 0 for package
ice_dev_init(): Failed to load the DDP package,Entering Safe Mode
ice_init_rss(): RSS is not supported in safe mode
As to build failure issue, I used "if, else if" previously and didn't
consider all cases other than "if, else if". So the test report shows:
error: 'parser' may be used uninitialized in this function. I changed it to
"if, else". Thanks!
Br
Simei
> >
> > return ice_register_parser(parser, ad); }
> > --
> > 1.8.3.1
>
