From: Denis V. Lunev <[email protected]>
Once the tap device behind a multiqueue netdev is deleted on the host,
the queue fds QEMU holds are no longer bound to any device and
TUNSETQUEUE fails with EINVAL, for attach and detach alike. Management
can delete the device under a running VM, e.g. OpenStack unplugs the
VIF of an instance it is deleting. The next
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET from the guest then kills QEMU:
could not enable queue
virtio_net_set_queue_pairs: Assertion `!r' failed.
A guest reset aborts the same way through set_features(): clearing the
features detaches the extra queues and the detach fails too.
Commit a1708203b583 ("net/tap-linux.c: avoid abort when setting invalid
fd") removed the earlier abort on the same path in
tap_set_vnet_hdr_len(); this is the next one.
tap_fd_enable() and tap_fd_disable() already report the error. Walk the
remaining queues anyway, return the failure and let the callers decide:
- the control command restores the previous number of queue pairs and
returns VIRTIO_NET_ERR, so the device matches the guest, which keeps
its old setting on error. The rollback cannot fail in turn: tap
tracks which queues are attached and skips the ioctl for a queue
whose attach or detach already failed;
- post-load fails the migration, so the source keeps running instead
of the destination aborting;
- set_features() has no way to report it and carries on with a dead
link, as a single queue NIC on a deleted tap already does.
The test attaches its own copy of the second queue fd before the guest
asks for two queue pairs, so the attach from QEMU fails with EINVAL on
a live device. Creating the tap needs CAP_NET_ADMIN, so the test is
skipped without it.
Fixes: fed699f9ca6a ("virtio-net: multiqueue support")
Signed-off-by: Denis V. Lunev <[email protected]>
Cc: "Michael S. Tsirkin" <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: Vladimir Sementsov-Ogievskiy <[email protected]>
Cc: Fabiano Rosas <[email protected]>
Cc: Laurent Vivier <[email protected]>
Cc: Paolo Bonzini <[email protected]>
---
hw/net/virtio-net.c | 25 +++++---
tests/qtest/virtio-net-test.c | 104 ++++++++++++++++++++++++++++++++++
2 files changed, 122 insertions(+), 7 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 54e42cfec9..82e3c22ca3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -766,24 +766,27 @@ static int peer_detach(VirtIONet *n, int index)
return tap_disable(nc->peer);
}
-static void virtio_net_set_queue_pairs(VirtIONet *n)
+static int virtio_net_set_queue_pairs(VirtIONet *n)
{
int i;
int r;
+ int ret = 0;
if (n->nic->peer_deleted) {
- return;
+ return 0;
}
for (i = 0; i < n->max_queue_pairs; i++) {
if (i < n->curr_queue_pairs) {
r = peer_attach(n, i);
- assert(!r);
} else {
r = peer_detach(n, i);
- assert(!r);
+ }
+ if (r < 0) {
+ ret = r;
}
}
+ return ret;
}
static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue);
@@ -1499,7 +1502,7 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
struct iovec *iov, unsigned int iov_cnt)
{
VirtIODevice *vdev = VIRTIO_DEVICE(n);
- uint16_t queue_pairs;
+ uint16_t queue_pairs, old_queue_pairs;
NetClientState *nc = qemu_get_queue(n->nic);
virtio_net_disable_rss(n);
@@ -1532,6 +1535,7 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
return VIRTIO_NET_ERR;
}
+ old_queue_pairs = n->curr_queue_pairs;
n->curr_queue_pairs = queue_pairs;
if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
/*
@@ -1543,7 +1547,12 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t
cmd,
/* stop the backend before changing the number of queue_pairs to avoid
handling a
* disabled queue */
virtio_net_set_status(vdev, vdev->status);
- virtio_net_set_queue_pairs(n);
+ if (virtio_net_set_queue_pairs(n) < 0) {
+ n->curr_queue_pairs = old_queue_pairs;
+ virtio_net_set_status(vdev, vdev->status);
+ virtio_net_set_queue_pairs(n);
+ return VIRTIO_NET_ERR;
+ }
return VIRTIO_NET_OK;
}
@@ -3298,7 +3307,9 @@ static int virtio_net_post_load_device(void *opaque, int
version_id)
*/
n->saved_guest_offloads = n->curr_guest_offloads;
- virtio_net_set_queue_pairs(n);
+ if (virtio_net_set_queue_pairs(n) < 0) {
+ return -EINVAL;
+ }
/* Find the first multicast entry in the saved MAC filter */
for (i = 0; i < n->mac_table.in_use; i++) {
diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index 60e5229a3d..9224ee05d9 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -16,6 +16,12 @@
#include "libqos/qgraph.h"
#include "libqos/virtio-net.h"
+#ifdef CONFIG_LINUX
+#include <net/if.h>
+#include <sys/ioctl.h>
+#include <linux/if_tun.h>
+#endif
+
#ifndef ETH_P_RARP
#define ETH_P_RARP 0x8035
#endif
@@ -285,6 +291,97 @@ static void *virtio_net_test_setup(GString *cmd_line, void
*arg)
return sv;
}
+#ifdef CONFIG_LINUX
+static void virtio_net_test_cleanup_tap(void *opaque)
+{
+ int *fds = opaque;
+ int i;
+
+ qos_invalidate_command_line();
+ for (i = 0; i < 2; i++) {
+ if (fds[i] >= 0) {
+ close(fds[i]);
+ }
+ }
+ g_free(fds);
+}
+
+/* arg is the number of tap queues, 1 or 2 */
+static void *virtio_net_test_setup_tap(GString *cmd_line, void *arg)
+{
+ struct ifreq ifr = {
+ .ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_MULTI_QUEUE,
+ };
+ int queues = GPOINTER_TO_INT(arg);
+ int *fds = g_new(int, 2);
+ int i;
+
+ fds[0] = fds[1] = -1;
+ /* TUNSETIFF returns the device name in ifr, the next call attaches to it
*/
+ for (i = 0; i < queues; i++) {
+ fds[i] = open("/dev/net/tun", O_RDWR);
+ if (fds[i] < 0 || ioctl(fds[i], TUNSETIFF, &ifr) < 0) {
+ break;
+ }
+ }
+
+ if (i < queues) {
+ for (i = 0; i < 2; i++) {
+ if (fds[i] >= 0) {
+ close(fds[i]);
+ fds[i] = -1;
+ }
+ }
+ g_string_append(cmd_line, " -netdev hubport,hubid=0,id=hs0 ");
+ } else if (queues == 1) {
+ g_string_append_printf(cmd_line, " -netdev tap,fd=%d,id=hs0 ",
+ fds[0]);
+ } else {
+ g_string_append_printf(cmd_line, " -netdev tap,fds=%d:%d,id=hs0 ",
+ fds[0], fds[1]);
+ }
+
+ g_test_queue_destroy(virtio_net_test_cleanup_tap, fds);
+ return fds;
+}
+
+static void mq_tap_attach_fail(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioNet *net = obj;
+ int *fds = data;
+ QTestState *qts = global_qtest;
+ QVirtQueue *vq = net->queues[net->n_queues - 1];
+ struct ifreq ifr = { .ifr_flags = IFF_ATTACH_QUEUE };
+ uint8_t cmd[4] = { VIRTIO_NET_CTRL_MQ, VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET };
+ uint64_t addr;
+ uint32_t head;
+
+ if (fds[0] < 0) {
+ g_test_skip("cannot create a multiqueue tap device");
+ return;
+ }
+
+ if (qvirtio_is_big_endian(net->vdev)) {
+ stw_be_p(cmd + 2, 2);
+ } else {
+ stw_le_p(cmd + 2, 2);
+ }
+
+ /* Grab queue 1 so that QEMU fails to attach it */
+ g_assert_cmpint(ioctl(fds[1], TUNSETQUEUE, &ifr), ==, 0);
+
+ addr = guest_alloc(t_alloc, sizeof(cmd) + 1);
+ memwrite(addr, cmd, sizeof(cmd));
+ head = qvirtqueue_add(qts, vq, addr, sizeof(cmd), false, true);
+ qvirtqueue_add(qts, vq, addr + sizeof(cmd), 1, true, false);
+ qvirtqueue_kick(qts, net->vdev, vq, head);
+ qvirtio_wait_used_elem(qts, net->vdev, vq, head, NULL,
+ QVIRTIO_NET_TIMEOUT_US);
+ g_assert_cmpint(readb(addr + sizeof(cmd)), ==, VIRTIO_NET_ERR);
+ guest_free(t_alloc, addr);
+}
+#endif /* CONFIG_LINUX */
+
#endif /* _WIN32 */
static void large_tx(void *obj, void *data, QGuestAllocator *t_alloc)
@@ -328,6 +425,13 @@ static void register_virtio_net_test(void)
qos_add_test("basic", "virtio-net", send_recv_test, &opts);
qos_add_test("rx_stop_cont", "virtio-net", stop_cont_test, &opts);
qos_add_test("announce-self", "virtio-net", announce_self, &opts);
+#ifdef CONFIG_LINUX
+ opts.before = virtio_net_test_setup_tap;
+ opts.arg = GINT_TO_POINTER(2);
+ opts.edge.extra_device_opts = "mq=on";
+ qos_add_test("mq-tap-attach-fail", "virtio-net", mq_tap_attach_fail,
&opts);
+ opts.edge.extra_device_opts = NULL;
+#endif
#endif
/* These tests do not need a loopback backend. */
base-commit: 76fdb3373c9a27b3f50ecc9e012cbdeb90af7151
--
2.53.0
