vhost: introduce API to start a specific driver
authorYuanhan Liu <yuanhan.liu@linux.intel.com>
Sat, 1 Apr 2017 07:22:56 +0000 (15:22 +0800)
committerYuanhan Liu <yuanhan.liu@linux.intel.com>
Sat, 1 Apr 2017 08:42:44 +0000 (10:42 +0200)
We used to use rte_vhost_driver_session_start() to trigger the vhost-user
session. It takes no argument, thus it's a global trigger. And it could
be problematic.

The issue is, currently, rte_vhost_driver_register(path, flags) actually
tries to put it into the session loop (by fdset_add). However, it needs
a set of APIs to set a vhost-user driver properly:
  * rte_vhost_driver_register(path, flags);
  * rte_vhost_driver_set_features(path, features);
  * rte_vhost_driver_callback_register(path, vhost_device_ops);

If a new vhost-user driver is registered after the trigger (think OVS-DPDK
that could add a port dynamically from cmdline), the current code will
effectively starts the session for the new driver just after the first
API rte_vhost_driver_register() is invoked, leaving later calls taking
no effect at all.

To handle the case properly, this patch introduce a new API,
rte_vhost_driver_start(path), to trigger a specific vhost-user driver.
To do that, the rte_vhost_driver_register(path, flags) is simplified
to create the socket only and let rte_vhost_driver_start(path) to
actually put it into the session loop.

Meanwhile, the rte_vhost_driver_session_start is removed: we could hide
the session thread internally (create the thread if it has not been
created). This would also simplify the application.

NOTE: the API order in prog guide is slightly adjusted for showing the
correct invoke order.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
doc/guides/prog_guide/vhost_lib.rst
doc/guides/rel_notes/release_17_05.rst
drivers/net/vhost/rte_eth_vhost.c
examples/tep_termination/main.c
examples/vhost/main.c
lib/librte_vhost/fd_man.c
lib/librte_vhost/fd_man.h
lib/librte_vhost/rte_vhost_version.map
lib/librte_vhost/rte_virtio_net.h
lib/librte_vhost/socket.c

index a4fb1f1..5979290 100644 (file)
@@ -116,12 +116,6 @@ The following is an overview of some key Vhost API functions:
   vhost-user driver could be vhost-user net, yet it could be something else,
   say, vhost-user SCSI.
 
-* ``rte_vhost_driver_session_start()``
-
-  This function starts the vhost session loop to handle vhost messages. It
-  starts an infinite loop, therefore it should be called in a dedicated
-  thread.
-
 * ``rte_vhost_driver_callback_register(path, vhost_device_ops)``
 
   This function registers a set of callbacks, to let DPDK applications take
@@ -149,6 +143,17 @@ The following is an overview of some key Vhost API functions:
     ``VHOST_F_LOG_ALL`` will be set/cleared at the start/end of live
     migration, respectively.
 
+* ``rte_vhost_driver_disable/enable_features(path, features))``
+
+  This function disables/enables some features. For example, it can be used to
+  disable mergeable buffers and TSO features, which both are enabled by
+  default.
+
+* ``rte_vhost_driver_start(path)``
+
+  This function triggers the vhost-user negotiation. It should be invoked at
+  the end of initializing a vhost-user driver.
+
 * ``rte_vhost_enqueue_burst(vid, queue_id, pkts, count)``
 
   Transmits (enqueues) ``count`` packets from host to guest.
@@ -157,13 +162,6 @@ The following is an overview of some key Vhost API functions:
 
   Receives (dequeues) ``count`` packets from guest, and stored them at ``pkts``.
 
-* ``rte_vhost_driver_disable/enable_features(path, features))``
-
-  This function disables/enables some features. For example, it can be used to
-  disable mergeable buffers and TSO features, which both are enabled by
-  default.
-
-
 Vhost-user Implementations
 --------------------------
 
index 7f89b1c..0373def 100644 (file)
@@ -315,6 +315,10 @@ API Changes
   * The vhost struct ``virtio_net_device_ops`` is renamed to
     ``vhost_device_ops``
 
+  * The vhost API ``rte_vhost_driver_session_start`` is removed. Instead,
+    ``rte_vhost_driver_start`` should be used, and no need to create a
+    thread to call it.
+
 
 ABI Changes
 -----------
