DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Ouyang, Changchun" <changchun.ouyang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config	VMDQ offload register for multicast feature
Date: Thu, 30 Oct 2014 10:10:02 +0000
Message-ID: <20141030101002.GC4460@bricha3-MOBL3> (raw)
In-Reply-To: <F52918179C57134FAEC9EA62FA2F96251187ECAC@shsmsx102.ccr.corp.intel.com>

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

  reply	other threads:[~2014-10-30 10:01 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-25  2:09 [dpdk-dev] [PATCH 0/5] Support virtio " Ouyang Changchun
2014-08-25  2:09 ` [dpdk-dev] [PATCH 1/5] ethdev: Add new config field to config VMDQ offload register Ouyang Changchun
2014-08-25  2:09 ` [dpdk-dev] [PATCH 2/5] e1000: config VMDQ offload register to receive multicast packet Ouyang Changchun
2014-08-25  2:09 ` [dpdk-dev] [PATCH 3/5] examples/vhost: enable promisc mode and config VMDQ offload register for multicast feature Ouyang Changchun
2014-08-25  2:09 ` [dpdk-dev] [PATCH 4/5] virtio: New API to enable/disable multicast and promisc mode Ouyang Changchun
2014-08-26  0:13   ` Stephen Hemminger
2014-08-26  1:05     ` Ouyang, Changchun
2014-08-26  1:26       ` Stephen Hemminger
2014-08-25  2:09 ` [dpdk-dev] [PATCH 5/5] examples/vmdq: set default value to rx mode Ouyang Changchun
2014-09-24  9:07 ` [dpdk-dev] [PATCH 0/5] Support virtio multicast feature Zhang, XiaonanX
2014-10-27  3:45 ` [dpdk-dev] [PATCH v2 " Ouyang Changchun
2014-10-27  3:45   ` [dpdk-dev] [PATCH v2 1/5] ethdev: Add new config field to config VMDQ offload register Ouyang Changchun
2014-10-27  3:45   ` [dpdk-dev] [PATCH v2 2/5] e1000: config VMDQ offload register to receive multicast packet Ouyang Changchun
2014-10-27  3:45   ` [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config VMDQ offload register for multicast feature Ouyang Changchun
2014-10-29 23:26     ` Xie, Huawei
2014-10-30  0:49       ` Ouyang, Changchun
2014-10-30 10:10         ` Bruce Richardson [this message]
2014-10-30 15:18           ` Ouyang, Changchun
2015-01-08 10:07     ` Xie, Huawei
2015-01-09  5:21       ` Ouyang, Changchun
2015-01-14 16:37       ` Thomas Monjalon
2015-01-23  3:20         ` Xie, Huawei
2014-10-27  3:45   ` [dpdk-dev] [PATCH v2 4/5] virtio: New API to enable/disable multicast and promisc mode Ouyang Changchun
2014-10-27  3:45   ` [dpdk-dev] [PATCH v2 5/5] examples/vmdq: set default value to rx mode Ouyang Changchun
2014-10-31  5:19   ` [dpdk-dev] [PATCH v3 0/5] Support virtio multicast feature Ouyang Changchun
2014-10-31  5:19     ` [dpdk-dev] [PATCH v3 1/5] ethdev: add vmdq rx mode Ouyang Changchun
2014-11-06 13:55       ` Thomas Monjalon
2014-11-08  2:13         ` Ouyang, Changchun
2014-10-31  5:19     ` [dpdk-dev] [PATCH v3 2/5] igb: Config VM offload register Ouyang Changchun
2014-10-31  5:19     ` [dpdk-dev] [PATCH v3 3/5] ixgbe: Config PFVML2FLT register Ouyang Changchun
2014-11-06 13:57       ` Thomas Monjalon
2014-10-31  5:19     ` [dpdk-dev] [PATCH v3 4/5] virtio: New API for promisc and allmulticast Ouyang Changchun
2014-11-06 13:59       ` Thomas Monjalon
2014-10-31  5:19     ` [dpdk-dev] [PATCH v3 5/5] vhost: enable promisc mode and multicast Ouyang Changchun
2014-11-08  4:26     ` [dpdk-dev] [PATCH v4 0/5] Support virtio multicast feature Ouyang Changchun
2014-11-08  4:26       ` [dpdk-dev] [PATCH v4 1/5] ethdev: Add vmdq rx mode Ouyang Changchun
2014-11-08  4:26       ` [dpdk-dev] [PATCH v4 2/5] igb: Config VM offload register Ouyang Changchun
2014-11-08  4:26       ` [dpdk-dev] [PATCH v4 3/5] ixgbe: Configure Rx mode for VMDQ Ouyang Changchun
2014-11-08  4:26       ` [dpdk-dev] [PATCH v4 4/5] virtio: Support promiscuous and allmulticast Ouyang Changchun
2014-11-08  4:26       ` [dpdk-dev] [PATCH v4 5/5] vhost: Enable promisc mode and multicast Ouyang Changchun
2014-11-11 23:16       ` [dpdk-dev] [PATCH v4 0/5] Support virtio multicast feature Thomas Monjalon
2014-11-12  0:29         ` Ouyang, Changchun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141030101002.GC4460@bricha3-MOBL3 \
    --to=bruce.richardson@intel.com \
    --cc=changchun.ouyang@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git