All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] event/rx_adapter: fix ignore return of event start
@ 2018-01-30 22:56 Vipin Varghese
  2018-01-31  5:32 ` Rao, Nikhil
  2018-02-04 18:18 ` [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb Nikhil Rao
  0 siblings, 2 replies; 9+ messages in thread
From: Vipin Varghese @ 2018-01-30 22:56 UTC (permalink / raw)
  To: dev, nikhil.rao; +Cc: deepak.k.jain, Vipin Varghese

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);
 		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);
 	rx_adapter->default_cb_arg = 1;
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] event/rx_adapter: fix ignore return of event start
  2018-01-30 22:56 [PATCH] event/rx_adapter: fix ignore return of event start Vipin Varghese
@ 2018-01-31  5:32 ` Rao, Nikhil
  2018-01-31  6:54   ` Jerin Jacob
  2018-02-04 18:18 ` [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb Nikhil Rao
  1 sibling, 1 reply; 9+ messages in thread
From: Rao, Nikhil @ 2018-01-31  5:32 UTC (permalink / raw)
  To: Varghese, Vipin, dev, Jacob,  Jerin, Van Haaren, Harry, Hemant Agrawal
  Cc: Jain, Deepak K, Rao, Nikhil


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.

>  		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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] event/rx_adapter: fix ignore return of event start
  2018-01-31  5:32 ` Rao, Nikhil
@ 2018-01-31  6:54   ` Jerin Jacob
  2018-02-02  8:08     ` Varghese, Vipin
  0 siblings, 1 reply; 9+ messages in thread
From: Jerin Jacob @ 2018-01-31  6:54 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: Varghese, Vipin, dev, Jacob,  Jerin, Van Haaren, Harry,
	Hemant Agrawal, Jain, Deepak K

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] event/rx_adapter: fix ignore return of event start
  2018-01-31  6:54   ` Jerin Jacob
@ 2018-02-02  8:08     ` Varghese, Vipin
  2018-02-02 10:04       ` Rao, Nikhil
  0 siblings, 1 reply; 9+ messages in thread
From: Varghese, Vipin @ 2018-02-02  8:08 UTC (permalink / raw)
  To: Jerin Jacob, Rao, Nikhil
  Cc: dev, Jacob,  Jerin, Van Haaren, Harry, Hemant Agrawal, Jain, Deepak K



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] event/rx_adapter: fix ignore return of event start
  2018-02-02  8:08     ` Varghese, Vipin
@ 2018-02-02 10:04       ` Rao, Nikhil
  2018-02-02 12:12         ` Jerin Jacob
  0 siblings, 1 reply; 9+ messages in thread
From: Rao, Nikhil @ 2018-02-02 10:04 UTC (permalink / raw)
  To: Varghese, Vipin, Jerin Jacob
  Cc: dev, Jacob,  Jerin, Van Haaren, Harry, Hemant Agrawal, Jain, Deepak K


> -----Original Message-----
> From: Varghese, Vipin
> Sent: Friday, February 2, 2018 1:39 PM
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Cc: 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-----
> > 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? 

Sorry, if my original email wasn't clear,  if rte_event_dev_configure() returns an error and rte_eventdev_start() returns success that would be a problem, i.e., the fix is incorrect.

Of the 2 options suggested by Jerin - Since ret is not a bitmask  ret |= wouldn't work, if I understand the option correctly . A new error would work.

How about EIO ? and we also update the documentation to indicate that the event device would be in a stopped state if the return code is EIO.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] event/rx_adapter: fix ignore return of event start
  2018-02-02 10:04       ` Rao, Nikhil
@ 2018-02-02 12:12         ` Jerin Jacob
  0 siblings, 0 replies; 9+ messages in thread
From: Jerin Jacob @ 2018-02-02 12:12 UTC (permalink / raw)
  To: Rao, Nikhil
  Cc: Varghese, Vipin, dev, Jacob,  Jerin, Van Haaren, Harry,
	Hemant Agrawal, Jain, Deepak K

-----Original Message-----
> Date: Fri, 2 Feb 2018 10:04:20 +0000
> From: "Rao, Nikhil" <nikhil.rao@intel.com>
> To: "Varghese, Vipin" <vipin.varghese@intel.com>, Jerin Jacob
>  <jerin.jacob@caviumnetworks.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: [PATCH] event/rx_adapter: fix ignore return of event start
> 
> 
> > -----Original Message-----
> > From: Varghese, Vipin
> > Sent: Friday, February 2, 2018 1:39 PM
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Rao, Nikhil
> > <nikhil.rao@intel.com>
> > Cc: 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-----
> > > 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? 
> 
> Sorry, if my original email wasn't clear,  if rte_event_dev_configure() returns an error and rte_eventdev_start() returns success that would be a problem, i.e., the fix is incorrect.
> 
> Of the 2 options suggested by Jerin - Since ret is not a bitmask  ret |= wouldn't work, if I understand the option correctly . A new error would work.
> 
> How about EIO ? and we also update the documentation to indicate that the event device would be in a stopped state if the return code is EIO.

