[dpdk-dev,2/4] net/ena: fix delayed cleanup of Rx descriptors

  From: Michal Krawczyk <mk@semihalf.com>

On RX path, after receiving bunch of packets, variable tracking
available descriptors in HW queue was not updated.

To fix this issue, additional variable was added which is storing number
of depleted descriptors updated by number of descriptors used in this
cycle.

Finally whole number is substracted by one to do not refill all
descriptors what is required by the driver.

Fixes: 1daff5260ff8 ("net/ena: use unmasked head and tail")

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
  

On Fri, Apr 7, 2017 at 12:48 PM, Marcin Wojtas <mw@semihalf.com> wrote:

> From: Michal Krawczyk <mk@semihalf.com>
>
> On RX path, after receiving bunch of packets, variable tracking
> available descriptors in HW queue was not updated.
>
> To fix this issue, additional variable was added which is storing number
> of depleted descriptors updated by number of descriptors used in this
> cycle.
>
> Finally whole number is substracted by one to do not refill all
> descriptors what is required by the driver.
>
> Fixes: 1daff5260ff8 ("net/ena: use unmasked head and tail")
>
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 2345bab..d875a2b 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1144,7 +1144,7 @@ static int ena_populate_rx_queue(struct ena_ring
> *rxq, unsigned int count)
>                 return 0;
>
>         in_use = rxq->next_to_use - rxq->next_to_clean;
> -       ena_assert_msg(((in_use + count) <= ring_size), "bad ring state");
> +       ena_assert_msg(((in_use + count) < ring_size), "bad ring state");
>
>         count = RTE_MIN(count,
>                         (uint16_t)(ring_size - (next_to_use & ring_mask)));
> @@ -1504,7 +1504,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts,
>         unsigned int ring_size = rx_ring->ring_size;
>         unsigned int ring_mask = ring_size - 1;
>         uint16_t next_to_clean = rx_ring->next_to_clean;
> -       uint16_t desc_in_use = 0;
> +       uint16_t desc_in_use, desc_to_refill;
>         unsigned int recv_idx = 0;
>         struct rte_mbuf *mbuf = NULL;
>         struct rte_mbuf *mbuf_head = NULL;
> @@ -1575,12 +1575,13 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts,
>                 recv_idx++;
>         }
>
> -       /* Burst refill to save doorbells, memory barriers, const interval
> */
> -       if (ring_size - desc_in_use > ENA_RING_DESCS_RATIO(ring_size))
> -               ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
> -
>         rx_ring->next_to_clean = next_to_clean;
>
> +       desc_to_refill = ring_size - desc_in_use + completed - 1;
> +       /* Burst refill to save doorbells, memory barriers, const interval
> */
> +       if (desc_to_refill > ENA_RING_DESCS_RATIO(ring_size))
> +               ena_populate_rx_queue(rx_ring, desc_to_refill);
> +
>         return recv_idx;
>  }
>
> --
> 1.8.3.1
>
>
​
Good catch! I would suggest, however, to slightly rework this this way:
adjust the desc_in_use to reflect the actual state:
desc_in_use -= completed;
so that it would be the only change in this patch, and should do the trick.
As for the adjustment "-1" I will refer to 1/4 separately.

Jakub


​