From: "Varghese, Vipin" <vipin.varghese@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>,
"Rao, Nikhil" <nikhil.rao@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Jacob, Jerin" <Jerin.JacobKollanukkaran@cavium.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>,
Hemant Agrawal <hemant.agrawal@nxp.com>,
"Jain, Deepak K" <deepak.k.jain@intel.com>
Subject: Re: [dpdk-dev] [PATCH] event/rx_adapter: fix ignore return of event start
Date: Fri, 2 Feb 2018 08:08:41 +0000 [thread overview]
Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D193E87@BGSMSX101.gar.corp.intel.com> (raw)
In-Reply-To: <20180131065407.GC24795@jerin>
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Wednesday, January 31, 2018 6:54 AM
> To: Rao, Nikhil <nikhil.rao@intel.com>
> Cc: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org; Jacob, Jerin
> <Jerin.JacobKollanukkaran@cavium.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Jain, Deepak K <deepak.k.jain@intel.com>
> Subject: Re: [PATCH] event/rx_adapter: fix ignore return of event start
>
> -----Original Message-----
> >
> >
> > Adding eventdev PMD folks for their suggestions on how to handle the return
> value from rte_event_dev_start() below.
> >
> > > -----Original Message-----
> > > From: Varghese, Vipin
> > > Sent: Wednesday, January 31, 2018 4:26 AM
> > > To: dev@dpdk.org; Rao, Nikhil <nikhil.rao@intel.com>
> > > Cc: Jain, Deepak K <deepak.k.jain@intel.com>; Varghese, Vipin
> > > <vipin.varghese@intel.com>
> > > Subject: [PATCH] event/rx_adapter: fix ignore return of event start
> > >
> > > Capture the return value for rte_event_dev_start. Return the result
> > > back to user.
> > >
> > > Coverity issue: 257000
> > > Fixes: 9c38b704d280 ("eventdev: add eth Rx adapter implementation")
> > > Cc: nikhil.rao@intel.com
> > >
> > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > ---
> > > lib/librte_eventdev/rte_event_eth_rx_adapter.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> > > b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> > > index 90106e6..a818bef 100644
> > > --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> > > +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> > > @@ -603,7 +603,7 @@ static uint16_t gcd_u16(uint16_t a, uint16_t b)
> > > RTE_EDEV_LOG_ERR("failed to configure event dev %u\n",
> > > dev_id);
> > > if (started)
> > > - rte_event_dev_start(dev_id);
> > > + ret = rte_event_dev_start(dev_id);
> >
> > Currently the a non-zero return value at this point signifies an error returned
> from rte_event_dev_configure(), so I suggest that the return value is typecasted
> to void.
>
> If I understand it correctly, Any one of the failure(configure() or start()) should
> result in bad state. Right?
> i.e If some reason PMD is not able to start() even after failure configuration()
> would result in bad state.
> If so, one option could be combine the error like ret |= operation or create a
> new logical error in Rx adapter which denotes this new error.
>
So do we agree to ACK these changes to get the code fix to the mainline? Then rework the logic as required?
> >
> > > return ret;
> > > }
> > >
> > > @@ -617,7 +617,7 @@ static uint16_t gcd_u16(uint16_t a, uint16_t b)
> > > conf->event_port_id = port_id;
> > > conf->max_nb_rx = 128;
> > > if (started)
> > > - rte_event_dev_start(dev_id);
> > > + ret = rte_event_dev_start(dev_id);
> > This change looks good to me.
> >
> > > rx_adapter->default_cb_arg = 1;
> > > return ret;
> > > }
> > > --
> > > 1.9.1
> >
next prev parent reply other threads:[~2018-02-02 8:08 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-30 22:56 Vipin Varghese
2018-01-31 5:32 ` Rao, Nikhil
2018-01-31 6:54 ` Jerin Jacob
2018-02-02 8:08 ` Varghese, Vipin [this message]
2018-02-02 10:04 ` Rao, Nikhil
2018-02-02 12:12 ` Jerin Jacob
2018-02-04 18:18 ` [dpdk-dev] [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb Nikhil Rao
2018-02-06 19:01 ` Jerin Jacob
2018-02-06 20:24 ` Thomas Monjalon
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=4C9E0AB70F954A408CC4ADDBF0F8FA7D4D193E87@BGSMSX101.gar.corp.intel.com \
--to=vipin.varghese@intel.com \
--cc=Jerin.JacobKollanukkaran@cavium.com \
--cc=deepak.k.jain@intel.com \
--cc=dev@dpdk.org \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=nikhil.rao@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).