index 34c241d..65c5ef2 100644 (file)
@@ -128,9 +128,6 @@ static struct internal_list_head internal_list =
 
 static pthread_mutex_t internal_list_lock = PTHREAD_MUTEX_INITIALIZER;
 
-static rte_atomic16_t nb_started_ports;
-static pthread_t session_th;
-
 static struct rte_eth_link pmd_link = {
                .link_speed = 10000,
                .link_duplex = ETH_LINK_FULL_DUPLEX,
@@ -769,42 +766,6 @@ rte_eth_vhost_get_vid_from_port_id(uint8_t port_id)
        return vid;
 }
 
-static void *
-vhost_driver_session(void *param __rte_unused)
-{
-       /* start event handling */
-       rte_vhost_driver_session_start();
-
-       return NULL;
-}
-
-static int
-vhost_driver_session_start(void)
-{
-       int ret;
-
-       ret = pthread_create(&session_th,
-                       NULL, vhost_driver_session, NULL);
-       if (ret)
-               RTE_LOG(ERR, PMD, "Can't create a thread\n");
-
-       return ret;
-}
-
-static void
-vhost_driver_session_stop(void)
-{
-       int ret;
-
-       ret = pthread_cancel(session_th);
-       if (ret)
-               RTE_LOG(ERR, PMD, "Can't cancel the thread\n");
-
-       ret = pthread_join(session_th, NULL);
-       if (ret)
-               RTE_LOG(ERR, PMD, "Can't join the thread\n");
-}
-
 static int
 eth_dev_start(struct rte_eth_dev *dev)
 {
@@ -1120,10 +1081,10 @@ eth_dev_vhost_create(const char *name, char *iface_name, int16_t queues,
                goto error;
        }
 
-       /* We need only one message handling thread */
-       if (rte_atomic16_add_return(&nb_started_ports, 1) == 1) {
-               if (vhost_driver_session_start())
-                       goto error;
+       if (rte_vhost_driver_start(iface_name) < 0) {
+               RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
+                       iface_name);
+               goto error;
        }
 
        return data->port_id;
@@ -1250,9 +1211,6 @@ rte_pmd_vhost_remove(const char *name)
 
        eth_dev_close(eth_dev);
 
-       if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
-               vhost_driver_session_stop();
-
        rte_free(vring_states[eth_dev->data->port_id]);
        vring_states[eth_dev->data->port_id] = NULL;
 
index 738f2d2..24c62cd 100644 (file)
@@ -1263,7 +1263,13 @@ main(int argc, char *argv[])
                        "failed to register vhost driver callbacks.\n");
        }
 
-       rte_vhost_driver_session_start();
+       if (rte_vhost_driver_start(dev_basename) < 0) {
+               rte_exit(EXIT_FAILURE,
+                       "failed to start vhost driver.\n");
+       }
+
+       RTE_LCORE_FOREACH_SLAVE(lcore_id)
+               rte_eal_wait_lcore(lcore_id);
 
        return 0;
 }
index 4395306..64b3eea 100644 (file)
@@ -1545,9 +1545,16 @@ main(int argc, char *argv[])
                        rte_exit(EXIT_FAILURE,
                                "failed to register vhost driver callbacks.\n");
                }
+
+               if (rte_vhost_driver_start(file) < 0) {
+                       rte_exit(EXIT_FAILURE,
+                               "failed to start vhost driver.\n");
+               }
        }
 
-       rte_vhost_driver_session_start();
+       RTE_LCORE_FOREACH_SLAVE(lcore_id)
+               rte_eal_wait_lcore(lcore_id);
+
        return 0;
 
 }
index c7a4490..2ceacc9 100644 (file)
@@ -210,8 +210,8 @@ fdset_del(struct fdset *pfdset, int fd)
  * will wait until the flag is reset to zero(which indicates the callback is
  * finished), then it could free the context after fdset_del.
  */
-void
-fdset_event_dispatch(struct fdset *pfdset)
+void *
+fdset_event_dispatch(void *arg)
 {
        int i;
        struct pollfd *pfd;
@@ -221,9 +221,10 @@ fdset_event_dispatch(struct fdset *pfdset)
        int fd, numfds;
        int remove1, remove2;
        int need_shrink;
+       struct fdset *pfdset = arg;
 
        if (pfdset == NULL)
-               return;
+               return NULL;
 
        while (1) {
 
@@ -294,4 +295,6 @@ fdset_event_dispatch(struct fdset *pfdset)
                if (need_shrink)
                        fdset_shrink(pfdset);
        }
+
+       return NULL;
 }
