diff mbox series

[05/31] net/bnxt: fix dev close operation

Message ID 20180619213058.12273-6-ajit.khaparde@broadcom.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series bnxt patchset |

Checks

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

Commit Message

Ajit Khaparde June 19, 2018, 9:30 p.m. UTC 
  We are not cleaning up all the memory and also not unregistering
the driver during device close operation. This patch fixes the issue.

Fixes: 893074951314 (net/bnxt: free memory in close operation)
Cc: stable@dpdk.org

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Ferruh Yigit June 26, 2018, 3:28 p.m. UTC | #1 
On 6/19/2018 10:30 PM, Ajit Khaparde wrote:
> We are not cleaning up all the memory and also not unregistering
> the driver during device close operation. This patch fixes the issue.
> 
> Fixes: 893074951314 (net/bnxt: free memory in close operation)
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

<...>

> @@ -3408,13 +3410,15 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
>  }
>  
>  static int
> -bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
> +bnxt_dev_uninit(struct rte_eth_dev *eth_dev)
> +{
>  	struct bnxt *bp = eth_dev->data->dev_private;
>  	int rc;
>  
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return -EPERM;
>  
> +	PMD_DRV_LOG(INFO, "Calling Device uninit\n");

This looks like can be a debug message, what do you think?

<...>

> @@ -3456,7 +3469,7 @@ static int bnxt_pci_remove(struct rte_pci_device *pci_dev)
>  static struct rte_pci_driver bnxt_rte_pmd = {
>  	.id_table = bnxt_pci_id_map,
>  	.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> -		RTE_PCI_DRV_INTR_LSC,
> +		RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_INTR_RMV,

Is Remove interrupts really supported? I can't find the related code in the driver.

You need to call _rte_eth_dev_callback_process() for RTE_ETH_EVENT_INTR_RMV
where you handle the interrupt.

And announce the feature "Removal event" in bnxt.ini
Ajit Khaparde June 28, 2018, 8:16 p.m. UTC | #2 
>
> > +     PMD_DRV_LOG(INFO, "Calling Device uninit\n");
>
> This looks like can be a debug message, what do you think?
>
​Yes. Changed it to debug.
​


>
> <...>
>
> > @@ -3456,7 +3469,7 @@ static int bnxt_pci_remove(struct rte_pci_device
> *pci_dev)
> >  static struct rte_pci_driver bnxt_rte_pmd = {
> >       .id_table = bnxt_pci_id_map,
> >       .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> > -             RTE_PCI_DRV_INTR_LSC,
> > +             RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_INTR_RMV,
>
> Is Remove interrupts really supported? I can't find the related code in
> the driver.
>
​That was some left over code. I cleaned it up.

Thanks
​


>
> You need to call _rte_eth_dev_callback_process() for
> RTE_ETH_EVENT_INTR_RMV
> where you handle the interrupt.
>
> And announce the feature "Removal event" in bnxt.ini
diff mbox series

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 33560db0d..b3826360c 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -152,6 +152,7 @@  static const struct rte_pci_id bnxt_pci_id_map[] = {
 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
 static void bnxt_print_link_info(struct rte_eth_dev *eth_dev);
 static int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
+static int bnxt_dev_uninit(struct rte_eth_dev *eth_dev);
 
 /***********************/
 
@@ -668,6 +669,8 @@  static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 		rte_free(bp->grp_info);
 		bp->grp_info = NULL;
 	}
+
+	bnxt_dev_uninit(eth_dev);
 }
 
 static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
@@ -3116,7 +3119,6 @@  static int bnxt_init_board(struct rte_eth_dev *eth_dev)
 	return rc;
 }
 
-static int bnxt_dev_uninit(struct rte_eth_dev *eth_dev);
 
 #define ALLOW_FUNC(x)	\
 	{ \
@@ -3408,13 +3410,15 @@  bnxt_dev_init(struct rte_eth_dev *eth_dev)
 }
 
 static int
-bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
+bnxt_dev_uninit(struct rte_eth_dev *eth_dev)
+{
 	struct bnxt *bp = eth_dev->data->dev_private;
 	int rc;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return -EPERM;
 
+	PMD_DRV_LOG(INFO, "Calling Device uninit\n");
 	bnxt_disable_int(bp);
 	bnxt_free_int(bp);
 	bnxt_free_mem(bp);
@@ -3428,8 +3432,17 @@  bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
 	}
 	rc = bnxt_hwrm_func_driver_unregister(bp, 0);
 	bnxt_free_hwrm_resources(bp);
-	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
-	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
+
+	if (bp->tx_mem_zone) {
+		rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
+		bp->tx_mem_zone = NULL;
+	}
+
+	if (bp->rx_mem_zone) {
+		rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
+		bp->rx_mem_zone = NULL;
+	}
+
 	if (bp->dev_stopped == 0)
 		bnxt_dev_close_op(eth_dev);
 	if (bp->pf.vf_info)
@@ -3456,7 +3469,7 @@  static int bnxt_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver bnxt_rte_pmd = {
 	.id_table = bnxt_pci_id_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
-		RTE_PCI_DRV_INTR_LSC,
+		RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_INTR_RMV,
 	.probe = bnxt_pci_probe,
 	.remove = bnxt_pci_remove,
 };