From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 3C4147F00 for ; Mon, 9 May 2016 22:30:57 +0200 (CEST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD6EF8E23E; Mon, 9 May 2016 20:30:56 +0000 (UTC) Received: from redhat.com (ovpn-116-53.ams2.redhat.com [10.36.116.53]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id u49KUsEL005456; Mon, 9 May 2016 16:30:55 -0400 Date: Mon, 9 May 2016 23:30:53 +0300 From: "Michael S. Tsirkin" To: Yuanhan Liu Cc: Victor Kaplansky , dev@dpdk.org, huawei xie Message-ID: <20160509232954-mutt-send-email-mst@redhat.com> References: <1462603224-29510-1-git-send-email-yuanhan.liu@linux.intel.com> <1462603224-29510-3-git-send-email-yuanhan.liu@linux.intel.com> <182147492.28014082.1462790025700.JavaMail.zimbra@redhat.com> <20160509203308.GI5641@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160509203308.GI5641@yliu-dev.sh.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 09 May 2016 20:30:56 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 2/6] vhost: add vhost-user client mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 May 2016 20:30:57 -0000 On Mon, May 09, 2016 at 01:33:08PM -0700, Yuanhan Liu wrote: > On Mon, May 09, 2016 at 06:33:45AM -0400, Victor Kaplansky wrote: > > Adding a flag for a future extension seems fine to me. > > Could we manage without adding a flag? > > For example, could we always try a client mode for a while at first, > > and if unsuccessful, then come up with server mode? > > It's hard to define "how long is for a while". And I don't think > there is a way to switch it back to client mode from server mode > if you do so. > > So, assume you create a vhost-user port (as client), and you > start QEMU (as server) later, after the gap you mentioned of > auto-turn into server, it simply doesn't work then. > > --yliu I think I agree - I don't know of other software doing such an automatic switch. Correctly setting server/client mode seems easy enough. > > ----- Original Message ----- > > > From: "Yuanhan Liu" > > > To: dev@dpdk.org > > > Cc: "huawei xie" , "Yuanhan Liu" > > > Sent: Saturday, May 7, 2016 9:40:20 AM > > > Subject: [dpdk-dev] [PATCH 2/6] vhost: add vhost-user client mode > > > > > > Add a new paramter (flags) to rte_vhost_driver_register(). DPDK > > > vhost-user acts as client mode when RTE_VHOST_USER_CLIENT flag > > > is set. > > > > > > The flags would also allow future extensions without breaking the > > > API (again). > > > > > > The rest is straingfoward then: allocate a unix socket, and > > > bind/listen for server, connect for client. > > > > > > Signed-off-by: Yuanhan Liu > > > --- > > > drivers/net/vhost/rte_eth_vhost.c | 2 +- > > > examples/vhost/main.c | 2 +- > > > lib/librte_vhost/rte_virtio_net.h | 11 +- > > > lib/librte_vhost/vhost_user/vhost-net-user.c | 215 > > > ++++++++++++++++----------- > > > 4 files changed, 142 insertions(+), 88 deletions(-) > > > > > > diff --git a/drivers/net/vhost/rte_eth_vhost.c > > > b/drivers/net/vhost/rte_eth_vhost.c > > > index a9dada5..36697cf 100644 > > > --- a/drivers/net/vhost/rte_eth_vhost.c > > > +++ b/drivers/net/vhost/rte_eth_vhost.c > > > @@ -456,7 +456,7 @@ eth_dev_start(struct rte_eth_dev *dev) > > > int ret = 0; > > > > > > if (rte_atomic16_cmpset(&internal->once, 0, 1)) { > > > - ret = rte_vhost_driver_register(internal->iface_name); > > > + ret = rte_vhost_driver_register(internal->iface_name, 0); > > > if (ret) > > > return ret; > > > } > > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c > > > index bbf0d28..6899189 100644 > > > --- a/examples/vhost/main.c > > > +++ b/examples/vhost/main.c > > > @@ -1499,7 +1499,7 @@ main(int argc, char *argv[]) > > > rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_MRG_RXBUF); > > > > > > /* Register vhost(cuse or user) driver to handle vhost messages. */ > > > - ret = rte_vhost_driver_register((char *)&dev_basename); > > > + ret = rte_vhost_driver_register(dev_basename, 0); > > > if (ret != 0) > > > rte_exit(EXIT_FAILURE, "vhost driver register failure.\n"); > > > > > > diff --git a/lib/librte_vhost/rte_virtio_net.h > > > b/lib/librte_vhost/rte_virtio_net.h > > > index 4e50425..c84e7ab 100644 > > > --- a/lib/librte_vhost/rte_virtio_net.h > > > +++ b/lib/librte_vhost/rte_virtio_net.h > > > @@ -51,6 +51,8 @@ > > > #include > > > #include > > > > > > +#define RTE_VHOST_USER_CLIENT (1ULL << 0) > > > + > > > struct rte_mbuf; > > > > > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > > @@ -96,11 +98,14 @@ uint64_t rte_vhost_feature_get(void); > > > > > > int rte_vhost_enable_guest_notification(int vid, uint16_t queue_id, int > > > enable); > > > > > > -/* Register vhost driver. dev_name could be different for multiple instance > > > support. */ > > > -int rte_vhost_driver_register(const char *dev_name); > > > +/** > > > + * Register vhost driver. path could be different for multiple > > > + * instance support. > > > + */ > > > +int rte_vhost_driver_register(const char *path, uint64_t flags); > > > > > > /* Unregister vhost driver. This is only meaningful to vhost user. */ > > > -int rte_vhost_driver_unregister(const char *dev_name); > > > +int rte_vhost_driver_unregister(const char *path); > > > > > > /* Register callbacks. */ > > > int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * > > > const); > > > diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c > > > b/lib/librte_vhost/vhost_user/vhost-net-user.c > > > index f485a3b..aa98717 100644 > > > --- a/lib/librte_vhost/vhost_user/vhost-net-user.c > > > +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c > > > @@ -58,6 +58,7 @@ > > > struct vhost_user_socket { > > > char *path; > > > int listenfd; > > > + int is_server; > > > }; > > > > > > struct vhost_user_connection { > > > @@ -75,7 +76,7 @@ struct vhost_user { > > > > > > #define MAX_VIRTIO_BACKLOG 128 > > > > > > -static void vhost_user_new_connection(int fd, void *data, int *remove); > > > +static void vhost_user_server_new_connection(int fd, void *data, int > > > *remove); > > > static void vhost_user_msg_handler(int fd, void *dat, int *remove); > > > > > > static struct vhost_user vhost_user = { > > > @@ -111,48 +112,6 @@ static const char *vhost_message_str[VHOST_USER_MAX] = { > > > [VHOST_USER_SEND_RARP] = "VHOST_USER_SEND_RARP", > > > }; > > > > > > -/** > > > - * Create a unix domain socket, bind to path and listen for connection. > > > - * @return > > > - * socket fd or -1 on failure > > > - */ > > > -static int > > > -uds_socket(const char *path) > > > -{ > > > - struct sockaddr_un un; > > > - int sockfd; > > > - int ret; > > > - > > > - if (path == NULL) > > > - return -1; > > > - > > > - sockfd = socket(AF_UNIX, SOCK_STREAM, 0); > > > - if (sockfd < 0) > > > - return -1; > > > - RTE_LOG(INFO, VHOST_CONFIG, "socket created, fd:%d\n", sockfd); > > > - > > > - memset(&un, 0, sizeof(un)); > > > - un.sun_family = AF_UNIX; > > > - snprintf(un.sun_path, sizeof(un.sun_path), "%s", path); > > > - ret = bind(sockfd, (struct sockaddr *)&un, sizeof(un)); > > > - if (ret == -1) { > > > - RTE_LOG(ERR, VHOST_CONFIG, "fail to bind fd:%d, remove file:%s and try > > > again.\n", > > > - sockfd, path); > > > - goto err; > > > - } > > > - RTE_LOG(INFO, VHOST_CONFIG, "bind to %s\n", path); > > > - > > > - ret = listen(sockfd, MAX_VIRTIO_BACKLOG); > > > - if (ret == -1) > > > - goto err; > > > - > > > - return sockfd; > > > - > > > -err: > > > - close(sockfd); > > > - return -1; > > > -} > > > - > > > /* return bytes# of read on success or negative val on failure. */ > > > static int > > > read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num) > > > @@ -287,32 +246,24 @@ send_vhost_message(int sockfd, struct VhostUserMsg > > > *msg) > > > return ret; > > > } > > > > > > -/* call back when there is new vhost-user connection. */ > > > + > > > static void > > > -vhost_user_new_connection(int fd, void *dat, int *remove __rte_unused) > > > +vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) > > > { > > > - struct vhost_user_socket *vsocket = dat; > > > - int conn_fd; > > > - struct vhost_user_connection *conn; > > > int vid; > > > - unsigned int size; > > > - > > > - conn_fd = accept(fd, NULL, NULL); > > > - RTE_LOG(INFO, VHOST_CONFIG, > > > - "new virtio connection is %d\n", conn_fd); > > > - if (conn_fd < 0) > > > - return; > > > + size_t size; > > > + struct vhost_user_connection *conn; > > > > > > - conn = calloc(1, sizeof(*conn)); > > > + conn = malloc(sizeof(*conn)); > > > if (conn == NULL) { > > > - close(conn_fd); > > > + close(fd); > > > return; > > > } > > > > > > vid = vhost_new_device(); > > > if (vid == -1) { > > > + close(fd); > > > free(conn); > > > - close(conn_fd); > > > return; > > > } > > > > > > @@ -323,8 +274,21 @@ vhost_user_new_connection(int fd, void *dat, int *remove > > > __rte_unused) > > > > > > conn->vsocket = vsocket; > > > conn->vid = vid; > > > - fdset_add(&vhost_user.fdset, > > > - conn_fd, vhost_user_msg_handler, NULL, conn); > > > + fdset_add(&vhost_user.fdset, fd, vhost_user_msg_handler, NULL, conn); > > > +} > > > + > > > +/* call back when there is new vhost-user connection from client */ > > > +static void > > > +vhost_user_server_new_connection(int fd, void *dat, int *remove > > > __rte_unused) > > > +{ > > > + struct vhost_user_socket *vsocket = dat; > > > + > > > + fd = accept(fd, NULL, NULL); > > > + if (fd < 0) > > > + return; > > > + > > > + RTE_LOG(INFO, VHOST_CONFIG, "new vhost user connection is %d\n", fd); > > > + vhost_user_add_connection(fd, vsocket); > > > } > > > > > > /* callback when there is message on the connfd */ > > > @@ -452,50 +416,135 @@ vhost_user_msg_handler(int connfd, void *dat, int > > > *remove) > > > } > > > } > > > > > > -/** > > > - * Creates and initialise the vhost server. > > > - */ > > > -int > > > -rte_vhost_driver_register(const char *path) > > > +static int > > > +create_unix_socket(const char *path, struct sockaddr_un *un, int is_server) > > > { > > > - struct vhost_user_socket *vsocket; > > > + int fd; > > > > > > - pthread_mutex_lock(&vhost_user.mutex); > > > + 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); > > > > > > - if (vhost_user.vsocket_cnt == MAX_VHOST_SOCKET) { > > > - RTE_LOG(ERR, VHOST_CONFIG, > > > - "error: the number of servers reaches maximum\n"); > > > - pthread_mutex_unlock(&vhost_user.mutex); > > > + memset(un, 0, sizeof(*un)); > > > + un->sun_family = AF_UNIX; > > > + strncpy(un->sun_path, path, sizeof(un->sun_path)); > > > + > > > + return fd; > > > +} > > > + > > > +static int > > > +vhost_user_create_server(struct vhost_user_socket *vsocket) > > > +{ > > > + int fd; > > > + int ret; > > > + struct sockaddr_un un; > > > + 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)); > > > + if (ret < 0) { > > > + RTE_LOG(ERR, VHOST_CONFIG, > > > + "failed to bind to %s: %s; remove it and try again\n", > > > + path, strerror(errno)); > > > + goto err; > > > } > > > + RTE_LOG(INFO, VHOST_CONFIG, "bind to %s\n", path); > > > > > > - vsocket = calloc(sizeof(struct vhost_user_socket), 1); > > > - if (vsocket == NULL) { > > > - pthread_mutex_unlock(&vhost_user.mutex); > > > + ret = listen(fd, MAX_VIRTIO_BACKLOG); > > > + if (ret < 0) > > > + goto err; > > > + > > > + vsocket->listenfd = fd; > > > + fdset_add(&vhost_user.fdset, fd, vhost_user_server_new_connection, > > > + NULL, vsocket); > > > + > > > + return 0; > > > + > > > +err: > > > + close(fd); > > > + return -1; > > > +} > > > + > > > +static int > > > +vhost_user_create_client(struct vhost_user_socket *vsocket) > > > +{ > > > + int fd; > > > + int ret; > > > + struct sockaddr_un un; > > > + const char *path = vsocket->path; > > > + > > > + fd = create_unix_socket(path, &un, vsocket->is_server); > > > + if (fd < 0) > > > + return -1; > > > + > > > + ret = connect(fd, (struct sockaddr *)&un, sizeof(un)); > > > + if (ret < 0) { > > > + RTE_LOG(ERR, VHOST_CONFIG, "failed to connect to %s: %s\n", > > > + path, strerror(errno)); > > > + close(fd); > > > return -1; > > > } > > > > > > - vsocket->listenfd = uds_socket(path); > > > - if (vsocket->listenfd < 0) { > > > - free(vsocket); > > > - pthread_mutex_unlock(&vhost_user.mutex); > > > + vhost_user_add_connection(fd, vsocket); > > > + > > > + return 0; > > > +} > > > + > > > +/* > > > + * Register a new vhost-user socket; here we could act as server > > > + * (the default case), or client (when RTE_VHOST_USER_CLIENT) flag > > > + * is set. > > > + */ > > > +int > > > +rte_vhost_driver_register(const char *path, uint64_t flags) > > > +{ > > > + int ret = -1; > > > + struct vhost_user_socket *vsocket; > > > + > > > + if (!path) > > > return -1; > > > + > > > + pthread_mutex_lock(&vhost_user.mutex); > > > + > > > + if (vhost_user.vsocket_cnt == MAX_VHOST_SOCKET) { > > > + RTE_LOG(ERR, VHOST_CONFIG, > > > + "error: the number of vhost sockets reaches maximum\n"); > > > + goto out; > > > } > > > > > > + vsocket = malloc(sizeof(struct vhost_user_socket)); > > > + if (!vsocket) > > > + goto out; > > > + memset(vsocket, 0, sizeof(struct vhost_user_socket)); > > > vsocket->path = strdup(path); > > > > > > - fdset_add(&vhost_user.fdset, vsocket->listenfd, > > > - vhost_user_new_connection, NULL, vsocket); > > > + if ((flags & RTE_VHOST_USER_CLIENT) != 0) { > > > + ret = vhost_user_create_client(vsocket); > > > + } else { > > > + vsocket->is_server = 1; > > > + ret = vhost_user_create_server(vsocket); > > > + } > > > + if (ret < 0) { > > > + free(vsocket->path); > > > + free(vsocket); > > > + goto out; > > > + } > > > > > > vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket; > > > + > > > +out: > > > pthread_mutex_unlock(&vhost_user.mutex); > > > > > > - return 0; > > > + return ret; > > > } > > > > > > - > > > /** > > > - * Unregister the specified vhost server > > > + * Unregister the specified vhost socket > > > */ > > > int > > > rte_vhost_driver_unregister(const char *path) > > > -- > > > 1.9.0 > > > > > >