From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 37F81CFD6 for ; Tue, 28 Mar 2017 14:48:29 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 28 Mar 2017 05:48:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,236,1486454400"; d="scan'208";a="65961058" Received: from yliu-dev.sh.intel.com ([10.239.67.162]) by orsmga002.jf.intel.com with ESMTP; 28 Mar 2017 05:48:27 -0700 From: Yuanhan Liu To: dev@dpdk.org Cc: Maxime Coquelin , Harris James R , Liu Changpeng , Yuanhan Liu Date: Tue, 28 Mar 2017 20:45:38 +0800 Message-Id: <1490705142-893-19-git-send-email-yuanhan.liu@linux.intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1490705142-893-1-git-send-email-yuanhan.liu@linux.intel.com> References: <1490253059-28112-1-git-send-email-yuanhan.liu@linux.intel.com> <1490705142-893-1-git-send-email-yuanhan.liu@linux.intel.com> Subject: [dpdk-dev] [PATCH v3 18/22] vhost: introduce API to start a specific driver 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: , X-List-Received-Date: Tue, 28 Mar 2017 12:48:30 -0000 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 --- v3: - fix broken reconnect --- doc/guides/prog_guide/vhost_lib.rst | 24 +++++----- doc/guides/rel_notes/release_17_05.rst | 8 ++++ drivers/net/vhost/rte_eth_vhost.c | 50 ++------------------ examples/tep_termination/main.c | 8 +++- examples/vhost/main.c | 9 +++- lib/librte_vhost/fd_man.c | 9 ++-- lib/librte_vhost/fd_man.h | 2 +- lib/librte_vhost/rte_vhost_version.map | 2 +- lib/librte_vhost/rte_virtio_net.h | 15 +++++- lib/librte_vhost/socket.c | 84 ++++++++++++++++++++-------------- 10 files changed, 108 insertions(+), 103 deletions(-) diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst index a4fb1f1..5979290 100644 --- a/doc/guides/prog_guide/vhost_lib.rst +++ b/doc/guides/prog_guide/vhost_lib.rst @@ -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 -------------------------- diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst index 2efe292..8f06fc4 100644 --- a/doc/guides/rel_notes/release_17_05.rst +++ b/doc/guides/rel_notes/release_17_05.rst @@ -57,6 +57,11 @@ New Features * Enable Vhost PMD's MTU get feature. * Get max MTU value from host in Virtio PMD +* **Made the vhost lib be a generic vhost-user lib.** + + Now it could be used to implement any other vhost-user drivers, such + as, vhost-user SCSI. + Resolved Issues --------------- @@ -157,6 +162,9 @@ 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. + ABI Changes ----------- diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c index 97a765f..e6c0758 100644 --- a/drivers/net/vhost/rte_eth_vhost.c +++ b/drivers/net/vhost/rte_eth_vhost.c @@ -127,9 +127,6 @@ struct 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, @@ -743,42 +740,6 @@ struct vhost_xstats_name_off { 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) { @@ -1083,10 +1044,10 @@ struct vhost_xstats_name_off { 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; @@ -1213,9 +1174,6 @@ struct vhost_xstats_name_off { 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; diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c index 738f2d2..24c62cd 100644 --- a/examples/tep_termination/main.c +++ b/examples/tep_termination/main.c @@ -1263,7 +1263,13 @@ static inline void __attribute__((always_inline)) "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; } diff --git a/examples/vhost/main.c b/examples/vhost/main.c index 4395306..64b3eea 100644 --- a/examples/vhost/main.c +++ b/examples/vhost/main.c @@ -1545,9 +1545,16 @@ static inline void __attribute__((always_inline)) 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; } diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c index c7a4490..2ceacc9 100644 --- a/lib/librte_vhost/fd_man.c +++ b/lib/librte_vhost/fd_man.c @@ -210,8 +210,8 @@ * 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 @@ 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 @@ if (need_shrink) fdset_shrink(pfdset); } + + return NULL; } diff --git a/lib/librte_vhost/fd_man.h b/lib/librte_vhost/fd_man.h index d319cac..90d34db 100644 --- a/lib/librte_vhost/fd_man.h +++ b/lib/librte_vhost/fd_man.h @@ -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 diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map index f4b74da..0785873 100644 --- a/lib/librte_vhost/rte_vhost_version.map +++ b/lib/librte_vhost/rte_vhost_version.map @@ -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; diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h index 11b204d..627708d 100644 --- a/lib/librte_vhost/rte_virtio_net.h +++ b/lib/librte_vhost/rte_virtio_net.h @@ -250,8 +250,19 @@ void rte_vhost_log_used_vring(int vid, uint16_t vring_idx, /* 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. diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 9fb28b5..eb2aedd 100644 --- a/lib/librte_vhost/socket.c +++ b/lib/librte_vhost/socket.c @@ -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 @@ struct vhost_user { 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 @@ struct vhost_user { 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 @@ struct vhost_user { 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 @@ struct vhost_user_reconnect_list { } 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 @@ struct vhost_user_reconnect_list { close(fd); return -1; } - reconn->un = un; + reconn->un = vsocket->un; reconn->fd = fd; reconn->vsocket = vsocket; pthread_mutex_lock(&reconn_list.mutex); @@ -643,11 +638,10 @@ struct vhost_user_reconnect_list { 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); @@ -703,8 +697,8 @@ struct vhost_user_reconnect_list { 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); @@ -774,8 +768,28 @@ struct vhost_device_ops const * } 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); } -- 1.9.0