[v2,01/13] ethdev: change promiscuous mode controllers to return errors

Message ID 1568030331-16526-2-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: change promiscuous mode functions to return status |

Checks

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

Commit Message

Andrew Rybchenko Sept. 9, 2019, 11:58 a.m. UTC
  From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>

Change rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable()
return value from void to int and return negative errno values
in case of error conditions.
Modify usage of these functions across the ethdev according
to 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 -
 doc/guides/rel_notes/release_19_11.rst      |  4 ++
 doc/guides/sample_app_ug/flow_classify.rst  |  6 ++-
 doc/guides/sample_app_ug/flow_filtering.rst | 15 +++++-
 doc/guides/sample_app_ug/rxtx_callbacks.rst |  5 +-
 doc/guides/sample_app_ug/skeleton.rst       |  6 ++-
 lib/librte_ethdev/rte_ethdev.c              | 52 ++++++++++++++++-----
 lib/librte_ethdev/rte_ethdev.h              | 14 +++++-
 8 files changed, 80 insertions(+), 23 deletions(-)
  

Comments

Ferruh Yigit Sept. 13, 2019, 3:35 p.m. UTC | #1
On 9/9/2019 12:58 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> 
> Change rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable()
> return value from void to int and return negative errno values
> in case of error conditions.
> Modify usage of these functions across the ethdev according
> to new return type.
> 
> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> diff --git a/doc/guides/sample_app_ug/flow_filtering.rst b/doc/guides/sample_app_ug/flow_filtering.rst
> index 02fc675506..de3e4ab0b6 100644
> --- a/doc/guides/sample_app_ug/flow_filtering.rst
> +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> @@ -193,7 +193,13 @@ application is shown below:
>                     }
>            }
>  
> -           rte_eth_promiscuous_enable(port_id);
> +           ret = rte_eth_promiscuous_enable(port_id);
> +           if (ret != 0) {
> +                   rte_exit(EXIT_FAILURE,
> +                           ":: cannot enable promiscuous mode: err=%d, port=%u\n",
> +                           ret, port_id);
> +           }
> +

Not for this patch, for future, I am for not putting code into the documents...
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index cbb4c34efd..b2e0a1fc7c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -88,7 +88,6 @@  Deprecation Notices
   negative errno values to indicate various error conditions (e.g.
   invalid port ID, unsupported operation, failed operation):
 
-  - ``rte_eth_promiscuous_enable`` and ``rte_eth_promiscuous_disable``
   - ``rte_eth_allmulticast_enable`` and ``rte_eth_allmulticast_disable``
   - ``rte_eth_link_get`` and ``rte_eth_link_get_nowait``
   - ``rte_eth_dev_stop``
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index 152f120197..5299157df7 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -97,6 +97,10 @@  API Changes
 * ethdev: changed ``rte_eth_dev_infos_get`` return value from ``void`` to
   ``int`` to provide a way to report various error conditions.
 
+* ethdev: changed ``rte_eth_promiscuous_enable`` and
+  ``rte_eth_promiscuous_disable`` return value from ``void`` to ``int`` to
+  provide a way to report various error conditions.
+
 
 ABI Changes
 -----------
diff --git a/doc/guides/sample_app_ug/flow_classify.rst b/doc/guides/sample_app_ug/flow_classify.rst
index 96a5c66d0f..7c2b6dcf83 100644
--- a/doc/guides/sample_app_ug/flow_classify.rst
+++ b/doc/guides/sample_app_ug/flow_classify.rst
@@ -315,7 +315,9 @@  Forwarding application is shown below:
                addr.addr_bytes[4], addr.addr_bytes[5]);
 
         /* Enable RX in promiscuous mode for the Ethernet device. */
-        rte_eth_promiscuous_enable(port);
+        retval = rte_eth_promiscuous_enable(port);
+        if (retval != 0)
+                return retval;
 
         return 0;
     }
@@ -343,7 +345,7 @@  Finally the RX port is set in promiscuous mode:
 
 .. code-block:: c
 
-    rte_eth_promiscuous_enable(port);
+    retval = rte_eth_promiscuous_enable(port);
 
 The Add Rules function
 ~~~~~~~~~~~~~~~~~~~~~~
diff --git a/doc/guides/sample_app_ug/flow_filtering.rst b/doc/guides/sample_app_ug/flow_filtering.rst
index 02fc675506..de3e4ab0b6 100644
--- a/doc/guides/sample_app_ug/flow_filtering.rst
+++ b/doc/guides/sample_app_ug/flow_filtering.rst
@@ -193,7 +193,13 @@  application is shown below:
                    }
           }
 
