On Thu, Nov 18, 2021 at 9:40 PM Ajit Khaparde
<ajit.khapa...@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.khapa...@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.ko...@broadcom.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.pura...@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)
>
