From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 2324E5A50 for ; Wed, 3 Jun 2015 21:03:53 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 03 Jun 2015 12:03:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,548,1427785200"; d="scan'208";a="502273136" Received: from pgsmsx105.gar.corp.intel.com ([10.221.44.96]) by FMSMGA003.fm.intel.com with ESMTP; 03 Jun 2015 12:03:52 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by PGSMSX105.gar.corp.intel.com (10.221.44.96) with Microsoft SMTP Server (TLS) id 14.3.224.2; Thu, 4 Jun 2015 03:03:51 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.120]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.50]) with mapi id 14.03.0224.002; Thu, 4 Jun 2015 03:03:49 +0800 From: "Xie, Huawei" To: "Ouyang, Changchun" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Thread-Index: AdCeMAYymQKRHtSGQ4ma9MgRSHf2mA== Date: Wed, 3 Jun 2015 19:03:49 +0000 Message-ID: References: <1433209800-29091-1-git-send-email-huawei.xie@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "Sun, Peng A" Subject: Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket 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: Wed, 03 Jun 2015 19:03:54 -0000 On 6/3/2015 8:55 PM, Ouyang, Changchun wrote:=0A= >=0A= >> -----Original Message-----=0A= >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie=0A= >> Sent: Tuesday, June 2, 2015 9:50 AM=0A= >> To: dev@dpdk.org=0A= >> Cc: Sun, Peng A=0A= >> Subject: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost= =0A= >> unix domain socket=0A= >>=0A= >> rte_vhost_driver_unregister will remove the listenfd from event list, an= d=0A= >> then close it.=0A= >>=0A= >> Signed-off-by: Huawei Xie =0A= >> Signed-off-by: Peng Sun =0A= >> ---=0A= >> lib/librte_vhost/rte_virtio_net.h | 3 ++=0A= >> lib/librte_vhost/vhost_cuse/vhost-net-cdev.c | 9 ++++=0A= >> lib/librte_vhost/vhost_user/vhost-net-user.c | 70=0A= >> +++++++++++++++++++++++----- lib/librte_vhost/vhost_user/vhost-net-=0A= >> user.h | 2 +-=0A= >> 4 files changed, 71 insertions(+), 13 deletions(-)=0A= >>=0A= >> diff --git a/lib/librte_vhost/rte_virtio_net.h=0A= >> b/lib/librte_vhost/rte_virtio_net.h=0A= >> index 5d38185..5630fbc 100644=0A= >> --- a/lib/librte_vhost/rte_virtio_net.h=0A= >> +++ b/lib/librte_vhost/rte_virtio_net.h=0A= >> @@ -188,6 +188,9 @@ int rte_vhost_enable_guest_notification(struct=0A= >> virtio_net *dev, uint16_t queue_i=0A= >> /* Register vhost driver. dev_name could be different for multiple inst= ance=0A= >> support. */ int rte_vhost_driver_register(const char *dev_name);=0A= >>=0A= >> +/* Unregister vhost driver. This is only meaningful to vhost user. */= =0A= >> +int rte_vhost_driver_unregister(const char *dev_name);=0A= >> +=0A= >> /* Register callbacks. */=0A= >> int rte_vhost_driver_callback_register(struct virtio_net_device_ops con= st *=0A= >> const);=0A= >> /* Start vhost driver session blocking loop. */ diff --git=0A= >> a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c=0A= >> b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c=0A= >> index 6b68abf..1ae7c49 100644=0A= >> --- a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c=0A= >> +++ b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c=0A= >> @@ -405,6 +405,15 @@ rte_vhost_driver_register(const char *dev_name) }= =0A= >>=0A= >> /**=0A= >> + * An empty function for unregister=0A= >> + */=0A= >> +int=0A= >> +rte_vhost_driver_unregister(const char *dev_name __rte_unused) {=0A= >> + return 0;=0A= >> +}=0A= >> +=0A= >> +/**=0A= >> * The CUSE session is launched allowing the application to receive ope= n,=0A= >> * release and ioctl calls.=0A= >> */=0A= >> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c=0A= >> b/lib/librte_vhost/vhost_user/vhost-net-user.c=0A= >> index 31f1215..dff46ee 100644=0A= >> --- a/lib/librte_vhost/vhost_user/vhost-net-user.c=0A= >> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c=0A= >> @@ -66,6 +66,8 @@ struct connfd_ctx {=0A= >> struct _vhost_server {=0A= >> struct vhost_server *server[MAX_VHOST_SERVER];=0A= >> struct fdset fdset;=0A= >> + int vserver_cnt;=0A= >> + pthread_mutex_t server_mutex;=0A= >> };=0A= >>=0A= >> static struct _vhost_server g_vhost_server =3D { @@ -74,10 +76,10 @@ st= atic=0A= >> struct _vhost_server g_vhost_server =3D {=0A= >> .fd_mutex =3D PTHREAD_MUTEX_INITIALIZER,=0A= >> .num =3D 0=0A= >> },=0A= >> + .vserver_cnt =3D 0,=0A= >> + .server_mutex =3D PTHREAD_MUTEX_INITIALIZER,=0A= >> };=0A= >>=0A= >> -static int vserver_idx;=0A= >> -=0A= >> static const char *vhost_message_str[VHOST_USER_MAX] =3D {=0A= >> [VHOST_USER_NONE] =3D "VHOST_USER_NONE",=0A= >> [VHOST_USER_GET_FEATURES] =3D "VHOST_USER_GET_FEATURES",=0A= >> @@ -427,7 +429,6 @@ vserver_message_handler(int connfd, void *dat, int= =0A= >> *remove)=0A= >> }=0A= >> }=0A= >>=0A= >> -=0A= >> /**=0A= >> * Creates and initialise the vhost server.=0A= >> */=0A= >> @@ -436,34 +437,79 @@ rte_vhost_driver_register(const char *path) {=0A= >> struct vhost_server *vserver;=0A= >>=0A= >> - if (vserver_idx =3D=3D 0)=0A= >> + pthread_mutex_lock(&g_vhost_server.server_mutex);=0A= >> + if (ops =3D=3D NULL)=0A= >> ops =3D get_virtio_net_callbacks();=0A= >> - if (vserver_idx =3D=3D MAX_VHOST_SERVER)=0A= >> +=0A= >> + if (g_vhost_server.vserver_cnt =3D=3D MAX_VHOST_SERVER) {=0A= >> + RTE_LOG(ERR, VHOST_CONFIG,=0A= >> + "error: the number of servers reaches maximum\n");=0A= >> + pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >> return -1;=0A= >> + }=0A= >>=0A= >> vserver =3D calloc(sizeof(struct vhost_server), 1);=0A= >> - if (vserver =3D=3D NULL)=0A= >> + if (vserver =3D=3D NULL) {=0A= >> + pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >> return -1;=0A= >> -=0A= >> - unlink(path);=0A= >> + }=0A= >>=0A= >> vserver->listenfd =3D uds_socket(path);=0A= >> if (vserver->listenfd < 0) {=0A= >> free(vserver);=0A= >> + pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >> return -1;=0A= >> }=0A= >> - vserver->path =3D path;=0A= >> +=0A= >> + vserver->path =3D strdup(path);=0A= > Do we need check if there is existing server which has same path with th= e new one?=0A= =0A= This is considered. If there is existing server, binding will fail.=0A= >=0A= >> fdset_add(&g_vhost_server.fdset, vserver->listenfd,=0A= >> - vserver_new_vq_conn, NULL,=0A= >> - vserver);=0A= >> + vserver_new_vq_conn, NULL, vserver);=0A= >>=0A= >> - g_vhost_server.server[vserver_idx++] =3D vserver;=0A= >> + g_vhost_server.server[g_vhost_server.vserver_cnt++] =3D vserver;=0A= >> + pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >>=0A= >> return 0;=0A= >> }=0A= >>=0A= >>=0A= >> +/**=0A= >> + * Unregister the specified vhost server */ int=0A= >> +rte_vhost_driver_unregister(const char *path) {=0A= >> + int i;=0A= >> + int count;=0A= >> +=0A= >> + pthread_mutex_lock(&g_vhost_server.server_mutex);=0A= >> +=0A= >> + for (i =3D 0; i < g_vhost_server.vserver_cnt; i++) {=0A= >> + if (!strcmp(g_vhost_server.server[i]->path, path)) {=0A= >> + fdset_del(&g_vhost_server.fdset,=0A= >> + g_vhost_server.server[i]->listenfd);=0A= >> +=0A= >> + close(g_vhost_server.server[i]->listenfd);=0A= > Besides listenfd, do we need check other fd's status, all fds should be c= losed before driver unregistered. =0A= No, we don't need. Why do we check other fds? Here we are to unregister =0A= the listenfd.=0A= >=0A= >> + free(g_vhost_server.server[i]->path);=0A= >> + free(g_vhost_server.server[i]);=0A= >> +=0A= >> + unlink(path);=0A= >> +=0A= >> + count =3D --g_vhost_server.vserver_cnt;=0A= >> + g_vhost_server.server[i] =3D=0A= >> + g_vhost_server.server[count];=0A= > it is better to remove the unnecessary new line for easy to read, so does= the next 2 lines=0A= Thanks.=0A= >> + g_vhost_server.server[count] =3D=0A= >> + NULL;=0A= >> +=0A= >> pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >> +=0A= >> + return 0;=0A= >> + }=0A= >> + }=0A= >> + pthread_mutex_unlock(&g_vhost_server.server_mutex);=0A= >> +=0A= >> + return -1;=0A= >> +}=0A= > Do we have test case cover this new function?=0A= >=0A= >> +=0A= >> int=0A= >> rte_vhost_driver_session_start(void)=0A= >> {=0A= >> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h=0A= >> b/lib/librte_vhost/vhost_user/vhost-net-user.h=0A= >> index 1b6be6c..2e72f3c 100644=0A= >> --- a/lib/librte_vhost/vhost_user/vhost-net-user.h=0A= >> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.h=0A= >> @@ -41,7 +41,7 @@=0A= >> #include "fd_man.h"=0A= >>=0A= >> struct vhost_server {=0A= >> - const char *path; /**< The path the uds is bind to. */=0A= >> + char *path; /**< The path the uds is bind to. */=0A= >> int listenfd; /**< The listener sockfd. */=0A= >> };=0A= >>=0A= >> --=0A= >> 1.8.1.4=0A= >=0A= >=0A= =0A=