Skip to contentSkip to conversation

net/bnxt: fix resource cleanup

PATCH In review Ajit Khaparde 1 participant 4 messages Apr 19, 2021 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 net/bnxt: fix resource cleanup

Fix resource cleanup in port close.
Once the pointers are freed, set them to NULL. Make sure access to the pointers is validated before use.

Fixes bb81e07323bb ("net/bnxt: support LED on/off")
Fixes 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
Fixes 1d0704f4d793 ("net/bnxt: add device configure operation")
Fixes 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Fixes 322bd6e70272 ("net/bnxt: add port representor infrastructure")
Fixes 0bf5a0b5ebb8 ("net/bnxt: add a failure log")
---
 drivers/net/bnxt/bnxt_ethdev.c | 39 +++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_hwrm.c   |  3 +++
 2 files changed, 32 insertions(+), 10 deletions(-)
drivers/net/bnxt/bnxt_ethdev.c +29 -10
index 20ca57eae5..dc3b04110f 100644
@@ -219,16 +219,19 @@ uint16_t bnxt_rss_hash_tbl_size(const struct bnxt *bp)
 static void bnxt_free_parent_info(struct bnxt *bp)
 {
 	rte_free(bp->parent);
+	bp->parent = NULL;
 }
 
 static void bnxt_free_pf_info(struct bnxt *bp)
 {
 	rte_free(bp->pf);
+	bp->pf = NULL;
 }
 
 static void bnxt_free_link_info(struct bnxt *bp)
 {
 	rte_free(bp->link_info);
+	bp->link_info = NULL;
 }
 
 static void bnxt_free_leds_info(struct bnxt *bp)
@@ -249,7 +252,9 @@ static void bnxt_free_flow_stats_info(struct bnxt *bp)
 static void bnxt_free_cos_queues(struct bnxt *bp)
 {
 	rte_free(bp->rx_cos_queue);
+	bp->rx_cos_queue = NULL;
 	rte_free(bp->tx_cos_queue);
+	bp->tx_cos_queue = NULL;
 }
 
 static void bnxt_free_mem(struct bnxt *bp, bool reconfig)
@@ -849,9 +854,14 @@ static int bnxt_shutdown_nic(struct bnxt *bp)
 
 uint32_t bnxt_get_speed_capabilities(struct bnxt *bp)
 {
-	uint32_t link_speed = bp->link_info->support_speeds;
+	uint32_t link_speed = 0;
 	uint32_t speed_capa = 0;
 
+	if (bp->link_info == NULL)
+		return 0;
+
+	link_speed = bp->link_info->support_speeds;
+
 	/* If PAM4 is configured, use PAM4 supported speed */
 	if (link_speed == 0 && bp->link_info->support_pam4_speeds > 0)
 		link_speed = bp->link_info->support_pam4_speeds;
@@ -1293,12 +1303,13 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
 {
 	int rc = 0;
 
-	if (bp->switch_domain_id) {
-		rc = rte_eth_switch_domain_free(bp->switch_domain_id);
-		if (rc)
-			PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
-				    bp->switch_domain_id, rc);
-	}
+	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp)))
+		return;
+
+	rc = rte_eth_switch_domain_free(bp->switch_domain_id);
+	if (rc)
+		PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
+			    bp->switch_domain_id, rc);
 }
 
 static void bnxt_ptp_get_current_time(void *arg)
@@ -1545,11 +1556,9 @@ bnxt_uninit_locks(struct bnxt *bp)
 
 static void bnxt_drv_uninit(struct bnxt *bp)
 {
-	bnxt_free_switch_domain(bp);
 	bnxt_free_leds_info(bp);
 	bnxt_free_cos_queues(bp);
 	bnxt_free_link_info(bp);
-	bnxt_free_pf_info(bp);
 	bnxt_free_parent_info(bp);
 	bnxt_uninit_locks(bp);
 
@@ -1559,6 +1568,7 @@ static void bnxt_drv_uninit(struct bnxt *bp)
 	bp->rx_mem_zone = NULL;
 
 	bnxt_free_vf_info(bp);
+	bnxt_free_pf_info(bp);
 
 	rte_free(bp->grp_info);
 	bp->grp_info = NULL;
@@ -1720,6 +1730,10 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 		return rc;
 
 	memset(&new, 0, sizeof(new));
+
+	if (bp->link_info == NULL)
+		goto out;
+
 	do {
 		/* Retrieve link info from hardware */
 		rc = bnxt_get_hwrm_link_config(bp, &new);
@@ -5748,7 +5762,10 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
 	bnxt_free_mem(bp, reconfig_dev);
 
 	bnxt_hwrm_func_buf_unrgtr(bp);
-	rte_free(bp->pf->vf_req_buf);
+	if (bp->pf != NULL) {
+		rte_free(bp->pf->vf_req_buf);
+		bp->pf->vf_req_buf = NULL;
+	}
 
 	rc = bnxt_hwrm_func_driver_unregister(bp, 0);
 	bp->flags &= ~BNXT_FLAG_REGISTERED;
@@ -5761,6 +5778,8 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
 	bnxt_uninit_ctx_mem(bp);
 
 	bnxt_free_flow_stats_info(bp);
+	if (bp->rep_info != NULL)
+		bnxt_free_switch_domain(bp);
 	bnxt_free_rep_info(bp);
 	rte_free(bp->ptp_cfg);
 	bp->ptp_cfg = NULL;
drivers/net/bnxt/bnxt_hwrm.c +3 -0
index a119fc3a0b..242493982c 100644
@@ -795,6 +795,9 @@ void bnxt_free_vf_info(struct bnxt *bp)
 	if (bp->pf->vf_info == NULL)
 		return;
 
+	if (bp->pf == NULL)
+		return;
+
 	for (i = 0; i < bp->pf->max_vfs; i++) {
 		rte_free(bp->pf->vf_info[i].vlan_table);
 		bp->pf->vf_info[i].vlan_table = NULL;
Signature · 1 line
2.21.1 (Apple Git-122.3)
#1 lore raw

PATCH 2/2 net/bnxt: cap max Rx rings for Thor

For Thor devices, RSS table can only accommodate 512 Rx queues. When RSS is enabled, Cap the max Rx rings to 512.
For non-RSS case, the number will be limited by number of VNICs.

---
 drivers/net/bnxt/bnxt.h        | 13 +++++++++++++
 drivers/net/bnxt/bnxt_ethdev.c |  7 ++++---
 2 files changed, 17 insertions(+), 3 deletions(-)
drivers/net/bnxt/bnxt.h +13 -0
index 05f8804274..263ea86590 100644
@@ -844,6 +844,7 @@ struct bnxt {
 static
 inline uint16_t bnxt_max_rings(struct bnxt *bp)
 {
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	uint16_t max_tx_rings = bp->max_tx_rings;
 	uint16_t max_rx_rings = bp->max_rx_rings;
 	uint16_t max_cp_rings = bp->max_cp_rings;
@@ -861,6 +862,18 @@ inline uint16_t bnxt_max_rings(struct bnxt *bp)
 				       bp->max_stat_ctx / 2U);
 	}
 
+	if (BNXT_CHIP_P5(bp)) {
+		/* RSS table size in Thor is 512.
+		 * Cap max Rx rings to the same value for RSS.
+		 * For non-RSS case cap it to the max VNIC count.
+		 */
+		if(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+			max_rx_rings = RTE_MIN(max_rx_rings,
+					       BNXT_RSS_TBL_SIZE_P5);
+		else
+			max_rx_rings = RTE_MIN(max_rx_rings, bp->max_vnics);
+	}
+
 	max_tx_rings = RTE_MIN(max_tx_rings, max_rx_rings);
 	if (max_cp_rings > BNXT_NUM_ASYNC_CPR(bp))
 		max_cp_rings -= BNXT_NUM_ASYNC_CPR(bp);
drivers/net/bnxt/bnxt_ethdev.c +4 -3
index dc3b04110f..9da817f487 100644
@@ -422,12 +422,13 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id)
 	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
 		int j, nr_ctxs = bnxt_rss_ctxts(bp);
 
+		/* RSS table size in Thor is 512.
+		 * Cap max Rx rings to same value
+		 */
 		if (bp->rx_nr_rings > BNXT_RSS_TBL_SIZE_P5) {
 			PMD_DRV_LOG(ERR, "RxQ cnt %d > reta_size %d\n",
 				    bp->rx_nr_rings, BNXT_RSS_TBL_SIZE_P5);
-			PMD_DRV_LOG(ERR,
-				    "Only queues 0-%d will be in RSS table\n",
-				    BNXT_RSS_TBL_SIZE_P5 - 1);
+			goto err_out;
 		}
 
 		rc = 0;
Signature · 1 line
2.21.1 (Apple Git-122.3)
#2 lore raw

Re: [dpdk-dev] [PATCH 2/2] net/bnxt: cap max Rx rings for Thor

Quoted 7 lines · Ajit Khaparde

For Thor devices, RSS table can only accommodate 512 Rx queues. When RSS is enabled, Cap the max Rx rings to 512.
For non-RSS case, the number will be limited by number of VNICs.

Patch applied to dpdk-next-net-brcm.
Fixed the checkpatch error during merge.

Quoted 60 lines
---
 drivers/net/bnxt/bnxt.h        | 13 +++++++++++++
 drivers/net/bnxt/bnxt_ethdev.c |  7 ++++---
 2 files changed, 17 insertions(+), 3 deletions(-)
drivers/net/bnxt/bnxt.h +13 -0
index 05f8804274..263ea86590 100644
@@ -844,6 +844,7 @@ struct bnxt {
 static
 inline uint16_t bnxt_max_rings(struct bnxt *bp)
 {
+       struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
        uint16_t max_tx_rings = bp->max_tx_rings;
        uint16_t max_rx_rings = bp->max_rx_rings;
        uint16_t max_cp_rings = bp->max_cp_rings;
@@ -861,6 +862,18 @@ inline uint16_t bnxt_max_rings(struct bnxt *bp)
                                       bp->max_stat_ctx / 2U);
        }
 
+       if (BNXT_CHIP_P5(bp)) {
+               /* RSS table size in Thor is 512.
+                * Cap max Rx rings to the same value for RSS.
+                * For non-RSS case cap it to the max VNIC count.
+                */
+               if(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+                       max_rx_rings = RTE_MIN(max_rx_rings,
+                                              BNXT_RSS_TBL_SIZE_P5);
+               else
+                       max_rx_rings = RTE_MIN(max_rx_rings, bp->max_vnics);
+       }
+
        max_tx_rings = RTE_MIN(max_tx_rings, max_rx_rings);
        if (max_cp_rings > BNXT_NUM_ASYNC_CPR(bp))
                max_cp_rings -= BNXT_NUM_ASYNC_CPR(bp);
drivers/net/bnxt/bnxt_ethdev.c +4 -4
index dc3b04110f..9da817f487 100644
@@ -422,12 +422,13 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id)
        if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
                int j, nr_ctxs = bnxt_rss_ctxts(bp);
 
+               /* RSS table size in Thor is 512.
+                * Cap max Rx rings to same value
+                */
                if (bp->rx_nr_rings > BNXT_RSS_TBL_SIZE_P5) {
                        PMD_DRV_LOG(ERR, "RxQ cnt %d > reta_size %d\n",
                                    bp->rx_nr_rings, BNXT_RSS_TBL_SIZE_P5);
-                       PMD_DRV_LOG(ERR,
-                                   "Only queues 0-%d will be in RSS table\n",
-                                   BNXT_RSS_TBL_SIZE_P5 - 1);
+                       goto err_out;
                }
 
                rc = 0;
--

2.21.1 (Apple Git-122.3)

#3 lore raw
In reply to Ajit Khaparde
Quoted 13 lines · Ajit Khaparde

Fix resource cleanup in port close.
Once the pointers are freed, set them to NULL. Make sure access to the pointers is validated before use.

Fixes bb81e07323bb ("net/bnxt: support LED on/off")
Fixes 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
Fixes 1d0704f4d793 ("net/bnxt: add device configure operation")
Fixes 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Fixes 322bd6e70272 ("net/bnxt: add port representor infrastructure")
Fixes 0bf5a0b5ebb8 ("net/bnxt: add a failure log")
Quoted 1 line

Patch applied to dpdk-next-net-brcm.

Quoted 144 lines
---
 drivers/net/bnxt/bnxt_ethdev.c | 39 +++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_hwrm.c   |  3 +++
 2 files changed, 32 insertions(+), 10 deletions(-)
drivers/net/bnxt/bnxt_ethdev.c +29 -10
index 20ca57eae5..dc3b04110f 100644
@@ -219,16 +219,19 @@ uint16_t bnxt_rss_hash_tbl_size(const struct bnxt *bp)
 static void bnxt_free_parent_info(struct bnxt *bp)
 {
        rte_free(bp->parent);
+       bp->parent = NULL;
 }
 
 static void bnxt_free_pf_info(struct bnxt *bp)
 {
        rte_free(bp->pf);
+       bp->pf = NULL;
 }
 
 static void bnxt_free_link_info(struct bnxt *bp)
 {
        rte_free(bp->link_info);
+       bp->link_info = NULL;
 }
 
 static void bnxt_free_leds_info(struct bnxt *bp)
@@ -249,7 +252,9 @@ static void bnxt_free_flow_stats_info(struct bnxt *bp)
 static void bnxt_free_cos_queues(struct bnxt *bp)
 {
        rte_free(bp->rx_cos_queue);
+       bp->rx_cos_queue = NULL;
        rte_free(bp->tx_cos_queue);
+       bp->tx_cos_queue = NULL;
 }
 
 static void bnxt_free_mem(struct bnxt *bp, bool reconfig)
@@ -849,9 +854,14 @@ static int bnxt_shutdown_nic(struct bnxt *bp)
 
 uint32_t bnxt_get_speed_capabilities(struct bnxt *bp)
 {
-       uint32_t link_speed = bp->link_info->support_speeds;
+       uint32_t link_speed = 0;
        uint32_t speed_capa = 0;
 
+       if (bp->link_info == NULL)
+               return 0;
+
+       link_speed = bp->link_info->support_speeds;
+
        /* If PAM4 is configured, use PAM4 supported speed */
        if (link_speed == 0 && bp->link_info->support_pam4_speeds > 0)
                link_speed = bp->link_info->support_pam4_speeds;
@@ -1293,12 +1303,13 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
 {
        int rc = 0;
 
-       if (bp->switch_domain_id) {
-               rc = rte_eth_switch_domain_free(bp->switch_domain_id);
-               if (rc)
-                       PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
-                                   bp->switch_domain_id, rc);
-       }
+       if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp)))
+               return;
+
+       rc = rte_eth_switch_domain_free(bp->switch_domain_id);
+       if (rc)
+               PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
+                           bp->switch_domain_id, rc);
 }
 
 static void bnxt_ptp_get_current_time(void *arg)
