[dpdk-dev] [PATCH] net/cxgbe: free resources in dev_uninit function

[dpdk-dev] [PATCH] net/cxgbe: free resources in dev_uninit function

[Rahul Lakkireddy]

Move freeing up resources from dev_close() to dev_uninit(). This fixes
NULL pointer de-reference when accessing adapter context needed by
other ports under same PF, but had been freed up by the first port.
This can happen if only the first port is started up and the check
to free up all resources is still satisfied. When dev_close is
called for other ports, adapter context is NULL since it was freed
up by the first port.

Thus, by moving to dev_uninit() all the ports can be teared down
safely without need for extra checks.

Fixes: 2195df6d11bd ("net/cxgbe: rework ethdev device allocation")
Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_ethdev.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 14011bb..3ee7c44 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -256,7 +256,6 @@ void cxgbe_dev_close(struct rte_eth_dev *eth_dev)
 {
 	struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
 	struct adapter *adapter = pi->adapter;
-	int i, dev_down = 0;
 
 	CXGBE_FUNC_TRACE();
 
@@ -270,22 +269,6 @@ void cxgbe_dev_close(struct rte_eth_dev *eth_dev)
 	 *  have been disabled
 	 */
 	t4_sge_eth_clear_queues(pi);
-
-	/*  See if all ports are down */
-	for_each_port(adapter, i) {
-		pi = adap2pinfo(adapter, i);
-		/*
-		 * Skip first port of the adapter since it will be closed
-		 * by DPDK
-		 */
-		if (i == 0)
-			continue;
-		dev_down += (pi->eth_dev->data->dev_started == 0) ? 1 : 0;
-	}
-
-	/* If rest of the ports are stopped, then free up resources */
-	if (dev_down == (adapter->params.nports - 1))
-		cxgbe_close(adapter);
 }
 
 /* Start the device.
@@ -1143,6 +1126,16 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 	return err;
 }
 
+static int eth_cxgbe_dev_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
+	struct adapter *adap = pi->adapter;
+
+	/* Free up other ports and all resources */
+	cxgbe_close(adap);
+	return 0;
+}
+
 static int eth_cxgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
@@ -1152,7 +1145,7 @@ static int eth_cxgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 
 static int eth_cxgbe_pci_remove(struct rte_pci_device *pci_dev)
 {
-	return rte_eth_dev_pci_generic_remove(pci_dev, NULL);
+	return rte_eth_dev_pci_generic_remove(pci_dev, eth_cxgbe_dev_uninit);
 }
 
 static struct rte_pci_driver rte_cxgbe_pmd = {
-- 
2.5.3

Re: [dpdk-dev] [PATCH] net/cxgbe: free resources in dev_uninit function

[Ferruh Yigit]

On 5/12/2018 12:50 PM, Rahul Lakkireddy wrote:
> Move freeing up resources from dev_close() to dev_uninit(). This fixes
> NULL pointer de-reference when accessing adapter context needed by
> other ports under same PF, but had been freed up by the first port.
> This can happen if only the first port is started up and the check
> to free up all resources is still satisfied. When dev_close is
> called for other ports, adapter context is NULL since it was freed
> up by the first port.
> 
> Thus, by moving to dev_uninit() all the ports can be teared down
> safely without need for extra checks.
> 
> Fixes: 2195df6d11bd ("net/cxgbe: rework ethdev device allocation")
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>

Applied to dpdk-next-net/master, thanks.