diff mbox series

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

Message ID 1602682146-4722-2-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Superseded, 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. 14, 2020, 1:28 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>
---
 doc/guides/rel_notes/deprecation.rst |  1 -
 lib/librte_ethdev/rte_ethdev.c       | 27 ++++++++++++++++++++-------
 lib/librte_ethdev/rte_ethdev.h       |  2 +-
 3 files changed, 21 insertions(+), 9 deletions(-)

Comments

Thomas Monjalon Oct. 14, 2020, 1:33 p.m. UTC | #1 
14/10/2020 15:28, Andrew Rybchenko:
> 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>

Please no capital letter in the email address.

> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Thanks for taking care of this cleanup.

A note in the API changes of the release notes is missing.
Except that miss,
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Andrew Rybchenko Oct. 14, 2020, 1:40 p.m. UTC | #2 
On 10/14/20 4:33 PM, Thomas Monjalon wrote:
> 14/10/2020 15:28, Andrew Rybchenko:
>> 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>
> Please no capital letter in the email address.

OK

>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Thanks for taking care of this cleanup.
>
> A note in the API changes of the release notes is missing.

I'll wait for tomorrow for more reviews and send v2
with both notes fixed.

> Except that miss,
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Thanks.
Ferruh Yigit Oct. 14, 2020, 5:29 p.m. UTC | #3 
On 10/14/2020 2:28 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>

<...>

> @@ -2278,7 +2278,7 @@ int rte_eth_dev_start(uint16_t port_id);
>    * @param port_id
>    *   The port identifier of the Ethernet device.
>    */
> -void rte_eth_dev_stop(uint16_t port_id);
> +int rte_eth_dev_stop(uint16_t port_id);
>   

Doxygen comment should be updated for return value.
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 584e720879..f0bf656b47 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/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 5b7979a3b8..2226e429ba 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1662,7 +1662,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);
 
@@ -1696,7 +1696,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;
 	}
 
@@ -1709,26 +1715,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
@@ -1784,7 +1792,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 f4cc5917b9..e8eedaf0af 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2278,7 +2278,7 @@  int rte_eth_dev_start(uint16_t port_id);
  * @param port_id
  *   The port identifier of the Ethernet device.
  */
-void rte_eth_dev_stop(uint16_t port_id);
+int rte_eth_dev_stop(uint16_t port_id);
 
 /**
  * Link up an Ethernet device.