From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 21548A04DD; Wed, 21 Oct 2020 07:33:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F3D6AAC7C; Wed, 21 Oct 2020 07:33:25 +0200 (CEST) Received: from mail-oo1-f68.google.com (mail-oo1-f68.google.com [209.85.161.68]) by dpdk.org (Postfix) with ESMTP id 4E21FAC6D for ; Wed, 21 Oct 2020 07:33:23 +0200 (CEST) Received: by mail-oo1-f68.google.com with SMTP id c10so257472oon.6 for ; Tue, 20 Oct 2020 22:33:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VnbdJVxbqH+ibyocOLXkdP6fpC497FCqQjqYttwt7og=; b=G6GymBkCdMUhn6EJ/vb/qQUzwSSr/+BmhiCnxKaCVtWu/RLnRdQ/PWAzuGhD3KABaQ I2ZltmB9Qq8GRzg2zFuPJN1rblimcd9/98bhtANcGFs4xTfX/aGA06yrigFXji0kX8xL vdRovHYxAfw6hK9Vn/gJXyidx9fAI+vW+1GQg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VnbdJVxbqH+ibyocOLXkdP6fpC497FCqQjqYttwt7og=; b=bU1hrxpZWxnoQeOyfdg8Kj3azAx29tYyt4aKSzrcXbAn46q0VfvUYYwW1bAgEUGyV/ RMhYTwc1VuQwezs60n+1RzGe4Dc1meJts5Q7JWmddfQZ24OWLOePFWKM0FRgcGJdjlR1 CEDQgmsOFq6l6B6ipuIsgARspZPSklphne5UNlQK8GwPdbtFSWpvwNW/Var3Me0rDVsC w9Sg2eDMUCKs5I2TTS2dClB9k94JDHvkLI/fdfVPlfG+Qf2BMydYRf0LJeAYYG+pgCqu PW1WiAm4LG34jnGjJZJ80GD9wIwKA/kPnZ/M4q5EWojvXCz69pVL5/6VU+1SUBwIrGC3 pyog== X-Gm-Message-State: AOAM531SG79AINBKFOChdcE3I6mlDIlUkpcNUE/Av63xVp2qd2EzCnNh ObeoWd7BQBO5inpraafaSjOFJ4pAXjABcccFw6IGPQ== X-Google-Smtp-Source: ABdhPJyhDUz3ii6oNtXeZnSi/SA7C+/Sf0xPqywhPrxwT63HMJBC6ryDLo9M77+3/bnuHz7ezzTKkEcsaEviu7wU11c= X-Received: by 2002:a4a:9e87:: with SMTP id u7mr1174977ook.15.1603258402079; Tue, 20 Oct 2020 22:33:22 -0700 (PDT) MIME-Version: 1.0 References: <20201020041118.23547-1-somnath.kotur@broadcom.com> In-Reply-To: <20201020041118.23547-1-somnath.kotur@broadcom.com> From: Ajit Khaparde Date: Tue, 20 Oct 2020 22:33:05 -0700 Message-ID: To: Somnath Kotur Cc: dpdk-dev , Ferruh Yigit , dpdk stable Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Oct 19, 2020 at 9:19 PM Somnath Kotur wrote: > > Some of the ring related memory was not being freed in both the release > ops. Fix to free them now > Add some more NULL ptr checks in the corresponding queue_release_mbufs() > and queue_release_op() respectively. > Also call queue_release_op() in the error path of the corresponding > queue_setup_op() > > Fixes: 6133f207970c ("net/bnxt: add Rx queue create/destroy") > Fixes: 51c87ebafc7d ("net/bnxt: add Tx queue create/destroy") > > Cc: stable@dpdk.org > > Signed-off-by: Somnath Kotur Patch applied to dpdk-next-net-brcm. > --- > drivers/net/bnxt/bnxt_rxq.c | 44 +++++++++++++++++++++++++++----------------- > drivers/net/bnxt/bnxt_txq.c | 36 +++++++++++++++++++++--------------- > 2 files changed, 48 insertions(+), 32 deletions(-) > > diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c > index 1003ca6..7851414 100644 > --- a/drivers/net/bnxt/bnxt_rxq.c > +++ b/drivers/net/bnxt/bnxt_rxq.c > @@ -207,7 +207,7 @@ void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq) > struct bnxt_tpa_info *tpa_info; > uint16_t i; > > - if (!rxq) > + if (!rxq || !rxq->rx_ring) > return; > > rte_spinlock_lock(&rxq->lock); > @@ -273,12 +273,21 @@ void bnxt_rx_queue_release_op(void *rx_queue) > bnxt_rx_queue_release_mbufs(rxq); > > /* Free RX ring hardware descriptors */ > - bnxt_free_ring(rxq->rx_ring->rx_ring_struct); > - /* Free RX Agg ring hardware descriptors */ > - bnxt_free_ring(rxq->rx_ring->ag_ring_struct); > - > + if (rxq->rx_ring) { > + bnxt_free_ring(rxq->rx_ring->rx_ring_struct); > + rte_free(rxq->rx_ring->rx_ring_struct); > + /* Free RX Agg ring hardware descriptors */ > + bnxt_free_ring(rxq->rx_ring->ag_ring_struct); > + rte_free(rxq->rx_ring->ag_ring_struct); > + > + rte_free(rxq->rx_ring); > + } > /* Free RX completion ring hardware descriptors */ > - bnxt_free_ring(rxq->cp_ring->cp_ring_struct); > + if (rxq->cp_ring) { > + bnxt_free_ring(rxq->cp_ring->cp_ring_struct); > + rte_free(rxq->cp_ring->cp_ring_struct); > + rte_free(rxq->cp_ring); > + } > > bnxt_free_rxq_stats(rxq); > rte_memzone_free(rxq->mz); > @@ -314,8 +323,7 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, > > if (nb_desc < BNXT_MIN_RING_DESC || nb_desc > MAX_RX_DESC_CNT) { > PMD_DRV_LOG(ERR, "nb_desc %d is invalid\n", nb_desc); > - rc = -EINVAL; > - goto out; > + return -EINVAL; > } > > if (eth_dev->data->rx_queues) { > @@ -327,8 +335,7 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, > RTE_CACHE_LINE_SIZE, socket_id); > if (!rxq) { > PMD_DRV_LOG(ERR, "bnxt_rx_queue allocation failed!\n"); > - rc = -ENOMEM; > - goto out; > + return -ENOMEM; > } > rxq->bp = bp; > rxq->mb_pool = mp; > @@ -344,8 +351,11 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, > PMD_DRV_LOG(DEBUG, "RX Buf MTU %d\n", eth_dev->data->mtu); > > rc = bnxt_init_rx_ring_struct(rxq, socket_id); > - if (rc) > - goto out; > + if (rc) { > + PMD_DRV_LOG(ERR, > + "init_rx_ring_struct failed!\n"); > + goto err; > + } > > PMD_DRV_LOG(DEBUG, "RX Buf size is %d\n", rxq->rx_buf_size); > rxq->queue_id = queue_idx; > @@ -360,10 +370,8 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, > if (bnxt_alloc_rings(bp, queue_idx, NULL, rxq, rxq->cp_ring, NULL, > "rxr")) { > PMD_DRV_LOG(ERR, > - "ring_dma_zone_reserve for rx_ring failed!\n"); > - bnxt_rx_queue_release_op(rxq); > - rc = -ENOMEM; > - goto out; > + "ring_dma_zone_reserve for rx_ring failed!\n"); > + goto err; > } > rte_atomic64_init(&rxq->rx_mbuf_alloc_fail); > > @@ -387,7 +395,9 @@ int bnxt_rx_queue_setup_op(struct rte_eth_dev *eth_dev, > if (!queue_idx) > bnxt_mtu_set_op(eth_dev, eth_dev->data->mtu); > > -out: > + return 0; > +err: > + bnxt_rx_queue_release_op(rxq); > return rc; > } > > diff --git a/drivers/net/bnxt/bnxt_txq.c b/drivers/net/bnxt/bnxt_txq.c > index c8d75ac..c9792a2 100644 > --- a/drivers/net/bnxt/bnxt_txq.c > +++ b/drivers/net/bnxt/bnxt_txq.c > @@ -27,7 +27,7 @@ static void bnxt_tx_queue_release_mbufs(struct bnxt_tx_queue *txq) > struct bnxt_sw_tx_bd *sw_ring; > uint16_t i; > > - if (!txq) > + if (!txq || !txq->tx_ring) > return; > > sw_ring = txq->tx_ring->tx_buf_ring; > @@ -62,10 +62,18 @@ void bnxt_tx_queue_release_op(void *tx_queue) > > /* Free TX ring hardware descriptors */ > bnxt_tx_queue_release_mbufs(txq); > - bnxt_free_ring(txq->tx_ring->tx_ring_struct); > + if (txq->tx_ring) { > + bnxt_free_ring(txq->tx_ring->tx_ring_struct); > + rte_free(txq->tx_ring->tx_ring_struct); > + rte_free(txq->tx_ring); > + } > > /* Free TX completion ring hardware descriptors */ > - bnxt_free_ring(txq->cp_ring->cp_ring_struct); > + if (txq->cp_ring) { > + bnxt_free_ring(txq->cp_ring->cp_ring_struct); > + rte_free(txq->cp_ring->cp_ring_struct); > + rte_free(txq->cp_ring); > + } > > bnxt_free_txq_stats(txq); > rte_memzone_free(txq->mz); > @@ -99,8 +107,7 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > > if (nb_desc < BNXT_MIN_RING_DESC || nb_desc > MAX_TX_DESC_CNT) { > PMD_DRV_LOG(ERR, "nb_desc %d is invalid", nb_desc); > - rc = -EINVAL; > - goto out; > + return -EINVAL; > } > > if (eth_dev->data->tx_queues) { > @@ -114,8 +121,7 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > RTE_CACHE_LINE_SIZE, socket_id); > if (!txq) { > PMD_DRV_LOG(ERR, "bnxt_tx_queue allocation failed!"); > - rc = -ENOMEM; > - goto out; > + return -ENOMEM; > } > > txq->free = rte_zmalloc_socket(NULL, > @@ -123,9 +129,8 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > RTE_CACHE_LINE_SIZE, socket_id); > if (!txq->free) { > PMD_DRV_LOG(ERR, "allocation of tx mbuf free array failed!"); > - rte_free(txq); > rc = -ENOMEM; > - goto out; > + goto err; > } > txq->bp = bp; > txq->nb_tx_desc = nb_desc; > @@ -138,7 +143,7 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > > rc = bnxt_init_tx_ring_struct(txq, socket_id); > if (rc) > - goto out; > + goto err; > > txq->queue_id = queue_idx; > txq->port_id = eth_dev->data->port_id; > @@ -147,16 +152,14 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > if (bnxt_alloc_rings(bp, queue_idx, txq, NULL, txq->cp_ring, NULL, > "txr")) { > PMD_DRV_LOG(ERR, "ring_dma_zone_reserve for tx_ring failed!"); > - bnxt_tx_queue_release_op(txq); > rc = -ENOMEM; > - goto out; > + goto err; > } > > if (bnxt_init_one_tx_ring(txq)) { > PMD_DRV_LOG(ERR, "bnxt_init_one_tx_ring failed!"); > - bnxt_tx_queue_release_op(txq); > rc = -ENOMEM; > - goto out; > + goto err; > } > > eth_dev->data->tx_queues[queue_idx] = txq; > @@ -165,6 +168,9 @@ int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev, > txq->tx_started = false; > else > txq->tx_started = true; > -out: > + > + return 0; > +err: > + bnxt_tx_queue_release_op(txq); > return rc; > } > -- > 2.7.4 >