* [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files
@ 2016-08-16 16:14 Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Jiayu Hu @ 2016-08-16 16:14 UTC (permalink / raw)
To: dev; +Cc: Jiayu Hu
Patch 1: rename the CLI option "dev-basename".
Patch 2: add multiple socket files support.
Jiayu Hu (2):
examples/vhost: rename dev-basename
examples/vhost: support multiple socket files
examples/vhost/main.c | 81 ++++++++++++++++++++++++++++++++++-----------------
1 file changed, 54 insertions(+), 27 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-16 16:14 [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files Jiayu Hu
@ 2016-08-16 16:14 ` Jiayu Hu
2016-08-18 8:22 ` Maxime Coquelin
2016-08-20 10:10 ` Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
2016-08-20 10:08 ` [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and " Jiayu Hu
2 siblings, 2 replies; 14+ messages in thread
From: Jiayu Hu @ 2016-08-16 16:14 UTC (permalink / raw)
To: dev; +Cc: Jiayu Hu
In examples/vhost, "dev-basename" is a program option, which is to set
the vhost-net socket used by vhost-user, or the character device used
by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
is not a suitable name for the vhost-net socket. Therefore, this patch
is to change this option name for examples/vhost.
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
examples/vhost/main.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 92a9823..a718577 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -90,9 +90,6 @@
/* Size of buffers used for snprintfs. */
#define MAX_PRINT_BUFF 6072
-/* Maximum character device basename size. */
-#define MAX_BASENAME_SZ 10
-
/* Maximum long option length for option parsing. */
#define MAX_LONG_OPT_SZ 64
@@ -139,8 +136,8 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
/* Specify the number of retries on RX. */
static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
-/* Character device basename. Can be set by user. */
-static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
+/* Socket file path. Can be set by user */
+static char socket_file[PATH_MAX] = "vhost-net";
/* empty vmdq configuration structure. Filled in programatically */
static struct rte_eth_conf vmdq_conf_default = {
@@ -392,17 +389,17 @@ port_init(uint8_t port)
}
/*
- * Set character device basename.
+ * Set socket file path.
*/
static int
-us_vhost_parse_basename(const char *q_arg)
+us_vhost_parse_socket_path(const char *q_arg)
{
/* parse number string */
- if (strnlen(q_arg, MAX_BASENAME_SZ) > MAX_BASENAME_SZ)
+ if (strnlen(q_arg, PATH_MAX) > PATH_MAX)
return -1;
else
- snprintf((char*)&dev_basename, MAX_BASENAME_SZ, "%s", q_arg);
+ snprintf((char *)&socket_file, PATH_MAX, "%s", q_arg);
return 0;
}
@@ -462,7 +459,7 @@ us_vhost_usage(const char *prgname)
RTE_LOG(INFO, VHOST_CONFIG, "%s [EAL options] -- -p PORTMASK\n"
" --vm2vm [0|1|2]\n"
" --rx_retry [0|1] --mergeable [0|1] --stats [0-N]\n"
- " --dev-basename <name>\n"
+ " --socket-file <path>\n"
" --nb-devices ND\n"
" -p PORTMASK: Set mask for ports to be used by application\n"
" --vm2vm [0|1|2]: disable/software(default)/hardware vm2vm comms\n"
@@ -472,7 +469,7 @@ us_vhost_usage(const char *prgname)
" --mergeable [0|1]: disable(default)/enable RX mergeable buffers\n"
" --vlan-strip [0|1]: disable/enable(default) RX VLAN strip on host\n"
" --stats [0-N]: 0: Disable stats, N: Time in seconds to print stats\n"
- " --dev-basename: The basename to be used for the character device.\n"
+ " --socket-file: The path of the socket file.\n"
" --tx-csum [0|1] disable/enable TX checksum offload.\n"
" --tso [0|1] disable/enable TCP segment offload.\n"
" --client register a vhost-user socket as client mode.\n",
@@ -497,7 +494,7 @@ us_vhost_parse_args(int argc, char **argv)
{"mergeable", required_argument, NULL, 0},
{"vlan-strip", required_argument, NULL, 0},
{"stats", required_argument, NULL, 0},
- {"dev-basename", required_argument, NULL, 0},
+ {"socket-file", required_argument, NULL, 0},
{"tx-csum", required_argument, NULL, 0},
{"tso", required_argument, NULL, 0},
{"client", no_argument, &client_mode, 1},
@@ -638,7 +635,8 @@ us_vhost_parse_args(int argc, char **argv)
if (!strncmp(long_option[option_index].name, "stats", MAX_LONG_OPT_SZ)) {
ret = parse_num_opt(optarg, INT32_MAX);
if (ret == -1) {
- RTE_LOG(INFO, VHOST_CONFIG, "Invalid argument for stats [0..N]\n");
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "Invalid argument for stats [0..N]\n");
us_vhost_usage(prgname);
return -1;
} else {
@@ -646,10 +644,13 @@ us_vhost_parse_args(int argc, char **argv)
}
}
- /* Set character device basename. */
- if (!strncmp(long_option[option_index].name, "dev-basename", MAX_LONG_OPT_SZ)) {
- if (us_vhost_parse_basename(optarg) == -1) {
- RTE_LOG(INFO, VHOST_CONFIG, "Invalid argument for character device basename (Max %d characters)\n", MAX_BASENAME_SZ);
+ /* Set socket file path. */
+ if (!strncmp(long_option[option_index].name,
+ "socket-file", MAX_LONG_OPT_SZ)) {
+ if (us_vhost_parse_socket_path(optarg) == -1) {
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "Invalid argument for socket name (Max %d characters)\n",
+ PATH_MAX);
us_vhost_usage(prgname);
return -1;
}
@@ -1345,7 +1346,7 @@ static void
sigint_handler(__rte_unused int signum)
{
/* Unregister vhost driver. */
- int ret = rte_vhost_driver_unregister((char *)&dev_basename);
+ int ret = rte_vhost_driver_unregister((char *)&socket_file);
if (ret != 0)
rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
exit(0);
@@ -1509,8 +1510,8 @@ main(int argc, char *argv[])
if (client_mode)
flags |= RTE_VHOST_USER_CLIENT;
- /* Register vhost(cuse or user) driver to handle vhost messages. */
- ret = rte_vhost_driver_register(dev_basename, flags);
+ /* Register vhost user driver to handle vhost messages. */
+ ret = rte_vhost_driver_register(socket_file, flags);
if (ret != 0)
rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-16 16:14 [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
@ 2016-08-16 16:14 ` Jiayu Hu
2016-08-18 8:01 ` Yuanhan Liu
` (2 more replies)
2016-08-20 10:08 ` [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and " Jiayu Hu
2 siblings, 3 replies; 14+ messages in thread
From: Jiayu Hu @ 2016-08-16 16:14 UTC (permalink / raw)
To: dev; +Cc: Jiayu Hu
When examples/vhost runs in client mode, only one QEMU can be connected.
This is because that examples/vhost just supports one socket file. This
patch is to add multiple sockets support for examples/vhost.
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
examples/vhost/main.c | 50 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index a718577..9974f0b 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -136,8 +136,9 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
/* Specify the number of retries on RX. */
static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
-/* Socket file path. Can be set by user */
-static char socket_file[PATH_MAX] = "vhost-net";
+/* Socket file paths. Can be set by user */
+static char *socket_files;
+int nb_sockets;
/* empty vmdq configuration structure. Filled in programatically */
static struct rte_eth_conf vmdq_conf_default = {
@@ -395,11 +396,12 @@ static int
us_vhost_parse_socket_path(const char *q_arg)
{
/* parse number string */
-
if (strnlen(q_arg, PATH_MAX) > PATH_MAX)
return -1;
- else
- snprintf((char *)&socket_file, PATH_MAX, "%s", q_arg);
+
+ socket_files = realloc(socket_files, PATH_MAX * (nb_sockets + 1));
+ snprintf(socket_files + nb_sockets * PATH_MAX, PATH_MAX, "%s", q_arg);
+ nb_sockets++;
return 0;
}
@@ -1341,14 +1343,30 @@ print_stats(void)
}
}
+/*
+ * This function is used to unregister drivers.
+ */
+static void
+unregister_drivers(int socket_num)
+{
+ int i, ret;
+
+ for (i = 0; i < socket_num; i++) {
+ ret = rte_vhost_driver_unregister(socket_files + i * PATH_MAX);
+ if (ret != 0)
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "Fail to unregister vhost driver for %s.\n",
+ socket_files + i * PATH_MAX);
+ }
+}
+
/* 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 *)&socket_file);
- if (ret != 0)
- rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
+ unregister_drivers(nb_sockets);
+
exit(0);
}
@@ -1412,12 +1430,15 @@ main(int argc, char *argv[])
{
unsigned lcore_id, core_id = 0;
unsigned nb_ports, valid_num_ports;
- int ret;
+ int ret, i;
uint8_t portid;
static pthread_t tid;
char thread_name[RTE_MAX_THREAD_NAME_LEN];
uint64_t flags = 0;
+ nb_sockets = 0;
+ socket_files = NULL;
+
signal(SIGINT, sigint_handler);
/* init EAL */
@@ -1511,9 +1532,14 @@ main(int argc, char *argv[])
flags |= RTE_VHOST_USER_CLIENT;
/* Register vhost user driver to handle vhost messages. */
- ret = rte_vhost_driver_register(socket_file, flags);
- if (ret != 0)
- rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
+ for (i = 0; i < nb_sockets; i++) {
+ ret = rte_vhost_driver_register
+ (socket_files + i * PATH_MAX, flags);
+ if (ret != 0) {
+ unregister_drivers(i);
+ rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
+ }
+ }
rte_vhost_driver_callback_register(&virtio_net_device_ops);
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
@ 2016-08-18 8:01 ` Yuanhan Liu
2016-08-18 8:27 ` Maxime Coquelin
2016-08-20 10:11 ` Jiayu Hu
2 siblings, 0 replies; 14+ messages in thread
From: Yuanhan Liu @ 2016-08-18 8:01 UTC (permalink / raw)
To: Jiayu Hu; +Cc: dev
On Tue, Aug 16, 2016 at 12:14:39PM -0400, Jiayu Hu wrote:
> +/*
> + * This function is used to unregister drivers.
> + */
> +static void
> +unregister_drivers(int socket_num)
> +{
Redundant comment. The function name already explains it well.
> /* Register vhost user driver to handle vhost messages. */
> - ret = rte_vhost_driver_register(socket_file, flags);
> - if (ret != 0)
> - rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
> + for (i = 0; i < nb_sockets; i++) {
> + ret = rte_vhost_driver_register
> + (socket_files + i * PATH_MAX, flags);
> + if (ret != 0) {
> + unregister_drivers(i);
> + rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
Lines over 80 chars.
Besides, please cc corresponding maintainers while sending patches, say
cc me for virtio/vhost changes. From MAINTAINERS you could find the
names.
So, please make a v2, with above 2 minor fixed. And also, please follow
the guide on http://dpdk.org/dev to send v2:
If a previous version of the patch has already been sent, a version
number and changelog annotations are helpful:
git send-email -1 -v2 --annotate --in-reply-to <Message-ID of the previous patch>
--to dev@dpdk.org --cc <everybody discussing the patch>
--yliu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
@ 2016-08-18 8:22 ` Maxime Coquelin
2016-08-18 8:35 ` Yuanhan Liu
2016-08-20 10:10 ` Jiayu Hu
1 sibling, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2016-08-18 8:22 UTC (permalink / raw)
To: Jiayu Hu, dev
Hi Jiayu,
On 08/16/2016 06:14 PM, Jiayu Hu wrote:
> In examples/vhost, "dev-basename" is a program option, which is to set
> the vhost-net socket used by vhost-user, or the character device used
> by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
> is not a suitable name for the vhost-net socket. Therefore, this patch
> is to change this option name for examples/vhost.
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
> examples/vhost/main.c | 41 +++++++++++++++++++++--------------------
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 92a9823..a718577 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -90,9 +90,6 @@
> /* Size of buffers used for snprintfs. */
> #define MAX_PRINT_BUFF 6072
>
> -/* Maximum character device basename size. */
> -#define MAX_BASENAME_SZ 10
> -
> /* Maximum long option length for option parsing. */
> #define MAX_LONG_OPT_SZ 64
>
> @@ -139,8 +136,8 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
> /* Specify the number of retries on RX. */
> static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
>
> -/* Character device basename. Can be set by user. */
> -static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
> +/* Socket file path. Can be set by user */
> +static char socket_file[PATH_MAX] = "vhost-net";
Not very important, but now that we only support vhost-user,
maybe we could default the name to "vhost-user"?
There is no real convention I think, but this is what OVS is
used to use in its examples.
Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
2016-08-18 8:01 ` Yuanhan Liu
@ 2016-08-18 8:27 ` Maxime Coquelin
2016-08-18 8:43 ` Yuanhan Liu
2016-08-20 10:11 ` Jiayu Hu
2 siblings, 1 reply; 14+ messages in thread
From: Maxime Coquelin @ 2016-08-18 8:27 UTC (permalink / raw)
To: Jiayu Hu, dev
On 08/16/2016 06:14 PM, Jiayu Hu wrote:
> When examples/vhost runs in client mode, only one QEMU can be connected.
> This is because that examples/vhost just supports one socket file. This
> patch is to add multiple sockets support for examples/vhost.
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
> examples/vhost/main.c | 50 ++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index a718577..9974f0b 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -136,8 +136,9 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
> /* Specify the number of retries on RX. */
> static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
>
> -/* Socket file path. Can be set by user */
> -static char socket_file[PATH_MAX] = "vhost-net";
Default name being removed, you can drop my comment on patch 1. :)
> +/* Socket file paths. Can be set by user */
> +static char *socket_files;
> +int nb_sockets;
Any reason not to make it static?
> /* empty vmdq configuration structure. Filled in programatically */
> static struct rte_eth_conf vmdq_conf_default = {
> @@ -395,11 +396,12 @@ static int
> us_vhost_parse_socket_path(const char *q_arg)
> {
> /* parse number string */
> -
> if (strnlen(q_arg, PATH_MAX) > PATH_MAX)
> return -1;
> - else
> - snprintf((char *)&socket_file, PATH_MAX, "%s", q_arg);
> +
> + socket_files = realloc(socket_files, PATH_MAX * (nb_sockets + 1));
> + snprintf(socket_files + nb_sockets * PATH_MAX, PATH_MAX, "%s", q_arg);
> + nb_sockets++;
>
> return 0;
> }
> @@ -1341,14 +1343,30 @@ print_stats(void)
> }
> }
>
> +/*
> + * This function is used to unregister drivers.
> + */
> +static void
> +unregister_drivers(int socket_num)
> +{
> + int i, ret;
> +
> + for (i = 0; i < socket_num; i++) {
> + ret = rte_vhost_driver_unregister(socket_files + i * PATH_MAX);
> + if (ret != 0)
> + RTE_LOG(ERR, VHOST_CONFIG,
> + "Fail to unregister vhost driver for %s.\n",
> + socket_files + i * PATH_MAX);
> + }
> +}
> +
> /* 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 *)&socket_file);
> - if (ret != 0)
> - rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
> + unregister_drivers(nb_sockets);
> +
> exit(0);
> }
>
> @@ -1412,12 +1430,15 @@ main(int argc, char *argv[])
> {
> unsigned lcore_id, core_id = 0;
> unsigned nb_ports, valid_num_ports;
> - int ret;
> + int ret, i;
> uint8_t portid;
> static pthread_t tid;
> char thread_name[RTE_MAX_THREAD_NAME_LEN];
> uint64_t flags = 0;
>
> + nb_sockets = 0;
> + socket_files = NULL;
Since socket_files is static, no need to initialize it to NULL.
If you staticize nb_sockets, same remark will apply.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-18 8:35 ` Yuanhan Liu
@ 2016-08-18 8:29 ` Maxime Coquelin
0 siblings, 0 replies; 14+ messages in thread
From: Maxime Coquelin @ 2016-08-18 8:29 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Jiayu Hu, dev
On 08/18/2016 10:35 AM, Yuanhan Liu wrote:
> On Thu, Aug 18, 2016 at 10:22:38AM +0200, Maxime Coquelin wrote:
>> Hi Jiayu,
>>
>> On 08/16/2016 06:14 PM, Jiayu Hu wrote:
>>> In examples/vhost, "dev-basename" is a program option, which is to set
>>> the vhost-net socket used by vhost-user, or the character device used
>>> by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
>>> is not a suitable name for the vhost-net socket. Therefore, this patch
>>> is to change this option name for examples/vhost.
>>>
>>> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
>>> ---
>>> examples/vhost/main.c | 41 +++++++++++++++++++++--------------------
>>> 1 file changed, 21 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
>>> index 92a9823..a718577 100644
>>> --- a/examples/vhost/main.c
>>> +++ b/examples/vhost/main.c
>>> @@ -90,9 +90,6 @@
>>> /* Size of buffers used for snprintfs. */
>>> #define MAX_PRINT_BUFF 6072
>>>
>>> -/* Maximum character device basename size. */
>>> -#define MAX_BASENAME_SZ 10
>>> -
>>> /* Maximum long option length for option parsing. */
>>> #define MAX_LONG_OPT_SZ 64
>>>
>>> @@ -139,8 +136,8 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
>>> /* Specify the number of retries on RX. */
>>> static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
>>>
>>> -/* Character device basename. Can be set by user. */
>>> -static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
>>> +/* Socket file path. Can be set by user */
>>> +static char socket_file[PATH_MAX] = "vhost-net";
>>
>> Not very important, but now that we only support vhost-user,
>> maybe we could default the name to "vhost-user"?
>>
>> There is no real convention I think, but this is what OVS is
>> used to use in its examples.
>
> I think it doesn't matter now, since since the 2nd patch, --socket-path
> is a must but not optional any more, meaning there is no default
> socket file path.
Yes, just noticed it while reviewing patch 2.
So this is all good to me for this patch.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-18 8:22 ` Maxime Coquelin
@ 2016-08-18 8:35 ` Yuanhan Liu
2016-08-18 8:29 ` Maxime Coquelin
0 siblings, 1 reply; 14+ messages in thread
From: Yuanhan Liu @ 2016-08-18 8:35 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: Jiayu Hu, dev
On Thu, Aug 18, 2016 at 10:22:38AM +0200, Maxime Coquelin wrote:
> Hi Jiayu,
>
> On 08/16/2016 06:14 PM, Jiayu Hu wrote:
> >In examples/vhost, "dev-basename" is a program option, which is to set
> >the vhost-net socket used by vhost-user, or the character device used
> >by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
> >is not a suitable name for the vhost-net socket. Therefore, this patch
> >is to change this option name for examples/vhost.
> >
> >Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> >---
> > examples/vhost/main.c | 41 +++++++++++++++++++++--------------------
> > 1 file changed, 21 insertions(+), 20 deletions(-)
> >
> >diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> >index 92a9823..a718577 100644
> >--- a/examples/vhost/main.c
> >+++ b/examples/vhost/main.c
> >@@ -90,9 +90,6 @@
> > /* Size of buffers used for snprintfs. */
> > #define MAX_PRINT_BUFF 6072
> >
> >-/* Maximum character device basename size. */
> >-#define MAX_BASENAME_SZ 10
> >-
> > /* Maximum long option length for option parsing. */
> > #define MAX_LONG_OPT_SZ 64
> >
> >@@ -139,8 +136,8 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
> > /* Specify the number of retries on RX. */
> > static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
> >
> >-/* Character device basename. Can be set by user. */
> >-static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
> >+/* Socket file path. Can be set by user */
> >+static char socket_file[PATH_MAX] = "vhost-net";
>
> Not very important, but now that we only support vhost-user,
> maybe we could default the name to "vhost-user"?
>
> There is no real convention I think, but this is what OVS is
> used to use in its examples.
I think it doesn't matter now, since since the 2nd patch, --socket-path
is a must but not optional any more, meaning there is no default
socket file path.
--yliu
>
> Other than that:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Thanks,
> Maxime
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-18 8:27 ` Maxime Coquelin
@ 2016-08-18 8:43 ` Yuanhan Liu
0 siblings, 0 replies; 14+ messages in thread
From: Yuanhan Liu @ 2016-08-18 8:43 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: Jiayu Hu, dev
On Thu, Aug 18, 2016 at 10:27:55AM +0200, Maxime Coquelin wrote:
>
>
> On 08/16/2016 06:14 PM, Jiayu Hu wrote:
> >When examples/vhost runs in client mode, only one QEMU can be connected.
> >This is because that examples/vhost just supports one socket file. This
> >patch is to add multiple sockets support for examples/vhost.
> >
> >Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> >---
> > examples/vhost/main.c | 50 ++++++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 38 insertions(+), 12 deletions(-)
> >
> >diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> >index a718577..9974f0b 100644
> >--- a/examples/vhost/main.c
> >+++ b/examples/vhost/main.c
> >@@ -136,8 +136,9 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
> > /* Specify the number of retries on RX. */
> > static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
> >
> >-/* Socket file path. Can be set by user */
> >-static char socket_file[PATH_MAX] = "vhost-net";
> Default name being removed, you can drop my comment on patch 1. :)
>
> >+/* Socket file paths. Can be set by user */
> >+static char *socket_files;
> >+int nb_sockets;
> Any reason not to make it static?
Right, it should be "static". Hmm, I missed it in review :(
--yliu
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files
2016-08-16 16:14 [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
@ 2016-08-20 10:08 ` Jiayu Hu
2 siblings, 0 replies; 14+ messages in thread
From: Jiayu Hu @ 2016-08-20 10:08 UTC (permalink / raw)
To: dev; +Cc: maxime.coquelin, yuanhan.liu, Jiayu Hu
Patch 1: rename the CLI option "dev-basename".
Patch 2: add multiple socket files support.
---
Changes in v2:
-Change "int nb_sockets" into "static int nb_sockets".
-Remove the initialization of socket_files and nb_sockets.
-Remove redundant comments.
-Break the long line which is over 80 characters into two lines.
Jiayu Hu (2):
examples/vhost: rename dev-basename
examples/vhost: support multiple socket files
examples/vhost/main.c | 76 +++++++++++++++++++++++++++++++++------------------
1 file changed, 49 insertions(+), 27 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
2016-08-18 8:22 ` Maxime Coquelin
@ 2016-08-20 10:10 ` Jiayu Hu
2016-08-23 6:01 ` Yuanhan Liu
1 sibling, 1 reply; 14+ messages in thread
From: Jiayu Hu @ 2016-08-20 10:10 UTC (permalink / raw)
To: dev; +Cc: maxime.coquelin, yuanhan.liu, Jiayu Hu
In examples/vhost, "dev-basename" is a program option, which is to set
the vhost-net socket used by vhost-user, or the character device used
by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
is not a suitable name for the vhost-net socket. Therefore, this patch
is to change this option name for examples/vhost.
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
examples/vhost/main.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 92a9823..a718577 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -90,9 +90,6 @@
/* Size of buffers used for snprintfs. */
#define MAX_PRINT_BUFF 6072
-/* Maximum character device basename size. */
-#define MAX_BASENAME_SZ 10
-
/* Maximum long option length for option parsing. */
#define MAX_LONG_OPT_SZ 64
@@ -139,8 +136,8 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
/* Specify the number of retries on RX. */
static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
-/* Character device basename. Can be set by user. */
-static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
+/* Socket file path. Can be set by user */
+static char socket_file[PATH_MAX] = "vhost-net";
/* empty vmdq configuration structure. Filled in programatically */
static struct rte_eth_conf vmdq_conf_default = {
@@ -392,17 +389,17 @@ port_init(uint8_t port)
}
/*
- * Set character device basename.
+ * Set socket file path.
*/
static int
-us_vhost_parse_basename(const char *q_arg)
+us_vhost_parse_socket_path(const char *q_arg)
{
/* parse number string */
- if (strnlen(q_arg, MAX_BASENAME_SZ) > MAX_BASENAME_SZ)
+ if (strnlen(q_arg, PATH_MAX) > PATH_MAX)
return -1;
else
- snprintf((char*)&dev_basename, MAX_BASENAME_SZ, "%s", q_arg);
+ snprintf((char *)&socket_file, PATH_MAX, "%s", q_arg);
return 0;
}
@@ -462,7 +459,7 @@ us_vhost_usage(const char *prgname)
RTE_LOG(INFO, VHOST_CONFIG, "%s [EAL options] -- -p PORTMASK\n"
" --vm2vm [0|1|2]\n"
" --rx_retry [0|1] --mergeable [0|1] --stats [0-N]\n"
- " --dev-basename <name>\n"
+ " --socket-file <path>\n"
" --nb-devices ND\n"
" -p PORTMASK: Set mask for ports to be used by application\n"
" --vm2vm [0|1|2]: disable/software(default)/hardware vm2vm comms\n"
@@ -472,7 +469,7 @@ us_vhost_usage(const char *prgname)
" --mergeable [0|1]: disable(default)/enable RX mergeable buffers\n"
" --vlan-strip [0|1]: disable/enable(default) RX VLAN strip on host\n"
" --stats [0-N]: 0: Disable stats, N: Time in seconds to print stats\n"
- " --dev-basename: The basename to be used for the character device.\n"
+ " --socket-file: The path of the socket file.\n"
" --tx-csum [0|1] disable/enable TX checksum offload.\n"
" --tso [0|1] disable/enable TCP segment offload.\n"
" --client register a vhost-user socket as client mode.\n",
@@ -497,7 +494,7 @@ us_vhost_parse_args(int argc, char **argv)
{"mergeable", required_argument, NULL, 0},
{"vlan-strip", required_argument, NULL, 0},
{"stats", required_argument, NULL, 0},
- {"dev-basename", required_argument, NULL, 0},
+ {"socket-file", required_argument, NULL, 0},
{"tx-csum", required_argument, NULL, 0},
{"tso", required_argument, NULL, 0},
{"client", no_argument, &client_mode, 1},
@@ -638,7 +635,8 @@ us_vhost_parse_args(int argc, char **argv)
if (!strncmp(long_option[option_index].name, "stats", MAX_LONG_OPT_SZ)) {
ret = parse_num_opt(optarg, INT32_MAX);
if (ret == -1) {
- RTE_LOG(INFO, VHOST_CONFIG, "Invalid argument for stats [0..N]\n");
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "Invalid argument for stats [0..N]\n");
us_vhost_usage(prgname);
return -1;
} else {
@@ -646,10 +644,13 @@ us_vhost_parse_args(int argc, char **argv)
}
}
- /* Set character device basename. */
- if (!strncmp(long_option[option_index].name, "dev-basename", MAX_LONG_OPT_SZ)) {
- if (us_vhost_parse_basename(optarg) == -1) {
- RTE_LOG(INFO, VHOST_CONFIG, "Invalid argument for character device basename (Max %d characters)\n", MAX_BASENAME_SZ);
+ /* Set socket file path. */
+ if (!strncmp(long_option[option_index].name,
+ "socket-file", MAX_LONG_OPT_SZ)) {
+ if (us_vhost_parse_socket_path(optarg) == -1) {
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "Invalid argument for socket name (Max %d characters)\n",
+ PATH_MAX);
us_vhost_usage(prgname);
return -1;
}
@@ -1345,7 +1346,7 @@ static void
sigint_handler(__rte_unused int signum)
{
/* Unregister vhost driver. */
- int ret = rte_vhost_driver_unregister((char *)&dev_basename);
+ int ret = rte_vhost_driver_unregister((char *)&socket_file);
if (ret != 0)
rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
exit(0);
@@ -1509,8 +1510,8 @@ main(int argc, char *argv[])
if (client_mode)
flags |= RTE_VHOST_USER_CLIENT;
- /* Register vhost(cuse or user) driver to handle vhost messages. */
- ret = rte_vhost_driver_register(dev_basename, flags);
+ /* Register vhost user driver to handle vhost messages. */
+ ret = rte_vhost_driver_register(socket_file, flags);
if (ret != 0)
rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
2016-08-18 8:01 ` Yuanhan Liu
2016-08-18 8:27 ` Maxime Coquelin
@ 2016-08-20 10:11 ` Jiayu Hu
2016-08-23 6:05 ` Yuanhan Liu
2 siblings, 1 reply; 14+ messages in thread
From: Jiayu Hu @ 2016-08-20 10:11 UTC (permalink / raw)
To: dev; +Cc: maxime.coquelin, yuanhan.liu, Jiayu Hu
When examples/vhost runs in client mode, only one QEMU can be connected.
This is because that examples/vhost just supports one socket file. This
patch is to add multiple sockets support for examples/vhost.
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
examples/vhost/main.c | 45 +++++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index a718577..71f6d92 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -136,8 +136,9 @@ static uint32_t burst_rx_delay_time = BURST_RX_WAIT_US;
/* Specify the number of retries on RX. */
static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
-/* Socket file path. Can be set by user */
-static char socket_file[PATH_MAX] = "vhost-net";
+/* Socket file paths. Can be set by user */
+static char *socket_files;
+static int nb_sockets;
/* empty vmdq configuration structure. Filled in programatically */
static struct rte_eth_conf vmdq_conf_default = {
@@ -395,11 +396,12 @@ static int
us_vhost_parse_socket_path(const char *q_arg)
{
/* parse number string */
-
if (strnlen(q_arg, PATH_MAX) > PATH_MAX)
return -1;
- else
- snprintf((char *)&socket_file, PATH_MAX, "%s", q_arg);
+
+ socket_files = realloc(socket_files, PATH_MAX * (nb_sockets + 1));
+ snprintf(socket_files + nb_sockets * PATH_MAX, PATH_MAX, "%s", q_arg);
+ nb_sockets++;
return 0;
}
@@ -1341,14 +1343,27 @@ print_stats(void)
}
}
+static void
+unregister_drivers(int socket_num)
+{
+ int i, ret;
+
+ for (i = 0; i < socket_num; i++) {
+ ret = rte_vhost_driver_unregister(socket_files + i * PATH_MAX);
+ if (ret != 0)
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "Fail to unregister vhost driver for %s.\n",
+ socket_files + i * PATH_MAX);
+ }
+}
+
/* 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 *)&socket_file);
- if (ret != 0)
- rte_exit(EXIT_FAILURE, "vhost driver unregister failure.\n");
+ unregister_drivers(nb_sockets);
+
exit(0);
}
@@ -1412,7 +1427,7 @@ main(int argc, char *argv[])
{
unsigned lcore_id, core_id = 0;
unsigned nb_ports, valid_num_ports;
- int ret;
+ int ret, i;
uint8_t portid;
static pthread_t tid;
char thread_name[RTE_MAX_THREAD_NAME_LEN];
@@ -1511,9 +1526,15 @@ main(int argc, char *argv[])
flags |= RTE_VHOST_USER_CLIENT;
/* Register vhost user driver to handle vhost messages. */
- ret = rte_vhost_driver_register(socket_file, flags);
- if (ret != 0)
- rte_exit(EXIT_FAILURE, "vhost driver register failure.\n");
+ for (i = 0; i < nb_sockets; i++) {
+ ret = rte_vhost_driver_register
+ (socket_files + i * PATH_MAX, flags);
+ if (ret != 0) {
+ unregister_drivers(i);
+ rte_exit(EXIT_FAILURE,
+ "vhost driver register failure.\n");
+ }
+ }
rte_vhost_driver_callback_register(&virtio_net_device_ops);
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename
2016-08-20 10:10 ` Jiayu Hu
@ 2016-08-23 6:01 ` Yuanhan Liu
0 siblings, 0 replies; 14+ messages in thread
From: Yuanhan Liu @ 2016-08-23 6:01 UTC (permalink / raw)
To: Jiayu Hu; +Cc: dev, maxime.coquelin
On Sat, Aug 20, 2016 at 06:10:33AM -0400, Jiayu Hu wrote:
> In examples/vhost, "dev-basename" is a program option, which is to set
> the vhost-net socket used by vhost-user, or the character device used
> by vhost-cuse. Since vhost-cuse should be dropped, and "dev-basename"
> is not a suitable name for the vhost-net socket. Therefore, this patch
> is to change this option name for examples/vhost.
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Applied to dpdk-next-virtio.
However, I got few more minor notes for you:
- about --in-reply-to
You should send all patches once, and link it to the former
version's cover letter.
- You should add v2 prefix. This could be done by:
$ git format-patch -v 2 ...
--yliu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files
2016-08-20 10:11 ` Jiayu Hu
@ 2016-08-23 6:05 ` Yuanhan Liu
0 siblings, 0 replies; 14+ messages in thread
From: Yuanhan Liu @ 2016-08-23 6:05 UTC (permalink / raw)
To: Jiayu Hu; +Cc: dev, maxime.coquelin
On Sat, Aug 20, 2016 at 06:11:36AM -0400, Jiayu Hu wrote:
> When examples/vhost runs in client mode, only one QEMU can be connected.
> This is because that examples/vhost just supports one socket file. This
> patch is to add multiple sockets support for examples/vhost.
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Applied to dpdk-next-virtio.
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Please don't add it when it's not explicitly given. You got some
comments from some one doesn't mean you got the "Reviewed-by".
So, I dropped it while applying it.
> ---
And, you should put v2 change log here, under the line of "---"
--yliu
> examples/vhost/main.c | 45 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 33 insertions(+), 12 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-08-23 5:55 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16 16:14 [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and support multiple socket files Jiayu Hu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 1/2] examples/vhost: rename dev-basename Jiayu Hu
2016-08-18 8:22 ` Maxime Coquelin
2016-08-18 8:35 ` Yuanhan Liu
2016-08-18 8:29 ` Maxime Coquelin
2016-08-20 10:10 ` Jiayu Hu
2016-08-23 6:01 ` Yuanhan Liu
2016-08-16 16:14 ` [dpdk-dev] [PATCH 2/2] examples/vhost: support multiple socket files Jiayu Hu
2016-08-18 8:01 ` Yuanhan Liu
2016-08-18 8:27 ` Maxime Coquelin
2016-08-18 8:43 ` Yuanhan Liu
2016-08-20 10:11 ` Jiayu Hu
2016-08-23 6:05 ` Yuanhan Liu
2016-08-20 10:08 ` [dpdk-dev] [PATCH 0/2] examples/vhost: rename a CLI option and " Jiayu Hu
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).