From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6E152A04C1; Tue, 19 Nov 2019 07:29:17 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 226F62BB1; Tue, 19 Nov 2019 07:29:16 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 6E90A2956; Tue, 19 Nov 2019 07:29:13 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Nov 2019 22:29:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,322,1569308400"; d="scan'208";a="204301051" Received: from dpdk-virtio-tbie-2.sh.intel.com ([10.67.104.74]) by fmsmga008.fm.intel.com with ESMTP; 18 Nov 2019 22:29:11 -0800 From: Tiwei Bie To: dev@dpdk.org, maxime.coquelin@redhat.com, zhihong.wang@intel.com Cc: stephen@networkplumber.org, stable@dpdk.org Date: Tue, 19 Nov 2019 14:29:48 +0800 Message-Id: <20191119062948.15834-1-tiwei.bie@intel.com> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH 20.02] net/virtio-user: do not close tap when disabling queue pairs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Do not close the tap fds when disabling queue pairs, instead, we just need to unbind the backend. Otherwise, tap port can be destroyed unexpectedly. Fixes: e3b434818bbb ("net/virtio-user: support kernel vhost") Cc: stable@dpdk.org Reported-by: Stephen Hemminger Signed-off-by: Tiwei Bie --- drivers/net/virtio/virtio_user/vhost_kernel.c | 34 +++++++++++--- .../net/virtio/virtio_user/vhost_kernel_tap.c | 44 +++++++++++++------ .../net/virtio/virtio_user/vhost_kernel_tap.h | 3 ++ drivers/net/virtio/virtio_user/vhost_user.c | 4 ++ .../net/virtio/virtio_user/virtio_user_dev.c | 5 ++- .../net/virtio/virtio_user/virtio_user_dev.h | 1 + 6 files changed, 70 insertions(+), 21 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c index 5c81e8dd9..d48d284d4 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel.c +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c @@ -330,16 +330,34 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev *dev, vhostfd = dev->vhostfds[pair_idx]; + if (dev->qp_enabled[pair_idx] == enable) + return 0; + if (!enable) { - if (dev->tapfds[pair_idx] >= 0) { - close(dev->tapfds[pair_idx]); - dev->tapfds[pair_idx] = -1; + tapfd = dev->tapfds[pair_idx]; + if (vhost_kernel_set_backend(vhostfd, -1) < 0) { + PMD_DRV_LOG(ERR, "fail to set backend for vhost kernel"); + return -1; } - return vhost_kernel_set_backend(vhostfd, -1); - } else if (dev->tapfds[pair_idx] >= 0) { + if (req_mq && vhost_kernel_tap_detach_queue(tapfd) < 0) { + PMD_DRV_LOG(ERR, "fail to disable tap for vhost kernel"); + return -1; + } + dev->qp_enabled[pair_idx] = false; return 0; } + if (dev->tapfds[pair_idx] >= 0) { + tapfd = dev->tapfds[pair_idx]; + if (vhost_kernel_tap_set_offload(tapfd, dev->features) == -1) + return -1; + if (req_mq && vhost_kernel_tap_attach_queue(tapfd) < 0) { + PMD_DRV_LOG(ERR, "fail to enable tap for vhost kernel"); + return -1; + } + goto set_backend; + } + if ((dev->features & (1ULL << VIRTIO_NET_F_MRG_RXBUF)) || (dev->features & (1ULL << VIRTIO_F_VERSION_1))) hdr_size = sizeof(struct virtio_net_hdr_mrg_rxbuf); @@ -353,13 +371,15 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev *dev, return -1; } + dev->tapfds[pair_idx] = tapfd; + +set_backend: if (vhost_kernel_set_backend(vhostfd, tapfd) < 0) { PMD_DRV_LOG(ERR, "fail to set backend for vhost kernel"); - close(tapfd); return -1; } - dev->tapfds[pair_idx] = tapfd; + dev->qp_enabled[pair_idx] = true; return 0; } diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c index 76bf75423..529f81133 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c @@ -18,7 +18,27 @@ #include "../virtio_logs.h" #include "../virtio_pci.h" -static int +int +vhost_kernel_tap_attach_queue(int fd) +{ + struct ifreq ifr; + + memset(&ifr, 0, sizeof(ifr)); + ifr.ifr_flags = IFF_ATTACH_QUEUE; + return ioctl(fd, TUNSETQUEUE, (void *)&ifr); +} + +int +vhost_kernel_tap_detach_queue(int fd) +{ + struct ifreq ifr; + + memset(&ifr, 0, sizeof(ifr)); + ifr.ifr_flags = IFF_DETACH_QUEUE; + return ioctl(fd, TUNSETQUEUE, (void *)&ifr); +} + +int vhost_kernel_tap_set_offload(int fd, uint64_t features) { unsigned int offload = 0; @@ -37,20 +57,18 @@ vhost_kernel_tap_set_offload(int fd, uint64_t features) offload |= TUN_F_UFO; } - if (offload != 0) { - /* Check if our kernel supports TUNSETOFFLOAD */ - if (ioctl(fd, TUNSETOFFLOAD, 0) != 0 && errno == EINVAL) { - PMD_DRV_LOG(ERR, "Kernel does't support TUNSETOFFLOAD\n"); - return -ENOTSUP; - } + /* Check if our kernel supports TUNSETOFFLOAD */ + if (ioctl(fd, TUNSETOFFLOAD, 0) != 0 && errno == EINVAL) { + PMD_DRV_LOG(ERR, "Kernel does't support TUNSETOFFLOAD\n"); + return -ENOTSUP; + } + if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) { + offload &= ~TUN_F_UFO; if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) { - offload &= ~TUN_F_UFO; - if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) { - PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s\n", - strerror(errno)); - return -1; - } + PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s\n", + strerror(errno)); + return -1; } } diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h index e0e95b4f5..1202932f7 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h @@ -37,3 +37,6 @@ int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq, const char *mac, uint64_t features); +int vhost_kernel_tap_attach_queue(int fd); +int vhost_kernel_tap_detach_queue(int fd); +int vhost_kernel_tap_set_offload(int fd, uint64_t features); diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index a4b5c25cd..d8e083ba8 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -456,6 +456,9 @@ vhost_user_enable_queue_pair(struct virtio_user_dev *dev, { int i; + if (dev->qp_enabled[pair_idx] == enable) + return 0; + for (i = 0; i < 2; ++i) { struct vhost_vring_state state = { .index = pair_idx * 2 + i, @@ -466,6 +469,7 @@ vhost_user_enable_queue_pair(struct virtio_user_dev *dev, return -1; } + dev->qp_enabled[pair_idx] = enable; return 0; } diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index a4400e772..177e829c9 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -545,8 +545,11 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev) } if (dev->vhostfds) { - for (i = 0; i < dev->max_queue_pairs; ++i) + for (i = 0; i < dev->max_queue_pairs; ++i) { close(dev->vhostfds[i]); + if (dev->tapfds[i] >= 0) + close(dev->tapfds[i]); + } free(dev->vhostfds); free(dev->tapfds); } diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h index ad8683771..3b6b6065a 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h @@ -49,6 +49,7 @@ struct virtio_user_dev { struct vring_packed packed_vrings[VIRTIO_MAX_VIRTQUEUES]; }; struct virtio_user_queue packed_queues[VIRTIO_MAX_VIRTQUEUES]; + bool qp_enabled[VIRTIO_MAX_VIRTQUEUE_PAIRS]; struct virtio_user_backend_ops *ops; pthread_mutex_t mutex; -- 2.23.0