index d319cac..90d34db 100644 (file)
@@ -64,6 +64,6 @@ int fdset_add(struct fdset *pfdset, int fd,
 
 void *fdset_del(struct fdset *pfdset, int fd);
 
-void fdset_event_dispatch(struct fdset *pfdset);
+void *fdset_event_dispatch(void *arg);
 
 #endif
index f4b74da..0785873 100644 (file)
@@ -4,7 +4,6 @@ DPDK_2.0 {
        rte_vhost_dequeue_burst;
        rte_vhost_driver_callback_register;
        rte_vhost_driver_register;
-       rte_vhost_driver_session_start;
        rte_vhost_enable_guest_notification;
        rte_vhost_enqueue_burst;
 
@@ -35,6 +34,7 @@ DPDK_17.05 {
        rte_vhost_driver_enable_features;
        rte_vhost_driver_get_features;
        rte_vhost_driver_set_features;
+       rte_vhost_driver_start;
        rte_vhost_get_mem_table;
        rte_vhost_get_mtu;
        rte_vhost_get_negotiated_features;
index 1fde822..fd9cbb0 100644 (file)
@@ -256,8 +256,19 @@ int rte_vhost_get_negotiated_features(int vid, uint64_t *features);
 /* Register callbacks. */
 int rte_vhost_driver_callback_register(const char *path,
        struct vhost_device_ops const * const ops);
-/* Start vhost driver session blocking loop. */
-int rte_vhost_driver_session_start(void);
+
+/**
+ *
+ * Start the vhost-user driver.
+ *
+ * This function triggers the vhost-user negotiation.
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_driver_start(const char *path);
 
 /**
  * Get the MTU value of the device if set in QEMU.
index 3b68fc9..66fd335 100644 (file)
@@ -63,7 +63,8 @@ struct vhost_user_socket {
        struct vhost_user_connection_list conn_list;
        pthread_mutex_t conn_mutex;
        char *path;
-       int listenfd;
+       int socket_fd;
+       struct sockaddr_un un;
        bool is_server;
        bool reconnect;
        bool dequeue_zero_copy;
@@ -101,7 +102,8 @@ struct vhost_user {
 
 static void vhost_user_server_new_connection(int fd, void *data, int *remove);
 static void vhost_user_read_cb(int fd, void *dat, int *remove);
-static int vhost_user_create_client(struct vhost_user_socket *vsocket);
+static int create_unix_socket(struct vhost_user_socket *vsocket);
+static int vhost_user_start_client(struct vhost_user_socket *vsocket);
 
 static struct vhost_user vhost_user = {
        .fdset = {
@@ -280,23 +282,26 @@ vhost_user_read_cb(int connfd, void *dat, int *remove)
 
                free(conn);
 
-               if (vsocket->reconnect)
-                       vhost_user_create_client(vsocket);
+               if (vsocket->reconnect) {
+                       create_unix_socket(vsocket);
+                       vhost_user_start_client(vsocket);
+               }
        }
 }
 
 static int
-create_unix_socket(const char *path, struct sockaddr_un *un, bool is_server)
+create_unix_socket(struct vhost_user_socket *vsocket)
 {
        int fd;
+       struct sockaddr_un *un = &vsocket->un;
 
        fd = socket(AF_UNIX, SOCK_STREAM, 0);
        if (fd < 0)
                return -1;
        RTE_LOG(INFO, VHOST_CONFIG, "vhost-user %s: socket created, fd: %d\n",
-               is_server ? "server" : "client", fd);
+               vsocket->is_server ? "server" : "client", fd);
 
-       if (!is_server && fcntl(fd, F_SETFL, O_NONBLOCK)) {
+       if (!vsocket->is_server && fcntl(fd, F_SETFL, O_NONBLOCK)) {
                RTE_LOG(ERR, VHOST_CONFIG,
                        "vhost-user: can't set nonblocking mode for socket, fd: "
                        "%d (%s)\n", fd, strerror(errno));
@@ -306,25 +311,21 @@ create_unix_socket(const char *path, struct sockaddr_un *un, bool is_server)
 
        memset(un, 0, sizeof(*un));
        un->sun_family = AF_UNIX;
-       strncpy(un->sun_path, path, sizeof(un->sun_path));
+       strncpy(un->sun_path, vsocket->path, sizeof(un->sun_path));
        un->sun_path[sizeof(un->sun_path) - 1] = '\0';
 
-       return fd;
+       vsocket->socket_fd = fd;
+       return 0;
 }
 
 static int
-vhost_user_create_server(struct vhost_user_socket *vsocket)
+vhost_user_start_server(struct vhost_user_socket *vsocket)
 {
-       int fd;
        int ret;
-       struct sockaddr_un un;
+       int fd = vsocket->socket_fd;
        const char *path = vsocket->path;
 
-       fd = create_unix_socket(path, &un, vsocket->is_server);
-       if (fd < 0)
-               return -1;
-
-       ret = bind(fd, (struct sockaddr *)&un, sizeof(un));
+       ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket->un));
        if (ret < 0) {
                RTE_LOG(ERR, VHOST_CONFIG,
                        "failed to bind to %s: %s; remove it and try again\n",
@@ -337,7 +338,6 @@ vhost_user_create_server(struct vhost_user_socket *vsocket)
        if (ret < 0)
                goto err;
 
-       vsocket->listenfd = fd;
        ret = fdset_add(&vhost_user.fdset, fd, vhost_user_server_new_connection,
                  NULL, vsocket);
        if (ret < 0) {
@@ -456,20 +456,15 @@ vhost_user_reconnect_init(void)
 }
 
 static int
-vhost_user_create_client(struct vhost_user_socket *vsocket)
+vhost_user_start_client(struct vhost_user_socket *vsocket)
 {
-       int fd;
        int ret;
-       struct sockaddr_un un;
+       int fd = vsocket->socket_fd;
        const char *path = vsocket->path;
        struct vhost_user_reconnect *reconn;
 
-       fd = create_unix_socket(path, &un, vsocket->is_server);
-       if (fd < 0)
-               return -1;
-
-       ret = vhost_user_connect_nonblock(fd, (struct sockaddr *)&un,
-                                         sizeof(un));
+       ret = vhost_user_connect_nonblock(fd, (struct sockaddr *)&vsocket->un,
+                                         sizeof(vsocket->un));
        if (ret == 0) {
                vhost_user_add_connection(fd, vsocket);
                return 0;
@@ -492,7 +487,7 @@ vhost_user_create_client(struct vhost_user_socket *vsocket)
                close(fd);
                return -1;
        }
-       reconn->un = un;
+       reconn->un = vsocket->un;
        reconn->fd = fd;
        reconn->vsocket = vsocket;
        pthread_mutex_lock(&reconn_list.mutex);
@@ -645,11 +640,10 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
                                goto out;
                        }
                }
-               ret = vhost_user_create_client(vsocket);
        } else {
                vsocket->is_server = true;
-               ret = vhost_user_create_server(vsocket);
        }
+       ret = create_unix_socket(vsocket);
        if (ret < 0) {
                free(vsocket->path);
                free(vsocket);
@@ -705,8 +699,8 @@ rte_vhost_driver_unregister(const char *path)
 
                if (!strcmp(vsocket->path, path)) {
                        if (vsocket->is_server) {
-                               fdset_del(&vhost_user.fdset, vsocket->listenfd);
-                               close(vsocket->listenfd);
+                               fdset_del(&vhost_user.fdset, vsocket->socket_fd);
+                               close(vsocket->socket_fd);
                                unlink(path);
                        } else if (vsocket->reconnect) {
                                vhost_user_remove_reconnect(vsocket);
@@ -776,8 +770,28 @@ vhost_driver_callback_get(const char *path)
 }
 
 int
-rte_vhost_driver_session_start(void)
+rte_vhost_driver_start(const char *path)
 {
-       fdset_event_dispatch(&vhost_user.fdset);
-       return 0;
+       struct vhost_user_socket *vsocket;
+       static pthread_t fdset_tid;
+
+       pthread_mutex_lock(&vhost_user.mutex);
+       vsocket = find_vhost_user_socket(path);
+       pthread_mutex_unlock(&vhost_user.mutex);
+
+       if (!vsocket)
+               return -1;
+
+       if (fdset_tid == 0) {
+               int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
+                                    &vhost_user.fdset);
+               if (ret < 0)
+                       RTE_LOG(ERR, VHOST_CONFIG,
+                               "failed to create fdset handling thread");
+       }
+
+       if (vsocket->is_server)
+               return vhost_user_start_server(vsocket);
+       else
+               return vhost_user_start_client(vsocket);
 }