From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id A932E7E0B for ; Thu, 30 Oct 2014 16:10:10 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 30 Oct 2014 08:18:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,286,1413270000"; d="scan'208";a="623534179" Received: from pgsmsx103.gar.corp.intel.com ([10.221.44.82]) by fmsmga002.fm.intel.com with ESMTP; 30 Oct 2014 08:18:56 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by PGSMSX103.gar.corp.intel.com (10.221.44.82) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 30 Oct 2014 23:18:52 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.156]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.44]) with mapi id 14.03.0195.001; Thu, 30 Oct 2014 23:18:50 +0800 From: "Ouyang, Changchun" To: "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH v2 3/5] vhost: enable promisc mode and config VMDQ offload register for multicast feature Thread-Index: AQHP88+7jLtLnqyHnUS1Zd1gXyKIqJxHzXbAgAAYJACAANoy0A== Date: Thu, 30 Oct 2014 15:18:49 +0000 Message-ID: References: <1408932572-10343-1-git-send-email-changchun.ouyang@intel.com> <1414381533-30370-1-git-send-email-changchun.ouyang@intel.com> <1414381533-30370-4-git-send-email-changchun.ouyang@intel.com> <20141030101002.GC4460@bricha3-MOBL3> In-Reply-To: <20141030101002.GC4460@bricha3-MOBL3> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Oct 2014 15:10:11 -0000 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 >=20 > 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 > > > > --- > > > > 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 =3D 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 =3D 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 = =3D > { > > > > .enable_default_pool =3D 0, > > > > .default_pool =3D 0, > > > > .nb_pool_maps =3D 0, > > > > + .rx_mode =3D 0, > > > > .pool_map =3D {{0, 0},}, > > > > }, > > > > }, > > > Same as above, do we need to initialize static var? > > Response same as above, no harm to initialize them to 0. >=20 > 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 =3D { .default_pool =3D 0 }, the user just has a single= line to > look at and can know that the rest of the values will be zero. Furthermor= e, > having the initializers all spelled out like that uses up screen space, w= hich, 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 ne= ed. >=20 > It's not a big deal either way, but that's my opinion on the matter. :-) >=20 Seems at least you 2 guys has strong objection on the initializing 0 to a s= tatic var. Ok for updating, and don't want to spend much time in arguing on the tiny s= tuff. :-) Pls waiting for my next version update. Changchun