[1/1] net/bnxt: fix Rx buffer posting

Message ID 20210318195213.683281-1-lance.richardson@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series [1/1] net/bnxt: fix Rx buffer posting |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Lance Richardson March 18, 2021, 7:52 p.m. UTC
  Remove early buffer posting logic from burst receive loop to address
several issues:
   - Posting receive descriptors without first posting completion
     entries risks overflowing the completion queue.
   - Posting receive descriptors without updating rx_raw_prod
     creates the possibility that the receive descriptor doorbell
     can be written twice with the same value.
   - Having this logic in the inner descriptor processing loop
     can impact performance.

Fixes: 637e34befd9c ("net/bnxt: optimize Rx processing")
Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation ring")
Cc: stable@dpdk.org

Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_rxr.c | 3 ---
 drivers/net/bnxt/bnxt_rxr.h | 2 --
 2 files changed, 5 deletions(-)
  

Comments

Ajit Khaparde March 19, 2021, 3:50 a.m. UTC | #1
On Thu, Mar 18, 2021 at 12:52 PM Lance Richardson
<lance.richardson@broadcom.com> wrote:
>
> Remove early buffer posting logic from burst receive loop to address
> several issues:
>    - Posting receive descriptors without first posting completion
>      entries risks overflowing the completion queue.
>    - Posting receive descriptors without updating rx_raw_prod
>      creates the possibility that the receive descriptor doorbell
>      can be written twice with the same value.
>    - Having this logic in the inner descriptor processing loop
>      can impact performance.
>
> Fixes: 637e34befd9c ("net/bnxt: optimize Rx processing")
> Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
> Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
Patch applied to dpdk-next-net-brcm.

> ---
>  drivers/net/bnxt/bnxt_rxr.c | 3 ---
>  drivers/net/bnxt/bnxt_rxr.h | 2 --
>  2 files changed, 5 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index c72545ada7..7179c6cb30 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -1018,9 +1018,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>                 raw_cons = NEXT_RAW_CMP(raw_cons);
>                 if (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt)
>                         break;
> -               /* Post some Rx buf early in case of larger burst processing */
> -               if (nb_rx_pkts == BNXT_RX_POST_THRESH)
> -                       bnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);
>         }
>
>         cpr->cp_raw_cons = raw_cons;
> diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> index a6fdd7767a..b43256e03e 100644
> --- a/drivers/net/bnxt/bnxt_rxr.h
> +++ b/drivers/net/bnxt/bnxt_rxr.h
> @@ -41,8 +41,6 @@ static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
>         (((cmp)->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK) >> \
>                 RX_PKT_CMPL_AGG_BUFS_SFT)
>
> -#define BNXT_RX_POST_THRESH    32
> -
>  /* Number of descriptors to process per inner loop in vector mode. */
>  #define RTE_BNXT_DESCS_PER_LOOP                4U
>
> --
> 2.25.1
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index c72545ada7..7179c6cb30 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1018,9 +1018,6 @@  uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		raw_cons = NEXT_RAW_CMP(raw_cons);
 		if (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt)
 			break;
-		/* Post some Rx buf early in case of larger burst processing */
-		if (nb_rx_pkts == BNXT_RX_POST_THRESH)
-			bnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);
 	}
 
 	cpr->cp_raw_cons = raw_cons;
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index a6fdd7767a..b43256e03e 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -41,8 +41,6 @@  static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
 	(((cmp)->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK) >> \
 		RX_PKT_CMPL_AGG_BUFS_SFT)
 
-#define BNXT_RX_POST_THRESH	32
-
 /* Number of descriptors to process per inner loop in vector mode. */
 #define RTE_BNXT_DESCS_PER_LOOP		4U