All of lore.kernel.org
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings
@ 2021-10-30  3:50 Kalesh A P
  2021-10-30  3:50 ` [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation Kalesh A P
  2021-10-31 16:04 ` [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Ajit Khaparde
  0 siblings, 2 replies; 4+ messages in thread
From: Kalesh A P @ 2021-10-30  3:50 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

During port stop, we clear "eth_dev->data->scattered_rx" at the
beginning. As a result, in bnxt_free_hwrm_rx_ring() the check
bnxt_need_agg_ring() returns false and we end up not freeing
the Rx aggregation rings which results in resource leak in the FW.

Fixes: 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 668e3aa..91f114e 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1476,7 +1476,6 @@ static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
 	int ret;
 
 	eth_dev->data->dev_started = 0;
-	eth_dev->data->scattered_rx = 0;
 
 	/* Prevent crashes when queues are still in use */
 	eth_dev->rx_pkt_burst = &bnxt_dummy_recv_pkts;
@@ -1533,6 +1532,8 @@ static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
 	if (BNXT_FLOW_XSTATS_EN(bp))
 		bp->flow_stat->flow_count = 0;
 
+	eth_dev->data->scattered_rx = 0;
+
 	return 0;
 }
 
-- 
2.10.1


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

* [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation
  2021-10-30  3:50 [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Kalesh A P
@ 2021-10-30  3:50 ` Kalesh A P
  2021-10-31 16:05   ` Ajit Khaparde
  2021-10-31 16:04 ` [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Ajit Khaparde
  1 sibling, 1 reply; 4+ messages in thread
From: Kalesh A P @ 2021-10-30  3:50 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

stat_ctx_alloc is called within the context of each rx/tx ring.
i.e from bnxt_alloc_hwrm_rx_ring and bnxt_alloc_hwrm_tx_ring().
So, there is no need to invoke bnxt_alloc_all_hwrm_stat_ctxs()
from bnxt_start_nic().

Fixes: 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c |  6 ------
 drivers/net/bnxt/bnxt_hwrm.c   | 31 -------------------------------
 drivers/net/bnxt/bnxt_hwrm.h   |  1 -
 3 files changed, 38 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 91f114e..c8dad8a 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -747,12 +747,6 @@ static int bnxt_start_nic(struct bnxt *bp)
 	if (BNXT_CHIP_P5(bp))
 		bp->max_ring_grps = BNXT_MAX_RSS_CTXTS_P5;
 
-	rc = bnxt_alloc_all_hwrm_stat_ctxs(bp);
-	if (rc) {
-		PMD_DRV_LOG(ERR, "HWRM stat ctx alloc failure rc: %x\n", rc);
-		goto err_out;
-	}
-
 	rc = bnxt_alloc_hwrm_rings(bp);
 	if (rc) {
 		PMD_DRV_LOG(ERR, "HWRM ring alloc failure rc: %x\n", rc);
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 82e89b7..55dcb1d 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2648,37 +2648,6 @@ bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 	return 0;
 }
 
-int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp)
-{
-	struct bnxt_cp_ring_info *cpr;
-	unsigned int i;
-	int rc = 0;
-
-	for (i = 0; i < bp->rx_cp_nr_rings; i++) {
-		struct bnxt_rx_queue *rxq = bp->rx_queues[i];
-
-		cpr = rxq->cp_ring;
-		if (cpr->hw_stats_ctx_id == HWRM_NA_SIGNATURE) {
-			rc = bnxt_hwrm_stat_ctx_alloc(bp, cpr);
-			if (rc)
-				return rc;
-		}
-	}
-
-	for (i = 0; i < bp->tx_cp_nr_rings; i++) {
-		struct bnxt_tx_queue *txq = bp->tx_queues[i];
-
-		cpr = txq->cp_ring;
-		if (cpr->hw_stats_ctx_id == HWRM_NA_SIGNATURE) {
-			rc = bnxt_hwrm_stat_ctx_alloc(bp, cpr);
-			if (rc)
-				return rc;
-		}
-	}
-
-	return rc;
-}
-
 static int
 bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
 {
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 6dc23b9..72d4864 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -188,7 +188,6 @@ int bnxt_hwrm_vnic_plcmode_cfg(struct bnxt *bp,
 int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
 			   struct bnxt_vnic_info *vnic, bool enable);
 
-int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp);
 int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp);
 int bnxt_alloc_all_hwrm_ring_grps(struct bnxt *bp);
 void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
-- 
2.10.1


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

* Re: [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings
  2021-10-30  3:50 [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Kalesh A P
  2021-10-30  3:50 ` [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation Kalesh A P
@ 2021-10-31 16:04 ` Ajit Khaparde
  1 sibling, 0 replies; 4+ messages in thread
From: Ajit Khaparde @ 2021-10-31 16:04 UTC (permalink / raw)
  To: Kalesh A P; +Cc: dpdk-dev, Ferruh Yigit

On Fri, Oct 29, 2021 at 8:30 PM Kalesh A P
<kalesh-anakkur.purayil@broadcom.com> wrote:
>
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>
> During port stop, we clear "eth_dev->data->scattered_rx" at the
> beginning. As a result, in bnxt_free_hwrm_rx_ring() the check
> bnxt_need_agg_ring() returns false and we end up not freeing
> the Rx aggregation rings which results in resource leak in the FW.
>
> Fixes: 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Patch applied to dpdk-next-net-brcm.Thanks

> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 668e3aa..91f114e 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -1476,7 +1476,6 @@ static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
>         int ret;
>
>         eth_dev->data->dev_started = 0;
> -       eth_dev->data->scattered_rx = 0;
>
>         /* Prevent crashes when queues are still in use */
>         eth_dev->rx_pkt_burst = &bnxt_dummy_recv_pkts;
> @@ -1533,6 +1532,8 @@ static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
>         if (BNXT_FLOW_XSTATS_EN(bp))
>                 bp->flow_stat->flow_count = 0;
>
> +       eth_dev->data->scattered_rx = 0;
> +
>         return 0;
>  }
>
> --
> 2.10.1
>

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

* Re: [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation
  2021-10-30  3:50 ` [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation Kalesh A P
@ 2021-10-31 16:05   ` Ajit Khaparde
  0 siblings, 0 replies; 4+ messages in thread
From: Ajit Khaparde @ 2021-10-31 16:05 UTC (permalink / raw)
  To: Kalesh A P; +Cc: dpdk-dev, Ferruh Yigit

On Fri, Oct 29, 2021 at 8:30 PM Kalesh A P
<kalesh-anakkur.purayil@broadcom.com> wrote:
>
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>
> stat_ctx_alloc is called within the context of each rx/tx ring.
> i.e from bnxt_alloc_hwrm_rx_ring and bnxt_alloc_hwrm_tx_ring().
> So, there is no need to invoke bnxt_alloc_all_hwrm_stat_ctxs()
> from bnxt_start_nic().
>
> Fixes: 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Patch applied to dpdk-next-net-brcm. Thanks

> ---
>  drivers/net/bnxt/bnxt_ethdev.c |  6 ------
>  drivers/net/bnxt/bnxt_hwrm.c   | 31 -------------------------------
>  drivers/net/bnxt/bnxt_hwrm.h   |  1 -
>  3 files changed, 38 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 91f114e..c8dad8a 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -747,12 +747,6 @@ static int bnxt_start_nic(struct bnxt *bp)
>         if (BNXT_CHIP_P5(bp))
>                 bp->max_ring_grps = BNXT_MAX_RSS_CTXTS_P5;
>
> -       rc = bnxt_alloc_all_hwrm_stat_ctxs(bp);
> -       if (rc) {
> -               PMD_DRV_LOG(ERR, "HWRM stat ctx alloc failure rc: %x\n", rc);
> -               goto err_out;
> -       }
> -
>         rc = bnxt_alloc_hwrm_rings(bp);
>         if (rc) {
>                 PMD_DRV_LOG(ERR, "HWRM ring alloc failure rc: %x\n", rc);
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 82e89b7..55dcb1d 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -2648,37 +2648,6 @@ bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
>         return 0;
>  }
>
> -int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp)
> -{
> -       struct bnxt_cp_ring_info *cpr;
> -       unsigned int i;
> -       int rc = 0;
> -
> -       for (i = 0; i < bp->rx_cp_nr_rings; i++) {
> -               struct bnxt_rx_queue *rxq = bp->rx_queues[i];
> -
> -               cpr = rxq->cp_ring;
> -               if (cpr->hw_stats_ctx_id == HWRM_NA_SIGNATURE) {
> -                       rc = bnxt_hwrm_stat_ctx_alloc(bp, cpr);
> -                       if (rc)
> -                               return rc;
> -               }
> -       }
> -
> -       for (i = 0; i < bp->tx_cp_nr_rings; i++) {
> -               struct bnxt_tx_queue *txq = bp->tx_queues[i];
> -
> -               cpr = txq->cp_ring;
> -               if (cpr->hw_stats_ctx_id == HWRM_NA_SIGNATURE) {
> -                       rc = bnxt_hwrm_stat_ctx_alloc(bp, cpr);
> -                       if (rc)
> -                               return rc;
> -               }
> -       }
> -
> -       return rc;
> -}
> -
>  static int
>  bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
>  {
> diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
> index 6dc23b9..72d4864 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.h
> +++ b/drivers/net/bnxt/bnxt_hwrm.h
> @@ -188,7 +188,6 @@ int bnxt_hwrm_vnic_plcmode_cfg(struct bnxt *bp,
>  int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
>                            struct bnxt_vnic_info *vnic, bool enable);
>
> -int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp);
>  int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp);
>  int bnxt_alloc_all_hwrm_ring_grps(struct bnxt *bp);
>  void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
> --
> 2.10.1
>

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

end of thread, other threads:[~2021-10-31 16:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-30  3:50 [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Kalesh A P
2021-10-30  3:50 ` [dpdk-dev] [PATCH 2/2] net/bnxt: fix stat context allocation Kalesh A P
2021-10-31 16:05   ` Ajit Khaparde
2021-10-31 16:04 ` [dpdk-dev] [PATCH 1/2] net/bnxt: fix freeing aggregation rings Ajit Khaparde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.