net/bnxt: create ring group array only when needed

Message ID 20190710171119.19501-1-lance.richardson@broadcom.com
State Accepted
Delegated to: Ferruh Yigit
Headers show
Series
  • net/bnxt: create ring group array only when needed
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Lance Richardson July 10, 2019, 5:11 p.m.
Fix an overrun of the ring group array with BCM5750X-based
adapters by ensuring that the ring group array is not allocated
or accessed for adapters that do not support ring groups.

Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 27 ++++++++++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c   | 14 ++++++++++----
 drivers/net/bnxt/bnxt_ring.c   | 30 ++++++++++++++++--------------
 3 files changed, 42 insertions(+), 29 deletions(-)

Comments

Ferruh Yigit July 16, 2019, 4:16 p.m. | #1
On 7/10/2019 6:11 PM, Lance Richardson wrote:
> Fix an overrun of the ring group array with BCM5750X-based
> adapters by ensuring that the ring group array is not allocated
> or accessed for adapters that do not support ring groups.
> 
> Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
> Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
> Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>

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

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 8fc510351..14b69a480 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -463,9 +463,11 @@  static int bnxt_init_nic(struct bnxt *bp)
 {
 	int rc;
 
-	rc = bnxt_init_ring_grps(bp);
-	if (rc)
-		return rc;
+	if (BNXT_HAS_RING_GRPS(bp)) {
+		rc = bnxt_init_ring_grps(bp);
+		if (rc)
+			return rc;
+	}
 
 	bnxt_init_vnics(bp);
 	bnxt_init_filters(bp);
@@ -3861,14 +3863,17 @@  bnxt_dev_init(struct rte_eth_dev *eth_dev)
 		goto error_free;
 	}
 
-	bp->grp_info = rte_zmalloc("bnxt_grp_info",
-				sizeof(*bp->grp_info) * bp->max_ring_grps, 0);
-	if (!bp->grp_info) {
-		PMD_DRV_LOG(ERR,
-			"Failed to alloc %zu bytes to store group info table\n",
-			sizeof(*bp->grp_info) * bp->max_ring_grps);
-		rc = -ENOMEM;
-		goto error_free;
+	if (BNXT_HAS_RING_GRPS(bp)) {
+		bp->grp_info = rte_zmalloc("bnxt_grp_info",
+					sizeof(*bp->grp_info) *
+						bp->max_ring_grps, 0);
+		if (!bp->grp_info) {
+			PMD_DRV_LOG(ERR,
+				"Failed to alloc %zu bytes for grp info tbl.\n",
+				sizeof(*bp->grp_info) * bp->max_ring_grps);
+			rc = -ENOMEM;
+			goto error_free;
+		}
 	}
 
 	/* Forward all requests if firmware is new enough */
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 71f03775a..8f0d33dca 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2004,7 +2004,8 @@  int bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 			cpr = bp->tx_queues[i - bp->rx_cp_nr_rings]->cp_ring;
 		} else {
 			cpr = bp->rx_queues[i]->cp_ring;
-			bp->grp_info[i].fw_stats_ctx = -1;
+			if (BNXT_HAS_RING_GRPS(bp))
+				bp->grp_info[i].fw_stats_ctx = -1;
 		}
 		if (cpr->hw_stats_ctx_id != HWRM_NA_SIGNATURE) {
 			rc = bnxt_hwrm_stat_ctx_free(bp, cpr, i);
@@ -2098,7 +2099,9 @@  void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 		bnxt_hwrm_ring_free(bp, ring,
 				    HWRM_RING_FREE_INPUT_RING_TYPE_RX);
 		ring->fw_ring_id = INVALID_HW_RING_ID;
-		bp->grp_info[queue_index].rx_fw_ring_id = INVALID_HW_RING_ID;
+		if (BNXT_HAS_RING_GRPS(bp))
+			bp->grp_info[queue_index].rx_fw_ring_id =
+							INVALID_HW_RING_ID;
 		memset(rxr->rx_desc_ring, 0,
 		       rxr->rx_ring_struct->ring_size *
 		       sizeof(*rxr->rx_desc_ring));
@@ -2118,7 +2121,9 @@  void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 		       rxr->ag_ring_struct->ring_size *
 		       sizeof(*rxr->ag_buf_ring));
 		rxr->ag_prod = 0;
-		bp->grp_info[queue_index].ag_fw_ring_id = INVALID_HW_RING_ID;
+		if (BNXT_HAS_RING_GRPS(bp))
+			bp->grp_info[queue_index].ag_fw_ring_id =
+							INVALID_HW_RING_ID;
 	}
 	if (cpr->cp_ring_struct->fw_ring_id != INVALID_HW_RING_ID) {
 		bnxt_free_cp_ring(bp, cpr);
@@ -2126,7 +2131,8 @@  void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 			bnxt_free_nq_ring(bp, rxq->nq_ring);
 	}
 
