From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 123851B1BD for ; Wed, 24 Jan 2018 16:42:52 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id BD45E2221C; Wed, 24 Jan 2018 10:42:51 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute1.internal (MEProxy); Wed, 24 Jan 2018 10:42:51 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fridaylinux.org; h=cc:date:from:in-reply-to:message-id:references:subject:to :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=W+hrVTNlLhmbh3Yu3 mpNC/5HKifvkNr1TE7eUQHzHDo=; b=KsTX84OG5FbtMjm8bj9oN74ZQBE0a35UU QbROgeaAq93ODfdXulCYNVeilpU0nStSTL2v0KeVeD/764+whBhKBuknz3G9yYbz cOzDIe/70c3MIzxFfYdUd2d4JSPKUcaFLmPveMIt0gFgrE+9jcu9NisUeNF7z1Ns 2qL4GGCZdU4uasI2SQSM/U3AOczQKO0YQ9lCRuvCCJfrUixtfOXTe3beweUkvKBi 34G0K+EIbYUGPT1c/8I8ttoKRwVPH+gWZ440QGTrn2GmbWBck1+LUrRf7ilTzCHM jlIxc4U1Fmig0ke4SqptB9prM7do5EfJbrpA4Lb2mDZw0lcP7/6gw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:in-reply-to:message-id :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; bh=W+hrVTNlLhmbh3Yu3mpNC/5HKifvkNr1TE7eUQHzHDo=; b=BuaE+QaV W77zdqd7fNbhbsB/aq/pnQudErYJ3YTOJ7YKiMMM5HY1APT5L3cIHyexpwVX0C6k OkmvMGdqmatvFTvOUlmSUCvYaIKVKsYE11mweXO9D8zepXrUkZGpllXgcj9RQ5/G 8X++Ae/j6d3YizLLWekmwqgQHUP/vOlpJMgEQQNldNJPbjGN0bfo3LrTpgyhk4KE g8Jf9eErmG47Azvt6A/ASKryB/qZjbIJhpZIfDdMDluSJ71nEFmPe87Hf97x2gfN DkHlG/2WqOGjFnuYUJTOwBcqNYZvhxqM0Qn5xf2oU4G5/cv5lzE01M+xCARuyN6p bReQ1YRvI66O0w== X-ME-Sender: Received: from localhost.localdomain (unknown [115.150.27.206]) by mail.messagingengine.com (Postfix) with ESMTPA id 3B7547E354; Wed, 24 Jan 2018 10:42:48 -0500 (EST) From: Yuanhan Liu To: Victor Kaplansky Cc: Maxime Coquelin , Yuanhan Liu , dpdk stable Date: Wed, 24 Jan 2018 23:33:43 +0800 Message-Id: <1516808026-25523-155-git-send-email-yliu@fridaylinux.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1516808026-25523-1-git-send-email-yliu@fridaylinux.org> References: <1516808026-25523-1-git-send-email-yliu@fridaylinux.org> Subject: [dpdk-stable] patch 'vhost: protect active rings from async ring changes' has been queued to LTS release 17.11.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 15:42:52 -0000 Hi, FYI, your patch has been queued to LTS release 17.11.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 01/26/18. So please shout if anyone has objections. Thanks. --yliu --- >>From b45011203c45861834237dd9103047d3b3009348 Mon Sep 17 00:00:00 2001 From: Victor Kaplansky Date: Wed, 17 Jan 2018 15:49:25 +0200 Subject: [PATCH] vhost: protect active rings from async ring changes [ upstream commit a3688046995f88c518fa27c45b39ae389260b18d ] When performing live migration or memory hot-plugging, the changes to the device and vrings made by message handler done independently from vring usage by PMD threads. This causes for example segfaults during live-migration with MQ enable, but in general virtually any request sent by qemu changing the state of device can cause problems. These patches fixes all above issues by adding a spinlock to every vring and requiring message handler to start operation only after ensuring that all PMD threads related to the device are out of critical section accessing the vring data. Each vring has its own lock in order to not create contention between PMD threads of different vrings and to prevent performance degradation by scaling queue pair number. See https://bugzilla.redhat.com/show_bug.cgi?id=1450680 Signed-off-by: Victor Kaplansky Reviewed-by: Maxime Coquelin Acked-by: Yuanhan Liu --- lib/librte_vhost/vhost.c | 1 + lib/librte_vhost/vhost.h | 6 ++-- lib/librte_vhost/vhost_user.c | 70 +++++++++++++++++++++++++++++++++++++++++++ lib/librte_vhost/virtio_net.c | 26 ++++++++++++++-- 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index 4f8b73a..dcc42fc 100644 --- a/lib/librte_vhost/vhost.c +++ b/lib/librte_vhost/vhost.c @@ -259,6 +259,7 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx) dev->virtqueue[vring_idx] = vq; init_vring_queue(dev, vring_idx); + rte_spinlock_init(&vq->access_lock); dev->nr_vring += 1; diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index 1cc81c1..c8f2a81 100644 --- a/lib/librte_vhost/vhost.h +++ b/lib/librte_vhost/vhost.h @@ -108,12 +108,14 @@ struct vhost_virtqueue { /* Backend value to determine if device should started/stopped */ int backend; + int enabled; + int access_ok; + rte_spinlock_t access_lock; + /* Used to notify the guest (trigger interrupt) */ int callfd; /* Currently unused as polling mode is enabled */ int kickfd; - int enabled; - int access_ok; /* Physical address of used ring, for logging */ uint64_t log_guest_addr; diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 6f3869c..3acaacf 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -1224,12 +1224,47 @@ vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, VhostUserMsg *msg) return alloc_vring_queue(dev, vring_idx); } +static void +vhost_user_lock_all_queue_pairs(struct virtio_net *dev) +{ + unsigned int i = 0; + unsigned int vq_num = 0; + + while (vq_num < dev->nr_vring) { + struct vhost_virtqueue *vq = dev->virtqueue[i]; + + if (vq) { + rte_spinlock_lock(&vq->access_lock); + vq_num++; + } + i++; + } +} + +static void +vhost_user_unlock_all_queue_pairs(struct virtio_net *dev) +{ + unsigned int i = 0; + unsigned int vq_num = 0; + + while (vq_num < dev->nr_vring) { + struct vhost_virtqueue *vq = dev->virtqueue[i]; + + if (vq) { + rte_spinlock_unlock(&vq->access_lock); + vq_num++; + } + i++; + } +} + int vhost_user_msg_handler(int vid, int fd) { struct virtio_net *dev; struct VhostUserMsg msg; int ret; + int unlock_required = 0; dev = get_device(vid); if (dev == NULL) @@ -1275,6 +1310,38 @@ vhost_user_msg_handler(int vid, int fd) return -1; } + /* + * Note: we don't lock all queues on VHOST_USER_GET_VRING_BASE, + * since it is sent when virtio stops and device is destroyed. + * destroy_device waits for queues to be inactive, so it is safe. + * Otherwise taking the access_lock would cause a dead lock. + */ + switch (msg.request.master) { + case VHOST_USER_SET_FEATURES: + case VHOST_USER_SET_PROTOCOL_FEATURES: + case VHOST_USER_SET_OWNER: + case VHOST_USER_RESET_OWNER: + case VHOST_USER_SET_MEM_TABLE: + case VHOST_USER_SET_LOG_BASE: + case VHOST_USER_SET_LOG_FD: + case VHOST_USER_SET_VRING_NUM: + case VHOST_USER_SET_VRING_ADDR: + case VHOST_USER_SET_VRING_BASE: + case VHOST_USER_SET_VRING_KICK: + case VHOST_USER_SET_VRING_CALL: + case VHOST_USER_SET_VRING_ERR: + case VHOST_USER_SET_VRING_ENABLE: + case VHOST_USER_SEND_RARP: + case VHOST_USER_NET_SET_MTU: + case VHOST_USER_SET_SLAVE_REQ_FD: + vhost_user_lock_all_queue_pairs(dev); + unlock_required = 1; + break; + default: + break; + + } + switch (msg.request.master) { case VHOST_USER_GET_FEATURES: msg.payload.u64 = vhost_user_get_features(dev); @@ -1376,6 +1443,9 @@ vhost_user_msg_handler(int vid, int fd) } + if (unlock_required) + vhost_user_unlock_all_queue_pairs(dev); + if (msg.flags & VHOST_USER_NEED_REPLY) { msg.payload.u64 = !!ret; msg.size = sizeof(msg.payload.u64); diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 17158c1..d347030 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -44,6 +44,7 @@ #include #include #include +#include #include "iotlb.h" #include "vhost.h" @@ -326,8 +327,11 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, } vq = dev->virtqueue[queue_id]; + + rte_spinlock_lock(&vq->access_lock); + if (unlikely(vq->enabled == 0)) - return 0; + goto out_access_unlock; if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_lock(vq); @@ -419,6 +423,9 @@ out: if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_unlock(vq); +out_access_unlock: + rte_spinlock_unlock(&vq->access_lock); + return count; } @@ -651,8 +658,11 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, } vq = dev->virtqueue[queue_id]; + + rte_spinlock_lock(&vq->access_lock); + if (unlikely(vq->enabled == 0)) - return 0; + goto out_access_unlock; if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_lock(vq); @@ -715,6 +725,9 @@ out: if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_unlock(vq); +out_access_unlock: + rte_spinlock_unlock(&vq->access_lock); + return pkt_idx; } @@ -1197,9 +1210,13 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, } vq = dev->virtqueue[queue_id]; - if (unlikely(vq->enabled == 0)) + + if (unlikely(rte_spinlock_trylock(&vq->access_lock) == 0)) return 0; + if (unlikely(vq->enabled == 0)) + goto out_access_unlock; + vq->batch_copy_nb_elems = 0; if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) @@ -1374,6 +1391,9 @@ out: if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) vhost_user_iotlb_rd_unlock(vq); +out_access_unlock: + rte_spinlock_unlock(&vq->access_lock); + if (unlikely(rarp_mbuf != NULL)) { /* * Inject it to the head of "pkts" array, so that switch's mac -- 2.7.4