patches for DPDK stable branches
 help / color / Atom feed
* [dpdk-stable] [PATCH] net/mlx5: fix potential null dereference in port close
@ 2020-05-15  8:42 Suanming Mou
  2020-05-28  6:59 ` [dpdk-stable] [PATCH v2] net/mlx5: fix secondary process resources release Suanming Mou
  0 siblings, 1 reply; 3+ messages in thread
From: Suanming Mou @ 2020-05-15  8:42 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: dev, rasland, stable

The mlx5_dev_close() might be called twice and the priv->sh is NULL on
the second pass.
The DRV_LOG does not check priv->sh and it causes potential NULL
dereference.

Check priv->sh before priv->sh->ctx to avoid the potential NULL
dereference."

Fixes: 942d13e6e7d1 ("net/mlx5: fix sharing context destroy order")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1445809..be16841 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1425,7 +1425,8 @@ struct mlx5_flow_id_pool *
 
 	DRV_LOG(DEBUG, "port %u closing device \"%s\"",
 		dev->data->port_id,
-		((priv->sh->ctx != NULL) ? priv->sh->ctx->device->name : ""));
+		((priv->sh && priv->sh->ctx) ?
+		priv->sh->ctx->device->name : ""));
 	/* In case mlx5_dev_stop() has not been called. */
 	mlx5_dev_interrupt_handler_uninstall(dev);
 	mlx5_dev_interrupt_handler_devx_uninstall(dev);
-- 
1.8.3.1


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

* [dpdk-stable] [PATCH v2] net/mlx5: fix secondary process resources release
  2020-05-15  8:42 [dpdk-stable] [PATCH] net/mlx5: fix potential null dereference in port close Suanming Mou
@ 2020-05-28  6:59 ` Suanming Mou
  2020-05-31 14:03   ` Raslan Darawsheh
  0 siblings, 1 reply; 3+ messages in thread
From: Suanming Mou @ 2020-05-28  6:59 UTC (permalink / raw)
  To: viacheslavo, matan, dev; +Cc: rasland, stable

When secondary process starts, it will allocate its own process private
data, and also does remap to UAR register of the Tx queue. Once the
secondary process exits, these resources should be released accordingly.
And the shared resources owned by primary should not be touched.

Currently, once one port in the secondary process spawn failed, all the
other spawned ports will also be released during process exits. However,
the mlx5_dev_close() function does not add the cases for secondary
process, it means call the mlx5_dev_close() function directly in
secondary process releases the resources it should not touch.

Add the case for secondary process release to its own resources in
mlx5_dev_close() function to help it quits gracefully.

Fixes: 942d13e6e7d1 ("net/mlx5: fix sharing context destroy order")
Fixes: 3a8207423a0f ("net/mlx5: close all ports on remove")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
v2: fix more issues in secondary process.
---
 drivers/net/mlx5/mlx5.c      | 47 +++++++++++++++++++++++++++++++-------------
 drivers/net/mlx5/mlx5_rxtx.h |  1 +
 drivers/net/mlx5/mlx5_txq.c  | 24 ++++++++++++++++++++++
 3 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 5589772..8110263 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1423,6 +1423,17 @@ struct mlx5_flow_id_pool *
 	unsigned int i;
 	int ret;
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		/* Check if process_private released. */
+		if (!dev->process_private)
+			return;
+		mlx5_tx_uar_uninit_secondary(dev);
+		mlx5_proc_priv_uninit(dev);
+		rte_eth_dev_release_port(dev);
+		return;
+	}
+	if (!priv->sh)
+		return;
 	DRV_LOG(DEBUG, "port %u closing device \"%s\"",
 		dev->data->port_id,
 		((priv->sh->ctx != NULL) ? priv->sh->ctx->device->name : ""));
@@ -1512,16 +1523,13 @@ struct mlx5_flow_id_pool *
 	if (ret)
 		DRV_LOG(WARNING, "port %u some flows still remain",
 			dev->data->port_id);
-	if (priv->sh) {
-		/*
-		 * Free the shared context in last turn, because the cleanup
-		 * routines above may use some shared fields, like
-		 * mlx5_nl_mac_addr_flush() uses ibdev_path for retrieveing
-		 * ifindex if Netlink fails.
-		 */
-		mlx5_free_shared_ibctx(priv->sh);
-		priv->sh = NULL;
-	}
+	/*
+	 * Free the shared context in last turn, because the cleanup
+	 * routines above may use some shared fields, like
+	 * mlx5_nl_mac_addr_flush() uses ibdev_path for retrieveing
+	 * ifindex if Netlink fails.
+	 */
+	mlx5_free_shared_ibctx(priv->sh);
 	if (priv->domain_id != RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID) {
 		unsigned int c = 0;
 		uint16_t port_id;
@@ -2409,11 +2417,11 @@ struct mlx5_flow_id_pool *
 		/* Receive command fd from primary process */
 		err = mlx5_mp_req_verbs_cmd_fd(&mp_id);
 		if (err < 0)
-			return NULL;
+			goto err_secondary;
 		/* Remap UAR for Tx queues. */
 		err = mlx5_tx_uar_init_secondary(eth_dev, err);
 		if (err)
-			return NULL;
+			goto err_secondary;
 		/*
 		 * Ethdev pointer is still required as input since
 		 * the primary device is not accessible from the
@@ -2422,6 +2430,9 @@ struct mlx5_flow_id_pool *
 		eth_dev->rx_pkt_burst = mlx5_select_rx_function(eth_dev);
 		eth_dev->tx_pkt_burst = mlx5_select_tx_function(eth_dev);
 		return eth_dev;
+err_secondary:
+		mlx5_dev_close(eth_dev);
+		return NULL;
 	}
 	/*
 	 * Some parameters ("tx_db_nc" in particularly) are needed in
@@ -3707,8 +3718,16 @@ struct mlx5_flow_id_pool *
 {
 	uint16_t port_id;
 
-	RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device)
-		rte_eth_dev_close(port_id);
+	RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device) {
+		/*
+		 * mlx5_dev_close() is not registered to secondary process,
+		 * call the close function explicitly for secondary process.
+		 */
+		if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+			mlx5_dev_close(&rte_eth_devices[port_id]);
+		else
+			rte_eth_dev_close(port_id);
+	}
 	return 0;
 }
 
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 48f2b79..26621ff 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -434,6 +434,7 @@ int mlx5_tx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	 const struct rte_eth_hairpin_conf *hairpin_conf);
 void mlx5_tx_queue_release(void *dpdk_txq);
 int mlx5_tx_uar_init_secondary(struct rte_eth_dev *dev, int fd);
+void mlx5_tx_uar_uninit_secondary(struct rte_eth_dev *dev);
 struct mlx5_txq_obj *mlx5_txq_obj_new(struct rte_eth_dev *dev, uint16_t idx,
 				      enum mlx5_txq_obj_type type);
 struct mlx5_txq_obj *mlx5_txq_obj_get(struct rte_eth_dev *dev, uint16_t idx);
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index a211fa9..616d2fa 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -428,6 +428,30 @@
 }
 
 /**
+ * Deinitialize Tx UAR registers for secondary process.
+ *
+ * @param dev
+ *   Pointer to Ethernet device.
+ */
+void
+mlx5_tx_uar_uninit_secondary(struct rte_eth_dev *dev)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_txq_data *txq;
+	struct mlx5_txq_ctrl *txq_ctrl;
+	unsigned int i;
+
+	MLX5_ASSERT(rte_eal_process_type() == RTE_PROC_SECONDARY);
+	for (i = 0; i != priv->txqs_n; ++i) {
+		if (!(*priv->txqs)[i])
+			continue;
+		txq = (*priv->txqs)[i];
+		txq_ctrl = container_of(txq, struct mlx5_txq_ctrl, txq);
+		txq_uar_uninit_secondary(txq_ctrl);
+	}
+}
+
+/**
  * Initialize Tx UAR registers for secondary process.
  *
  * @param dev
-- 
1.8.3.1


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

* Re: [dpdk-stable] [PATCH v2] net/mlx5: fix secondary process resources release
  2020-05-28  6:59 ` [dpdk-stable] [PATCH v2] net/mlx5: fix secondary process resources release Suanming Mou
