From 744b98d2735286531d87a1c47d46005fb30d5477 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 8 Oct 2008 17:02:33 -0300 Subject: [PATCH] virtio-net: Fix kick/wait logic The virtnet_transmit() logic for waiting the packet to be transmitted is reversed: we can't wait the packet to be transmitted if we didn't kick() the ring yet. The vring_more_used() while loop logic is reversed also, that explains why the code works today. The current code risks trying to free a buffer from the used ring when none was available, that will happen most times because KVM doesn't handle the packet immediately on kick(). Luckily it was working because it was unlikely to have a buffer still queued for transmit when virtnet_transmit() was called. Also, adds a BUG_ON() to vring_get_buf(), to catch cases where we try to free a buffer from the used ring when there was none available. Patch for Etherboot. gPXE has the same problem on the code, but I hadn't a chance to test gPXE using virtio-net yet. Signed-off-by: Eduardo Habkost Signed-off-by: Stefan Hajnoczi --- src/drivers/net/virtio-net.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/drivers/net/virtio-net.c b/src/drivers/net/virtio-net.c index 4ec154df..68c84d79 100644 --- a/src/drivers/net/virtio-net.c +++ b/src/drivers/net/virtio-net.c @@ -187,6 +187,8 @@ static int vring_get_buf(int queue_index, unsigned int *len) u32 id; int ret; + BUG_ON(!vring_more_used(queue_index)); + elem = &vr->used->ring[last_used_idx[queue_index] % vr->num]; wmb(); id = elem->id; @@ -365,6 +367,8 @@ static void virtnet_transmit(struct nic *nic, const char *destaddr, vring_add_buf(TX_INDEX, 0, 0); + vring_kick(nic, TX_INDEX, 1); + /* * http://www.etherboot.org/wiki/dev/devmanual * @@ -372,13 +376,11 @@ static void virtnet_transmit(struct nic *nic, const char *destaddr, * before returning from this routine" */ - while (vring_more_used(TX_INDEX)) { + while (!vring_more_used(TX_INDEX)) { mb(); udelay(10); } - vring_kick(nic, TX_INDEX, 1); - /* free desc */ (void)vring_get_buf(TX_INDEX, NULL);