Re: [dpdk-dev] [PATCH v9 08/10] eal: replace rte_panic instances in ethdev

[Kevin Traynor <ktraynor@redhat.com>]

On 04/26/2018 07:21 AM, Arnon Warshavsky wrote:
> Local function to this file,
> changing from void to int is non-abi-breaking
> 
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 42 ++++++++++++++++++++++++++++++------------
>  lib/librte_ether/rte_ethdev.h |  4 +++-
>  2 files changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index f0f53d4..940de15 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -194,7 +194,7 @@ enum {
>  	return port_id;
>  }
>  
> -static void
> +static int
>  rte_eth_dev_shared_data_prepare(void)
>  {
>  	const unsigned flags = 0;
> @@ -210,8 +210,12 @@ enum {
>  					rte_socket_id(), flags);
>  		} else
>  			mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
> -		if (mz == NULL)
> -			rte_panic("Cannot allocate ethdev shared data\n");
> +		if (mz == NULL) {
> +			rte_spinlock_unlock(&rte_eth_shared_data_lock);
> +			RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared data\n",
> +					__func__);
> +			return -1;
> +		}
>  
>  		rte_eth_dev_shared_data = mz->addr;
>  		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> @@ -224,6 +228,8 @@ enum {
>  	}
>  
>  	rte_spinlock_unlock(&rte_eth_shared_data_lock);
> +
> +	return 0;
>  }
>  
>  struct rte_eth_dev *
> @@ -274,7 +280,8 @@ struct rte_eth_dev *
>  	uint16_t port_id;
>  	struct rte_eth_dev *eth_dev = NULL;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return NULL;
>  
>  	/* Synchronize port creation between primary and secondary threads. */
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
> @@ -317,7 +324,8 @@ struct rte_eth_dev *
>  	uint16_t i;
>  	struct rte_eth_dev *eth_dev = NULL;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return NULL;
>  
>  	/* Synchronize port attachment to primary port creation and release. */
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
> @@ -345,7 +353,8 @@ struct rte_eth_dev *
>  	if (eth_dev == NULL)
>  		return -EINVAL;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return -1;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> @@ -399,7 +408,8 @@ struct rte_eth_dev *
>  int __rte_experimental
>  rte_eth_dev_owner_new(uint64_t *owner_id)
>  {
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return -1;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> @@ -450,7 +460,8 @@ struct rte_eth_dev *
>  {
>  	int ret;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return -1;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> @@ -467,7 +478,8 @@ struct rte_eth_dev *
>  			{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
>  	int ret;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return -1;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> @@ -477,12 +489,15 @@ struct rte_eth_dev *
>  	return ret;
>  }
>  
> -void __rte_experimental
> +int __rte_experimental
>  rte_eth_dev_owner_delete(const uint64_t owner_id)
>  {
>  	uint16_t port_id;
> +	int error;

it's inconsistent that this function returns the error code from
rte_eth_dev_shared_data_prepare() and all these other functions return -1

>  
> -	rte_eth_dev_shared_data_prepare();
> +	error = rte_eth_dev_shared_data_prepare();
> +	if (error != 0)
> +		return error;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> @@ -495,6 +510,8 @@ struct rte_eth_dev *
>  	}
>  
>  	rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
> +
> +	return 0;
>  }
>  
>  int __rte_experimental
> @@ -502,7 +519,8 @@ struct rte_eth_dev *
>  {
>  	int ret = 0;
>  
> -	rte_eth_dev_shared_data_prepare();
> +	if (rte_eth_dev_shared_data_prepare() != 0)
> +		return -1;
>  
>  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
>  
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index b9eb8ae..46e5947 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1354,8 +1354,10 @@ int __rte_experimental rte_eth_dev_owner_unset(const uint16_t port_id,
>   *
>   * @param	owner_id
>   *  The owner identifier.
> + *  @return
> + *  0 on success, negative errno value on error.
>   */
> -void __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
> +int __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
>  
>  /**
>   * @warning
>