Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic

Dor Laor
2008-07-24T23:24:30Z

Mark McLoughlin wrote:
> virtio_net tries to guess when it has received a tx
> notification from the guest whether it indicates that the
> guest has no more room in the tx ring and it should
> immediately flush the queued buffers.
>
> The heuristic is based on the fact that there are 128
> buffer entries in the ring and each packet uses 2 buffers
> (i.e. the virtio_net_hdr and the packet's linear data).
>
> Using GSO or increasing the size of the rings will break
> that heuristic, so let's remove it and assume that any
> notification from the guest after we've disabled
> notifications indicates that we should flush our buffers.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
> qemu/hw/virtio-net.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
> index 31867f1..4adfa42 100644
> --- a/qemu/hw/virtio-net.c
> +++ b/qemu/hw/virtio-net.c
> @@ -175,8 +175,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
> {
> VirtIONet *n = to_virtio_net(vdev);
>
> - if (n->tx_timer_active &&
> - (vq->vring.avail->idx - vq->last_avail_idx) == 64) {
> + if (n->tx_timer_active) {
>
Actually it was worthless anyway since if we set the timer, the flag
below would have been cleared,
causing the guest not to notify the host, thus the ==64 never called.
So I'm in favour too.
> vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
> qemu_del_timer(n->tx_timer);
> n->tx_timer_active = 0;
>
As stated by newer messages, we should handle the first tx notification
if the timer wasn't active to shorten latency.
Cheers, Dor