net/bnxt: fix Rx queue count implementation

Message ID 20210218181920.223463-1-lance.richardson@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series net/bnxt: fix Rx queue count implementation |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/travis-robot fail travis build: failed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation fail apply issues

Commit Message

Lance Richardson Feb. 18, 2021, 6:19 p.m. UTC
  bnxt_rx_queue_count_op() incorrectly returns the number of
filled but unprocessed completion queue entries instead of
the number of filled but unprocessed received packet
completions. Fix by properly accounting for the number of
completion ring entries used by the various received packet
completion types.

Fixes: 34c0ba839bae ("net/bnxt: fix Rx queue count")
Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Cc: stable@dpdk.org
---
 drivers/net/bnxt/bnxt_cpr.h    |  4 ++++
 drivers/net/bnxt/bnxt_ethdev.c | 38 +++++++++++++++++++++++++++++-----
 drivers/net/bnxt/bnxt_rxr.c    |  3 +--
 drivers/net/bnxt/bnxt_rxr.h    |  4 ++++
 4 files changed, 42 insertions(+), 7 deletions(-)
  

Comments

Ajit Khaparde March 1, 2021, 12:37 a.m. UTC | #1
On Thu, Feb 18, 2021 at 10:19 AM Lance Richardson
<lance.richardson@broadcom.com> wrote:
>
> bnxt_rx_queue_count_op() incorrectly returns the number of
> filled but unprocessed completion queue entries instead of
> the number of filled but unprocessed received packet
> completions. Fix by properly accounting for the number of
> completion ring entries used by the various received packet
> completion types.
>
> Fixes: 34c0ba839bae ("net/bnxt: fix Rx queue count")
> Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Cc: stable@dpdk.org

Patch applied to dpdk-next-net-brcm.

