net/bnxt: fail init when mbuf allocation fails

Message ID 20211119053821.93989-1-ajit.khaparde@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series net/bnxt: fail init when mbuf allocation fails |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Ajit Khaparde Nov. 19, 2021, 5:38 a.m. UTC
  Fix driver init when Rx mbuf allocation fails.
If we continue to use the driver with whatever rings were
created successfully, it can cause unexpected behavior.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 41 ++++++++++++++++++++++--------------
 drivers/net/bnxt/bnxt_ring.c |  5 +++--
 drivers/net/bnxt/bnxt_rxr.c  |  8 +++----
 3 files changed, 32 insertions(+), 22 deletions(-)
  

Comments

Ajit Khaparde Nov. 19, 2021, 6:24 a.m. UTC | #1
On Thu, Nov 18, 2021 at 9:40 PM Ajit Khaparde
<ajit.khaparde@broadcom.com> wrote:
>
> Fix driver init when Rx mbuf allocation fails.
> If we continue to use the driver with whatever rings were
> created successfully, it can cause unexpected behavior.
>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Patch applied to dpdk-next-net-brcm.


> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 41 ++++++++++++++++++++++--------------
>  drivers/net/bnxt/bnxt_ring.c |  5 +++--
>  drivers/net/bnxt/bnxt_rxr.c  |  8 +++----
>  3 files changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 7f51c61097..f53f8632fe 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -2633,6 +2633,8 @@ bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
>                 cpr = bp->rx_queues[i]->cp_ring;
>                 if (BNXT_HAS_RING_GRPS(bp))
>                         bp->grp_info[i].fw_stats_ctx = -1;
> +               if (cpr == NULL)
> +                       continue;
>                 rc = bnxt_hwrm_stat_ctx_free(bp, cpr);
>                 if (rc)
>                         return rc;
> @@ -2640,6 +2642,8 @@ bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
>
>         for (i = 0; i < bp->tx_cp_nr_rings; i++) {
>                 cpr = bp->tx_queues[i]->cp_ring;
> +               if (cpr == NULL)
> +                       continue;
>                 rc = bnxt_hwrm_stat_ctx_free(bp, cpr);
>                 if (rc)
>                         return rc;
> @@ -2697,16 +2701,17 @@ void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr)
>  void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
>  {
>         struct bnxt_rx_queue *rxq = bp->rx_queues[queue_index];
> -       struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
> -       struct bnxt_ring *ring = rxr->rx_ring_struct;
> -       struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
> +       struct bnxt_rx_ring_info *rxr = rxq ? rxq->rx_ring : NULL;
> +       struct bnxt_ring *ring = rxr ? rxr->rx_ring_struct : NULL;
> +       struct bnxt_cp_ring_info *cpr = rxq ? rxq->cp_ring : NULL;
>
>         if (BNXT_HAS_RING_GRPS(bp))
>                 bnxt_hwrm_ring_grp_free(bp, queue_index);
>
> -       bnxt_hwrm_ring_free(bp, ring,
> -                           HWRM_RING_FREE_INPUT_RING_TYPE_RX,
> -                           cpr->cp_ring_struct->fw_ring_id);
> +       if (ring != NULL && cpr != NULL)
> +               bnxt_hwrm_ring_free(bp, ring,
> +                                   HWRM_RING_FREE_INPUT_RING_TYPE_RX,
> +                                   cpr->cp_ring_struct->fw_ring_id);
>         if (BNXT_HAS_RING_GRPS(bp))
>                 bp->grp_info[queue_index].rx_fw_ring_id = INVALID_HW_RING_ID;
>
> @@ -2715,22 +2720,26 @@ void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
>          * but we may have to deal with agg ring struct before the offload
>          * flags are updated.
>          */
> -       if (!bnxt_need_agg_ring(bp->eth_dev) || rxr->ag_ring_struct == NULL)
> +       if (!bnxt_need_agg_ring(bp->eth_dev) ||
> +           (rxr && rxr->ag_ring_struct == NULL))
>                 goto no_agg;
>
> -       ring = rxr->ag_ring_struct;
> -       bnxt_hwrm_ring_free(bp, ring,
> -                           BNXT_CHIP_P5(bp) ?
> -                           HWRM_RING_FREE_INPUT_RING_TYPE_RX_AGG :
> -                           HWRM_RING_FREE_INPUT_RING_TYPE_RX,
> -                           cpr->cp_ring_struct->fw_ring_id);
> +       ring = rxr ? rxr->ag_ring_struct : NULL;
> +       if (ring != NULL && cpr != NULL) {
> +               bnxt_hwrm_ring_free(bp, ring,
> +                                   BNXT_CHIP_P5(bp) ?
> +                                   HWRM_RING_FREE_INPUT_RING_TYPE_RX_AGG :
> +                                   HWRM_RING_FREE_INPUT_RING_TYPE_RX,
> +                                   cpr->cp_ring_struct->fw_ring_id);
> +       }
>         if (BNXT_HAS_RING_GRPS(bp))
>                 bp->grp_info[queue_index].ag_fw_ring_id = INVALID_HW_RING_ID;
>
>  no_agg:
> -       bnxt_hwrm_stat_ctx_free(bp, cpr);
> -
> -       bnxt_free_cp_ring(bp, cpr);
> +       if (cpr != NULL) {
> +               bnxt_hwrm_stat_ctx_free(bp, cpr);
> +               bnxt_free_cp_ring(bp, cpr);
> +       }
>
>         if (BNXT_HAS_RING_GRPS(bp))
>                 bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
> diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
> index 7940d489a1..dc437f314e 100644
> --- a/drivers/net/bnxt/bnxt_ring.c
> +++ b/drivers/net/bnxt/bnxt_ring.c
> @@ -648,8 +648,9 @@ int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int queue_index)
>
>         if (rxq->rx_started) {
>                 if (bnxt_init_one_rx_ring(rxq)) {
> -                       PMD_DRV_LOG(ERR, "bnxt_init_one_rx_ring failed!\n");
> -                       bnxt_rx_queue_release_op(bp->eth_dev, queue_index);
> +                       PMD_DRV_LOG(ERR,
> +                                   "ring%d bnxt_init_one_rx_ring failed!\n",
> +                                   queue_index);
>                         rc = -ENOMEM;
>                         goto err_out;
>                 }
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index e2f09ad3a0..44247d7200 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -1332,9 +1332,9 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
>                 if (unlikely(!rxr->rx_buf_ring[i])) {
>                         if (bnxt_alloc_rx_data(rxq, rxr, raw_prod) != 0) {
>                                 PMD_DRV_LOG(WARNING,
> -                                           "init'ed rx ring %d with %d/%d mbufs only\n",
> +                                           "RxQ %d allocated %d of %d mbufs\n",
>                                             rxq->queue_id, i, ring->ring_size);
> -                               break;
> +                               return -ENOMEM;
>                         }
>                 }
>                 rxr->rx_raw_prod = raw_prod;
> @@ -1362,9 +1362,9 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
>                 if (unlikely(!rxr->ag_buf_ring[i])) {
>                         if (bnxt_alloc_ag_data(rxq, rxr, raw_prod) != 0) {
>                                 PMD_DRV_LOG(WARNING,
> -                                           "init'ed AG ring %d with %d/%d mbufs only\n",
> +                                           "RxQ %d allocated %d of %d mbufs\n",
>                                             rxq->queue_id, i, ring->ring_size);
> -                               break;
> +                               return -ENOMEM;
>                         }
>                 }
>                 rxr->ag_raw_prod = raw_prod;
> --
> 2.30.1 (Apple Git-130)
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 7f51c61097..f53f8632fe 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2633,6 +2633,8 @@  bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 		cpr = bp->rx_queues[i]->cp_ring;
 		if (BNXT_HAS_RING_GRPS(bp))
 			bp->grp_info[i].fw_stats_ctx = -1;
+		if (cpr == NULL)
+			continue;
 		rc = bnxt_hwrm_stat_ctx_free(bp, cpr);
 		if (rc)
 			return rc;
@@ -2640,6 +2642,8 @@  bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 
 	for (i = 0; i < bp->tx_cp_nr_rings; i++) {
 		cpr = bp->tx_queues[i]->cp_ring;
+		if (cpr == NULL)
+			continue;
 		rc = bnxt_hwrm_stat_ctx_free(bp, cpr);
 		if (rc)
 			return rc;
@@ -2697,16 +2701,17 @@  void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr)
 void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 {
 	struct bnxt_rx_queue *rxq = bp->rx_queues[queue_index];
-	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	struct bnxt_ring *ring = rxr->rx_ring_struct;
-	struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
+	struct bnxt_rx_ring_info *rxr = rxq ? rxq->rx_ring : NULL;
+	struct bnxt_ring *ring = rxr ? rxr->rx_ring_struct : NULL;
+	struct bnxt_cp_ring_info *cpr = rxq ? rxq->cp_ring : NULL;
 
 	if (BNXT_HAS_RING_GRPS(bp))
 		bnxt_hwrm_ring_grp_free(bp, queue_index);
 
-	bnxt_hwrm_ring_free(bp, ring,
-			    HWRM_RING_FREE_INPUT_RING_TYPE_RX,
-			    cpr->cp_ring_struct->fw_ring_id);
+	if (ring != NULL && cpr != NULL)
+		bnxt_hwrm_ring_free(bp, ring,
+				    HWRM_RING_FREE_INPUT_RING_TYPE_RX,
+				    cpr->cp_ring_struct->fw_ring_id);
 	if (BNXT_HAS_RING_GRPS(bp))
 		bp->grp_info[queue_index].rx_fw_ring_id = INVALID_HW_RING_ID;
 
@@ -2715,22 +2720,26 @@  void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 	 * but we may have to deal with agg ring struct before the offload
 	 * flags are updated.
 	 */
-	if (!bnxt_need_agg_ring(bp->eth_dev) || rxr->ag_ring_struct == NULL)
+	if (!bnxt_need_agg_ring(bp->eth_dev) ||
+	    (rxr && rxr->ag_ring_struct == NULL))
 		goto no_agg;
 
-	ring = rxr->ag_ring_struct;
-	bnxt_hwrm_ring_free(bp, ring,
-			    BNXT_CHIP_P5(bp) ?
-			    HWRM_RING_FREE_INPUT_RING_TYPE_RX_AGG :
-			    HWRM_RING_FREE_INPUT_RING_TYPE_RX,
-			    cpr->cp_ring_struct->fw_ring_id);
+	ring = rxr ? rxr->ag_ring_struct : NULL;
+	if (ring != NULL && cpr != NULL) {
+		bnxt_hwrm_ring_free(bp, ring,
+				    BNXT_CHIP_P5(bp) ?
+				    HWRM_RING_FREE_INPUT_RING_TYPE_RX_AGG :
+				    HWRM_RING_FREE_INPUT_RING_TYPE_RX,
+				    cpr->cp_ring_struct->fw_ring_id);
+	}
 	if (BNXT_HAS_RING_GRPS(bp))
 		bp->grp_info[queue_index].ag_fw_ring_id = INVALID_HW_RING_ID;
 
 no_agg:
-	bnxt_hwrm_stat_ctx_free(bp, cpr);
-
-	bnxt_free_cp_ring(bp, cpr);
+	if (cpr != NULL) {
+		bnxt_hwrm_stat_ctx_free(bp, cpr);
+		bnxt_free_cp_ring(bp, cpr);
+	}
 
 	if (BNXT_HAS_RING_GRPS(bp))
 		bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 7940d489a1..dc437f314e 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -648,8 +648,9 @@  int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 
 	if (rxq->rx_started) {
 		if (bnxt_init_one_rx_ring(rxq)) {
-			PMD_DRV_LOG(ERR, "bnxt_init_one_rx_ring failed!\n");
-			bnxt_rx_queue_release_op(bp->eth_dev, queue_index);
+			PMD_DRV_LOG(ERR,
+				    "ring%d bnxt_init_one_rx_ring failed!\n",
+				    queue_index);
 			rc = -ENOMEM;
 			goto err_out;
 		}
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index e2f09ad3a0..44247d7200 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1332,9 +1332,9 @@  int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
 		if (unlikely(!rxr->rx_buf_ring[i])) {
 			if (bnxt_alloc_rx_data(rxq, rxr, raw_prod) != 0) {
 				PMD_DRV_LOG(WARNING,
-					    "init'ed rx ring %d with %d/%d mbufs only\n",
+					    "RxQ %d allocated %d of %d mbufs\n",
 					    rxq->queue_id, i, ring->ring_size);
-				break;
+				return -ENOMEM;
 			}
 		}
 		rxr->rx_raw_prod = raw_prod;
@@ -1362,9 +1362,9 @@  int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
 		if (unlikely(!rxr->ag_buf_ring[i])) {
 			if (bnxt_alloc_ag_data(rxq, rxr, raw_prod) != 0) {
 				PMD_DRV_LOG(WARNING,
-					    "init'ed AG ring %d with %d/%d mbufs only\n",
+					    "RxQ %d allocated %d of %d mbufs\n",
 					    rxq->queue_id, i, ring->ring_size);
-				break;
+				return -ENOMEM;
 			}
 		}
 		rxr->ag_raw_prod = raw_prod;