From 94b52958b77a2a040564cf7ed716d3a9545d94e5 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Tue, 20 Mar 2018 11:44:56 +0100 Subject: [PATCH 1/2] virtio_net: flush uncompleted TX on reset If the backend could not transmit a packet right away for some reason, the packet is queued for asynchronous sending. The corresponding vq element is tracked in the async_tx.elem field of the VirtIONetQueue, for later freeing when the transmission is complete. If a reset happens before completion, virtio_net_tx_complete() will push async_tx.elem back to the guest anyway, and we end up with the inuse flag of the vq being equal to -1. The next call to virtqueue_pop() is then likely to fail with "Virtqueue size exceeded". This can be reproduced easily by starting a guest with an hubport backend that is not connected to a functional network, eg, -device virtio-net-pci,netdev=hub0 -netdev hubport,id=hub0,hubid=0 and no other -netdev hubport,hubid=0 on the command line. The appropriate fix is to ensure that such an asynchronous transmission cannot survive a device reset. So for all queues, we first try to send the packet again, and eventually we purge it if the backend still could not deliver it. CC: qemu-stable@nongnu.org Reported-by: R. Nageswara Sastry Buglink: https://github.com/open-power-host-os/qemu/issues/37 Signed-off-by: Greg Kurz Tested-by: R. Nageswara Sastry Signed-off-by: Jason Wang --- hw/net/virtio-net.c | 11 +++++++++++ include/net/net.h | 1 + net/net.c | 1 - 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 67ad38cfe4..90502fca7c 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -427,6 +427,7 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc) static void virtio_net_reset(VirtIODevice *vdev) { VirtIONet *n = VIRTIO_NET(vdev); + int i; /* Reset back to compatibility mode */ n->promisc = 1; @@ -450,6 +451,16 @@ static void virtio_net_reset(VirtIODevice *vdev) memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac)); qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac); memset(n->vlans, 0, MAX_VLAN >> 3); + + /* Flush any async TX */ + for (i = 0; i < n->max_queues; i++) { + NetClientState *nc = qemu_get_subqueue(n->nic, i); + + if (nc->peer) { + qemu_flush_or_purge_queued_packets(nc->peer, true); + assert(!virtio_net_get_subqueue(nc)->async_tx.elem); + } + } } static void peer_test_vnet_hdr(VirtIONet *n) diff --git a/include/net/net.h b/include/net/net.h index a943e968a3..1f7341e459 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -153,6 +153,7 @@ ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf, int size, NetPacketSent *sent_cb); void qemu_purge_queued_packets(NetClientState *nc); void qemu_flush_queued_packets(NetClientState *nc); +void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge); void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]); bool qemu_has_ufo(NetClientState *nc); bool qemu_has_vnet_hdr(NetClientState *nc); diff --git a/net/net.c b/net/net.c index 5222e45069..29f83983e5 100644 --- a/net/net.c +++ b/net/net.c @@ -595,7 +595,6 @@ void qemu_purge_queued_packets(NetClientState *nc) qemu_net_queue_purge(nc->peer->incoming_queue, nc); } -static void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge) { nc->receive_disabled = 0; From 7587855cd23755a7a6bd01b026611465f5584ecd Mon Sep 17 00:00:00 2001 From: Julia Suvorova via Qemu-devel Date: Thu, 15 Mar 2018 23:06:32 +0300 Subject: [PATCH 2/2] net/vde: print error on vde_open() failure Despite the fact that now when the initialization of vde fails, qemu does not end silently, no informative error is printed. The patch generates an error and pushes it through the calling function. Related bug: https://bugs.launchpad.net/qemu/+bug/676029 Signed-off-by: Julia Suvorova Signed-off-by: Jason Wang --- net/vde.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/vde.c b/net/vde.c index e50e5d6394..99189cccb6 100644 --- a/net/vde.c +++ b/net/vde.c @@ -30,6 +30,7 @@ #include "qemu-common.h" #include "qemu/option.h" #include "qemu/main-loop.h" +#include "qapi/error.h" typedef struct VDEState { NetClientState nc; @@ -76,7 +77,7 @@ static NetClientInfo net_vde_info = { static int net_vde_init(NetClientState *peer, const char *model, const char *name, const char *sock, - int port, const char *group, int mode) + int port, const char *group, int mode, Error **errp) { NetClientState *nc; VDEState *s; @@ -92,6 +93,7 @@ static int net_vde_init(NetClientState *peer, const char *model, vde = vde_open(init_sock, (char *)"QEMU", &args); if (!vde){ + error_setg_errno(errp, errno, "Could not open vde"); return -1; } @@ -112,7 +114,6 @@ static int net_vde_init(NetClientState *peer, const char *model, int net_init_vde(const Netdev *netdev, const char *name, NetClientState *peer, Error **errp) { - /* FIXME error_setg(errp, ...) on failure */ const NetdevVdeOptions *vde; assert(netdev->type == NET_CLIENT_DRIVER_VDE); @@ -120,7 +121,7 @@ int net_init_vde(const Netdev *netdev, const char *name, /* missing optional values have been initialized to "all bits zero" */ if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group, - vde->has_mode ? vde->mode : 0700) == -1) { + vde->has_mode ? vde->mode : 0700, errp) == -1) { return -1; }