DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] Fix vhost startup issue
@ 2015-07-02  3:33 Ouyang Changchun
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-02  3:33 UTC (permalink / raw)
  To: dev

The commit breaks vhost sample when it runs in second time:
292959c71961acde0cda6e77e737bb0a4df1559c
 
It should call api to unregister vhost driver when sample exit/quit, then
the socket file will be removed(by calling unlink), and thus make vhost sample
work correctly in second time startup.

Also add/refine some log infomation.

Changchun Ouyang (3):
  vhost: add log if fails to bind a socket
  vhost: fix the comments and log
  vhost: call api to unregister vhost driver

 examples/vhost/main.c                        | 22 ++++++++++++++++++++--
 lib/librte_vhost/vhost_user/vhost-net-user.c |  5 ++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket
  2015-07-02  3:33 [dpdk-dev] [PATCH 0/3] Fix vhost startup issue Ouyang Changchun
@ 2015-07-02  3:33 ` Ouyang Changchun
  2015-07-02  9:29   ` Xie, Huawei
  2015-07-02  9:31   ` Xie, Huawei
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log Ouyang Changchun
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-02  3:33 UTC (permalink / raw)
  To: dev

It adds more readable log info if a socket fails to bind to local device file name.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 lib/librte_vhost/vhost_user/vhost-net-user.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index 87a4711..f406a94 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -122,8 +122,11 @@ uds_socket(const char *path)
 	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)
+	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);
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log
  2015-07-02  3:33 [dpdk-dev] [PATCH 0/3] Fix vhost startup issue Ouyang Changchun
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
@ 2015-07-02  3:33 ` Ouyang Changchun
  2015-07-02  9:25   ` Xie, Huawei
  2015-07-02  9:32   ` Xie, Huawei
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
  3 siblings, 2 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-02  3:33 UTC (permalink / raw)
  To: dev

It fixes the wrong log info when fails to unregister vhost driver.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 examples/vhost/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 7863dcf..72c4773 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -3051,10 +3051,10 @@ main(int argc, char *argv[])
 	if (mergeable == 0)
 		rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_MRG_RXBUF);
 
-	/* Register CUSE device to handle IOCTLs. */
+	/* Register vhost driver to handle IOCTLs. */
 	ret = rte_vhost_driver_register((char *)&dev_basename);
 	if (ret != 0)
-		rte_exit(EXIT_FAILURE,"CUSE device setup failure.\n");
+		rte_exit(EXIT_FAILURE,"vhost driver register failure.\n");
 
 	rte_vhost_driver_callback_register(&virtio_net_device_ops);
 
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver
  2015-07-02  3:33 [dpdk-dev] [PATCH 0/3] Fix vhost startup issue Ouyang Changchun
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log Ouyang Changchun
@ 2015-07-02  3:33 ` Ouyang Changchun
  2015-07-02  9:38   ` Xie, Huawei
  2015-07-02 16:04   ` Xie, Huawei
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
  3 siblings, 2 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-02  3:33 UTC (permalink / raw)
  To: dev

The commit will break vhost sample when it runs in second time:
292959c71961acde0cda6e77e737bb0a4df1559c

It should call api to unregister vhost driver when sample exit/quit, then
the socket file will be removed(by calling unlink), and thus make vhost sample
work correctly in second time startup.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 examples/vhost/main.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 72c4773..90666b3 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -2871,6 +2871,16 @@ setup_mempool_tbl(int socket, uint32_t index, char *pool_name,
 	}
 }
 
+/* When we receive a HUP signal, unregister vhost driver */
+static void
+sighup_handler(__rte_unused int signum)
+{
+	/* Unregister vhost driver. */
+	int ret = rte_vhost_driver_unregister((char *)&dev_basename);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
+	exit(0);
+}
 
 /*
  * Main function, does initialisation and calls the per-lcore functions. The CUSE
@@ -2887,6 +2897,8 @@ main(int argc, char *argv[])
 	uint16_t queue_id;
 	static pthread_t tid;
 
+	signal(SIGINT, sighup_handler);
+
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
@@ -3060,6 +3072,12 @@ main(int argc, char *argv[])
 
 	/* Start CUSE session. */
 	rte_vhost_driver_session_start();
+
+	/* Unregister vhost driver. */
+	ret = rte_vhost_driver_unregister((char *)&dev_basename);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE,"vhost driver unregister failure.\n");
+
 	return 0;
 
 }
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log Ouyang Changchun
@ 2015-07-02  9:25   ` Xie, Huawei
  2015-07-03  1:55     ` Ouyang, Changchun
  2015-07-02  9:32   ` Xie, Huawei
  1 sibling, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02  9:25 UTC (permalink / raw)
  To: Ouyang, Changchun, dev