-	bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
+	if (BNXT_HAS_RING_GRPS(bp))
+		bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
 }
 
 int bnxt_free_all_hwrm_rings(struct bnxt *bp)
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 56bb463a6..16327dbe9 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -397,8 +397,7 @@  static int bnxt_alloc_cmpl_ring(struct bnxt *bp, int queue_index,
 }
 
 static int bnxt_alloc_nq_ring(struct bnxt *bp, int queue_index,
-			      struct bnxt_cp_ring_info *nqr,
-			      bool rx)
+			      struct bnxt_cp_ring_info *nqr)
 {
 	struct bnxt_ring *nq_ring = nqr->cp_ring_struct;
 	uint8_t ring_type;
@@ -414,9 +413,6 @@  static int bnxt_alloc_nq_ring(struct bnxt *bp, int queue_index,
 	if (rc)
 		return rc;
 
-	if (rx)
-		bp->grp_info[queue_index].cp_fw_ring_id = nq_ring->fw_ring_id;
-
 	bnxt_set_db(bp, &nqr->cp_db, ring_type, queue_index,
 		    nq_ring->fw_ring_id);
 	bnxt_db_nq(nqr);
@@ -443,7 +439,8 @@  static int bnxt_alloc_rx_ring(struct bnxt *bp, int queue_index)
 		return rc;
 
 	rxr->rx_prod = 0;
-	bp->grp_info[queue_index].rx_fw_ring_id = ring->fw_ring_id;
+	if (BNXT_HAS_RING_GRPS(bp))
+		bp->grp_info[queue_index].rx_fw_ring_id = ring->fw_ring_id;
 	bnxt_set_db(bp, &rxr->rx_db, ring_type, queue_index, ring->fw_ring_id);
 	bnxt_db_write(&rxr->rx_db, rxr->rx_prod);
 
@@ -478,7 +475,8 @@  static int bnxt_alloc_rx_agg_ring(struct bnxt *bp, int queue_index)
 		return rc;
 
 	rxr->ag_prod = 0;
-	bp->grp_info[queue_index].ag_fw_ring_id = ring->fw_ring_id;
+	if (BNXT_HAS_RING_GRPS(bp))
+		bp->grp_info[queue_index].ag_fw_ring_id = ring->fw_ring_id;
 	bnxt_set_db(bp, &rxr->ag_db, ring_type, map_idx, ring->fw_ring_id);
 	bnxt_db_write(&rxr->ag_db, rxr->ag_prod);
 
@@ -495,15 +493,17 @@  int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 	int rc = 0;
 
 	if (BNXT_HAS_NQ(bp)) {
-		if (bnxt_alloc_nq_ring(bp, queue_index, nqr, true))
+		if (bnxt_alloc_nq_ring(bp, queue_index, nqr))
 			goto err_out;
 	}
 
 	if (bnxt_alloc_cmpl_ring(bp, queue_index, cpr, nqr))
 		goto err_out;
 
-	bp->grp_info[queue_index].fw_stats_ctx = cpr->hw_stats_ctx_id;
-	bp->grp_info[queue_index].cp_fw_ring_id = cp_ring->fw_ring_id;
+	if (BNXT_HAS_RING_GRPS(bp)) {
+		bp->grp_info[queue_index].fw_stats_ctx = cpr->hw_stats_ctx_id;
+		bp->grp_info[queue_index].cp_fw_ring_id = cp_ring->fw_ring_id;
+	}
 
 	if (!queue_index) {
 		/*
@@ -570,15 +570,17 @@  int bnxt_alloc_hwrm_rings(struct bnxt *bp)
 		struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
 
 		if (BNXT_HAS_NQ(bp)) {
-			if (bnxt_alloc_nq_ring(bp, i, nqr, true))
+			if (bnxt_alloc_nq_ring(bp, i, nqr))
 				goto err_out;
 		}
 
 		if (bnxt_alloc_cmpl_ring(bp, i, cpr, nqr))
 			goto err_out;
 
-		bp->grp_info[i].fw_stats_ctx = cpr->hw_stats_ctx_id;
-		bp->grp_info[i].cp_fw_ring_id = cp_ring->fw_ring_id;
+		if (BNXT_HAS_RING_GRPS(bp)) {
+			bp->grp_info[i].fw_stats_ctx = cpr->hw_stats_ctx_id;
+			bp->grp_info[i].cp_fw_ring_id = cp_ring->fw_ring_id;
+		}
 
 		bnxt_hwrm_set_ring_coal(bp, &coal, cp_ring->fw_ring_id);
 
@@ -623,7 +625,7 @@  int bnxt_alloc_hwrm_rings(struct bnxt *bp)
 		unsigned int idx = i + bp->rx_cp_nr_rings;
 
 		if (BNXT_HAS_NQ(bp)) {
-			if (bnxt_alloc_nq_ring(bp, idx, nqr, false))
+			if (bnxt_alloc_nq_ring(bp, idx, nqr))
 				goto err_out;
 		}