[dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss

[Ouyang, Changchun]

Hi PawelX

> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Friday, January 30, 2015 12:14 AM
> To: Ouyang, Changchun; Thomas Monjalon; Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ouyang,
> Changchun
> > Sent: Wednesday, January 28, 2015 2:35 AM
> > To: Thomas Monjalon
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf
> > rss
> >
> > Hi Thomas,
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > Sent: Tuesday, January 27, 2015 8:13 PM
> > > To: Ouyang, Changchun
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in
> > > vf rss
> > >
> > > > To follow up the comments from Wodkowski, PawelX, remove this
> > > > unnecessary check, as check_mq_mode has already check the queue
> > > number
> > > > in device configure stage, if the queue number of vf is not
> > > > correct, it will return error code and exit, so it doesn't need
> > > > check again here in device start stage(note: pf_host_configure is
> > > > called in device start
> > > stage).
> > > >
> > > > This fixes commit 42d2f78abcb77ecb769be4149df550308169ef0f
> > > >
> > > > Signed-off-by: Changchun Ouyang <changchun.ouyang at intel.com>
> > >
> > > Suggested-by: Pawel Wodkowski <pawelx.wodkowski at intel.com>
> > > Fixes: 42d2f78abcb77 ("configure VF RSS")
> > >
> > > Applied
> > >
> >
> > Thanks very much for the applying!
> >
> > > Changchun, as you are working on ixgbe, maybe you would like to
> > > review some ixgbe patches from others?
> > >
> >
> > No problem, I will try to do it when my bandwidth allows me to do it,
> > :-) Thanks Changchun
> 
> Actually I was suggesting exactly opposite direction. Main issue is that the
> sriov field in rte_eth_dev_data is only used by igb and ixgbe drivers. In
> addition
> rte_eth_dev_check_mq_mode() is specialized for ixgbe driver.
> 
> I am thinking about moving sriov from rte_eth_dev_data to driver's private
> structure or at least move rte_eth_dev_check_mq_mode() to struct
> eth_dev_ops as optional driver configuration step.
> 
> What do you think about both steps?

Good opinion!
I prefer to move rte_eth_dev_check_mq_mode to eth_dev_ops as optional driver configure,
The reason is that in future other eth type may also need such kind of check or even refine some queue number values by their own way,
I can help review your patch after you send out. 
Thanks for your enhancing that.

Changchun