net/virtio-user: do not close tap when disabling queue pairs

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 <stephen@networkplumber.org>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
index 5c81e8d..2c80507 100644 (file)
--- 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_set_queue(tapfd, false) < 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_set_queue(tapfd, true) < 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 76bf754..2fbfecb 100644 (file)
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
@@ -18,7 +18,7 @@
 #include "../virtio_logs.h"
 #include "../virtio_pci.h"
 
-static int
+int
 vhost_kernel_tap_set_offload(int fd, uint64_t features)
 {
        unsigned int offload = 0;
@@ -37,26 +37,34 @@ 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;
                }
        }
 
        return 0;
 }
 
+int
+vhost_kernel_tap_set_queue(int fd, bool attach)
+{
+       struct ifreq ifr = {
+               .ifr_flags = attach ? IFF_ATTACH_QUEUE : IFF_DETACH_QUEUE,
+       };
+
+       return ioctl(fd, TUNSETQUEUE, &ifr);
+}
+
 int
 vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
                         const char *mac, uint64_t features)

diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
index e0e95b4..5c4447b 100644 (file)
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
@@ -2,6 +2,10 @@
  * Copyright(c) 2016 Intel Corporation
  */
 
+#ifndef _VHOST_KERNEL_TAP_H
+#define _VHOST_KERNEL_TAP_H
+
+#include <stdbool.h>
 #include <sys/ioctl.h>
 
 /* TUN ioctls */
@@ -37,3 +41,7 @@
 
 int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
                         const char *mac, uint64_t features);
+int vhost_kernel_tap_set_offload(int fd, uint64_t features);
+int vhost_kernel_tap_set_queue(int fd, bool attach);
+
+#endif

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index a4b5c25..d8e083b 100644 (file)
--- 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 ffbaa75..1c6b26f 100644 (file)
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -546,8 +546,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 ad86837..3b6b606 100644 (file)
--- 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;