From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36807) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFIGh-0007r2-3D for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:42:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFIGc-0000oU-TE for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:42:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53061) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFIGc-0000oK-L6 for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:42:14 -0400 Message-ID: <55A61CE1.2040200@redhat.com> Date: Wed, 15 Jul 2015 16:42:09 +0800 From: Jason Wang MIME-Version: 1.0 References: <55A617E9.9080503@cn.fujitsu.com> In-Reply-To: <55A617E9.9080503@cn.fujitsu.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 for-2.4] virtio-net: remove virtio queues if the guest doesn't support multiqueue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang , qemu-devl , "Michael S. Tsirkin" Cc: Fam Zheng , Stefan Hajnoczi On 07/15/2015 04:20 PM, Wen Congyang wrote: > commit da51a335 adds all queues in .realize(). But if the > guest doesn't support multiqueue, we forget to remove them. And > we cannot handle the ctrl vq corretly. The guest will hang. > > Signed-off-by: Wen Congyang > --- > hw/net/virtio-net.c | 93 ++++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 78 insertions(+), 15 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index e3c2db3..48c7705 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1306,9 +1306,86 @@ static void virtio_net_tx_bh(void *opaque) > } > } > > +static void virtio_net_add_queue(VirtIONet *n, int index) > +{ > + VirtIODevice *vdev = VIRTIO_DEVICE(n); > + > + n->vqs[index].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx); > + if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) { > + n->vqs[index].tx_vq = > + virtio_add_queue(vdev, 256, virtio_net_handle_tx_timer); > + n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > + virtio_net_tx_timer, > + &n->vqs[index]); > + } else { > + n->vqs[index].tx_vq = > + virtio_add_queue(vdev, 256, virtio_net_handle_tx_bh); > + n->vqs[index].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[index]); > + } > + > + n->vqs[index].tx_waiting = 0; > + n->vqs[index].n = n; > +} > + > +static void virtio_net_del_queue(VirtIONet *n, int index) > +{ > + VirtIODevice *vdev = VIRTIO_DEVICE(n); > + VirtIONetQueue *q = &n->vqs[index]; > + NetClientState *nc = qemu_get_subqueue(n->nic, index); > + > + qemu_purge_queued_packets(nc); > + > + virtio_del_queue(vdev, index * 2); > + if (q->tx_timer) { > + timer_del(q->tx_timer); > + timer_free(q->tx_timer); > + } else { > + qemu_bh_delete(q->tx_bh); > + } > + virtio_del_queue(vdev, index * 2 + 1); > +} Ok, then in unrealize() you may just want to delete bhs/timers up to curr_queues. Otherwise it may cause a use after free? > + > +static void virtio_net_change_num_queues(VirtIONet *n, int new_max_queues) > +{ > + VirtIODevice *vdev = VIRTIO_DEVICE(n); > + int old_num_queues = virtio_get_num_queues(vdev); > + int new_num_queues = new_max_queues * 2 + 1; > + int i; > + > + assert(old_num_queues >= 3); > + assert(old_num_queues % 2 == 1); > + > + if (old_num_queues == new_num_queues) { > + return; > + } > + > + /* > + * We always need to remove and add ctrl vq if > + * old_num_queues != new_num_queues. Remove ctrl_vq first, > + * and then we only enter one of the following too loops. > + */ > + virtio_del_queue(vdev, old_num_queues - 1); > + > + for (i = new_num_queues - 1; i < old_num_queues - 1; i += 2) { > + /* new_num_queues < old_num_queues */ > + virtio_net_del_queue(n, i / 2); > + } > + > + for (i = old_num_queues - 1; i < new_num_queues - 1; i += 2) { > + /* new_num_queues > old_num_queues */ > + virtio_net_add_queue(n, i / 2); > + } > + > + /* add ctrl_vq last */ > + n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); > +} > + > static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue) > { > + int max = multiqueue ? n->max_queues : 1; > + > n->multiqueue = multiqueue; > + virtio_net_change_num_queues(n, max); > > virtio_net_set_queues(n); > } > @@ -1583,21 +1660,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) > } > > for (i = 0; i < n->max_queues; i++) { > - n->vqs[i].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx); > - if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) { > - n->vqs[i].tx_vq = > - virtio_add_queue(vdev, 256, virtio_net_handle_tx_timer); > - n->vqs[i].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > - virtio_net_tx_timer, > - &n->vqs[i]); > - } else { > - n->vqs[i].tx_vq = > - virtio_add_queue(vdev, 256, virtio_net_handle_tx_bh); > - n->vqs[i].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[i]); > - } > - > - n->vqs[i].tx_waiting = 0; > - n->vqs[i].n = n; > + virtio_net_add_queue(n, i); > } > > n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);