[dpdk-dev,v2,3/5] vhost: enable promisc mode and config VMDQ offload register for multicast feature
Message ID | 1414381533-30370-4-git-send-email-changchun.ouyang@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 04D4F7FE6; Mon, 27 Oct 2014 04:37:13 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 6171A7FE2 for <dev@dpdk.org>; Mon, 27 Oct 2014 04:37:08 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 26 Oct 2014 20:45:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="406476886" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by FMSMGA003.fm.intel.com with ESMTP; 26 Oct 2014 20:37:51 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id s9R3jj0A010806; Mon, 27 Oct 2014 11:45:45 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id s9R3jh0l031047; Mon, 27 Oct 2014 11:45:45 +0800 Received: (from couyang@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id s9R3jh6G031043; Mon, 27 Oct 2014 11:45:43 +0800 From: Ouyang Changchun <changchun.ouyang@intel.com> To: dev@dpdk.org Date: Mon, 27 Oct 2014 11:45:31 +0800 Message-Id: <1414381533-30370-4-git-send-email-changchun.ouyang@intel.com> X-Mailer: git-send-email 1.7.12.2 In-Reply-To: <1414381533-30370-1-git-send-email-changchun.ouyang@intel.com> References: <1408932572-10343-1-git-send-email-changchun.ouyang@intel.com> <1414381533-30370-1-git-send-email-changchun.ouyang@intel.com> Subject: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config VMDQ offload register for multicast feature X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Ouyang Changchun
Oct. 27, 2014, 3:45 a.m. UTC
This patch is to let vhost receive and forward multicast and broadcast packets,
add promiscuous option into command line; and set VMDQ RX mode as:
ETH_VMDQ_ACCEPT_BROADCAST|ETH_VMDQ_ACCEPT_MULTICAST if promisc mode is on.
Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
examples/vhost/main.c | 25 ++++++++++++++++++++++---
lib/librte_vhost/virtio-net.c | 4 +++-
2 files changed, 25 insertions(+), 4 deletions(-)
Comments
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang Changchun > Sent: Sunday, October 26, 2014 8:46 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config > VMDQ offload register for multicast feature > > This patch is to let vhost receive and forward multicast and broadcast packets, > add promiscuous option into command line; and set VMDQ RX mode as: > ETH_VMDQ_ACCEPT_BROADCAST|ETH_VMDQ_ACCEPT_MULTICAST if promisc > mode is on. > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > --- > examples/vhost/main.c | 25 ++++++++++++++++++++++--- > lib/librte_vhost/virtio-net.c | 4 +++- > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c > index 291128e..c4947f7 100644 > --- a/examples/vhost/main.c > +++ b/examples/vhost/main.c > @@ -161,6 +161,9 @@ > /* mask of enabled ports */ > static uint32_t enabled_port_mask = 0; > > +/* Ports set in promiscuous mode off by default. */ comment is confusing > +static uint32_t promiscuous_on; > + > /*Number of switching cores enabled*/ > static uint32_t num_switching_cores = 0; Don't initialize static variables to zero/NULL > > @@ -274,6 +277,7 @@ static struct rte_eth_conf vmdq_conf_default = { > .enable_default_pool = 0, > .default_pool = 0, > .nb_pool_maps = 0, > + .rx_mode = 0, > .pool_map = {{0, 0},}, > }, > }, Same as above, do we need to initialize static var? > @@ -364,13 +368,15 @@ static inline int > get_eth_conf(struct rte_eth_conf *eth_conf, uint32_t num_devices) > { > struct rte_eth_vmdq_rx_conf conf; > + struct rte_eth_vmdq_rx_conf *def_conf = > + &vmdq_conf_default.rx_adv_conf.vmdq_rx_conf; > unsigned i; > > memset(&conf, 0, sizeof(conf)); > conf.nb_queue_pools = (enum rte_eth_nb_pools)num_devices; > conf.nb_pool_maps = num_devices; > - conf.enable_loop_back = > - > vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.enable_loop_back; > + conf.enable_loop_back = def_conf->enable_loop_back; > + conf.rx_mode = def_conf->rx_mode; > > for (i = 0; i < conf.nb_pool_maps; i++) { > conf.pool_map[i].vlan_id = vlan_tags[ i ]; > @@ -468,6 +474,9 @@ port_init(uint8_t port) > return retval; > } > > + if (promiscuous_on) > + rte_eth_promiscuous_enable(port); > + > rte_eth_macaddr_get(port, &vmdq_ports_eth_addr[port]); > RTE_LOG(INFO, VHOST_PORT, "Max virtio devices supported: %u\n", > num_devices); > RTE_LOG(INFO, VHOST_PORT, "Port %u > MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8 > @@ -598,7 +607,8 @@ us_vhost_parse_args(int argc, char **argv) > }; > > /* Parse command line */ > - while ((opt = getopt_long(argc, argv, "p:",long_option, > &option_index)) != EOF) { > + while ((opt = getopt_long(argc, argv, "p:P", > + long_option, &option_index)) != EOF) { > switch (opt) { > /* Portmask */ > case 'p': > @@ -610,6 +620,15 @@ us_vhost_parse_args(int argc, char **argv) > } > break; > > + case 'P': > + promiscuous_on = 1; > + > vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode = > + ETH_VMDQ_ACCEPT_BROADCAST | > + ETH_VMDQ_ACCEPT_MULTICAST; > + rte_vhost_feature_enable(1ULL << > VIRTIO_NET_F_CTRL_RX); Alignment? > + > + break; > + > case 0: > /* Enable/disable vm2vm comms. */ > if (!strncmp(long_option[option_index].name, "vm2vm", > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > index 27ba175..744156c 100644 > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -68,7 +68,9 @@ static struct virtio_net_device_ops const *notify_ops; > static struct virtio_net_config_ll *ll_root; > > /* Features supported by this application. RX merge buffers are enabled by > default. */ > -#define VHOST_SUPPORTED_FEATURES (1ULL << VIRTIO_NET_F_MRG_RXBUF) > +#define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) > | \ > + (1ULL << VIRTIO_NET_F_CTRL_RX)) > + > static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES; > > /* Line size for reading maps file. */ > -- > 1.8.4.2
Hi, > -----Original Message----- > From: Xie, Huawei > Sent: Thursday, October 30, 2014 7:26 AM > To: Ouyang, Changchun; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > config VMDQ offload register for multicast feature > > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang > Changchun > > Sent: Sunday, October 26, 2014 8:46 PM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > > config VMDQ offload register for multicast feature > > > > This patch is to let vhost receive and forward multicast and broadcast > > packets, add promiscuous option into command line; and set VMDQ RX > mode as: > > ETH_VMDQ_ACCEPT_BROADCAST|ETH_VMDQ_ACCEPT_MULTICAST if > promisc mode is > > on. > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > --- > > examples/vhost/main.c | 25 ++++++++++++++++++++++--- > > lib/librte_vhost/virtio-net.c | 4 +++- > > 2 files changed, 25 insertions(+), 4 deletions(-) > > > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c index > > 291128e..c4947f7 100644 > > --- a/examples/vhost/main.c > > +++ b/examples/vhost/main.c > > @@ -161,6 +161,9 @@ > > /* mask of enabled ports */ > > static uint32_t enabled_port_mask = 0; > > > > +/* Ports set in promiscuous mode off by default. */ > comment is confusing Don't think it is confusing, But I can refine it a bit. > > +static uint32_t promiscuous_on; > > + > > /*Number of switching cores enabled*/ static uint32_t > > num_switching_cores = 0; > Don't initialize static variables to zero/NULL I don't touch this line in my patch, and initialization to 0 is not an issue here. > > > > @@ -274,6 +277,7 @@ static struct rte_eth_conf vmdq_conf_default = { > > .enable_default_pool = 0, > > .default_pool = 0, > > .nb_pool_maps = 0, > > + .rx_mode = 0, > > .pool_map = {{0, 0},}, > > }, > > }, > Same as above, do we need to initialize static var? Response same as above, no harm to initialize them to 0. > > > @@ -364,13 +368,15 @@ static inline int get_eth_conf(struct > > rte_eth_conf *eth_conf, uint32_t num_devices) { > > struct rte_eth_vmdq_rx_conf conf; > > + struct rte_eth_vmdq_rx_conf *def_conf = > > + &vmdq_conf_default.rx_adv_conf.vmdq_rx_conf; > > unsigned i; > > > > memset(&conf, 0, sizeof(conf)); > > conf.nb_queue_pools = (enum rte_eth_nb_pools)num_devices; > > conf.nb_pool_maps = num_devices; > > - conf.enable_loop_back = > > - > > vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.enable_loop_back; > > + conf.enable_loop_back = def_conf->enable_loop_back; > > + conf.rx_mode = def_conf->rx_mode; > > > > for (i = 0; i < conf.nb_pool_maps; i++) { > > conf.pool_map[i].vlan_id = vlan_tags[ i ]; @@ -468,6 +474,9 > @@ > > port_init(uint8_t port) > > return retval; > > } > > > > + if (promiscuous_on) > > + rte_eth_promiscuous_enable(port); > > + > > rte_eth_macaddr_get(port, &vmdq_ports_eth_addr[port]); > > RTE_LOG(INFO, VHOST_PORT, "Max virtio devices supported: %u\n", > > num_devices); > > RTE_LOG(INFO, VHOST_PORT, "Port %u > > MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8 > > @@ -598,7 +607,8 @@ us_vhost_parse_args(int argc, char **argv) > > }; > > > > /* Parse command line */ > > - while ((opt = getopt_long(argc, argv, "p:",long_option, > > &option_index)) != EOF) { > > + while ((opt = getopt_long(argc, argv, "p:P", > > + long_option, &option_index)) != EOF) { > > switch (opt) { > > /* Portmask */ > > case 'p': > > @@ -610,6 +620,15 @@ us_vhost_parse_args(int argc, char **argv) > > } > > break; > > > > + case 'P': > > + promiscuous_on = 1; > > + > > vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode = > > + ETH_VMDQ_ACCEPT_BROADCAST | > > + ETH_VMDQ_ACCEPT_MULTICAST; > > + rte_vhost_feature_enable(1ULL << > > VIRTIO_NET_F_CTRL_RX); > > Alignment? Checked it again, already aligned, your mail display should has some issue. :-) > > + > > + break; > > + > > case 0: > > /* Enable/disable vm2vm comms. */ > > if (!strncmp(long_option[option_index].name, > "vm2vm", diff --git > > a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c index > > 27ba175..744156c 100644 > > --- a/lib/librte_vhost/virtio-net.c > > +++ b/lib/librte_vhost/virtio-net.c > > @@ -68,7 +68,9 @@ static struct virtio_net_device_ops const *notify_ops; > > static struct virtio_net_config_ll *ll_root; > > > > /* Features supported by this application. RX merge buffers are > > enabled by default. */ -#define VHOST_SUPPORTED_FEATURES (1ULL << > > VIRTIO_NET_F_MRG_RXBUF) > > +#define VHOST_SUPPORTED_FEATURES ((1ULL << > VIRTIO_NET_F_MRG_RXBUF) > > | \ > > + (1ULL << VIRTIO_NET_F_CTRL_RX)) > > + > > static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES; > > > > /* Line size for reading maps file. */ > > -- > > 1.8.4.2 Thanks Changchun
On Thu, Oct 30, 2014 at 12:49:13AM +0000, Ouyang, Changchun wrote: > Hi, > > > -----Original Message----- > > From: Xie, Huawei > > Sent: Thursday, October 30, 2014 7:26 AM > > To: Ouyang, Changchun; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > > config VMDQ offload register for multicast feature > > > > > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang > > Changchun > > > Sent: Sunday, October 26, 2014 8:46 PM > > > To: dev@dpdk.org > > > Subject: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > > > config VMDQ offload register for multicast feature > > > > > > This patch is to let vhost receive and forward multicast and broadcast > > > packets, add promiscuous option into command line; and set VMDQ RX > > mode as: > > > ETH_VMDQ_ACCEPT_BROADCAST|ETH_VMDQ_ACCEPT_MULTICAST if > > promisc mode is > > > on. > > > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > > --- > > > examples/vhost/main.c | 25 ++++++++++++++++++++++--- > > > lib/librte_vhost/virtio-net.c | 4 +++- > > > 2 files changed, 25 insertions(+), 4 deletions(-) > > > > > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c index > > > 291128e..c4947f7 100644 > > > --- a/examples/vhost/main.c > > > +++ b/examples/vhost/main.c > > > @@ -161,6 +161,9 @@ > > > /* mask of enabled ports */ > > > static uint32_t enabled_port_mask = 0; > > > > > > +/* Ports set in promiscuous mode off by default. */ > > comment is confusing > Don't think it is confusing, > But I can refine it a bit. > > > +static uint32_t promiscuous_on; > > > + > > > /*Number of switching cores enabled*/ static uint32_t > > > num_switching_cores = 0; > > Don't initialize static variables to zero/NULL > > I don't touch this line in my patch, and initialization to 0 is not an issue here. > > > > > > > @@ -274,6 +277,7 @@ static struct rte_eth_conf vmdq_conf_default = { > > > .enable_default_pool = 0, > > > .default_pool = 0, > > > .nb_pool_maps = 0, > > > + .rx_mode = 0, > > > .pool_map = {{0, 0},}, > > > }, > > > }, > > Same as above, do we need to initialize static var? > Response same as above, no harm to initialize them to 0. With this one, I would actually disagree. With something like this is it harder to read, since you have a whole list of values given here. The reader has to scan through the list of values to check in case any of them are non-zero. If there is no initializer, or just a single-value initializer, e.g. ... vmdq_conf_default = { .default_pool = 0 }, the user just has a single line to look at and can know that the rest of the values will be zero. Furthermore, having the initializers all spelled out like that uses up screen space, which, if the initializers weren't there would be filled instead by code which is meaningful. More meaningful code on screen again makes things easier for the reader, as they have to do less scrolling to find the context they need. It's not a big deal either way, but that's my opinion on the matter. :-) /Bruce
Hi , > -----Original Message----- > From: Richardson, Bruce > Sent: Thursday, October 30, 2014 6:10 PM > To: Ouyang, Changchun > Cc: Xie, Huawei; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > config VMDQ offload register for multicast feature > > On Thu, Oct 30, 2014 at 12:49:13AM +0000, Ouyang, Changchun wrote: > > Hi, > > > > > -----Original Message----- > > > From: Xie, Huawei > > > Sent: Thursday, October 30, 2014 7:26 AM > > > To: Ouyang, Changchun; dev@dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode > > > and config VMDQ offload register for multicast feature > > > > > > > > > > > > > -----Original Message----- > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang > > > Changchun > > > > Sent: Sunday, October 26, 2014 8:46 PM > > > > To: dev@dpdk.org > > > > Subject: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > > > > config VMDQ offload register for multicast feature > > > > > > > > This patch is to let vhost receive and forward multicast and > > > > broadcast packets, add promiscuous option into command line; and > > > > set VMDQ RX > > > mode as: > > > > ETH_VMDQ_ACCEPT_BROADCAST|ETH_VMDQ_ACCEPT_MULTICAST if > > > promisc mode is > > > > on. > > > > > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > > > --- > > > > examples/vhost/main.c | 25 ++++++++++++++++++++++--- > > > > lib/librte_vhost/virtio-net.c | 4 +++- > > > > 2 files changed, 25 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c index > > > > 291128e..c4947f7 100644 > > > > --- a/examples/vhost/main.c > > > > +++ b/examples/vhost/main.c > > > > @@ -161,6 +161,9 @@ > > > > /* mask of enabled ports */ > > > > static uint32_t enabled_port_mask = 0; > > > > > > > > +/* Ports set in promiscuous mode off by default. */ > > > comment is confusing > > Don't think it is confusing, > > But I can refine it a bit. > > > > +static uint32_t promiscuous_on; > > > > + > > > > /*Number of switching cores enabled*/ static uint32_t > > > > num_switching_cores = 0; > > > Don't initialize static variables to zero/NULL > > > > I don't touch this line in my patch, and initialization to 0 is not an issue here. > > > > > > > > > > @@ -274,6 +277,7 @@ static struct rte_eth_conf vmdq_conf_default = > { > > > > .enable_default_pool = 0, > > > > .default_pool = 0, > > > > .nb_pool_maps = 0, > > > > + .rx_mode = 0, > > > > .pool_map = {{0, 0},}, > > > > }, > > > > }, > > > Same as above, do we need to initialize static var? > > Response same as above, no harm to initialize them to 0. > > With this one, I would actually disagree. With something like this is it harder > to read, since you have a whole list of values given here. The reader has to > scan through the list of values to check in case any of them are non-zero. If > there is no initializer, or just a single-value initializer, e.g. ... > vmdq_conf_default = { .default_pool = 0 }, the user just has a single line to > look at and can know that the rest of the values will be zero. Furthermore, > having the initializers all spelled out like that uses up screen space, which, if > the initializers weren't there would be filled instead by code which is > meaningful. More meaningful code on screen again makes things easier for > the reader, as they have to do less scrolling to find the context they need. > > It's not a big deal either way, but that's my opinion on the matter. :-) > Seems at least you 2 guys has strong objection on the initializing 0 to a static var. Ok for updating, and don't want to spend much time in arguing on the tiny stuff. :-) Pls waiting for my next version update. Changchun
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > index 27ba175..744156c 100644 > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -68,7 +68,9 @@ static struct virtio_net_device_ops const *notify_ops; > static struct virtio_net_config_ll *ll_root; > > /* Features supported by this application. RX merge buffers are enabled by > default. */ > -#define VHOST_SUPPORTED_FEATURES (1ULL << VIRTIO_NET_F_MRG_RXBUF) > +#define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) > | \ > + (1ULL << VIRTIO_NET_F_CTRL_RX)) > + CTRL_RX is dependent on CTRL_VQ. CTRL_VQ should be enabled if CTRL_RX is enabled. Observed that virtio-net driver will crash if CTRL_VQ isn't enabled in vhost-user case. /* Caller should know better */ BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) || (out + in > VIRTNET_SEND_COMMAND_SG_MAX)); > static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES; > > /* Line size for reading maps file. */ > -- > 1.8.4.2
Hi Huawei, > -----Original Message----- > From: Xie, Huawei > Sent: Thursday, January 8, 2015 6:08 PM > To: Ouyang, Changchun; dev@dpdk.org > Cc: Tetsuya Mukawa > Subject: RE: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and > config VMDQ offload register for multicast feature > > > diff --git a/lib/librte_vhost/virtio-net.c > > b/lib/librte_vhost/virtio-net.c index 27ba175..744156c 100644 > > --- a/lib/librte_vhost/virtio-net.c > > +++ b/lib/librte_vhost/virtio-net.c > > @@ -68,7 +68,9 @@ static struct virtio_net_device_ops const *notify_ops; > > static struct virtio_net_config_ll *ll_root; > > > > /* Features supported by this application. RX merge buffers are > > enabled by default. */ -#define VHOST_SUPPORTED_FEATURES (1ULL << > > VIRTIO_NET_F_MRG_RXBUF) > > +#define VHOST_SUPPORTED_FEATURES ((1ULL << > VIRTIO_NET_F_MRG_RXBUF) > > | \ > > + (1ULL << VIRTIO_NET_F_CTRL_RX)) > > + > CTRL_RX is dependent on CTRL_VQ. > CTRL_VQ should be enabled if CTRL_RX is enabled. > Observed that virtio-net driver will crash if CTRL_VQ isn't enabled in vhost- > user case. > /* Caller should know better */ > BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) || > (out + in > VIRTNET_SEND_COMMAND_SG_MAX)); Thanks for identifying, after your patch sent out to fix it, I will act it. Changchun
Hi Huawei, 2015-01-08 10:07, Xie, Huawei: > CTRL_RX is dependent on CTRL_VQ. > CTRL_VQ should be enabled if CTRL_RX is enabled. > Observed that virtio-net driver will crash if CTRL_VQ isn't enabled in vhost-user case. > /* Caller should know better */ > BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) || > (out + in > VIRTNET_SEND_COMMAND_SG_MAX)); Do you plan to send a patch?
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, January 15, 2015 12:37 AM > To: dev@dpdk.org > Cc: Xie, Huawei; Ouyang, Changchun > Subject: Re: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config > VMDQ offload register for multicast feature > > Hi Huawei, > > 2015-01-08 10:07, Xie, Huawei: > > CTRL_RX is dependent on CTRL_VQ. > > CTRL_VQ should be enabled if CTRL_RX is enabled. > > Observed that virtio-net driver will crash if CTRL_VQ isn't enabled in vhost-user > case. > > /* Caller should know better */ > > BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) || > > (out + in > VIRTNET_SEND_COMMAND_SG_MAX)); > > Do you plan to send a patch? In the vhost-user patch, will temporarily turn this on. However, we don't create any control queues in vhost. Will investigate the root cause later. > > -- > Thomas
diff --git a/examples/vhost/main.c b/examples/vhost/main.c index 291128e..c4947f7 100644 --- a/examples/vhost/main.c +++ b/examples/vhost/main.c @@ -161,6 +161,9 @@ /* mask of enabled ports */ static uint32_t enabled_port_mask = 0; +/* Ports set in promiscuous mode off by default. */ +static uint32_t promiscuous_on; + /*Number of switching cores enabled*/ static uint32_t num_switching_cores = 0; @@ -274,6 +277,7 @@ static struct rte_eth_conf vmdq_conf_default = { .enable_default_pool = 0, .default_pool = 0, .nb_pool_maps = 0, + .rx_mode = 0, .pool_map = {{0, 0},}, }, }, @@ -364,13 +368,15 @@ static inline int get_eth_conf(struct rte_eth_conf *eth_conf, uint32_t num_devices) { struct rte_eth_vmdq_rx_conf conf; + struct rte_eth_vmdq_rx_conf *def_conf = + &vmdq_conf_default.rx_adv_conf.vmdq_rx_conf; unsigned i; memset(&conf, 0, sizeof(conf)); conf.nb_queue_pools = (enum rte_eth_nb_pools)num_devices; conf.nb_pool_maps = num_devices; - conf.enable_loop_back = - vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.enable_loop_back; + conf.enable_loop_back = def_conf->enable_loop_back; + conf.rx_mode = def_conf->rx_mode; for (i = 0; i < conf.nb_pool_maps; i++) { conf.pool_map[i].vlan_id = vlan_tags[ i ]; @@ -468,6 +474,9 @@ port_init(uint8_t port) return retval; } + if (promiscuous_on) + rte_eth_promiscuous_enable(port); + rte_eth_macaddr_get(port, &vmdq_ports_eth_addr[port]); RTE_LOG(INFO, VHOST_PORT, "Max virtio devices supported: %u\n", num_devices); RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8 @@ -598,7 +607,8 @@ us_vhost_parse_args(int argc, char **argv) }; /* Parse command line */ - while ((opt = getopt_long(argc, argv, "p:",long_option, &option_index)) != EOF) { + while ((opt = getopt_long(argc, argv, "p:P", + long_option, &option_index)) != EOF) { switch (opt) { /* Portmask */ case 'p': @@ -610,6 +620,15 @@ us_vhost_parse_args(int argc, char **argv) } break; + case 'P': + promiscuous_on = 1; + vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode = + ETH_VMDQ_ACCEPT_BROADCAST | + ETH_VMDQ_ACCEPT_MULTICAST; + rte_vhost_feature_enable(1ULL << VIRTIO_NET_F_CTRL_RX); + + break; + case 0: /* Enable/disable vm2vm comms. */ if (!strncmp(long_option[option_index].name, "vm2vm", diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c index 27ba175..744156c 100644 --- a/lib/librte_vhost/virtio-net.c +++ b/lib/librte_vhost/virtio-net.c @@ -68,7 +68,9 @@ static struct virtio_net_device_ops const *notify_ops; static struct virtio_net_config_ll *ll_root; /* Features supported by this application. RX merge buffers are enabled by default. */ -#define VHOST_SUPPORTED_FEATURES (1ULL << VIRTIO_NET_F_MRG_RXBUF) +#define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \ + (1ULL << VIRTIO_NET_F_CTRL_RX)) + static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES; /* Line size for reading maps file. */