* [PATCH] net/bnxt: fail init when mbuf allocation fails
@ 2021-11-19 5:38 Ajit Khaparde
2021-11-19 6:24 ` Ajit Khaparde
0 siblings, 1 reply; 2+ messages in thread
From: Ajit Khaparde @ 2021-11-19 5:38 UTC (permalink / raw)
To: dev; +Cc: Somnath Kotur, Kalesh AP
[-- Attachment #1: Type: text/plain, Size: 5100 bytes --]
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(-)
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)
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] net/bnxt: fail init when mbuf allocation fails
2021-11-19 5:38 [PATCH] net/bnxt: fail init when mbuf allocation fails Ajit Khaparde
@ 2021-11-19 6:24 ` Ajit Khaparde
0 siblings, 0 replies; 2+ messages in thread
From: Ajit Khaparde @ 2021-11-19 6:24 UTC (permalink / raw)
To: dpdk-dev; +Cc: Somnath Kotur, Kalesh AP, Ferruh Yigit
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)
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-19 6:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 5:38 [PATCH] net/bnxt: fail init when mbuf allocation fails Ajit Khaparde
2021-11-19 6:24 ` Ajit Khaparde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).