[v2,02/13] net/failsafe: check code of promiscuous mode switch

Message ID 1568030331-16526-3-git-send-email-arybchenko@solarflare.com
State Superseded
Delegated to: Ferruh Yigit
Headers show
Series
  • ethdev: change promiscuous mode functions to return status
Related show

Checks

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

Commit Message

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

rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() return
value was changed from void to int, so this patch modify usage
of these functions across net/failsafe according to new return type.

Try to keep promiscuous mode consistent across all active
devices in the case of failure.

Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_ether.c |  8 +++--
 drivers/net/failsafe/failsafe_ops.c   | 44 ++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 6 deletions(-)

Comments

Gaëtan Rivet Sept. 9, 2019, 12:48 p.m. | #1
Hi,

On Mon, Sep 09, 2019 at 12:58:39PM +0100, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> 
> rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() return
> value was changed from void to int, so this patch modify usage
> of these functions across net/failsafe according to new return type.
> 
> Try to keep promiscuous mode consistent across all active
> devices in the case of failure.
> 
> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

LGTM,
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Thanks,

Patch

diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 504c76edb0..bd38f1c1e4 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -126,9 +126,13 @@  fs_eth_dev_conf_apply(struct rte_eth_dev *dev,
 	if (dev->data->promiscuous != edev->data->promiscuous) {
 		DEBUG("Configuring promiscuous");
 		if (dev->data->promiscuous)
-			rte_eth_promiscuous_enable(PORT_ID(sdev));
+			ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
 		else
-			rte_eth_promiscuous_disable(PORT_ID(sdev));
+			ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
+		if (ret != 0) {
+			ERROR("Failed to apply promiscuous mode");
+			return ret;
+		}
 	} else {
 		DEBUG("promiscuous already set");
 	}
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 1e57633374..114d6a09f6 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -659,10 +659,28 @@  fs_promiscuous_enable(struct rte_eth_dev *dev)
 {
 	struct sub_device *sdev;
 	uint8_t i;
+	int ret = 0;
 
 	fs_lock(dev, 0);
-	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
-		rte_eth_promiscuous_enable(PORT_ID(sdev));
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
+		ret = fs_err(sdev, ret);
+		if (ret != 0) {
+			ERROR("Promiscuous mode enable failed for subdevice %d",
+				PORT_ID(sdev));
+			break;
+		}
+	}
+	if (ret != 0) {
+		/* Rollback in the case of failure */
+		FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+			ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
+			ret = fs_err(sdev, ret);
+			if (ret != 0)
+				ERROR("Promiscuous mode disable failed for subdevice %d",
+					PORT_ID(sdev));
+		}
+	}
 	fs_unlock(dev, 0);
 }
 
@@ -671,10 +689,28 @@  fs_promiscuous_disable(struct rte_eth_dev *dev)
 {
 	struct sub_device *sdev;
 	uint8_t i;
+	int ret = 0;
 
 	fs_lock(dev, 0);
-	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
-		rte_eth_promiscuous_disable(PORT_ID(sdev));
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
+		ret = fs_err(sdev, ret);
+		if (ret != 0) {
+			ERROR("Promiscuous mode disable failed for subdevice %d",
+				PORT_ID(sdev));
+			break;
+		}
+	}
+	if (ret != 0) {
+		/* Rollback in the case of failure */
+		FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+			ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
+			ret = fs_err(sdev, ret);
+			if (ret != 0)
+				ERROR("Promiscuous mode enable failed for subdevice %d",
+					PORT_ID(sdev));
+		}
+	}
 	fs_unlock(dev, 0);
 }