DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] mlx5: fix error unwind in device start
@ 2018-08-01 21:59 Stephen Hemminger
  2018-08-13  7:52 ` Shahaf Shuler
  0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2018-08-01 21:59 UTC (permalink / raw)
  To: shahafs, yskoh; +Cc: dev, Stephen Hemminger, Stephen Hemminger

The error handling in start of the mlx5 driver is buggy.
For example, if setting up the flows fails the device driver
will then get stuck in mlx5_flow_rxq_flags_clear waiting
for something that will never happen.

The problem is that the code jumps to a common error label
and does unwind for portions of the driver which have not
been setup.

This suggested patch breaks it into different labels with
each failure path only unwinding what was done.

Also, the ethdev driver should not be manipulating the
dev_started flag directly. That is handled by the common
ethdev layer.

The patch works for the success case, but furthur testing
is needed to actually exercise all the error paths.
This is left as exercise for the maintainers.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index e2a9bb703261..79a7b233986a 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -171,42 +171,42 @@ mlx5_dev_start(struct rte_eth_dev *dev)
 	if (ret) {
 		DRV_LOG(ERR, "port %u Rx queue allocation failed: %s",
 			dev->data->port_id, strerror(rte_errno));
-		mlx5_txq_stop(dev);
-		return -rte_errno;
+		goto error_txq_stop;
 	}
-	dev->data->dev_started = 1;
+
 	ret = mlx5_rx_intr_vec_enable(dev);
 	if (ret) {
 		DRV_LOG(ERR, "port %u Rx interrupt vector creation failed",
 			dev->data->port_id);
-		goto error;
+		goto error_rxq_stop;
 	}
 	mlx5_xstats_init(dev);
 	ret = mlx5_traffic_enable(dev);
 	if (ret) {
 		DRV_LOG(DEBUG, "port %u failed to set defaults flows",
 			dev->data->port_id);
-		goto error;
+		goto error_intr_vec_disable;
 	}
 	ret = mlx5_flow_start(dev, &priv->flows);
 	if (ret) {
 		DRV_LOG(DEBUG, "port %u failed to set flows",
 			dev->data->port_id);
-		goto error;
+		goto error_traffic_disable;
 	}
+
 	dev->tx_pkt_burst = mlx5_select_tx_function(dev);
 	dev->rx_pkt_burst = mlx5_select_rx_function(dev);
 	mlx5_dev_interrupt_handler_install(dev);
 	return 0;
-error:
-	ret = rte_errno; /* Save rte_errno before cleanup. */
-	/* Rollback. */
-	dev->data->dev_started = 0;
-	mlx5_flow_stop(dev, &priv->flows);
+
+error_traffic_disable:
 	mlx5_traffic_disable(dev);
-	mlx5_txq_stop(dev);
+error_intr_vec_disable:
+	mlx5_rx_intr_vec_disable(dev);
+error_rxq_stop:
 	mlx5_rxq_stop(dev);
-	rte_errno = ret; /* Restore rte_errno. */
+error_txq_stop:
+	mlx5_txq_stop(dev);
 	return -rte_errno;
 }
 
-- 
2.18.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-08-14  7:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-01 21:59 [dpdk-dev] [RFC] mlx5: fix error unwind in device start Stephen Hemminger
2018-08-13  7:52 ` Shahaf Shuler
2018-08-13 15:20   ` Stephen Hemminger
2018-08-14  7:35     ` Shahaf Shuler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).