From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <Jerin.Jacob@cavium.com>
Received: from NAM02-CY1-obe.outbound.protection.outlook.com
(mail-cys01nam02on0080.outbound.protection.outlook.com [104.47.37.80])
by dpdk.org (Postfix) with ESMTP id 4BEEB9E3
for <dev@dpdk.org>; Mon, 4 Jul 2016 14:16:19 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;
bh=lAbBPigceTD9fDlKPBVOhCP4s9ddLMl3SKL9oGaHXsk=;
b=c9NI1dt7rDHaRO8M/Ue82TkB5qoJmh0j9gPOYhMqpU3zl7aPS+WSC8Y+aI8NqahukrUCXHwWvYzDga57VWE5D3qpdJ0lY//N9Fz0rARM75UKaHS1DOEPsEP15ixBrMEsD38Np/OUOdM+vQldk8SNp+JEf2tsLuPFmcdsGvwTa9g=
Authentication-Results: spf=none (sender IP is )
smtp.mailfrom=Jerin.Jacob@cavium.com;
Received: from localhost.localdomain (122.167.47.22) by
BLUPR0701MB1714.namprd07.prod.outlook.com (10.163.85.140) with Microsoft SMTP
Server (TLS) id 15.1.534.14; Mon, 4 Jul 2016 12:16:14 +0000
Date: Mon, 4 Jul 2016 17:45:57 +0530
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
CC: <dev@dpdk.org>, <thomas.monjalon@6wind.com>, <bruce.richardson@intel.com>,
<jianbo.liu@linaro.org>, <huawei.xie@intel.com>
Message-ID: <20160704121556.GA5050@localhost.localdomain>
References: <1467028448-8914-1-git-send-email-jerin.jacob@caviumnetworks.com>
<1467371814-26754-1-git-send-email-jerin.jacob@caviumnetworks.com>
<1467371814-26754-2-git-send-email-jerin.jacob@caviumnetworks.com>
<20160704073648.GV2831@yliu-dev.sh.intel.com>
<20160704083626.GA12596@localhost.localdomain>
<20160704084232.GY2831@yliu-dev.sh.intel.com>
<20160704090754.GD12596@localhost.localdomain>
<20160704110225.GA2831@yliu-dev.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20160704110225.GA2831@yliu-dev.sh.intel.com>
User-Agent: Mutt/1.6.1 (2016-04-27)
X-Originating-IP: [122.167.47.22]
X-ClientProxiedBy: BM1PR01CA0056.INDPRD01.PROD.OUTLOOK.COM (10.163.199.28) To
BLUPR0701MB1714.namprd07.prod.outlook.com (10.163.85.140)
X-MS-Office365-Filtering-Correlation-Id: 35419d72-1c02-4971-b0b9-08d3a405002d
X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714;
2:OnkRs2BTgsjsHi765QpwArAvMZ12zFo374tTioWTyqd/YWJxsDbyj8TP6kYhqrnYQA8Fn28RQhdgXvbHrfd1S9YcUCRJxPyOsdzcGjhX+alq46MHGcA/BZxmmhL9xThDh0LiNd1FAonesrk+z7Vl3AtnzBPM2Lj1RpiDcCipT7fwE86GI4hq8twKvlbNFTBK;
3:IcRoYUHSp/tvINhLIIEZlLVFZ+4E2e2jTO1J/Ka+k6Ft6CEls6jowPQImYe2UcCoWFmRTqM4dJFuqXu/z3WwZxdXSJtjouEzE08v3XU7QT2OmQGY9asHoYq8nzq/jDly;
25:B52K7ozBCBO8MluwP5cs+Mhi0NUN1wJxjZ0LvcxTpUK2UwExActbL9YBvzsF6eFkHPmqssofHakTrdJEd89vV7x0XBmT7WxGoOmwh213Xoa9st8IK3Si0Q6//C8QxJaJKveIdMEYwClL9aa6jGLa0zlJOQdLfR6hxg0l+yxzbPlWPgXW7awtaYO5oB/shcPX7IaS5CQoczouRlcbwUMyI4GBjzfUIJxMnFvL5+vBLQzInLu1/dJLi4zLbETveuJC57auelYpwa7iXqXgcwH+/wPodpWEK7jXPQt/XkO3zomY4MqUUlgLLerKO6fPWAwmq+a2R1SwioWpp8jR1kE0IyEAE69H9wlmyhuKZ2ITA8jIRUN4Uhu3bSL9Y4tsFl90SSs7upYfd0HaJ7Wt3PVRfX5hqDWeScFbXWdQVcJE8g4=
X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR0701MB1714;
X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714;
31:aVD32rhyIGhh9gNWomJWUFYdMRkIUtClIQLjy5VoNzV4RKCY1rMmPd69ikl93y/Gr2I1Q0+CJFG1SOto2asLoA+UcFS1dP8a5h5sM27XrlfoRsmFgk9ikanyOqmDE5ubjsDOKRSj5r4ADvnFG3y5jc0wPtdR0NOSaIWgDzLUPZP4NlOrlVzXHLDsCgyUBcxVRBu0KFrLCJ+cpODluhpKDQ==;
20:Fs5EyzFNZin+vWycYh3OCtgOa16xs3pjG2K7STyPG0pdGzUL68n9AJelfONyz4MaU8jy+6CQ+39gHCzAf/dZPBTvqUBwTJAfjmRoNiR37AwJjb4KIR7ugbdysjIasoFLoSJbMZa9BY5FHjfN37/pfu0Yc7qdInLJDPKM3hhkTeqOYP4V/XaOxFXrw2xC/lhF4Yxhy/63eXG36flenq8q86M0/EHR5NeTc4dN9U9npHdYtBe2Yzi0i8JJGLD80fpOhsUZil5kEegL40fS2CkabOqfVG8pWcF75SM6rvz6ekVPlZE+RwDAwMQhGPqLBxYGktXhggPkZJMTR9nQQD9s1oJ3nxoSeLKFo6FHJxzpgtHUfFMjd+Wu7INYVTTrXXXzPuhpLMbgwe7oDKinb6IwmBo7O4XJQ/izyd53EQI7o8/TO6WFg+7b6P0UhBE0o1lc8NT8QuqrVVfRADe+L5gQt9Nw1xWVJDoTIpTU3t0I/zoqmHl3vG+FczjiDE6Uz5D006nJqK5abKMDm3lAyuxOx+Irb2aA4++uYcilXYJvMpKmwbWP2SiSCYVBZTHWQ3n7CWR2MknN4DfjOdTJwJbL+BeeNbFKWyVqkxuRaICy9OE=
X-Microsoft-Antispam-PRVS: <BLUPR0701MB1714DB3389622073FEEA4E8881380@BLUPR0701MB1714.namprd07.prod.outlook.com>
X-Exchange-Antispam-Report-Test: UriScan:(17755550239193);
X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0;
RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001);
SRVR:BLUPR0701MB1714; BCL:0; PCL:0; RULEID:; SRVR:BLUPR0701MB1714;
X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714;
4:3Pxbgds9N1LtxQzJ/XB6AQIUsN16rSD+tOZDYtUBqi5stHXxF8uBN03UUDEOHgfhuBtDq3s3VZmWStkFk/Xic4QZ+u4bKwzpswLlMl8Wfd1Igb+x4D6Ml3g9pQ+R3IDKe3JnedioDdnUDAxzV+zNKT+WwFmKSSHxfA5PapGEC454l0gFa9+En3HCXKUaXilOvtUlp4e1jAbZjbdssq9xDh8B68r4cuPz3WoShENbYcaKNpWSf/O5Sngf1px2eIf61lURCbls1xmLGf2qdRiBdCIh7i/dac3kfDIykZEtNAJKpggbP3Ik78GCFLWIn/sA6Rj2mutaXJ5fa9a1aL84x7xwbQRSx2WL7knTJZVLegfPKiR7cQVxhlcRcygkQs6k/DJCEw9pxXImVr0IzZjRwfrXj9I5AZVJ9ihYYItoN54=
X-Forefront-PRVS: 0993689CD1
X-Forefront-Antispam-Report: SFV:NSPM;
SFS:(10009020)(4630300001)(6069001)(6009001)(7916002)(189002)(199003)(24454002)(61506002)(7846002)(77096005)(9686002)(97736004)(4326007)(4001350100001)(2950100001)(2906002)(101416001)(33656002)(50986999)(54356999)(305945005)(7736002)(50466002)(76176999)(92566002)(42186005)(93886004)(97756001)(47776003)(66066001)(23726003)(6116002)(81166006)(81156014)(8676002)(3846002)(68736007)(586003)(1076002)(189998001)(46406003)(106356001)(105586002)(83506001)(110136002)(7099028)(18370500001);
DIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR0701MB1714; H:localhost.localdomain; FPR:;
SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en;
Received-SPF: None (protection.outlook.com: cavium.com does not designate
permitted sender hosts)
X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; BLUPR0701MB1714;
23:jMo+AL6yzzvKHmh63ns9X37aFbdg/axxTT9eT9Z?=
=?us-ascii?Q?czXTFAVNYzzXkQJIGiM2m2xfkzwaRaHbB0Hv92GebAHB5BuMz3fHlERMx0kp?=
=?us-ascii?Q?2Ckk23d3T77hnVgtetNfdc8dIkijy6naSoPtPLCHwhUnpbiCj0408pzy2xsw?=
=?us-ascii?Q?mwcbbrnbN2DUb+LstuVXTIZqDhjWgTAsMm379ydUMQcXaeaaearFbD/dmX8x?=
=?us-ascii?Q?5p8QdFjpN/l9pp6lh275oYMT8dkh7pppNDNHHWYYmK/YWe102XNJjJl4eEEF?=
=?us-ascii?Q?QnwdBpOd6s6IBR5skzKhbRbOjQaa+WUSVZDVybwFXB+RwC+n/GTuu2kYBrvm?=
=?us-ascii?Q?8MisOPrNzoD55AgXv1oo9bqVTFYQyDWc0vrb/iT6CvfeGPS0kg+I7SMRtnqL?=
=?us-ascii?Q?s6UIASb9mRU5Jcs9BZLPPeNWtNwnxrR91LDtNxMZU0hld/7eTMwyn1obLWVs?=
=?us-ascii?Q?fo2PeAAFI7tjYd4apT4S3Tg1TIxVv3bZqREeJpyzow0hhRqQB71PK8FpAPMC?=
=?us-ascii?Q?tAbBfYLr6FmH4HY0EHw/vtxowMugBfnDH5sr1GWv788cppDxXoyz6eiGOAUb?=
=?us-ascii?Q?8NIewDQ4AhZCnTtcDGXHv1k70vPqO5aWl2olDKS6heOfk/ccYrCb4gJmHRp6?=
=?us-ascii?Q?flhGlP/wdD3G24BEzhJUi2STBPUuFqvWkCVetJuTulmKqZ18o4o4Z/CkXuko?=
=?us-ascii?Q?dmpLs7ppbro2JBFocGs/AbKqnR4K/N/a04nO6CSmN+rk6F32ojWV9U9/WmPx?=
=?us-ascii?Q?ZTrTD8R7U+qB+cLmXsoiZsCZKsOcCjHd76YDZyTOfOw/MDPZSnjfKNtor50V?=
=?us-ascii?Q?jeKdyCCHzDrsIe0CSDejjQI4HhLzAXTQy9y6E8uVFYLtMM0Ws9CpRI8SPAWU?=
=?us-ascii?Q?Uv2sTz/ZoNBpd6OZLtOWYpY2+/kS19T3DaJexy+sWh0Oj7SfEEQqRZ37dK1I?=
=?us-ascii?Q?2M1EGZrTCQep25vehEx0IcKWwVId+9pRHk0T0vK033WV/Cf7/rkQDKMWBCFh?=
=?us-ascii?Q?39QQqrTnMtozr35qWDTCVpSGkGONm1PW07zVdwvhiOeMigLVDRKrXIcNjROO?=
=?us-ascii?Q?MomqA4Qdm3GOp6HipdVeGmv34YMGNN2Cyz1AZAgcDs1X9QAPTvaFbWQYdt7t?=
=?us-ascii?Q?fbntEUsJj1MW7vCpqSGMAARoriEo5dsUYK01djxOwViRXHd0QJw7Ziw=3D?=
=?us-ascii?Q?=3D?=
X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714;
6:6ZWo1XBU+dAWt1qLxH+WXWhqJ4UDQlcbfjr5v5KCVbB0/eAjA0VzPeNMouQfH8BSKyO2DEg3c58mts3xCracun8xBUKo2BSHpF4TRXt/0sKT40qNR3xFGmCOgSEPws5iajRplSEatSIfbO08V3zh34a51wJg1x0njoHW1efvCoCPqp7wMQRT597OAYh76ftZ+IGt1HEFKRtPvUpHoCDnuasa6TZAGalUsI4lVsBjzUpK17a/z2Gl6Qi+qcJ/wnxgEvhu/6w/OXsyhytnGDFtpS5uH8DqKe3urJFeFpl1Feg=;
5:oPr4+RWVq7kPycuMd+sbUa1uVCh8PWvqMe4cD2tPnHyhkx1HNhWCbPklTiLk8llmLRjC3hbatpr6Lh1QKWm35Q26U91FQX1uktZBGsVv3hN00mhKWMr0w+syEYIrmkm5gDmh7GL+XK37vSCe2kvM6Q==;
24:yeLq8WjdPjSwjZS3NJbDeKzRxNphIqK+Q/tGQWLWjiV/Sgw5U0Y7ejqNSXwDU6EZQjv5bbKUEaF5GTOo2FT7dtf5JpGtUSs/92V+sYlUQVU=;
7:kekvWJzBgA+kFxndAY+N/hKjcaWIfcZI6DmIbTs4rRjsAUD8YUkhQQFQxF1/YdLoPeyTlrPPkQAY1ouhRKS/aWnht9/ibf71etyu3gNtbhSnYW6r0RMSvfAhpLEp199YtcKsbdcXmgZ7cPP/hLJMknYBXOmZKAHCHGtoCEdeiRQQcGkHNr9ZnwB0uE8AbC0x6Wwug+IU329sEJR/J2vU6C38cc7XhSixHwihDQEwW590ub52+BHP9yQA2NE+VHMr
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-OriginatorOrg: caviumnetworks.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jul 2016 12:16:14.5870 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0701MB1714
Subject: Re: [dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation
cleanup
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>
X-List-Received-Date: Mon, 04 Jul 2016 12:16:19 -0000
On Mon, Jul 04, 2016 at 07:02:25PM +0800, Yuanhan Liu wrote:
> On Mon, Jul 04, 2016 at 02:37:55PM +0530, Jerin Jacob wrote:
> > On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote:
> > > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote:
> > > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote:
> > > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote:
> > > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
> > > > > > {
> > > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
> > > > > >
> > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3
> > > > > > - struct virtio_hw *hw = dev->data->dev_private;
> > > > > > -#endif
> > > > > > struct virtnet_tx *txvq;
> > > > > > struct virtqueue *vq;
> > > > > > uint16_t tx_free_thresh;
> > > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
> > > > > > }
> > > > > >
> > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3
> > > > > > + struct virtio_hw *hw = dev->data->dev_private;
> > > > >
> > > > > I'd suggest to move above declaration to ...
> > > > >
> > > > > > /* Use simple rx/tx func if single segment and no offloads */
> > > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
> > > > > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
> > > > >
> > > > > here: we should try to avoid declaring vars in the middle of a code block.
> > > >
> > > > Next patch in this series, moving all rxtx handler selection code to
> > > > separate function(virtio_update_rxtx_handler()) where declaration comes
> > > > as first line in the function.i.e the comment is taken care of in the
> > > > series.
> > >
> > > Yes, I saw that. But in principle, each patch is atomic: it's not a
> > > good idea/practice to introduce issues in path A and then fix it in
> > > path B.
> >
> > In my view it was not an issue as I was removing all possible
> > conditional compilation flag. If I were to move the declaration to top
> > then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3
> > flag I need to add around declaring the variable.
>
> Nope, I was suggesting to move it inside the "if" block. So, this
> is actually consistent with what you are trying to do. Besides, it
> removes an declaration in the middle.
Just to get the clarity on "moving inside the 'if' block"
Are you suggesting to have like below?
#ifdef RTE_MACHINE_CPUFLAG_SSSE3
+ struct virtio_hw *hw;
/* Use simple rx/tx func if single segment and no offloads */
if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) ==
VIRTIO_SIMPLE_FLAGS &&
!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
PMD_INIT_LOG(INFO, "Using simple rx/tx path");
dev->tx_pkt_burst = virtio_xmit_pkts_simple;
dev->rx_pkt_burst = virtio_recv_pkts_vec;
- use_simple_rxtx = 1;
+ hw = dev->data->dev_private;
+ hw->use_simple_rxtx = 1;
}
#endif
Instead of following scheme in existing patch,
#ifdef RTE_MACHINE_CPUFLAG_SSSE3
+ struct virtio_hw *hw = dev->data->dev_private;
/* Use simple rx/tx func if single segment and no offloads */
if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) ==
VIRTIO_SIMPLE_FLAGS &&
!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
PMD_INIT_LOG(INFO, "Using simple rx/tx path");
dev->tx_pkt_burst = virtio_xmit_pkts_simple;
dev->rx_pkt_burst = virtio_recv_pkts_vec;
- use_simple_rxtx = 1;
+ hw->use_simple_rxtx = 1;
}
#endif
The former case will have issue as "hw" been used in "if" with vtpci_with_feature.
OR
if you meant just floating "struct virtio_hw *hw" without RTE_MACHINE_CPUFLAG_SSSE3
then it comes error on non x86 as unused "hw" variable.
If you meant something else then let me know?
>
> --yliu
>
> > Hope this justifies the reason. If you are not convinced then let me know,
> > if will add the change in next revision.
> >
> > Jerin
> >
> > >
> > > --yliu