net/bnxt: fix regression in non-vector mode Rx data path on Thor

Message ID 20211103160432.8052-1-somnath.kotur@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series net/bnxt: fix regression in non-vector mode Rx data path on Thor |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Somnath Kotur Nov. 3, 2021, 4:04 p.m. UTC
  The patch introduced by
commit 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")
ended up shortening the return code path from the function thereby
resulting in not executing the line of code at the end of the function
that was resetting the next consumer index to 0.
This would result in an application crash when error recovery or other
port stop/start scenarios were invoked on Thor which is what this
commit 61cd4384fabf ("net/bnxt: fix crash after port stop/start")
was addressing.
Fix it by moving the resetting line of code before the return path
in the case when aggregration rings are not used (default case).

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

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_rxr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Comments

Ajit Khaparde Nov. 5, 2021, 1:01 a.m. UTC | #1
On Wed, Nov 3, 2021 at 9:10 AM Somnath Kotur <somnath.kotur@broadcom.com> wrote:
>
> The patch introduced by
> commit 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")
> ended up shortening the return code path from the function thereby
> resulting in not executing the line of code at the end of the function
> that was resetting the next consumer index to 0.
> This would result in an application crash when error recovery or other
> port stop/start scenarios were invoked on Thor which is what this
> commit 61cd4384fabf ("net/bnxt: fix crash after port stop/start")
> was addressing.
> Fix it by moving the resetting line of code before the return path
> in the case when aggregration rings are not used (default case).
>
> Fixes: 657c2a7f1dd4 ("net/bnxt: create aggregation rings when needed")
>
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Fixed typo in commit log.
Patch applied to dpdk-next-net-brcm.

> ---
>  drivers/net/bnxt/bnxt_rxr.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 992f97388c..8bc8ddc353 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -1369,6 +1369,9 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
>                 rxr->rx_buf_ring[i] = &rxq->fake_mbuf;
>         }
>
> +       /* Explicitly reset this driver internal tracker on a ring init */
> +       rxr->rx_next_cons = 0;
> +
>         if (!bnxt_need_agg_ring(rxq->bp->eth_dev))
>                 return 0;
>
> @@ -1407,9 +1410,6 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
>         }
>         PMD_DRV_LOG(DEBUG, "TPA alloc Done!\n");
>
> -       /* Explicitly reset this driver internal tracker on a ring init */
> -       rxr->rx_next_cons = 0;
> -
>         return 0;
>  }
>
> --
> 2.28.0.497.g54e85e7
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 992f97388c..8bc8ddc353 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1369,6 +1369,9 @@  int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
 		rxr->rx_buf_ring[i] = &rxq->fake_mbuf;
 	}
 
+	/* Explicitly reset this driver internal tracker on a ring init */
+	rxr->rx_next_cons = 0;
+
 	if (!bnxt_need_agg_ring(rxq->bp->eth_dev))
 		return 0;
 
@@ -1407,9 +1410,6 @@  int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
 	}
 	PMD_DRV_LOG(DEBUG, "TPA alloc Done!\n");
 
-	/* Explicitly reset this driver internal tracker on a ring init */
-	rxr->rx_next_cons = 0;
-
 	return 0;
 }