net/bnxt: fail init when mbuf allocation fails
Checks
Commit Message
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
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)
>
@@ -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;
@@ -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;
}
@@ -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;