From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 To: Yuanhan Liu CC: , , , , 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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