From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Subject: Re: [PATCH v2 2/3] virtio: move SSE based Rx
 implementation to separate file
Date: Mon, 4 Jul 2016 14:08:14 +0530
Message-ID: <20160704083813.GB12596@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-3-git-send-email-jerin.jacob@caviumnetworks.com>
 <20160704074247.GW2831@yliu-dev.sh.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: <dev@dpdk.org>, <thomas.monjalon@6wind.com>, <bruce.richardson@intel.com>,
 <jianbo.liu@linaro.org>, <huawei.xie@intel.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM03-BY2-obe.outbound.protection.outlook.com
 (mail-by2nam03on0049.outbound.protection.outlook.com [104.47.42.49])
 by dpdk.org (Postfix) with ESMTP id AD5D12C06
 for <dev@dpdk.org>; Mon,  4 Jul 2016 10:38:37 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <20160704074247.GW2831@yliu-dev.sh.intel.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Jul 04, 2016 at 03:42:47PM +0800, Yuanhan Liu wrote:
> On Fri, Jul 01, 2016 at 04:46:37PM +0530, Jerin Jacob wrote:
> > * Introduced cpuflag based run-time detection to
> > select the SSE based simple Rx handler
> > * Split out SSE instruction based virtio simple Rx
> > implementation to a separate file
> 
> As your commit log says, it does two things, therefore, I'd suggest you
> to do it in two patches, with each just does one thing as you mentioned.

OK. Will fix it in next revision.

> 
> > +static void
> > +virtio_update_rxtx_handler(struct rte_eth_dev *dev,
> > +			   const struct rte_eth_txconf *tx_conf)
> > +{
> > +	uint8_t use_simple_rxtx = 0;
> > +	struct virtio_hw *hw = dev->data->dev_private;
> > +
> > +#if defined RTE_ARCH_X86
> > +	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE3))
> > +		use_simple_rxtx = 1;
> > +#endif
> > +	/* Use simple rx/tx func if single segment and no offloads */
> > +	if (use_simple_rxtx &&
> > +	   (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
> > +		!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
> 
> The alignment here is not consistent, something like following is what
> I'd suggest:
> 
> 	if (use_simple_rxtx &&
> 	    (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
> 	    !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {

OK. Will fix it in next revision.

> 
> 	--yliu