From 199ee608f0d08510b5c6c37f31a7fbff211d63c4 Mon Sep 17 00:00:00 2001 From: Luigi Rizzo Date: Tue, 5 Feb 2013 17:53:31 +0100 Subject: [PATCH 1/7] net: fix qemu_flush_queued_packets() in presence of a hub When frontend and backend are connected through a hub as below (showing only one direction), and the frontend (or in general, all output ports of the hub) cannot accept more traffic, the backend queues packets in queue-A. When the frontend (or in general, one output port) becomes ready again, quemu tries to flush packets from queue-B, which is unfortunately empty. e1000.0 <--[queue B]-- hub0port0(hub)hub0port1 <--[queue A]-- tap.0 To fix this i propose to introduce a new function net_hub_flush() which is called when trying to flush a queue connected to a hub. Signed-off-by: Luigi Rizzo Signed-off-by: Stefan Hajnoczi --- net/hub.c | 14 ++++++++++++++ net/hub.h | 1 + net/net.c | 6 ++++++ 3 files changed, 21 insertions(+) diff --git a/net/hub.c b/net/hub.c index a24c9d17f7..df32074de0 100644 --- a/net/hub.c +++ b/net/hub.c @@ -338,3 +338,17 @@ void net_hub_check_clients(void) } } } + +bool net_hub_flush(NetClientState *nc) +{ + NetHubPort *port; + NetHubPort *source_port = DO_UPCAST(NetHubPort, nc, nc); + int ret = 0; + + QLIST_FOREACH(port, &source_port->hub->ports, next) { + if (port != source_port) { + ret += qemu_net_queue_flush(port->nc.send_queue); + } + } + return ret ? true : false; +} diff --git a/net/hub.h b/net/hub.h index 583ada89d8..a625effe00 100644 --- a/net/hub.h +++ b/net/hub.h @@ -21,5 +21,6 @@ NetClientState *net_hub_add_port(int hub_id, const char *name); NetClientState *net_hub_find_client_by_name(int hub_id, const char *name); void net_hub_info(Monitor *mon); void net_hub_check_clients(void); +bool net_hub_flush(NetClientState *nc); #endif /* NET_HUB_H */ diff --git a/net/net.c b/net/net.c index be03a8dd14..a66aa02472 100644 --- a/net/net.c +++ b/net/net.c @@ -441,6 +441,12 @@ void qemu_flush_queued_packets(NetClientState *nc) { nc->receive_disabled = 0; + if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) { + if (net_hub_flush(nc->peer)) { + qemu_notify_event(); + } + return; + } if (qemu_net_queue_flush(nc->send_queue)) { /* We emptied the queue successfully, signal to the IO thread to repoll * the file descriptor (for tap, for example). From 7d91ddd25e3a4e5008a2ac16127d51a34fd56bf1 Mon Sep 17 00:00:00 2001 From: Luigi Rizzo Date: Tue, 5 Feb 2013 18:29:09 +0100 Subject: [PATCH 2/7] net: fix unbounded NetQueue In the current implementation of qemu, running without a network backend will cause the queue to grow unbounded when the guest is transmitting traffic. This patch fixes the problem by implementing bounded size NetQueue, used with an arbitrary limit of 10000 packets, and dropping packets when the queue is full _and_ the sender does not pass a callback. The second condition makes sure that we never drop packets that contains a callback (which would be tricky, because the producer expects the callback to be run when all previous packets have been consumed; so we cannot run it when the packet is dropped). If documentation is correct, producers that submit a callback should stop sending when their packet is queued, so there is no real risk that the queue exceeds the max size by large values. Signed-off-by: Luigi Rizzo Signed-off-by: Stefan Hajnoczi --- net/queue.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/net/queue.c b/net/queue.c index 6eaf5b63c0..859d02a136 100644 --- a/net/queue.c +++ b/net/queue.c @@ -50,6 +50,8 @@ struct NetPacket { struct NetQueue { void *opaque; + uint32_t nq_maxlen; + uint32_t nq_count; QTAILQ_HEAD(packets, NetPacket) packets; @@ -63,6 +65,8 @@ NetQueue *qemu_new_net_queue(void *opaque) queue = g_malloc0(sizeof(NetQueue)); queue->opaque = opaque; + queue->nq_maxlen = 10000; + queue->nq_count = 0; QTAILQ_INIT(&queue->packets); @@ -92,6 +96,9 @@ static void qemu_net_queue_append(NetQueue *queue, { NetPacket *packet; + if (queue->nq_count >= queue->nq_maxlen && !sent_cb) { + return; /* drop if queue full and no callback */ + } packet = g_malloc(sizeof(NetPacket) + size); packet->sender = sender; packet->flags = flags; @@ -99,6 +106,7 @@ static void qemu_net_queue_append(NetQueue *queue, packet->sent_cb = sent_cb; memcpy(packet->data, buf, size); + queue->nq_count++; QTAILQ_INSERT_TAIL(&queue->packets, packet, entry); } @@ -113,6 +121,9 @@ static void qemu_net_queue_append_iov(NetQueue *queue, size_t max_len = 0; int i; + if (queue->nq_count >= queue->nq_maxlen && !sent_cb) { + return; /* drop if queue full and no callback */ + } for (i = 0; i < iovcnt; i++) { max_len += iov[i].iov_len; } @@ -130,6 +141,7 @@ static void qemu_net_queue_append_iov(NetQueue *queue, packet->size += len; } + queue->nq_count++; QTAILQ_INSERT_TAIL(&queue->packets, packet, entry); } @@ -220,6 +232,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from) QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) { if (packet->sender == from) { QTAILQ_REMOVE(&queue->packets, packet, entry); + queue->nq_count--; g_free(packet); } } @@ -233,6 +246,7 @@ bool qemu_net_queue_flush(NetQueue *queue) packet = QTAILQ_FIRST(&queue->packets); QTAILQ_REMOVE(&queue->packets, packet, entry); + queue->nq_count--; ret = qemu_net_queue_deliver(queue, packet->sender, @@ -240,6 +254,7 @@ bool qemu_net_queue_flush(NetQueue *queue) packet->data, packet->size); if (ret == 0) { + queue->nq_count++; QTAILQ_INSERT_HEAD(&queue->packets, packet, entry); return false; } From ce675a7579fea498397c5d2da3c5367671e9f02a Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Thu, 21 Feb 2013 11:05:56 +0800 Subject: [PATCH 3/7] tap: forbid creating multiqueue tap when hub is used Obviously, hub does not support multiqueue tap. So this patch forbids creating multiple queue tap when hub is used to prevent the crash when command line such as "-net tap,queues=2" is used. Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- net/tap.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/net/tap.c b/net/tap.c index 48c254ed85..daab350efc 100644 --- a/net/tap.c +++ b/net/tap.c @@ -693,6 +693,13 @@ int net_init_tap(const NetClientOptions *opts, const char *name, queues = tap->has_queues ? tap->queues : 1; vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL; + /* QEMU vlans does not support multiqueue tap, in this case peer is set. + * For -netdev, peer is always NULL. */ + if (peer && (tap->has_queues || tap->has_fds || tap->has_vhostfds)) { + error_report("Multiqueue tap cannnot be used with QEMU vlans"); + return -1; + } + if (tap->has_fd) { if (tap->has_ifname || tap->has_script || tap->has_downscript || tap->has_vnet_hdr || tap->has_helper || tap->has_queues || From d26e445c80fddcc7483b83f3115e5067fef28fe6 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 25 Feb 2013 10:17:08 +0100 Subject: [PATCH 4/7] tap: set IFF_ONE_QUEUE per default historically the kernel queues packets two times. once at the device and second in qdisc. this is believed to cause interface stalls if one of these queues overruns. setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the flag is ignored since then. see kernel commit 5d097109257c03a71845729f8db6b5770c4bbedc Signed-off-by: Peter Lieven Acked-by: Michael S. Tsirkin Signed-off-by: Stefan Hajnoczi --- net/tap-linux.c | 10 ++++++---- net/tap-linux.h | 9 +++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/net/tap-linux.c b/net/tap-linux.c index a9531892a6..36c09e24d8 100644 --- a/net/tap-linux.c +++ b/net/tap-linux.c @@ -42,6 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, struct ifreq ifr; int fd, ret; int len = sizeof(struct virtio_net_hdr); + unsigned int features; TFR(fd = open(PATH_NET_TUN, O_RDWR)); if (fd < 0) { @@ -51,9 +52,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, memset(&ifr, 0, sizeof(ifr)); ifr.ifr_flags = IFF_TAP | IFF_NO_PI; - if (*vnet_hdr) { - unsigned int features; + if (ioctl(fd, TUNGETFEATURES, &features) == 0 && + features & IFF_ONE_QUEUE) { + ifr.ifr_flags |= IFF_ONE_QUEUE; + } + if (*vnet_hdr) { if (ioctl(fd, TUNGETFEATURES, &features) == 0 && features & IFF_VNET_HDR) { *vnet_hdr = 1; @@ -78,8 +82,6 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, } if (mq_required) { - unsigned int features; - if ((ioctl(fd, TUNGETFEATURES, &features) != 0) || !(features & IFF_MULTI_QUEUE)) { error_report("multiqueue required, but no kernel " diff --git a/net/tap-linux.h b/net/tap-linux.h index 65087e1419..1cf35d41bd 100644 --- a/net/tap-linux.h +++ b/net/tap-linux.h @@ -34,10 +34,11 @@ #endif /* TUNSETIFF ifr flags */ -#define IFF_TAP 0x0002 -#define IFF_NO_PI 0x1000 -#define IFF_VNET_HDR 0x4000 -#define IFF_MULTI_QUEUE 0x0100 +#define IFF_TAP 0x0002 +#define IFF_NO_PI 0x1000 +#define IFF_ONE_QUEUE 0x2000 +#define IFF_VNET_HDR 0x4000 +#define IFF_MULTI_QUEUE 0x0100 #define IFF_ATTACH_QUEUE 0x0200 #define IFF_DETACH_QUEUE 0x0400 From f6b26cf257232e5854c0e5c98a8685c625bf986e Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Fri, 22 Feb 2013 23:15:06 +0800 Subject: [PATCH 5/7] net: reduce the unnecessary memory allocation of multiqueue Edivaldo reports a problem that the array of NetClientState in NICState is too large - MAX_QUEUE_NUM(1024) which will wastes memory even if multiqueue is not used. Instead of static arrays, solving this issue by allocating the queues on demand for both the NetClientState array in NICState and VirtIONetQueue array in VirtIONet. Tested by myself, with single virtio-net-pci device. The memory allocation is almost the same as when multiqueue is not merged. Cc: Edivaldo de Araujo Pereira Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- hw/virtio-net.c | 6 ++++-- include/net/net.h | 2 +- net/net.c | 19 +++++++++---------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 573c669d15..bb2c26c483 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -44,7 +44,7 @@ typedef struct VirtIONet VirtIODevice vdev; uint8_t mac[ETH_ALEN]; uint16_t status; - VirtIONetQueue vqs[MAX_QUEUE_NUM]; + VirtIONetQueue *vqs; VirtQueue *ctrl_vq; NICState *nic; uint32_t tx_timeout; @@ -1326,8 +1326,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, n->vdev.set_status = virtio_net_set_status; n->vdev.guest_notifier_mask = virtio_net_guest_notifier_mask; n->vdev.guest_notifier_pending = virtio_net_guest_notifier_pending; + n->max_queues = MAX(conf->queues, 1); + n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues); n->vqs[0].rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx); - n->max_queues = conf->queues; n->curr_queues = 1; n->vqs[0].n = n; n->tx_timeout = net->txtimer; @@ -1412,6 +1413,7 @@ void virtio_net_exit(VirtIODevice *vdev) } } + g_free(n->vqs); qemu_del_nic(n->nic); virtio_cleanup(&n->vdev); } diff --git a/include/net/net.h b/include/net/net.h index 43a045e052..cb049a16a3 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -72,7 +72,7 @@ struct NetClientState { }; typedef struct NICState { - NetClientState ncs[MAX_QUEUE_NUM]; + NetClientState *ncs; NICConf *conf; void *opaque; bool peer_deleted; diff --git a/net/net.c b/net/net.c index a66aa02472..f3d67f8322 100644 --- a/net/net.c +++ b/net/net.c @@ -235,23 +235,20 @@ NICState *qemu_new_nic(NetClientInfo *info, const char *name, void *opaque) { - NetClientState *nc; NetClientState **peers = conf->peers.ncs; NICState *nic; - int i; + int i, queues = MAX(1, conf->queues); assert(info->type == NET_CLIENT_OPTIONS_KIND_NIC); assert(info->size >= sizeof(NICState)); - nc = qemu_new_net_client(info, peers[0], model, name); - nc->queue_index = 0; - - nic = qemu_get_nic(nc); + nic = g_malloc0(info->size + sizeof(NetClientState) * queues); + nic->ncs = (void *)nic + info->size; nic->conf = conf; nic->opaque = opaque; - for (i = 1; i < conf->queues; i++) { - qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, nc->name, + for (i = 0; i < queues; i++) { + qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name, NULL); nic->ncs[i].queue_index = i; } @@ -261,7 +258,7 @@ NICState *qemu_new_nic(NetClientInfo *info, NetClientState *qemu_get_subqueue(NICState *nic, int queue_index) { - return &nic->ncs[queue_index]; + return nic->ncs + queue_index; } NetClientState *qemu_get_queue(NICState *nic) @@ -273,7 +270,7 @@ NICState *qemu_get_nic(NetClientState *nc) { NetClientState *nc0 = nc - nc->queue_index; - return DO_UPCAST(NICState, ncs[0], nc0); + return (NICState *)((void *)nc0 - nc->info->size); } void *qemu_get_nic_opaque(NetClientState *nc) @@ -368,6 +365,8 @@ void qemu_del_nic(NICState *nic) qemu_cleanup_net_client(nc); qemu_free_net_client(nc); } + + g_free(nic); } void qemu_foreach_nic(qemu_nic_foreach func, void *opaque) From 40e8c26d7b7e260cc3566c6b68cee969e816970e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 26 Feb 2013 11:07:16 +0100 Subject: [PATCH 6/7] doc: document -netdev hubport Reviewed-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- hmp-commands.hx | 2 +- qemu-options.hx | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index cef7708e3a..69c707d332 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1169,7 +1169,7 @@ ETEXI { .name = "netdev_add", .args_type = "netdev:O", - .params = "[user|tap|socket],id=str[,prop=value][,...]", + .params = "[user|tap|socket|hubport],id=str[,prop=value][,...]", .help = "add host network device", .mhandler.cmd = hmp_netdev_add, }, diff --git a/qemu-options.hx b/qemu-options.hx index 797d992804..863069f293 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1408,7 +1408,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, #ifdef CONFIG_VDE "vde|" #endif - "socket],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL) + "socket|" + "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL) STEXI @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}] @findex -net @@ -1730,6 +1731,14 @@ vde_switch -F -sock /tmp/myswitch qemu-system-i386 linux.img -net nic -net vde,sock=/tmp/myswitch @end example +@item -netdev hubport,id=@var{id},hubid=@var{hubid} + +Create a hub port on QEMU "vlan" @var{hubid}. + +The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single +netdev. @code{-net} and @code{-device} with parameter @option{vlan} create the +required hub automatically. + @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}] Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default). At most @var{len} bytes (64k by default) per packet are stored. The file format is From af347aa5a521555f5342e67993eb717d4f542ba8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 22 Feb 2013 18:31:51 +0100 Subject: [PATCH 7/7] qmp: netdev_add is like -netdev, not -net, fix documentation Cc: qemu-stable@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Signed-off-by: Stefan Hajnoczi --- qmp-commands.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index 799adea1b7..95022e259f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -822,7 +822,7 @@ Example: -> { "execute": "netdev_add", "arguments": { "type": "user", "id": "netdev1" } } <- { "return": {} } -Note: The supported device options are the same ones supported by the '-net' +Note: The supported device options are the same ones supported by the '-netdev' command-line argument, which are listed in the '-help' output or QEMU's manual