On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> It fixes the wrong log info when fails to unregister vhost driver.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>  examples/vhost/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 7863dcf..72c4773 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -3051,10 +3051,10 @@ main(int argc, char *argv[])
>  	if (mergeable == 0)
>  		rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_MRG_RXBUF);
>  
> -	/* Register CUSE device to handle IOCTLs. */
> +	/* Register vhost driver to handle IOCTLs. */

Also update IOCTLS.
or:  register vhost [cuse or user] driver to handle vhost message.
>  	ret = rte_vhost_driver_register((char *)&dev_basename);
>  	if (ret != 0)
> -		rte_exit(EXIT_FAILURE,"CUSE device setup failure.\n");
> +		rte_exit(EXIT_FAILURE,"vhost driver register failure.\n");
>  
>  	rte_vhost_driver_callback_register(&virtio_net_device_ops);
>  


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
@ 2015-07-02  9:29   ` Xie, Huawei
  2015-07-03  1:57     ` Ouyang, Changchun
  2015-07-02  9:31   ` Xie, Huawei
  1 sibling, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02  9:29 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> It adds more readable log info if a socket fails to bind to local device file name.
local socket file, not device file. :).
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>  lib/librte_vhost/vhost_user/vhost-net-user.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
> index 87a4711..f406a94 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.c
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
> @@ -122,8 +122,11 @@ uds_socket(const char *path)
>  	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)
> +	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);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
  2015-07-02  9:29   ` Xie, Huawei
@ 2015-07-02  9:31   ` Xie, Huawei
  1 sibling, 0 replies; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02  9:31 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> It adds more readable log info if a socket fails to bind to local device file name.

fails in binding, not fail to bind, :).

>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log Ouyang Changchun
  2015-07-02  9:25   ` Xie, Huawei
@ 2015-07-02  9:32   ` Xie, Huawei
  1 sibling, 0 replies; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02  9:32 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> It fixes the wrong log info when fails to unregister vhost driver.
As commented elsewhere, fails in unregistering, not fails to unregister. :).



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
@ 2015-07-02  9:38   ` Xie, Huawei
  2015-07-03  2:03     ` Ouyang, Changchun
  2015-07-02 16:04   ` Xie, Huawei
  1 sibling, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02  9:38 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
>  
>  	/* Start CUSE session. */
>  	rte_vhost_driver_session_start();
> +
> +	/* Unregister vhost driver. */
> +	ret = rte_vhost_driver_unregister((char *)&dev_basename);
> +	if (ret != 0)
> +		rte_exit(EXIT_FAILURE,"vhost driver unregister failure.\n");
> +
Better remove the above code.
It is duplicated with signal handler and actually
rte_vhost_driver_session_start never returns.

