Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Yliu,

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, October 21, 2015 2:01 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; Michael S. Tsirkin; marcel@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
> 
> > >  static int
> > > +ixgbe_config_vmdq_rss(struct rte_eth_dev *dev)
> > > +{
> > > +	struct ixgbe_hw *hw;
> > > +	uint32_t mrqc;
> > > +
> > > +	ixgbe_rss_configure(dev);
> > > +
> > > +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > > +
> > > +	/* MRQC: enable VMDQ RSS */
> > > +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> > > +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> > > +
> > > +	switch (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) {
> > > +	case 2:
> > > +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> > > +		break;
> > > +
> > > +	case 4:
> > > +		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
> > > +		break;
> > > +
> > > +	default:
> > > +		PMD_INIT_LOG(ERR, "Invalid pool number in non-IOV mode with VMDQ RSS");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> > > +
> > > +	ixgbe_vmdq_pool_configure(dev);
> > > +
> > > +	return 0;
> > > +}
> >
> > So ixgbe_config_vmdq_rss() checks nb_q_per_pool value, and might return an error if the value is invalid.
> > Though this return value seems just ignored by ixgbe_dev_mq_rx_configure() below.
> > Probably, it is better to move nb_q_per_pool value checking into rte_eth_dev_check_mq_mode(),
> > as is done for other modes?
> > I know it is no ideal, as it probably should be HW specific check,
> > but seems anyway better than just ignoring the error.
> >
> 
> Hi,
> 
> Thanks for the review. However, as you might have seen, I dropped this
> patch in the lastest version. This patch is for using the NIC VMDq
> feature to link with VM multiple queue to demonstrate the multiple
> queue feature. However, this introduces too much limitation. So,
> I dropped it.

Ah sorry, didn't notice it - too many patches are flying around these days.

> 
> However, despite of above usage, if you think it's necessary to add
> such ability (support VMDq RSS in non-SRIOV), I could update this patch
> based on your comments, and sent it out as a standalone patch. Otherwise,
> I'll simply drop it.

That's ok by me.
Konstantin

> 
> 	--yliu