diff mbox series

[v2,01/11] ethdev: change eth dev stop function to return int

Message ID 1602768646-13142-2-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: change device stop to return status |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andrew Rybchenko Oct. 15, 2020, 1:30 p.m. UTC 
  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>
---
 doc/guides/rel_notes/deprecation.rst   |  1 -
 doc/guides/rel_notes/release_20_11.rst |  3 +++
 lib/librte_ethdev/rte_ethdev.c         | 27 +++++++++++++++++++-------
 lib/librte_ethdev/rte_ethdev.h         |  5 ++++-
 4 files changed, 27 insertions(+), 9 deletions(-)

Comments

Ferruh Yigit Oct. 16, 2020, 9:22 a.m. UTC | #1 
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...
Ray Kinsella Oct. 16, 2020, 11:20 a.m. UTC | #2 
On 15/10/2020 14:30, 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>
> ---
>  doc/guides/rel_notes/deprecation.rst   |  1 -
>  doc/guides/rel_notes/release_20_11.rst |  3 +++
>  lib/librte_ethdev/rte_ethdev.c         | 27 +++++++++++++++++++-------
>  lib/librte_ethdev/rte_ethdev.h         |  5 ++++-
>  4 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d1f5ed39db..2e04e24374 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -127,7 +127,6 @@ Deprecation Notices
>    negative errno values to indicate various error conditions (e.g.
>    invalid port ID, unsupported operation, failed operation):
>  
> -  - ``rte_eth_dev_stop``
>    - ``rte_eth_dev_close``
>  
>  * ethdev: New offload flags ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> 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.
> +
>  
>  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));
> +		}
> +
>  		return ret;
>  	}
>  
> @@ -1708,26 +1714,28 @@ rte_eth_dev_start(uint16_t port_id)
>  	return 0;
>  }
>  
> -void
> +int
>  rte_eth_dev_stop(uint16_t port_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	RTE_ETH_VALID_PORTID_OR_RET(port_id);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  	dev = &rte_eth_devices[port_id];
>  
> -	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop);
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_stop, -ENOTSUP);
>  
>  	if (dev->data->dev_started == 0) {
>  		RTE_ETHDEV_LOG(INFO,
>  			"Device with port_id=%"PRIu16" already stopped\n",
>  			port_id);
> -		return;
> +		return 0;
>  	}
>  
>  	dev->data->dev_started = 0;
>  	(*dev->dev_ops->dev_stop)(dev);
>  	rte_ethdev_trace_stop(port_id);
> +
> +	return 0;
>  }
>  
>  int
> @@ -1783,7 +1791,12 @@ rte_eth_dev_reset(uint16_t port_id)
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
>  
> -	rte_eth_dev_stop(port_id);
> +	ret = rte_eth_dev_stop(port_id);
> +	if (ret != 0) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"Failed to stop device (port %u) before reset: %s - ignore\n",
> +			port_id, rte_strerror(-ret));

ABI change is 100%,
Just question the logic of continuing here to do a reset, if you failed to stop the device.


