[dpdk-dev,12/13] i40e: return -errno when intr setup fails

Message ID 7f9c82cc9331585b82fcf680ffe873700808408f.1481590851.git.mirq-linux@rere.qmqm.pl (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Michał Mirosław Dec. 13, 2016, 1:08 a.m. UTC
  Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
---
 drivers/net/i40e/i40e_ethdev.c               | 5 +++--
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
  

Comments

Ferruh Yigit Dec. 22, 2016, 3:45 p.m. UTC | #1
On 12/13/2016 1:08 AM, Michał Mirosław wrote:
> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> ---
>  drivers/net/i40e/i40e_ethdev.c               | 5 +++--
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 67778ba..39fbcfe 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1692,8 +1692,9 @@ i40e_dev_start(struct rte_eth_dev *dev)
>  	     !RTE_ETH_DEV_SRIOV(dev).active) &&
>  	    dev->data->dev_conf.intr_conf.rxq != 0) {
>  		intr_vector = dev->data->nb_rx_queues;
> -		if (rte_intr_efd_enable(intr_handle, intr_vector))
> -			return -1;
> +		ret = rte_intr_efd_enable(intr_handle, intr_vector);
> +		if (ret)
> +			return ret;

What is the benefit of returning -errno instead of -1?

>  	}
>  
>  	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
> diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> index 47a3b20..f7a8ce3 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> @@ -1157,7 +1157,7 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
>  				RTE_LOG(ERR, EAL,
>  					"can't setup eventfd, error %i (%s)\n",
>  					errno, strerror(errno));
> -				return -1;
> +				return -errno;
>  			}
>  			intr_handle->efds[i] = fd;
>  		}
>
  
Michał Mirosław Dec. 23, 2016, 1:55 a.m. UTC | #2
On Thu, Dec 22, 2016 at 03:45:35PM +0000, Ferruh Yigit wrote:
> On 12/13/2016 1:08 AM, Michał Mirosław wrote:
> > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c               | 5 +++--
> >  lib/librte_eal/linuxapp/eal/eal_interrupts.c | 2 +-
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index 67778ba..39fbcfe 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1692,8 +1692,9 @@ i40e_dev_start(struct rte_eth_dev *dev)
> >  	     !RTE_ETH_DEV_SRIOV(dev).active) &&
> >  	    dev->data->dev_conf.intr_conf.rxq != 0) {
> >  		intr_vector = dev->data->nb_rx_queues;
> > -		if (rte_intr_efd_enable(intr_handle, intr_vector))
> > -			return -1;
> > +		ret = rte_intr_efd_enable(intr_handle, intr_vector);
> > +		if (ret)
> > +			return ret;
> 
> What is the benefit of returning -errno instead of -1?

Information. Besides, all other error returns from i40e_dev_start return
negated error code (-1 happens to be -EPERM, which further confuses
the poor developer who's diagnosing the failure).

Best Regards,
Michał Mirosław
  
Jingjing Wu Dec. 28, 2016, 3:47 a.m. UTC | #3
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw

> Sent: Tuesday, December 13, 2016 9:08 AM

> To: dev@dpdk.org

> Subject: [dpdk-dev] [PATCH 12/13] i40e: return -errno when intr setup fails

> 

> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>

> ---

>  drivers/net/i40e/i40e_ethdev.c               | 5 +++--

>  lib/librte_eal/linuxapp/eal/eal_interrupts.c | 2 +-

>  2 files changed, 4 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c

> index 67778ba..39fbcfe 100644

> --- a/drivers/net/i40e/i40e_ethdev.c

> +++ b/drivers/net/i40e/i40e_ethdev.c

> @@ -1692,8 +1692,9 @@ i40e_dev_start(struct rte_eth_dev *dev)

>  	     !RTE_ETH_DEV_SRIOV(dev).active) &&

>  	    dev->data->dev_conf.intr_conf.rxq != 0) {

>  		intr_vector = dev->data->nb_rx_queues;

> -		if (rte_intr_efd_enable(intr_handle, intr_vector))

> -			return -1;

> +		ret = rte_intr_efd_enable(intr_handle, intr_vector);

> +		if (ret)

> +			return ret;

>  	}

> 

>  	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) { diff --git

> a/lib/librte_eal/linuxapp/eal/eal_interrupts.c

> b/lib/librte_eal/linuxapp/eal/eal_interrupts.c

> index 47a3b20..f7a8ce3 100644

> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c

> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c

> @@ -1157,7 +1157,7 @@ rte_intr_efd_enable(struct rte_intr_handle

> *intr_handle, uint32_t nb_efd)

>  				RTE_LOG(ERR, EAL,

>  					"can't setup eventfd, error %i (%s)\n",

>  					errno, strerror(errno));

> -				return -1;

> +				return -errno;

>  			}

>  			intr_handle->efds[i] = fd;

>  		}

> --


Reviewed-by: Jingjing Wu <jingjing.wu@intel.com>
  
Ferruh Yigit Jan. 11, 2017, 4:09 p.m. UTC | #4
On 12/28/2016 3:47 AM, Wu, Jingjing wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
>> Sent: Tuesday, December 13, 2016 9:08 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH 12/13] i40e: return -errno when intr setup fails
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>

> Reviewed-by: Jingjing Wu <jingjing.wu@intel.com>
> 

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..39fbcfe 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1692,8 +1692,9 @@  i40e_dev_start(struct rte_eth_dev *dev)
 	     !RTE_ETH_DEV_SRIOV(dev).active) &&
 	    dev->data->dev_conf.intr_conf.rxq != 0) {
 		intr_vector = dev->data->nb_rx_queues;
-		if (rte_intr_efd_enable(intr_handle, intr_vector))
-			return -1;
+		ret = rte_intr_efd_enable(intr_handle, intr_vector);
+		if (ret)
+			return ret;
 	}
 
 	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 47a3b20..f7a8ce3 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -1157,7 +1157,7 @@  rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
 				RTE_LOG(ERR, EAL,
 					"can't setup eventfd, error %i (%s)\n",
 					errno, strerror(errno));
-				return -1;
+				return -errno;
 			}
 			intr_handle->efds[i] = fd;
 		}