DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
@ 2015-01-21  3:38 Ouyang Changchun
  2015-01-27 12:12 ` Thomas Monjalon
  0 siblings, 1 reply; 6+ messages in thread
From: Ouyang Changchun @ 2015-01-21  3:38 UTC (permalink / raw)
  To: dev

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@intel.com>
---
 lib/librte_pmd_ixgbe/ixgbe_pf.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c b/lib/librte_pmd_ixgbe/ixgbe_pf.c
index 93f6e43..dbda9b5 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
@@ -187,21 +187,6 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
 	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw->mac.num_rar_entries), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw->mac.num_rar_entries), 0);
 
-	/*
-	 * VF RSS can support at most 4 queues for each VF, even if
-	 * 8 queues are available for each VF, it need refine to 4
-	 * queues here due to this limitation, otherwise no queue
-	 * will receive any packet even RSS is enabled.
-	 */
-	if (eth_dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_RSS) {
-		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
-			RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_32_POOLS;
-			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
-			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
-				dev_num_vf(eth_dev) * 4;
-		}
-	}
-
 	/* set VMDq map to default PF pool */
 	hw->mac.ops.set_vmdq(hw, 0, RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
 
-- 
1.8.4.2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
  2015-01-21  3:38 [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss Ouyang Changchun
@ 2015-01-27 12:12 ` Thomas Monjalon
  2015-01-28  1:35   ` Ouyang, Changchun
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2015-01-27 12:12 UTC (permalink / raw)
  To: Ouyang Changchun; +Cc: dev

> 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@intel.com>

Suggested-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Fixes: 42d2f78abcb77 ("configure VF RSS")

Applied

Changchun, as you are working on ixgbe, maybe you would like to review
some ixgbe patches from others?

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
  2015-01-27 12:12 ` Thomas Monjalon
@ 2015-01-28  1:35   ` Ouyang, Changchun
  2015-01-29 16:13     ` Wodkowski, PawelX
  0 siblings, 1 reply; 6+ messages in thread
From: Ouyang, Changchun @ 2015-01-28  1:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, January 27, 2015 8:13 PM
> To: Ouyang, Changchun
> Cc: dev@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@intel.com>
> 
> Suggested-by: Pawel Wodkowski <pawelx.wodkowski@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
  2015-01-28  1:35   ` Ouyang, Changchun
@ 2015-01-29 16:13     ` Wodkowski, PawelX
  2015-01-30  1:20       ` Ouyang, Changchun
  0 siblings, 1 reply; 6+ messages in thread
From: Wodkowski, PawelX @ 2015-01-29 16:13 UTC (permalink / raw)
  To: Ouyang, Changchun, Thomas Monjalon, Richardson, Bruce; +Cc: dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang, Changchun
> Sent: Wednesday, January 28, 2015 2:35 AM
> To: Thomas Monjalon
> Cc: dev@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@6wind.com]
> > Sent: Tuesday, January 27, 2015 8:13 PM
> > To: Ouyang, Changchun
> > Cc: dev@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@intel.com>
> >
> > Suggested-by: Pawel Wodkowski <pawelx.wodkowski@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?

Pawel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
  2015-01-29 16:13     ` Wodkowski, PawelX
@ 2015-01-30  1:20       ` Ouyang, Changchun
  2015-01-30 10:16         ` Wodkowski, PawelX
  0 siblings, 1 reply; 6+ messages in thread
From: Ouyang, Changchun @ 2015-01-30  1:20 UTC (permalink / raw)
  To: Wodkowski, PawelX, Thomas Monjalon, Richardson, Bruce; +Cc: dev

Hi PawelX

> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Friday, January 30, 2015 12:14 AM
> To: Ouyang, Changchun; Thomas Monjalon; Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang,
> Changchun
> > Sent: Wednesday, January 28, 2015 2:35 AM
> > To: Thomas Monjalon
> > Cc: dev@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@6wind.com]
> > > Sent: Tuesday, January 27, 2015 8:13 PM
> > > To: Ouyang, Changchun
> > > Cc: dev@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@intel.com>
> > >
> > > Suggested-by: Pawel Wodkowski <pawelx.wodkowski@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

 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
  2015-01-30  1:20       ` Ouyang, Changchun
@ 2015-01-30 10:16         ` Wodkowski, PawelX
  0 siblings, 0 replies; 6+ messages in thread
From: Wodkowski, PawelX @ 2015-01-30 10:16 UTC (permalink / raw)
  To: Ouyang, Changchun, Thomas Monjalon, Richardson, Bruce; +Cc: dev

> -----Original Message-----
> From: Ouyang, Changchun
> Sent: Friday, January 30, 2015 2:20 AM
> To: Wodkowski, PawelX; Thomas Monjalon; Richardson, Bruce
> Cc: dev@dpdk.org; Ouyang, Changchun
> Subject: RE: [dpdk-dev] [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@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang,
> > Changchun
> > > Sent: Wednesday, January 28, 2015 2:35 AM
> > > To: Thomas Monjalon
> > > Cc: dev@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@6wind.com]
> > > > Sent: Tuesday, January 27, 2015 8:13 PM
> > > > To: Ouyang, Changchun
> > > > Cc: dev@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@intel.com>
> > > >
> > > > Suggested-by: Pawel Wodkowski <pawelx.wodkowski@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.
> 

What about about moving sriov from rte_eth_dev_data to driver's private
structure?

Pawel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-01-30 10:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21  3:38 [dpdk-dev] [PATCH] ixgbe: Fix an unnecessary check in vf rss Ouyang Changchun
2015-01-27 12:12 ` Thomas Monjalon
2015-01-28  1:35   ` Ouyang, Changchun
2015-01-29 16:13     ` Wodkowski, PawelX
2015-01-30  1:20       ` Ouyang, Changchun
2015-01-30 10:16         ` Wodkowski, PawelX

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).