> +	}
>  	ret = dev->dev_ops->dev_reset(dev);
>  
>  	return eth_err(port_id, ret);
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index a61ca115a0..b85861cf2b 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2277,8 +2277,11 @@ int rte_eth_dev_start(uint16_t port_id);
>   *
>   * @param port_id
>   *   The port identifier of the Ethernet device.
> + * @return
> + *   - 0: Success, Ethernet device stopped.
> + *   - <0: Error code of the driver device stop function.
>   */
> -void rte_eth_dev_stop(uint16_t port_id);
> +int rte_eth_dev_stop(uint16_t port_id);
>  
>  /**
>   * Link up an Ethernet device.
>
Ferruh Yigit Oct. 16, 2020, 4:20 p.m. UTC | #3 
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>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Andrew Rybchenko Oct. 16, 2020, 5:13 p.m. UTC | #4 
On 10/16/20 2:20 PM, Kinsella, Ray wrote:
>
> On 15/10/2020 14:30, 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>
>> ---
>>   doc/guides/rel_notes/deprecation.rst   |  1 -
>>   doc/guides/rel_notes/release_20_11.rst |  3 +++
>>   lib/librte_ethdev/rte_ethdev.c         | 27 +++++++++++++++++++-------
>>   lib/librte_ethdev/rte_ethdev.h         |  5 ++++-
>>   4 files changed, 27 insertions(+), 9 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>> index d1f5ed39db..2e04e24374 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -127,7 +127,6 @@ Deprecation Notices
>>     negative errno values to indicate various error conditions (e.g.
>>     invalid port ID, unsupported operation, failed operation):
>>   
>> -  - ``rte_eth_dev_stop``
>>     - ``rte_eth_dev_close``
>>   
>>   * ethdev: New offload flags ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
>> 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.
>> +
>>   
>>   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));
>> +		}
>> +
>>   		return ret;
>>   	}
>>   
>> @@ -1708,26 +1714,28 @@ rte_eth_dev_start(uint16_t port_id)
>>   	return 0;
>>   }
>>   
>> -void
>> +int
>>   rte_eth_dev_stop(uint16_t port_id)
>>   {
>>   	struct rte_eth_dev *dev;
>>   
>> -	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>   	dev = &rte_eth_devices[port_id];
>>   
>> -	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop);
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_stop, -ENOTSUP);
>>   
>>   	if (dev->data->dev_started == 0) {
>>   		RTE_ETHDEV_LOG(INFO,
>>   			"Device with port_id=%"PRIu16" already stopped\n",
>>   			port_id);
>> -		return;
>> +		return 0;
>>   	}
>>   
>>   	dev->data->dev_started = 0;
>>   	(*dev->dev_ops->dev_stop)(dev);
>>   	rte_ethdev_trace_stop(port_id);
>> +
>> +	return 0;
>>   }
>>   
>>   int
>> @@ -1783,7 +1791,12 @@ rte_eth_dev_reset(uint16_t port_id)
>>   
>>   	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
>>   
>> -	rte_eth_dev_stop(port_id);
>> +	ret = rte_eth_dev_stop(port_id);
>> +	if (ret != 0) {
>> +		RTE_ETHDEV_LOG(ERR,
>> +			"Failed to stop device (port %u) before reset: %s - ignore\n",
>> +			port_id, rte_strerror(-ret));
> ABI change is 100%,
> Just question the logic of continuing here to do a reset, if you failed to stop the device.

In the case of reset I'm sure that we should ignore stop failure here.
Typically reset is required to recover from bad state etc and stop
failure in such condition could definitely happen.
Ray Kinsella Oct. 19, 2020, 9:37 a.m. UTC | #5 
On 16/10/2020 18:13, Andrew Rybchenko wrote:
> On 10/16/20 2:20 PM, Kinsella, Ray wrote:
>> On 15/10/2020 14:30, 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>
>>> ---
>>>  doc/guides/rel_notes/deprecation.rst   |  1 -
>>>  doc/guides/rel_notes/release_20_11.rst |  3 +++
>>>  lib/librte_ethdev/rte_ethdev.c         | 27 +++++++++++++++++++-------
>>>  lib/librte_ethdev/rte_ethdev.h         |  5 ++++-
>>>  4 files changed, 27 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>>> index d1f5ed39db..2e04e24374 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -127,7 +127,6 @@ Deprecation Notices
>>>    negative errno values to indicate various error conditions (e.g.
>>>    invalid port ID, unsupported operation, failed operation):
>>>  
>>> -  - ``rte_eth_dev_stop``
>>>    - ``rte_eth_dev_close``
>>>  
>>>  * ethdev: New offload flags ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
>>> 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.
>>> +
>>>  
>>>  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));
>>> +		}
>>> +
>>>  		return ret;
>>>  	}
>>>  
>>> @@ -1708,26 +1714,28 @@ rte_eth_dev_start(uint16_t port_id)
>>>  	return 0;
>>>  }
>>>  
>>> -void
>>> +int
>>>  rte_eth_dev_stop(uint16_t port_id)
>>>  {
>>>  	struct rte_eth_dev *dev;
>>>  
>>> -	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>  	dev = &rte_eth_devices[port_id];
>>>  
>>> -	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop);
>>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_stop, -ENOTSUP);
>>>  
>>>  	if (dev->data->dev_started == 0) {
>>>  		RTE_ETHDEV_LOG(INFO,
>>>  			"Device with port_id=%"PRIu16" already stopped\n",
>>>  			port_id);
>>> -		return;
>>> +		return 0;
>>>  	}
>>>  
>>>  	dev->data->dev_started = 0;
>>>  	(*dev->dev_ops->dev_stop)(dev);
>>>  	rte_ethdev_trace_stop(port_id);
>>> +
>>> +	return 0;
>>>  }
>>>  
>>>  int
>>> @@ -1783,7 +1791,12 @@ rte_eth_dev_reset(uint16_t port_id)
>>>  
>>>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
>>>  
>>> -	rte_eth_dev_stop(port_id);
>>> +	ret = rte_eth_dev_stop(port_id);
>>> +	if (ret != 0) {
>>> +		RTE_ETHDEV_LOG(ERR,
>>> +			"Failed to stop device (port %u) before reset: %s - ignore\n",
>>> +			port_id, rte_strerror(-ret));
>> ABI change is 100%,
>> Just question the logic of continuing here to do a reset, if you failed to stop the device.
> 
> In the case of reset I'm sure that we should ignore stop failure here.
> Typically reset is required to recover from bad state etc and stop
> failure in such condition could definitely happen.

Ok - thanks for that.
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d1f5ed39db..2e04e24374 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -127,7 +127,6 @@  Deprecation Notices
   negative errno values to indicate various error conditions (e.g.
   invalid port ID, unsupported operation, failed operation):
 
-  - ``rte_eth_dev_stop``
   - ``rte_eth_dev_close``
 
 * ethdev: New offload flags ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
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.
+
 
 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));
+		}
+
 		return ret;
 	}
 
@@ -1708,26 +1714,28 @@  rte_eth_dev_start(uint16_t port_id)
 	return 0;
 }
 
-void
+int
 rte_eth_dev_stop(uint16_t port_id)
 {
 	struct rte_eth_dev *dev;
 
-	RTE_ETH_VALID_PORTID_OR_RET(port_id);
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
-	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_stop, -ENOTSUP);
 
 	if (dev->data->dev_started == 0) {
 		RTE_ETHDEV_LOG(INFO,
 			"Device with port_id=%"PRIu16" already stopped\n",
 			port_id);
-		return;
+		return 0;
 	}
 
 	dev->data->dev_started = 0;
 	(*dev->dev_ops->dev_stop)(dev);
 	rte_ethdev_trace_stop(port_id);
+
+	return 0;
 }
 
 int
@@ -1783,7 +1791,12 @@  rte_eth_dev_reset(uint16_t port_id)
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
 
-	rte_eth_dev_stop(port_id);
+	ret = rte_eth_dev_stop(port_id);
+	if (ret != 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"Failed to stop device (port %u) before reset: %s - ignore\n",
+			port_id, rte_strerror(-ret));
+	}
 	ret = dev->dev_ops->dev_reset(dev);
 
 	return eth_err(port_id, ret);
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index a61ca115a0..b85861cf2b 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2277,8 +2277,11 @@  int rte_eth_dev_start(uint16_t port_id);
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
+ * @return
+ *   - 0: Success, Ethernet device stopped.
+ *   - <0: Error code of the driver device stop function.
  */
-void rte_eth_dev_stop(uint16_t port_id);
+int rte_eth_dev_stop(uint16_t port_id);
 
 /**
  * Link up an Ethernet device.