Hi , > -----Original Message----- > From: Richardson, Bruce > Sent: Thursday, October 30, 2014 6:10 PM > To: Ouyang, Changchun > Cc: Xie, Huawei; dev at 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 at 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 at dpdk.org] On Behalf Of Ouyang > > > Changchun > > > > Sent: Sunday, October 26, 2014 8:46 PM > > > > To: dev at 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 at 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