+1 for new error. You may consider EBUSY or EINPROGRESS also.No strong opinion for the name.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb
  2018-01-30 22:56 [PATCH] event/rx_adapter: fix ignore return of event start Vipin Varghese
  2018-01-31  5:32 ` Rao, Nikhil
@ 2018-02-04 18:18 ` Nikhil Rao
  2018-02-06 19:01   ` Jerin Jacob
  1 sibling, 1 reply; 9+ messages in thread
From: Nikhil Rao @ 2018-02-04 18:18 UTC (permalink / raw)
  To: dev; +Cc: nikhil.rao, vipin.varghese, deepak.k.jain, jerin.jacob

The default adapter configuration callback is invoked when a Rx
queue is added to the adapter and the adapter detects that a SW
service is needed. The adapter needs to re-configure the device
with an additional port and to do do, it needs to stop the
device and restart it after it is done reconfiguring it. This
patch adds code to check the return code of
rte_event_dev_start() for both when the reconfiguration fails
and when it succeeds and introduces a new error code (-EIO)
for the first case.

Fixes: 9c38b704d280 ("eventdev: add eth Rx adapter implementation")
Coverity issue: 257000

Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
 lib/librte_eventdev/rte_event_eth_rx_adapter.h | 6 ++++++
 lib/librte_eventdev/rte_event_eth_rx_adapter.c | 8 +++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
index 6a9e7ed..c20507b 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
@@ -321,6 +321,12 @@ int rte_event_eth_rx_adapter_free(uint8_t id);
  * @return
  *  - 0: Success, Receive queue added correctly.
  *  - <0: Error code on failure.
+ *  - (-EIO) device reconfiguration and restart error. The adapter reconfigures
+ *  the event device with an additional port if it is required to use a service
+ *  function for packet transfer from the ethernet device to the event device.
+ *  If the device had been started before this call, this error code indicates
+ *  an error in restart following an error in reconfiguration, i.e., a
+ *  combination of the two error codes.
  */
 int rte_event_eth_rx_adapter_queue_add(uint8_t id,
 			uint8_t eth_dev_id,
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index 90106e6..9aece9f 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -602,8 +602,10 @@ default_conf_cb(uint8_t id, uint8_t dev_id,
 	if (ret) {
 		RTE_EDEV_LOG_ERR("failed to configure event dev %u\n",
 						dev_id);
-		if (started)
-			rte_event_dev_start(dev_id);
+		if (started) {
+			if (rte_event_dev_start(dev_id))
+				return -EIO;
+		}
 		return ret;
 	}
 
@@ -617,7 +619,7 @@ default_conf_cb(uint8_t id, uint8_t dev_id,
 	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);
 	rx_adapter->default_cb_arg = 1;
 	return ret;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb
  2018-02-04 18:18 ` [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
  0 siblings, 1 reply; 9+ messages in thread
From: Jerin Jacob @ 2018-02-06 19:01 UTC (permalink / raw)
  To: Nikhil Rao; +Cc: dev, vipin.varghese, deepak.k.jain

-----Original Message-----
> Date: Sun, 4 Feb 2018 23:48:31 +0530
> From: Nikhil Rao <nikhil.rao@intel.com>
> To: dev@dpdk.org
> CC: nikhil.rao@intel.com, vipin.varghese@intel.com,
>  deepak.k.jain@intel.com, jerin.jacob@caviumnetworks.com
> Subject: [PATCH v2] eventdev: fix unchecked return in default Rx adapter
>  conf cb
> X-Mailer: git-send-email 2.7.4
> 
> The default adapter configuration callback is invoked when a Rx
> queue is added to the adapter and the adapter detects that a SW
> service is needed. The adapter needs to re-configure the device
> with an additional port and to do do, it needs to stop the
> device and restart it after it is done reconfiguring it. This
> patch adds code to check the return code of
> rte_event_dev_start() for both when the reconfiguration fails
> and when it succeeds and introduces a new error code (-EIO)
> for the first case.
> 
> Fixes: 9c38b704d280 ("eventdev: add eth Rx adapter implementation")
> Coverity issue: 257000
> 
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>

Cc: stable@dpdk.org
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] eventdev: fix unchecked return in default Rx adapter conf cb
  2018-02-06 19:01   ` Jerin Jacob
@ 2018-02-06 20:24     ` Thomas Monjalon
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2018-02-06 20:24 UTC (permalink / raw)
  To: Nikhil Rao; +Cc: dev, Jerin Jacob, vipin.varghese, deepak.k.jain

> > The default adapter configuration callback is invoked when a Rx
> > queue is added to the adapter and the adapter detects that a SW
> > service is needed. The adapter needs to re-configure the device
> > with an additional port and to do do, it needs to stop the
> > device and restart it after it is done reconfiguring it. This
> > patch adds code to check the return code of
> > rte_event_dev_start() for both when the reconfiguration fails
> > and when it succeeds and introduces a new error code (-EIO)
> > for the first case.
> > 
> > Fixes: 9c38b704d280 ("eventdev: add eth Rx adapter implementation")
> > Coverity issue: 257000
> > 
> > Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> 
> Cc: stable@dpdk.org
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-02-06 20:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-30 22:56 [PATCH] event/rx_adapter: fix ignore return of event start Vipin Varghese
2018-01-31  5:32 ` Rao, Nikhil
2018-01-31  6:54   ` Jerin Jacob
2018-02-02  8:08     ` Varghese, Vipin
2018-02-02 10:04       ` Rao, Nikhil
2018-02-02 12:12         ` Jerin Jacob
2018-02-04 18:18 ` [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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.