@@ -1545,11 +1556,9 @@ bnxt_uninit_locks(struct bnxt *bp)
 
 static void bnxt_drv_uninit(struct bnxt *bp)
 {
-       bnxt_free_switch_domain(bp);
        bnxt_free_leds_info(bp);
        bnxt_free_cos_queues(bp);
        bnxt_free_link_info(bp);
-       bnxt_free_pf_info(bp);
        bnxt_free_parent_info(bp);
        bnxt_uninit_locks(bp);
 
@@ -1559,6 +1568,7 @@ static void bnxt_drv_uninit(struct bnxt *bp)
        bp->rx_mem_zone = NULL;
 
        bnxt_free_vf_info(bp);
+       bnxt_free_pf_info(bp);
 
        rte_free(bp->grp_info);
        bp->grp_info = NULL;
@@ -1720,6 +1730,10 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
                return rc;
 
        memset(&new, 0, sizeof(new));
+
+       if (bp->link_info == NULL)
+               goto out;
+
        do {
                /* Retrieve link info from hardware */
                rc = bnxt_get_hwrm_link_config(bp, &new);
@@ -5748,7 +5762,10 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
        bnxt_free_mem(bp, reconfig_dev);
 
        bnxt_hwrm_func_buf_unrgtr(bp);
-       rte_free(bp->pf->vf_req_buf);
+       if (bp->pf != NULL) {
+               rte_free(bp->pf->vf_req_buf);
+               bp->pf->vf_req_buf = NULL;
+       }
 
        rc = bnxt_hwrm_func_driver_unregister(bp, 0);
        bp->flags &= ~BNXT_FLAG_REGISTERED;
@@ -5761,6 +5778,8 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
        bnxt_uninit_ctx_mem(bp);
 
        bnxt_free_flow_stats_info(bp);
+       if (bp->rep_info != NULL)
+               bnxt_free_switch_domain(bp);
        bnxt_free_rep_info(bp);
        rte_free(bp->ptp_cfg);
        bp->ptp_cfg = NULL;
drivers/net/bnxt/bnxt_hwrm.c +3 -1
index a119fc3a0b..242493982c 100644
@@ -795,6 +795,9 @@ void bnxt_free_vf_info(struct bnxt *bp)
        if (bp->pf->vf_info == NULL)
                return;
 
+       if (bp->pf == NULL)
+               return;
+
        for (i = 0; i < bp->pf->max_vfs; i++) {
                rte_free(bp->pf->vf_info[i].vlan_table);
                bp->pf->vf_info[i].vlan_table = NULL;
--

2.21.1 (Apple Git-122.3)