Thread: [kvm-devel] [PATCH 1/3] Revert virtio tap hack (v2)

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

While it has served us well, it is long overdue that we eliminate the
virtio-net tap hack.  It turns out that zero-copy has very little impact on
performance.  The tap hack was gaining such a significant performance boost
not because of zero-copy, but because it avoided dropping packets on receive
which is apparently a significant problem with the tap implementation in QEMU.

Patches 3 and 4 in this series address the packet dropping issue and the net
result is a 25% boost in RX performance even in the absence of zero-copy.

Also worth mentioning, is that this makes merging virtio into upstream QEMU
significantly easier.

Since v1, we're just rebasing on the new io thread patch set.

This series depends on my IO thread series.

Signed-off-by: Anthony Liguori <ali...@us...>

diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h
index 57d2123..f5157bd 100644
--- a/qemu/hw/pc.h
+++ b/qemu/hw/pc.h
@@ -154,7 +154,6 @@ void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd);
 /* virtio-net.c */
 
 PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
-void virtio_net_poll(void);
 
 /* virtio-blk.h */
 void *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t device,
diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index f727b14..8d26832 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -13,7 +13,6 @@
 
 #include "virtio.h"
 #include "net.h"
-#include "pc.h"
 #include "qemu-timer.h"
 
 /* from Linux's virtio_net.h */
@@ -62,15 +61,10 @@ typedef struct VirtIONet
     VirtQueue *tx_vq;
     VLANClientState *vc;
     int can_receive;
-    int tap_fd;
-    struct VirtIONet *next;
-    int do_notify;
     QEMUTimer *tx_timer;
     int tx_timer_active;
 } VirtIONet;
 
-static VirtIONet *VirtIONetHead = NULL;
-
 static VirtIONet *to_virtio_net(VirtIODevice *vdev)
 {
     return (VirtIONet *)vdev;
@@ -105,7 +99,6 @@ static int virtio_net_can_receive(void *opaque)
     return (n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) && n->can_receive;
 }
 
-/* -net user receive function */
 static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
 {
     VirtIONet *n = opaque;
@@ -144,87 +137,6 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
     virtio_notify(&n->vdev, n->rx_vq);
 }
 
-/* -net tap receive handler */
-void virtio_net_poll(void)
-{
-    VirtIONet *vnet;
-    int len;
-    fd_set rfds;
-    struct timeval tv;
-    int max_fd = -1;
-    VirtQueueElement elem;
-    struct virtio_net_hdr *hdr;
-    int did_notify;
-
-    FD_ZERO(&rfds);
-    tv.tv_sec = 0;
-    tv.tv_usec = 0;
-
-    while (1) {
-
-        // Prepare the set of device to select from
-        for (vnet = VirtIONetHead; vnet; vnet = vnet->next) {
-
-            if (vnet->tap_fd == -1)
-                continue;
-
-            vnet->do_notify = 0;
-            //first check if the driver is ok
-            if (!virtio_net_can_receive(vnet))
-                continue;
-
-            /* FIXME: the drivers really need to set their status better */
-            if (vnet->rx_vq->vring.avail == NULL) {
-                vnet->can_receive = 0;
-                continue;
-            }
-
-            FD_SET(vnet->tap_fd, &rfds);
-            if (max_fd < vnet->tap_fd) max_fd = vnet->tap_fd;
-        }
-
-        if (select(max_fd + 1, &rfds, NULL, NULL, &tv) <= 0)
-            break;
-
-        // Now check who has data pending in the tap
-        for (vnet = VirtIONetHead; vnet; vnet = vnet->next) {
-
-            if (!FD_ISSET(vnet->tap_fd, &rfds))
-                continue;
-
-            if (virtqueue_pop(vnet->rx_vq, &elem) == 0) {
-                vnet->can_receive = 0;
-                continue;
-            }
-
-            hdr = (void *)elem.in_sg[0].iov_base;
-            hdr->flags = 0;
-            hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
-again:
-            len = readv(vnet->tap_fd, &elem.in_sg[1], elem.in_num - 1);
-            if (len == -1) {
-                if (errno == EINTR || errno == EAGAIN)
-                    goto again;
-                else
-                    fprintf(stderr, "reading network error %d", len);
-            }
-            virtqueue_push(vnet->rx_vq, &elem, sizeof(*hdr) + len);
-            vnet->do_notify = 1;
-        }
-
-        /* signal other side */
-        did_notify = 0;
-        for (vnet = VirtIONetHead; vnet; vnet = vnet->next)
-            if (vnet->do_notify) {
-                virtio_notify(&vnet->vdev, vnet->rx_vq);
-                did_notify++;
-            }
-        if (!did_notify)
-            break;
-     }
-
-}
-
 /* TX */
 static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
 {
@@ -303,12 +215,6 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     memcpy(n->mac, nd->macaddr, 6);
     n->vc = qemu_new_vlan_client(nd->vlan, virtio_net_receive,
                                  virtio_net_can_receive, n);
-    n->tap_fd = hack_around_tap(n->vc->vlan->first_client);
-    if (n->tap_fd != -1) {
-        n->next = VirtIONetHead;
-        //push the device on top of the list
-        VirtIONetHead = n;
-    }
 
     n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
     n->tx_timer_active = 0;
diff --git a/qemu/vl.c b/qemu/vl.c
index bcf893f..b8ce485 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -3966,15 +3966,8 @@ typedef struct TAPState {
     VLANClientState *vc;
     int fd;
     char down_script[1024];
-    int no_poll;
 } TAPState;
 
-static int tap_read_poll(void *opaque)
-{
-    TAPState *s = opaque;
-    return (!s->no_poll);
-}
-
 static void tap_receive(void *opaque, const uint8_t *buf, int size)
 {
     TAPState *s = opaque;
@@ -4008,22 +4001,6 @@ static void tap_send(void *opaque)
     }
 }
 
-int hack_around_tap(void *opaque)
-{
-    VLANClientState *vc = opaque;
-    TAPState *ts = vc->opaque;
-
-    if (vc->fd_read != tap_receive)
-        return -1;
-
-    if (ts) {
-       ts->no_poll = 1;
-       return ts->fd;
-    }
-
-    return -1;
-}
-
 /* fd support */
 
 static TAPState *net_tap_fd_init(VLANState *vlan, int fd)
@@ -4034,10 +4011,8 @@ static TAPState *net_tap_fd_init(VLANState *vlan, int fd)
     if (!s)
         return NULL;
     s->fd = fd;
-    s->no_poll = 0;
-    enable_sigio_timer(fd);
     s->vc = qemu_new_vlan_client(vlan, tap_receive, NULL, s);
-    qemu_set_fd_handler2(s->fd, tap_read_poll, tap_send, NULL, s);
+    qemu_set_fd_handler2(s->fd, NULL, tap_send, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "tap: fd=%d", fd);
     return s;
 }
@@ -7972,10 +7947,7 @@ void main_loop_wait(int timeout)
         slirp_select_poll(&rfds, &wfds, &xfds);
     }
 #endif
-    virtio_net_poll();
-
     qemu_aio_poll();
-
     if (vm_running) {
         qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
                         qemu_get_clock(vm_clock));