@ 2020-05-31 14:03   ` Raslan Darawsheh
  0 siblings, 0 replies; 3+ messages in thread
From: Raslan Darawsheh @ 2020-05-31 14:03 UTC (permalink / raw)
  To: Suanming Mou, Slava Ovsiienko, Matan Azrad, dev; +Cc: stable

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Thursday, May 28, 2020 10:00 AM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>; stable@dpdk.org
> Subject: [PATCH v2] net/mlx5: fix secondary process resources release
> 
> When secondary process starts, it will allocate its own process private
> data, and also does remap to UAR register of the Tx queue. Once the
> secondary process exits, these resources should be released accordingly.
> And the shared resources owned by primary should not be touched.
> 
> Currently, once one port in the secondary process spawn failed, all the
> other spawned ports will also be released during process exits. However,
> the mlx5_dev_close() function does not add the cases for secondary
> process, it means call the mlx5_dev_close() function directly in
> secondary process releases the resources it should not touch.
> 
> Add the case for secondary process release to its own resources in
> mlx5_dev_close() function to help it quits gracefully.
> 
> Fixes: 942d13e6e7d1 ("net/mlx5: fix sharing context destroy order")
> Fixes: 3a8207423a0f ("net/mlx5: close all ports on remove")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>
> ---
> v2: fix more issues in secondary process.
> ---
>  drivers/net/mlx5/mlx5.c      | 47 +++++++++++++++++++++++++++++++-----
> --------


Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15  8:42 [dpdk-stable] [PATCH] net/mlx5: fix potential null dereference in port close Suanming Mou
2020-05-28  6:59 ` [dpdk-stable] [PATCH v2] net/mlx5: fix secondary process resources release Suanming Mou
2020-05-31 14:03   ` Raslan Darawsheh

patches for DPDK stable branches

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox