[PATCH 2/4] Revert virtio tap hack

Anthony Liguori
2008-05-04T02:47:11Z

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.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

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));