-           rte_eth_promiscuous_enable(port_id);
+           ret = rte_eth_promiscuous_enable(port_id);
+           if (ret != 0) {
+                   rte_exit(EXIT_FAILURE,
+                           ":: cannot enable promiscuous mode: err=%d, port=%u\n",
+                           ret, port_id);
+           }
+
            ret = rte_eth_dev_start(port_id);
            if (ret < 0) {
                    rte_exit(EXIT_FAILURE,
@@ -278,7 +284,12 @@  We are setting the RX port to promiscuous mode:
 
 .. code-block:: c
 
-   rte_eth_promiscuous_enable(port_id);
+   ret = rte_eth_promiscuous_enable(port_id);
+   if (ret != 0) {
+        rte_exit(EXIT_FAILURE,
+                 ":: cannot enable promiscuous mode: err=%d, port=%u\n",
+                 ret, port_id);
+   }
 
 The last step is to start the port.
 
diff --git a/doc/guides/sample_app_ug/rxtx_callbacks.rst b/doc/guides/sample_app_ug/rxtx_callbacks.rst
index 32c120992f..0a69ec71ab 100644
--- a/doc/guides/sample_app_ug/rxtx_callbacks.rst
+++ b/doc/guides/sample_app_ug/rxtx_callbacks.rst
@@ -117,8 +117,9 @@  comments:
             return retval;
 
         /* Enable RX in promiscuous mode for the Ethernet device. */
-        rte_eth_promiscuous_enable(port);
-
+        retval = rte_eth_promiscuous_enable(port);
+        if (retval != 0)
+            return retval;
 
         /* Add the callbacks for RX and TX.*/
         rte_eth_add_rx_callback(port, 0, add_timestamps, NULL);
diff --git a/doc/guides/sample_app_ug/skeleton.rst b/doc/guides/sample_app_ug/skeleton.rst
index 59ca511d33..1d0a2760d4 100644
--- a/doc/guides/sample_app_ug/skeleton.rst
+++ b/doc/guides/sample_app_ug/skeleton.rst
@@ -149,7 +149,9 @@  Forwarding application is shown below:
             return retval;
 
         /* Enable RX in promiscuous mode for the Ethernet device. */
-        rte_eth_promiscuous_enable(port);
+        retval = rte_eth_promiscuous_enable(port);
+        if (retval != 0)
+            return retval;
 
         return 0;
     }
@@ -177,7 +179,7 @@  Finally the RX port is set in promiscuous mode:
 
 .. code-block:: c
 
-        rte_eth_promiscuous_enable(port);
+        retval = rte_eth_promiscuous_enable(port);
 
 
 The Lcores Main
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 30b0c7803f..b97dd8aa85 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1381,24 +1381,41 @@  rte_eth_dev_mac_restore(struct rte_eth_dev *dev,
 	}
 }
 
-static void
+static int
 rte_eth_dev_config_restore(struct rte_eth_dev *dev,
 			   struct rte_eth_dev_info *dev_info, uint16_t port_id)
 {
+	int ret;
+
 	if (!(*dev_info->dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR))
 		rte_eth_dev_mac_restore(dev, dev_info);
 
 	/* replay promiscuous configuration */
-	if (rte_eth_promiscuous_get(port_id) == 1)
-		rte_eth_promiscuous_enable(port_id);
-	else if (rte_eth_promiscuous_get(port_id) == 0)
-		rte_eth_promiscuous_disable(port_id);
+	if (rte_eth_promiscuous_get(port_id) == 1) {
+		ret = rte_eth_promiscuous_enable(port_id);
+		if (ret != 0 && ret != -ENOTSUP) {
+			RTE_ETHDEV_LOG(ERR,
+				"Failed to enable promiscuous mode for device (port %u): %s\n",
+				port_id, rte_strerror(-ret));
+			return ret;
+		}
+	} else if (rte_eth_promiscuous_get(port_id) == 0) {
+		ret = rte_eth_promiscuous_disable(port_id);
+		if (ret != 0 && ret != -ENOTSUP) {
+			RTE_ETHDEV_LOG(ERR,
+				"Failed to disable promiscuous mode for device (port %u): %s\n",
+				port_id, rte_strerror(-ret));
+			return ret;
+		}
+	}
 
 	/* replay all multicast configuration */
 	if (rte_eth_allmulticast_get(port_id) == 1)
 		rte_eth_allmulticast_enable(port_id);
 	else if (rte_eth_allmulticast_get(port_id) == 0)
 		rte_eth_allmulticast_disable(port_id);
+
+	return 0;
 }
 
 int
@@ -1436,7 +1453,14 @@  rte_eth_dev_start(uint16_t port_id)
 	else
 		return eth_err(port_id, diag);
 
-	rte_eth_dev_config_restore(dev, &dev_info, port_id);
+	ret = rte_eth_dev_config_restore(dev, &dev_info, port_id);
+	if (ret != 0) {
+		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);
+		return ret;
+	}
 
 	if (dev->data->dev_conf.intr_conf.lsc == 0) {
 		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
@@ -1864,30 +1888,34 @@  rte_eth_tx_done_cleanup(uint16_t port_id, uint16_t queue_id, uint32_t free_cnt)
 	return eth_err(port_id, ret);
 }
 
-void
+int
 rte_eth_promiscuous_enable(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->promiscuous_enable);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_enable, -ENOTSUP);
 	(*dev->dev_ops->promiscuous_enable)(dev);
 	dev->data->promiscuous = 1;
+
+	return 0;
 }
 
-void
+int
 rte_eth_promiscuous_disable(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->promiscuous_disable);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_disable, -ENOTSUP);
 	dev->data->promiscuous = 0;
 	(*dev->dev_ops->promiscuous_disable)(dev);
+
+	return 0;
 }
 
 int
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 475dbdae17..f07a829b29 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2022,16 +2022,26 @@  int rte_eth_dev_reset(uint16_t port_id);
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for promiscuous_enable() does not exist
+ *     for the device.
+ *   - (-ENODEV) if *port_id* invalid.
  */
-void rte_eth_promiscuous_enable(uint16_t port_id);
+int rte_eth_promiscuous_enable(uint16_t port_id);
 
 /**
  * Disable receipt in promiscuous mode for an Ethernet device.
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for promiscuous_disable() does not exist
+ *     for the device.
+ *   - (-ENODEV) if *port_id* invalid.
  */
-void rte_eth_promiscuous_disable(uint16_t port_id);
+int rte_eth_promiscuous_disable(uint16_t port_id);
 
 /**
  * Return the value of promiscuous mode for an Ethernet device.