Re: [PATCH] ixgbe: Fix an unnecessary check in vf rss

Hi PawelX

-----Original Message-----
From: Wodkowski, PawelX
Sent: Friday, January 30, 2015 12:14 AM
To: Ouyang, Changchun; Thomas Monjalon; Richardson, Bruce
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: RE: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss

quoted

-----Original Message-----
From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Ouyang,

Changchun

quoted

Sent: Wednesday, January 28, 2015 2:35 AM
To: Thomas Monjalon
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf
rss

Hi Thomas,

quoted

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
Sent: Tuesday, January 27, 2015 8:13 PM
To: Ouyang, Changchun
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in
vf rss

quoted

To follow up the comments from Wodkowski, PawelX, remove this
unnecessary check, as check_mq_mode has already check the queue

number

quoted

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).

quoted

This fixes commit 42d2f78abcb77ecb769be4149df550308169ef0f

Signed-off-by: Changchun Ouyang <redacted>

Suggested-by: Pawel Wodkowski <redacted>
Fixes: 42d2f78abcb77 ("configure VF RSS")

Applied

Thanks very much for the applying!

quoted

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