On Thu, Oct 13, 2022 at 04:00:57PM +0200, Laurent Vivier wrote:
When virtio-net is used with the socket netdev backend, the backend
can be busy and not able to collect new packets.
In this case, net_socket_receive() returns 0 and registers a poll function
to detect when the socket is ready again.
In virtio_net_tx_bh(), virtio_net_flush_tx() forwards the 0, the virtio
notifications are disabled and the function is not rescheduled, waiting
for the backend to be ready.
When the socket netdev backend is again able to send packets, the poll
function re-starts to flush remaining packets. This is done by
calling virtio_net_tx_complete(). It re-enables notifications and calls
again virtio_net_flush_tx().
But it seems if virtio_net_flush_tx() reaches the tx_burst value all
the queue is not flushed and no new notification is sent to reschedule
virtio_net_tx_bh(). Nothing re-start to flush the queue and remaining packets
are stuck in the queue.
To fix that, detect in virtio_net_tx_complete() if virtio_net_flush_tx()
has been stopped by tx_burst and if yes reschedule the bottom half
function virtio_net_tx_bh() to flush the remaining packets.
This is what is done in virtio_net_tx_bh() when the virtio_net_flush_tx()
is synchronous, and completely by-passed when the operation needs to be
asynchronous.
RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC
Do we need to reschedule the function in the other case managed
in virtio_net_tx_bh() and by-passed when the completion is asynchronous?
I am guessing no.
/* If less than a full burst, re-enable notification and flush
* anything that may have come in while we weren't looking. If
* we find something, assume the guest is still active and reschedule */
virtio_queue_set_notification(q->tx_vq, 1);
ret = virtio_net_flush_tx(q);
if (ret == -EINVAL) {
return;
} else if (ret > 0) {
virtio_queue_set_notification(q->tx_vq, 0);
qemu_bh_schedule(q->tx_bh);
q->tx_waiting = 1;
}
RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC RFC
Fixes: a697a334b3c4 ("virtio-net: Introduce a new bottom half packet TX")
Cc: alex.williamson@redhat.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Looks ok superficially
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Jason, your area.
---
hw/net/virtio-net.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index e9f696b4cfeb..1fbf2f3e19a7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2526,6 +2526,7 @@ static void virtio_net_tx_complete(NetClientState *nc,
ssize_t len)
VirtIONet *n = qemu_get_nic_opaque(nc);
VirtIONetQueue *q = virtio_net_get_subqueue(nc);
VirtIODevice *vdev = VIRTIO_DEVICE(n);
+ int ret;
virtqueue_push(q->tx_vq, q->async_tx.elem, 0);
virtio_notify(vdev, q->tx_vq);
@@ -2534,7 +2535,17 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t
len)
q->async_tx.elem = NULL;
virtio_queue_set_notification(q->tx_vq, 1);
- virtio_net_flush_tx(q);
+ ret = virtio_net_flush_tx(q);
+ if (q->tx_bh && ret >= n->tx_burst) {
+ /*
+ * the flush has been stopped by tx_burst
+ * we will not receive notification for the
+ * remainining part, so re-schedule
+ */
+ virtio_queue_set_notification(q->tx_vq, 0);
+ qemu_bh_schedule(q->tx_bh);
+ q->tx_waiting = 1;
+ }