>  	return 0;
>  
>  }


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
  2015-07-02  9:38   ` Xie, Huawei
@ 2015-07-02 16:04   ` Xie, Huawei
  2015-07-03  2:04     ` Ouyang, Changchun
  1 sibling, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-07-02 16:04 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> The commit will break vhost sample when it runs in second time:
> 292959c71961acde0cda6e77e737bb0a4df1559c
>
> It should call api to unregister vhost driver when sample exit/quit, then
> the socket file will be removed(by calling unlink), and thus make vhost sample
> work correctly in second time startup.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>  examples/vhost/main.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 72c4773..90666b3 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -2871,6 +2871,16 @@ setup_mempool_tbl(int socket, uint32_t index, char *pool_name,
>  	}
>  }
>  
> +/* When we receive a HUP signal, unregister vhost driver */
> +static void
> +sighup_handler(__rte_unused int signum)
> +{
> +	/* Unregister vhost driver. */
> +	int ret = rte_vhost_driver_unregister((char *)&dev_basename);
> +	if (ret != 0)
> +		rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
> +	exit(0);
> +}
>  
>  /*
>   * Main function, does initialisation and calls the per-lcore functions. The CUSE
> @@ -2887,6 +2897,8 @@ main(int argc, char *argv[])
>  	uint16_t queue_id;
>  	static pthread_t tid;
>  
> +	signal(SIGINT, sighup_handler);
> +

ignor if duplciated.
sighup->sigint

>  	/* init EAL */
>  	ret = rte_eal_init(argc, argv);
>  	if (ret < 0)
> @@ -3060,6 +3072,12 @@ main(int argc, char *argv[])
>  
>  	/* Start CUSE session. */
>  	rte_vhost_driver_session_start();
> +
> +	/* Unregister vhost driver. */
> +	ret = rte_vhost_driver_unregister((char *)&dev_basename);
> +	if (ret != 0)
> +		rte_exit(EXIT_FAILURE,"vhost driver unregister failure.\n");
> +
>  	return 0;
>  
>  }


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log
  2015-07-02  9:25   ` Xie, Huawei
@ 2015-07-03  1:55     ` Ouyang, Changchun
  0 siblings, 0 replies; 22+ messages in thread
From: Ouyang, Changchun @ 2015-07-03  1:55 UTC (permalink / raw)
  To: Xie, Huawei, dev



> -----Original Message-----
> From: Xie, Huawei
> Sent: Thursday, July 2, 2015 5:25 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Cc: Cao, Waterman; Xu, Qian Q
> Subject: Re: [PATCH 2/3] vhost: fix the comments and log
> 
> 
> On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> > It fixes the wrong log info when fails to unregister vhost driver.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> > ---
> >  examples/vhost/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/examples/vhost/main.c b/examples/vhost/main.c index
> > 7863dcf..72c4773 100644
> > --- a/examples/vhost/main.c
> > +++ b/examples/vhost/main.c
> > @@ -3051,10 +3051,10 @@ main(int argc, char *argv[])
> >  	if (mergeable == 0)
> >  		rte_vhost_feature_disable(1ULL <<
> VIRTIO_NET_F_MRG_RXBUF);
> >
> > -	/* Register CUSE device to handle IOCTLs. */
> > +	/* Register vhost driver to handle IOCTLs. */
> 
> Also update IOCTLS.
> or:  register vhost [cuse or user] driver to handle vhost message.

Make sense, will update it, thanks

> >  	ret = rte_vhost_driver_register((char *)&dev_basename);
> >  	if (ret != 0)
> > -		rte_exit(EXIT_FAILURE,"CUSE device setup failure.\n");
> > +		rte_exit(EXIT_FAILURE,"vhost driver register failure.\n");
> >
> >  	rte_vhost_driver_callback_register(&virtio_net_device_ops);
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket
  2015-07-02  9:29   ` Xie, Huawei
@ 2015-07-03  1:57     ` Ouyang, Changchun
  0 siblings, 0 replies; 22+ messages in thread
From: Ouyang, Changchun @ 2015-07-03  1:57 UTC (permalink / raw)
  To: Xie, Huawei, dev



> -----Original Message-----
> From: Xie, Huawei
> Sent: Thursday, July 2, 2015 5:29 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Cc: Cao, Waterman; Xu, Qian Q
> Subject: Re: [PATCH 1/3] vhost: add log if fails to bind a socket
> 
> On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> > It adds more readable log info if a socket fails to bind to local device file
> name.
> local socket file, not device file. :).

Make sense, will update it, thanks

> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> > ---
> >  lib/librte_vhost/vhost_user/vhost-net-user.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c
> b/lib/librte_vhost/vhost_user/vhost-net-user.c
> > index 87a4711..f406a94 100644
> > --- a/lib/librte_vhost/vhost_user/vhost-net-user.c
> > +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
> > @@ -122,8 +122,11 @@ uds_socket(const char *path)
> >  	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)
> > +	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);

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver
  2015-07-02  9:38   ` Xie, Huawei
@ 2015-07-03  2:03     ` Ouyang, Changchun
  0 siblings, 0 replies; 22+ messages in thread
From: Ouyang, Changchun @ 2015-07-03  2:03 UTC (permalink / raw)
  To: Xie, Huawei, dev


> -----Original Message-----
> From: Xie, Huawei
> Sent: Thursday, July 2, 2015 5:38 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Cc: Cao, Waterman; Xu, Qian Q
> Subject: Re: [PATCH 3/3] vhost: call api to unregister vhost driver
> 
> On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> >
> >  	/* Start CUSE session. */
> >  	rte_vhost_driver_session_start();
> > +
> > +	/* Unregister vhost driver. */
> > +	ret = rte_vhost_driver_unregister((char *)&dev_basename);
> > +	if (ret != 0)
> > +		rte_exit(EXIT_FAILURE,"vhost driver unregister failure.\n");
> > +
> Better remove the above code.
> It is duplicated with signal handler and actually
> rte_vhost_driver_session_start never returns.

How about call one function to replace the code snippet?
I think we need unregister there, it give us a clear example what the vhost lib caller need to do at the ramp down stage. 
Maybe 'never return' will be changed some day.

> 
> >  	return 0;
> >
> >  }

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver
  2015-07-02 16:04   ` Xie, Huawei
@ 2015-07-03  2:04     ` Ouyang, Changchun
  0 siblings, 0 replies; 22+ messages in thread
From: Ouyang, Changchun @ 2015-07-03  2:04 UTC (permalink / raw)
  To: Xie, Huawei, dev



> -----Original Message-----
> From: Xie, Huawei
> Sent: Friday, July 3, 2015 12:04 AM
> To: Ouyang, Changchun; dev@dpdk.org
> Cc: Cao, Waterman; Xu, Qian Q
> Subject: Re: [PATCH 3/3] vhost: call api to unregister vhost driver
> 
> On 7/2/2015 11:33 AM, Ouyang, Changchun wrote:
> > The commit will break vhost sample when it runs in second time:
> > 292959c71961acde0cda6e77e737bb0a4df1559c
> >
> > It should call api to unregister vhost driver when sample exit/quit,
> > then the socket file will be removed(by calling unlink), and thus make
> > vhost sample work correctly in second time startup.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> > ---
> >  examples/vhost/main.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/examples/vhost/main.c b/examples/vhost/main.c index
> > 72c4773..90666b3 100644
> > --- a/examples/vhost/main.c
> > +++ b/examples/vhost/main.c
> > @@ -2871,6 +2871,16 @@ setup_mempool_tbl(int socket, uint32_t index,
> char *pool_name,
> >  	}
> >  }
> >
> > +/* When we receive a HUP signal, unregister vhost driver */ static
> > +void sighup_handler(__rte_unused int signum) {
> > +	/* Unregister vhost driver. */
> > +	int ret = rte_vhost_driver_unregister((char *)&dev_basename);
> > +	if (ret != 0)
> > +		rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
> > +	exit(0);
> > +}
> >
> >  /*
> >   * Main function, does initialisation and calls the per-lcore
> > functions. The CUSE @@ -2887,6 +2897,8 @@ main(int argc, char *argv[])
> >  	uint16_t queue_id;
> >  	static pthread_t tid;
> >
> > +	signal(SIGINT, sighup_handler);
> > +
> 
> ignor if duplciated.
> sighup->sigint

Make sense, will update it in v2

> 
> >  	/* init EAL */
> >  	ret = rte_eal_init(argc, argv);
> >  	if (ret < 0)
> > @@ -3060,6 +3072,12 @@ main(int argc, char *argv[])
> >
> >  	/* Start CUSE session. */
> >  	rte_vhost_driver_session_start();
> > +
> > +	/* Unregister vhost driver. */
> > +	ret = rte_vhost_driver_unregister((char *)&dev_basename);
> > +	if (ret != 0)
> > +		rte_exit(EXIT_FAILURE,"vhost driver unregister failure.\n");
> > +
> >  	return 0;
> >
> >  }

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue
  2015-07-02  3:33 [dpdk-dev] [PATCH 0/3] Fix vhost startup issue Ouyang Changchun
                   ` (2 preceding siblings ...)
  2015-07-02  3:33 ` [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
@ 2015-07-06  2:26 ` Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 1/3] vhost: add log when failing to bind a socket Ouyang Changchun
                     ` (5 more replies)
  3 siblings, 6 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-06  2:26 UTC (permalink / raw)
  To: dev

The patch set fix vhost sample fails to start up in second time:
 
It should call api to unregister vhost driver when sample exit/quit, then
the socket file will be removed(by calling unlink), and thus make vhost sample
work correctly in second time startup.
 
It also adds/refines some log information.

Changchun Ouyang (3):
  vhost: add log when failing to bind a socket
  vhost: fix the comments and log
  vhost: call api to unregister vhost driver

 examples/vhost/main.c                        | 16 ++++++++++++++--
 lib/librte_vhost/vhost_user/vhost-net-user.c |  5 ++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 1/3] vhost: add log when failing to bind a socket
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
@ 2015-07-06  2:26   ` Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 2/3] vhost: fix the comments and log Ouyang Changchun
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-06  2:26 UTC (permalink / raw)
  To: dev

It adds more readable log info if a socket fails to bind to local socket file name.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 lib/librte_vhost/vhost_user/vhost-net-user.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index 87a4711..f406a94 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -122,8 +122,11 @@ uds_socket(const char *path)
 	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)
+	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);
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 2/3] vhost: fix the comments and log
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 1/3] vhost: add log when failing to bind a socket Ouyang Changchun
@ 2015-07-06  2:26   ` Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-06  2:26 UTC (permalink / raw)
  To: dev

It fixes the wrong log info when failing to unregister vhost driver.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 examples/vhost/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

change in v2:
  - refine the comment
  - fix checkpatch issue

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 7863dcf..56a5c70 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -3051,10 +3051,10 @@ main(int argc, char *argv[])
 	if (mergeable == 0)
 		rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_MRG_RXBUF);
 
-	/* Register CUSE device to handle IOCTLs. */
+	/* Register vhost(cuse or user) driver to handle vhost messages. */
 	ret = rte_vhost_driver_register((char *)&dev_basename);
 	if (ret != 0)
-		rte_exit(EXIT_FAILURE,"CUSE device setup failure.\n");
+		rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
 
 	rte_vhost_driver_callback_register(&virtio_net_device_ops);
 
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 3/3] vhost: call api to unregister vhost driver
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 1/3] vhost: add log when failing to bind a socket Ouyang Changchun
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 2/3] vhost: fix the comments and log Ouyang Changchun
@ 2015-07-06  2:26   ` Ouyang Changchun
  2015-07-06  8:09   ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Xu, Qian Q
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Ouyang Changchun @ 2015-07-06  2:26 UTC (permalink / raw)
  To: dev

The following commit broke vhost sample when it runs in second time:
292959c71961acde0cda6e77e737bb0a4df1559c

It should call api to unregister vhost driver when sample exit/quit, then
the socket file will be removed(by calling unlink), and thus make vhost sample
work correctly in the second time startup.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 examples/vhost/main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

