Re: [dpdk-dev] [PATCH v2 01/11] ethdev: change eth dev stop function to return int

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/15/2020 2:30 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> 
> Change rte_eth_dev_stop() return value from void to int
> and return negative errno values in case of error conditions.
> Also update the usage of the function in ethdev according to
> the new return type.
> 
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

<...>

> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
> index f8686a50db..c8c30937fa 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -355,6 +355,9 @@ API Changes
>   * vhost: Add a new function ``rte_vhost_crypto_driver_start`` to be called
>     instead of ``rte_vhost_driver_start`` by crypto applications.
>   
> +* ethdev: changed ``rte_eth_dev_stop`` return value from ``void`` to
> +  ``int`` to provide a way to report various error conditions.
> +
>  

If there will be a new version, there is a ethdev block already in this section 
can you please move the paragraph up there?

>   ABI Changes
>   -----------
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index d9b82df073..b8cf04ef4d 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1661,7 +1661,7 @@ rte_eth_dev_start(uint16_t port_id)
>   	struct rte_eth_dev *dev;
>   	struct rte_eth_dev_info dev_info;
>   	int diag;
> -	int ret;
> +	int ret, ret_stop;
>   
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>   
> @@ -1695,7 +1695,13 @@ rte_eth_dev_start(uint16_t port_id)
>   		RTE_ETHDEV_LOG(ERR,
>   			"Error during restoring configuration for device (port %u): %s\n",
>   			port_id, rte_strerror(-ret));
> -		rte_eth_dev_stop(port_id);
> +		ret_stop = rte_eth_dev_stop(port_id);
> +		if (ret_stop != 0) {
> +			RTE_ETHDEV_LOG(ERR,
> +				"Failed to stop device (port %u): %s\n",
> +				port_id, rte_strerror(-ret_stop));
> +		}
> +


Again, if there will be a new version already,
This is the 'rte_eth_dev_start()' function and error log is "Failed to stop 
device .." :)
What do you think about adding a little more detail, like "failed to stop back 
on error" etc...