> ---
>  drivers/net/bnxt/bnxt_cpr.h    |  4 ++++
>  drivers/net/bnxt/bnxt_ethdev.c | 38 +++++++++++++++++++++++++++++-----
>  drivers/net/bnxt/bnxt_rxr.c    |  3 +--
>  drivers/net/bnxt/bnxt_rxr.h    |  4 ++++
>  4 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
> index d7e0d4621a..28c0a9049c 100644
> --- a/drivers/net/bnxt/bnxt_cpr.h
> +++ b/drivers/net/bnxt/bnxt_cpr.h
> @@ -26,6 +26,10 @@ struct bnxt_db_info;
>  #define CMP_TYPE(cmp)                                          \
>         (((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
>
> +/* Get completion length from completion type, in 16-byte units. */
> +#define CMP_LEN(cmp_type) (((cmp_type) & 1) + 1)
> +
> +
>  #define ADV_RAW_CMP(idx, n)    ((idx) + (n))
>  #define NEXT_RAW_CMP(idx)      ADV_RAW_CMP(idx, 1)
>  #define RING_CMP(ring, idx)    ((idx) & (ring)->ring_mask)
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 22c880c5c3..9824cdb6d8 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -2942,8 +2942,8 @@ static uint32_t
>  bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  {
>         struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> -       uint32_t desc = 0, raw_cons = 0, cons;
>         struct bnxt_cp_ring_info *cpr;
> +       uint32_t desc = 0, raw_cons;
>         struct bnxt_rx_queue *rxq;
>         struct rx_pkt_cmpl *rxcmp;
>         int rc;
> @@ -2957,15 +2957,43 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>         raw_cons = cpr->cp_raw_cons;
>
>         while (1) {
> +               uint32_t agg_cnt, cons, cmpl_type;
> +
>                 cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
> -               rte_prefetch0(&cpr->cp_desc_ring[cons]);
>                 rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
>
> -               if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct)) {
> +               if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
>                         break;
> -               } else {
> -                       raw_cons++;
> +
> +               cmpl_type = CMP_TYPE(rxcmp);
> +
> +               switch (cmpl_type) {
> +               case CMPL_BASE_TYPE_RX_L2:
> +               case CMPL_BASE_TYPE_RX_L2_V2:
> +                       agg_cnt = BNXT_RX_L2_AGG_BUFS(rxcmp);
> +                       raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
> +                       desc++;
> +                       break;
> +
> +               case CMPL_BASE_TYPE_RX_TPA_END:
> +                       if (BNXT_CHIP_P5(rxq->bp)) {
> +                               struct rx_tpa_v2_end_cmpl_hi *p5_tpa_end;
> +
> +                               p5_tpa_end = (void *)rxcmp;
> +                               agg_cnt = BNXT_TPA_END_AGG_BUFS_TH(p5_tpa_end);
> +                       } else {
> +                               struct rx_tpa_end_cmpl *tpa_end;
> +
> +                               tpa_end = (void *)rxcmp;
> +                               agg_cnt = BNXT_TPA_END_AGG_BUFS(tpa_end);
> +                       }
> +
> +                       raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
>                         desc++;
> +                       break;
> +
> +               default:
> +                       raw_cons += CMP_LEN(cmpl_type);
>                 }
>         }
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 4674f7cea2..e8f174fc57 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -827,8 +827,7 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
>                 goto next_rx;
>         }
>
> -       agg_buf = (rxcmp->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK)
> -                       >> RX_PKT_CMPL_AGG_BUFS_SFT;
> +       agg_buf = BNXT_RX_L2_AGG_BUFS(rxcmp);
>         if (agg_buf && !bnxt_agg_bufs_valid(cpr, agg_buf, tmp_raw_cons))
>                 return -EBUSY;
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> index 0e31b37cad..06d10848da 100644
> --- a/drivers/net/bnxt/bnxt_rxr.h
> +++ b/drivers/net/bnxt/bnxt_rxr.h
> @@ -37,6 +37,10 @@ static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
>  #define BNXT_TPA_END_AGG_ID_TH(cmp) \
>         rte_le_to_cpu_16((cmp)->agg_id)
>
> +#define BNXT_RX_L2_AGG_BUFS(cmp) \
> +       (((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. */
> --
> 2.25.1
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index d7e0d4621a..28c0a9049c 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -26,6 +26,10 @@  struct bnxt_db_info;
 #define CMP_TYPE(cmp)						\
 	(((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
 
+/* Get completion length from completion type, in 16-byte units. */
+#define CMP_LEN(cmp_type) (((cmp_type) & 1) + 1)
+
+
 #define ADV_RAW_CMP(idx, n)	((idx) + (n))
 #define NEXT_RAW_CMP(idx)	ADV_RAW_CMP(idx, 1)
 #define RING_CMP(ring, idx)	((idx) & (ring)->ring_mask)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 22c880c5c3..9824cdb6d8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2942,8 +2942,8 @@  static uint32_t
 bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
 	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
-	uint32_t desc = 0, raw_cons = 0, cons;
 	struct bnxt_cp_ring_info *cpr;
+	uint32_t desc = 0, raw_cons;
 	struct bnxt_rx_queue *rxq;
 	struct rx_pkt_cmpl *rxcmp;
 	int rc;
@@ -2957,15 +2957,43 @@  bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 	raw_cons = cpr->cp_raw_cons;
 
 	while (1) {
+		uint32_t agg_cnt, cons, cmpl_type;
+
 		cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
-		rte_prefetch0(&cpr->cp_desc_ring[cons]);
 		rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
 
-		if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct)) {
+		if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
 			break;
-		} else {
-			raw_cons++;
+
+		cmpl_type = CMP_TYPE(rxcmp);
+
+		switch (cmpl_type) {
+		case CMPL_BASE_TYPE_RX_L2:
+		case CMPL_BASE_TYPE_RX_L2_V2:
+			agg_cnt = BNXT_RX_L2_AGG_BUFS(rxcmp);
+			raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
+			desc++;
+			break;
+
+		case CMPL_BASE_TYPE_RX_TPA_END:
+			if (BNXT_CHIP_P5(rxq->bp)) {
+				struct rx_tpa_v2_end_cmpl_hi *p5_tpa_end;
+
+				p5_tpa_end = (void *)rxcmp;
+				agg_cnt = BNXT_TPA_END_AGG_BUFS_TH(p5_tpa_end);
+			} else {
+				struct rx_tpa_end_cmpl *tpa_end;
+
+				tpa_end = (void *)rxcmp;
+				agg_cnt = BNXT_TPA_END_AGG_BUFS(tpa_end);
+			}
+
+			raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
 			desc++;
+			break;
+
+		default:
+			raw_cons += CMP_LEN(cmpl_type);
 		}
 	}
 
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 4674f7cea2..e8f174fc57 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -827,8 +827,7 @@  static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		goto next_rx;
 	}
 
-	agg_buf = (rxcmp->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK)
-			>> RX_PKT_CMPL_AGG_BUFS_SFT;
+	agg_buf = BNXT_RX_L2_AGG_BUFS(rxcmp);
 	if (agg_buf && !bnxt_agg_bufs_valid(cpr, agg_buf, tmp_raw_cons))
 		return -EBUSY;
 
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index 0e31b37cad..06d10848da 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -37,6 +37,10 @@  static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
 #define BNXT_TPA_END_AGG_ID_TH(cmp) \
 	rte_le_to_cpu_16((cmp)->agg_id)
 
+#define BNXT_RX_L2_AGG_BUFS(cmp) \
+	(((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. */