change in v2:
 - refine the signal handler name

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 56a5c70..1b137b9 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -2871,6 +2871,16 @@ setup_mempool_tbl(int socket, uint32_t index, char *pool_name,
 	}
 }
 
+/* When we receive a INT signal, unregister vhost driver */
+static void
+sigint_handler(__rte_unused int signum)
+{
+	/* Unregister vhost driver. */
+	int ret = rte_vhost_driver_unregister((char *)&dev_basename);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
+	exit(0);
+}
 
 /*
  * Main function, does initialisation and calls the per-lcore functions. The CUSE
@@ -2887,6 +2897,8 @@ main(int argc, char *argv[])
 	uint16_t queue_id;
 	static pthread_t tid;
 
+	signal(SIGINT, sigint_handler);
+
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
                     ` (2 preceding siblings ...)
  2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
@ 2015-07-06  8:09   ` Xu, Qian Q
  2015-07-07  2:31   ` Ouyang, Changchun
  2015-07-10 14:20   ` Xie, Huawei
  5 siblings, 0 replies; 22+ messages in thread
From: Xu, Qian Q @ 2015-07-06  8:09 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

Tested-by: Qian Xu <qian.q.xu@intel.com>

- Test Commit: b283164694b6ed18be1856b949e91a80371e44d4
- OS: Fedora 21
- GCC: gcc (GCC) 4.9.2 20141101 (Red Hat 4.9.2-1)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
- NIC: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01)
- Target: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01)
- Total 1 cases(Test Case7 plus rerun ), 1 passed, 0 failed.

Test Case 1:  test_perf_virtio_one_vm_dpdk_fwd_vhost-cuse_jumboframe
====================================================================

On host:

1. Start up vhost-switch, mergeable 1 means the jubmo frame feature is enabled. vm2vm 0 means only one vm without vm to vm communication::

    taskset -c 1-3 <dpdk_folder>/examples/vhost/build/vhost-switch -c 0xf -n 4 --huge-dir /mnt/huge --socket-mem 1024,1024 -- -p 1 --mergeable 1 --zero-copy 0 --vm2vm 0
   

2. Start VM with vhost cuse as backend::

    taskset -c 4-6  /home/qxu10/qemu-2.2.0/x86_64-softmmu/qemu-system-x86_64 -object memory-backend-file, id=mem,size=2048M,mem-path=/mnt/huge,share=on -numa node,memdev=mem -mem-prealloc \
    -enable-kvm -m 2048 -smp 4 -cpu host -name dpdk1-vm1 \
    -drive file=/home/img/dpdk1-vm1.img \
    -netdev tap,id=vhost3,ifname=tap_vhost3,vhost=on,script=no \
    -device virtio-net pci,netdev=vhost3,mac=52:54:00:00:00:01,id=net3,csum=off,gso=off,guest_csum=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
    -netdev tap,id=vhost4,ifname=tap_vhost4,vhost=on,script=no \
    -device virtio-net-pci,netdev=vhost4,mac=52:54:00:00:00:02,id=net4,csum=off,gso=off,guest_csum=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
    -netdev tap,id=ipvm1,ifname=tap3,script=/etc/qemu-ifup -device rtl8139,netdev=ipvm1,id=net0,mac=00:00:00:00:00:01 \
    -localtime -nographic

On guest:

3. ensure the dpdk folder copied to the guest with the same config file and build process as host. Then bind 2 virtio devices to igb_uio and start testpmd, below is the step for reference::

    ./<dpdk_folder>/tools/dpdk_nic_bind.py --bind igb_uio 00:03.0 00:04.0

    ./<dpdk_folder>/x86_64-native-linuxapp-gcc/app/test-pmd/testpmd -c f -n 4 -- -i --txqflags 0x0f00 --max-pkt-len 9000 
    
    $ >set fwd mac
    
    $ >start tx_first

4. After typing start tx_first in testpmd, user can see there would be 2 virtio device with MAC and vlan id registered in vhost-user, the log would be shown in host's vhost-sample output.

5. Send traffic(30second) to virtio1 and virtio2, and set the packet size from 64 to 1518 as well as the jumbo frame 3000. Check the performance in Mpps. The traffic sent to virtio1 should have the DEST MAC of Virtio1's MAC, Vlan id of Virtio1. The traffic sent to virtio2 should have the DEST MAC of Virtio2's MAC, Vlan id of Virtio2. As to the functionality criteria, The received rate should not be zero. As to the performance criteria, need check it with developer or design doc/PRD.

Test Case 7:  test_perf_virtio_one_vm_dpdk_fwd_vhost-user_jumboframe
====================================================================

This case is similar to TestCase1, just change the backend from vhost cuse to vhost-user, so need rebuild the dpdk in vhost-user on host, other steps are same as TestCase1. The command to launch vm is different, see below as example:: 

    <qemu-2.2.0_folder>/x86_64-softmmu/qemu-system-x86_64 -name us-vhost-vm1 -cpu host -enable-kvm -m 2048 -object memory-backend-file,id=mem,size=2048M,mem-path=/mnt/huge,share=on -numa node,memdev=mem -mem-prealloc -smp 2 -drive file=/home/img/dpdk1-vm1.img -chardev socket,id=char0,path=<dpdk/vhost-net -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,mac=52:54:00:00:00:01,netdev=mynet1 -chardev socket,id=char1,path=/home/qxu10/dpdk/vhost-net -netdev type=vhost-user,id=mynet2,chardev=char1,vhostforce -device virtio-net-pci,mac=52:54:00:00:00:02,netdev=mynet2 -netdev tap,id=ipvm1,ifname=tap3,script=/etc/qemu-ifup -device rtl8139,netdev=ipvm1,id=net0,mac=00:00:00:00:00:09 -nographic

On the guest, need add one parameter at the end of testpmd command line: --disable-hw-vlan-filter.

Rerun the case and make sure no vhost-register issue. 


Thanks
Qian

-----Original Message-----
From: Ouyang, Changchun 
Sent: Monday, July 06, 2015 10:27 AM
To: dev@dpdk.org
Cc: Xie, Huawei; Cao, Waterman; Xu, Qian Q; Ouyang, Changchun
Subject: [PATCH v2 0/3] Fix vhost startup issue

The patch set fix vhost sample fails to start up in second time:
 
It should call api to unregister vhost driver when sample exit/quit, then the socket file will be removed(by calling unlink), and thus make vhost sample work correctly in second time startup.
 
It also adds/refines some log information.

Changchun Ouyang (3):
  vhost: add log when failing to bind a socket
  vhost: fix the comments and log
  vhost: call api to unregister vhost driver

 examples/vhost/main.c                        | 16 ++++++++++++++--
 lib/librte_vhost/vhost_user/vhost-net-user.c |  5 ++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

--
1.8.4.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
                     ` (3 preceding siblings ...)
  2015-07-06  8:09   ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Xu, Qian Q
@ 2015-07-07  2:31   ` Ouyang, Changchun
  2015-07-10 14:20   ` Xie, Huawei
  5 siblings, 0 replies; 22+ messages in thread
From: Ouyang, Changchun @ 2015-07-07  2:31 UTC (permalink / raw)
  To: dev


> -----Original Message-----
> From: Ouyang, Changchun
> Sent: Monday, July 6, 2015 10:27 AM
> To: dev@dpdk.org
> Cc: Xie, Huawei; Cao, Waterman; Xu, Qian Q; Ouyang, Changchun
> Subject: [PATCH v2 0/3] Fix vhost startup issue
> 
> The patch set fix vhost sample fails to start up in second time:
> 
> It should call api to unregister vhost driver when sample exit/quit, then the
> socket file will be removed(by calling unlink), and thus make vhost sample
> work correctly in second time startup.
> 
> It also adds/refines some log information.
> 
> Changchun Ouyang (3):
>   vhost: add log when failing to bind a socket
>   vhost: fix the comments and log
>   vhost: call api to unregister vhost driver
> 
>  examples/vhost/main.c                        | 16 ++++++++++++++--
>  lib/librte_vhost/vhost_user/vhost-net-user.c |  5 ++++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> --
> 1.8.4.2

Any more comments for this patch set?

Thanks
Changchun

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue
  2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
                     ` (4 preceding siblings ...)
  2015-07-07  2:31   ` Ouyang, Changchun
@ 2015-07-10 14:20   ` Xie, Huawei
  2015-07-17 13:13     ` Thomas Monjalon
  5 siblings, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-07-10 14:20 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

Acked-by: Huawei Xie <huawei.xie@intel.com>

On 7/6/2015 10:27 AM, Ouyang, Changchun wrote:
> The patch set fix vhost sample fails to start up in second time:
>  
> It should call api to unregister vhost driver when sample exit/quit, then
> the socket file will be removed(by calling unlink), and thus make vhost sample
> work correctly in second time startup.
>  
> It also adds/refines some log information.
>
> Changchun Ouyang (3):
>   vhost: add log when failing to bind a socket
>   vhost: fix the comments and log
>   vhost: call api to unregister vhost driver
>
>  examples/vhost/main.c                        | 16 ++++++++++++++--
>  lib/librte_vhost/vhost_user/vhost-net-user.c |  5 ++++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue
  2015-07-10 14:20   ` Xie, Huawei
@ 2015-07-17 13:13     ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2015-07-17 13:13 UTC (permalink / raw)
  To: Ouyang, Changchun; +Cc: dev

2015-07-10 14:20, Xie, Huawei:
> On 7/6/2015 10:27 AM, Ouyang, Changchun wrote:
> > The patch set fix vhost sample fails to start up in second time:
> >  
> > It should call api to unregister vhost driver when sample exit/quit, then
> > the socket file will be removed(by calling unlink), and thus make vhost sample
> > work correctly in second time startup.
> >  
> > It also adds/refines some log information.
> >
> > Changchun Ouyang (3):
> >   vhost: add log when failing to bind a socket
> >   vhost: fix the comments and log
> >   vhost: call api to unregister vhost driver
> 
> Acked-by: Huawei Xie <huawei.xie@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-07-17 13:14 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-02  3:33 [dpdk-dev] [PATCH 0/3] Fix vhost startup issue Ouyang Changchun
2015-07-02  3:33 ` [dpdk-dev] [PATCH 1/3] vhost: add log if fails to bind a socket Ouyang Changchun
2015-07-02  9:29   ` Xie, Huawei
2015-07-03  1:57     ` Ouyang, Changchun
2015-07-02  9:31   ` Xie, Huawei
2015-07-02  3:33 ` [dpdk-dev] [PATCH 2/3] vhost: fix the comments and log Ouyang Changchun
2015-07-02  9:25   ` Xie, Huawei
2015-07-03  1:55     ` Ouyang, Changchun
2015-07-02  9:32   ` Xie, Huawei
2015-07-02  3:33 ` [dpdk-dev] [PATCH 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
2015-07-02  9:38   ` Xie, Huawei
2015-07-03  2:03     ` Ouyang, Changchun
2015-07-02 16:04   ` Xie, Huawei
2015-07-03  2:04     ` Ouyang, Changchun
2015-07-06  2:26 ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Ouyang Changchun
2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 1/3] vhost: add log when failing to bind a socket Ouyang Changchun
2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 2/3] vhost: fix the comments and log Ouyang Changchun
2015-07-06  2:26   ` [dpdk-dev] [PATCH v2 3/3] vhost: call api to unregister vhost driver Ouyang Changchun
2015-07-06  8:09   ` [dpdk-dev] [PATCH v2 0/3] Fix vhost startup issue Xu, Qian Q
2015-07-07  2:31   ` Ouyang, Changchun
2015-07-10 14:20   ` Xie, Huawei
2015-07-17 13:13     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).