* [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
* 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 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 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
* [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
* 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
* [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 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 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 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 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
* [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
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).