Re: [dpdk-dev] [PATCH v2 2/3] virtio: move SSE based Rx implementation to separate file

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

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