From: Ajit Khaparde <ajit.khaparde@broadcom.com>
To: dpdk-dev <dev@dpdk.org>
Cc: Somnath Kotur <somnath.kotur@broadcom.com>,
Kalesh AP <kalesh-anakkur.purayil@broadcom.com>,
Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH] net/bnxt: fail init when mbuf allocation fails
Date: Thu, 18 Nov 2021 22:24:44 -0800 [thread overview]
Message-ID: <CACZ4nhsBOAoOiuP94sD1RMHuqU+vCTA=AohhY5tDL42pdwLvJQ@mail.gmail.com> (raw)
In-Reply-To: <20211119053821.93989-1-ajit.khaparde@broadcom.com>
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)
>
prev parent reply other threads:[~2021-11-19 6:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-19 5:38 Ajit Khaparde
2021-11-19 6:24 ` Ajit Khaparde [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACZ4nhsBOAoOiuP94sD1RMHuqU+vCTA=AohhY5tDL42pdwLvJQ@mail.gmail.com' \
--to=ajit.khaparde@broadcom.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=kalesh-anakkur.purayil@broadcom.com \
--cc=somnath.kotur@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).