On 2/23/2016 12:23 AM, Tom Kiely wrote:

Hi,
    Sorry I missed the last few messages until now. I'm happy with
just removing the "if". Kyle, when you say you fixed it, do you mean
that you will push the patch or have already done so ?
   Thanks,
       Tom

On 02/18/2016 02:03 PM, Kyle Larose wrote:

quoted

On Tue, Jan 5, 2016 at 2:13 AM, Xie, Huawei [off-list ref]
wrote:

quoted

On 12/17/2015 7:18 PM, Tom Kiely wrote:

quoted

On 11/25/2015 05:32 PM, Xie, Huawei wrote:

quoted

On 11/13/2015 5:33 PM, Tom Kiely wrote:

quoted

If all rx descriptors are processed while transient
mbuf exhaustion is present, the rx ring ends up with
no available descriptors. Thus no packets are received
on that ring. Since descriptor refill is performed post
rx descriptor processing, in this case no refill is
ever subsequently performed resulting in permanent rx
traffic drop.

Signed-off-by: Tom Kiely <redacted>
---
   drivers/net/virtio/virtio_rxtx.c |    6 ++++--
   1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c

b/drivers/net/virtio/virtio_rxtx.c
index 5770fa2..a95e234 100644

--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c

@@ -586,7 +586,8 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf

**rx_pkts, uint16_t nb_pkts)
       if (likely(num > DESC_PER_CACHELINE))
           num = num - ((rxvq->vq_used_cons_idx + num) %
DESC_PER_CACHELINE);
   -    if (num == 0)
+    /* Refill free descriptors even if no pkts recvd */
+    if (num == 0 && virtqueue_full(rxvq))

Should the return condition be that no used buffers and we have avail
descs in avail ring, i.e,
      num == 0 && rxvq->vq_free_cnt != rxvq->vq_nentries

rather than
      num == 0 && rxvq->vq_free_cnt == 0

Yes we could do that but I don't see a good reason to wait until the
vq_free_cnt == vq_nentries
before attempting the refill. The existing code will attempt refill
even if only 1 packet was received
and the free count is small. To me it seems safer to extend that to
try refill even if no packet was received
but the free count is non-zero.

The existing code attempt to refill only if 1 packet was received.

If we want to refill even no packet was received, then the strict
condition should be
     num == 0 && rxvq->vq_free_cnt != rxvq->vq_nentries

The safer condition, what you want to use,  should be
     num == 0 && !virtqueue_full(...)
rather than
     num == 0 && virtqueue_full(...)

We could simplify things a bit, just remove this check, if the
following
receiving code already takes care of the "num == 0" condition.

FWIW, I fixed this issue myself by just removing the if(num == 0)
checks entirely. I didn't see any benefit in short-circuiting a loop
which pretty much does nothing anyway when num == 0. Further, we only
hit this case when there's no packets to receive, which means there's
probably a few cycles to spare. This is even simpler.

Yes, as i said, that is the simplest fix.

quoted

quoted

I find virtqueue_full is confusing, maybe we could change it to some
other meaningful name.

quoted

    Tom

quoted

quoted

           return 0;
         num = virtqueue_dequeue_burst_rx(rxvq, rcv_pkts, len, num);

@@ -683,7 +684,8 @@ virtio_recv_mergeable_pkts(void *rx_queue,
         virtio_rmb();
   -    if (nb_used == 0)
+    /* Refill free descriptors even if no pkts recvd */
+    if (nb_used == 0 && virtqueue_full(rxvq))
           return 0;
         PMD_RX_LOG(DEBUG, "used:%d\n", nb_used);