Re: [dpdk-stable] [PATCH] net/bnxt: fix Rx queue count implementation

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 5318 bytes --]

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
>