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 E8E3BA04DD for ; Wed, 21 Oct 2020 07:33:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9ABC2ACB7; Wed, 21 Oct 2020 07:33:27 +0200 (CEST) Received: from mail-oo1-f67.google.com (mail-oo1-f67.google.com [209.85.161.67]) by dpdk.org (Postfix) with ESMTP id 53400AC7C for ; Wed, 21 Oct 2020 07:33:23 +0200 (CEST) Received: by mail-oo1-f67.google.com with SMTP id c25so250865ooe.13 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=1Dbs6itheckEz+iET/cR2DiKe1C807cMRedSH8c8ctE=; b=M4MBA3EcSqzelZ4TBFJ0o5ZyfQ9PQKPc8HtMDGOMkWH3DW6aVe5anWYZOgR/gQzls6 9UvYdMTK8GzgwDmd/QQOheKjwMPvEnbfe00xqy/J2giaHPTJK5CSy7cn81WK1PSgBuae iVZ6R0i/bImmVTHbvWoXaJAF0X/MzydBKUL5E= 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=1Dbs6itheckEz+iET/cR2DiKe1C807cMRedSH8c8ctE=; b=Zhy/9MPMX/j/00bznpxHB0UMsU7kz3swrBtZ5YLiy0GdYs6mek10NE0dkurhS04T58 vJ28tqZmdzAapn8HZx3Y7u8kXOK5UFwgL8iBl7hnYE0DdTbPT1tOksFELVPKH0jgyVMK sPzD7nTgM8HjqTBDJNlKVSCsRgSD4IuCNeeMSk1cdVWqYnr8SOLOrYsfsdQ4/dxczR8U 4/BGC9nro8RDvQLVYyDObsE6ozfrD5wxEOiVm4JA2EiX+qGNIYVLhjYoMJf1KBOQy0Om 15wKEvIJgfcwlSteCGcvhJUzkHXlUKYDFNkSogUp1aQ097vfVaTtb2BGHflciTO4dTH/ Ci1g== X-Gm-Message-State: AOAM531+Def17SlT5jVCSRug400Iz0OzreeTXdeaNHliYkdb/jCrYCBC ksbmBnA43TPpOIZGu2wUwk+iEKNqs2CsEKhfIiFEbA== 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-stable] [dpdk-dev] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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 >