patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops
@ 2020-10-20  4:11 Somnath Kotur
  2020-10-21  5:33 ` [dpdk-stable] [dpdk-dev] " Ajit Khaparde
  0 siblings, 1 reply; 2+ messages in thread
From: Somnath Kotur @ 2020-10-20  4:11 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

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 <somnath.kotur@broadcom.com>
---
 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops
  2020-10-20  4:11 [dpdk-stable] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops Somnath Kotur
@ 2020-10-21  5:33 ` Ajit Khaparde
  0 siblings, 0 replies; 2+ messages in thread
From: Ajit Khaparde @ 2020-10-21  5:33 UTC (permalink / raw)
  To: Somnath Kotur; +Cc: dpdk-dev, Ferruh Yigit, dpdk stable

On Mon, Oct 19, 2020 at 9:19 PM Somnath Kotur
<somnath.kotur@broadcom.com> 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 <somnath.kotur@broadcom.com>
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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-21  5:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20  4:11 [dpdk-stable] [PATCH] net/bnxt: cleanup and fixes in Rx/Tx queue release ops Somnath Kotur
2020-10-21  5:33 ` [dpdk-stable] [dpdk-dev] " Ajit Khaparde

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git