Re: [dpdk-dev] [PATCH] event/rx_adapter: fix ignore return of event start

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----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.

>
> >  		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
>