[dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

[dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

[Daniel Kan]

---
 app/test-pmd/testpmd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b11eb2e..355db0f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1546,6 +1546,9 @@ init_port_config(void)
 		if (nb_rxq > 0) {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
+			if (nb_rxq > 1 && rss_hf != 0) {
+				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+			}
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

[Thomas Monjalon]

Hello,

11/01/2014 03:46, Daniel Kan:
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1546,6 +1546,9 @@ init_port_config(void)
>  		if (nb_rxq > 0) {
>  			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
>  			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
> +			if (nb_rxq > 1 && rss_hf != 0) {
> +				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> +			}

The fix looks good. Thanks.

Please, could you add a detailed explanation and a signed-off as described in 
the guidelines:
	http://dpdk.org/dev#send

I think it's a good idea to point to the commit which has introduced the bug.
Note also that "RSS rx" may be simply "RSS".

Thank you
-- 
Thomas

Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

[Ananyev, Konstantin]

Hi Daniel,

Good catch :)
We plan to deliver a fix for that bug in coming DPDK release.
It is very similar to yours one, though we need extra check that there are no VFs enabled for that port.
Thanks
Konstantin 

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
Sent: Monday, January 13, 2014 10:31 AM
To: Daniel Kan
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

Hello,

11/01/2014 03:46, Daniel Kan:
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1546,6 +1546,9 @@ init_port_config(void)
>  		if (nb_rxq > 0) {
>  			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
>  			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
> +			if (nb_rxq > 1 && rss_hf != 0) {
> +				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> +			}

The fix looks good. Thanks.

Please, could you add a detailed explanation and a signed-off as described in the guidelines:
	http://dpdk.org/dev#send

I think it's a good idea to point to the commit which has introduced the bug.
Note also that "RSS rx" may be simply "RSS".

Thank you
--
Thomas
--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS rx by setting mq_mode

[Thomas Monjalon]

13/01/2014 12:18, Ananyev, Konstantin:
> We plan to deliver a fix for that bug in coming DPDK release.
> It is very similar to yours one, though we need extra check that there are
> no VFs enabled for that port.

Thanks for your comments about the VF. I think it will be clearer in a patch.
Please, send a v2 patch or a new one on top of this one, following the 
guidelines:
	http://dpdk.org/dev#send
so it can be tracked with git.

Note: please remove your "confidential emails" footer for mailing list.
-- 
Thomas

[dpdk-dev] [PATCH v2] app/testpmd: fix RSS by setting mq_mode

[Daniel Kan]

The mq_mode was not set when rxq is > 1; it's defaulted to ETH_MQ_RX_NONE.
As a result, RSS remains inactive. The fix is to set mq_mode to ETH_MQ_RX_RSS
when rxq is > 1 and hf is non-zero.

This bug was introduced by commit 243db2ddee3094a2cb39fdd4b17e26df4e7735e1
igb/ixgbe: ETH_MQ_RX_NONE should disable RSS

Signed-off-by: Daniel Kan <dan@nyansa.com>
---
Updated commit log in accordance with dpdk guidelines

 app/test-pmd/testpmd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b11eb2e..355db0f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1546,6 +1546,9 @@ init_port_config(void)
 		if (nb_rxq > 0) {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
+			if (nb_rxq > 1 && rss_hf != 0) {
+				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+			}
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS by setting mq_mode

[Maxime Leroy]

Hello,

Thanks for your patch fixing the regression introduced by my commit
(igb/ixgbe: ETH_MQ_RX_NONE should disable RSS).

I have one comment about your fix. I don't think there are any reasons
to not enable RSS with only one RX queue in testpmd.

RSS is mainly used in testpmd to spread traffic on the different rx queues.
But you can use RSS with one rx queue for debbugging purpose.
For example, you can use the rxonly forward engine of the testpmd to display
the RSS hash. (see pkt_burst_receive in app/test-pmd/rxonly.c)

An other issue about not enabling RSS with 1 queue, when you use the command
show_rss_key in the testpmd, this one will display that the RSS is enabled.
(because rss_hf != 0; see port_rss_hash_conf_show function in
app/test-pmd/config.c)

Do you agree with my analyze ?

Thanks.

-- 
Maxime Leroy

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS by setting mq_mode

[Daniel Kan]

Maxime,
Thanks for your comment. If we want testpmd to have RSS always enabled out of box (note that rxq=1), then your suggestion makes sense. I’m new to dpdk so I was trying to preserve the current behavior. Now looking back, I guess that’s why we have an explicit disable-rss option. If I don’t hear any resistance by tomorrow, I will submit a new patch. Thanks.

Dan

PS. I don’t see port_rss_hash_conf_show in config.c, at not in 1.5.1 branch. Which branch is this in?

On Jan 14, 2014, at 9:42 AM, Maxime Leroy <maxime.leroy@6wind.com> wrote:

> Hello,
> 
> Thanks for your patch fixing the regression introduced by my commit
> (igb/ixgbe: ETH_MQ_RX_NONE should disable RSS).
> 
> I have one comment about your fix. I don't think there are any reasons
> to not enable RSS with only one RX queue in testpmd.
> 
> RSS is mainly used in testpmd to spread traffic on the different rx queues.
> But you can use RSS with one rx queue for debbugging purpose.
> For example, you can use the rxonly forward engine of the testpmd to display
> the RSS hash. (see pkt_burst_receive in app/test-pmd/rxonly.c)
> 
> An other issue about not enabling RSS with 1 queue, when you use the command
> show_rss_key in the testpmd, this one will display that the RSS is enabled.
> (because rss_hf != 0; see port_rss_hash_conf_show function in
> app/test-pmd/config.c)
> 
> Do you agree with my analyze ?
> 
> Thanks.
> 
> -- 
> Maxime Leroy

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix RSS by setting mq_mode

[Thomas Monjalon]

14/01/2014 11:57, Daniel Kan :
> On Jan 14, 2014, at 9:42 AM, Maxime Leroy <maxime.leroy@6wind.com> wrote:
> > An other issue about not enabling RSS with 1 queue, when you use the
> > command show_rss_key in the testpmd, this one will display that the RSS
> > is enabled. (because rss_hf != 0; see port_rss_hash_conf_show function in
> > app/test-pmd/config.c)
> 
> PS. I don’t see port_rss_hash_conf_show in config.c, at not in 1.5.1 branch.
> Which branch is this in?

Maxime is talking about a local development branch. The function 
port_rss_hash_conf_show() is in a patch which should be ready for the next 
version. By the way, your fix will be integrated in version 1.5.2.

Daniel, thanks to provide a new version of your patch which allow RSS 
debugging with only 1 queue.

-- 
Thomas

[dpdk-dev] [PATCH v3]

[Daniel Kan]

The mq_mode was not set when rxq is > 0; it's defaulted to ETH_MQ_RX_NONE. 
As a result, RSS remains inactive. The fix is to set mq_mode to ETH_MQ_RX_RSS 
when hf is non-zero.

This bug was introduced by commit 243db2ddee3094a2cb39fdd4b17e26df4e7735e1
igb/ixgbe: ETH_MQ_RX_NONE should disable RSS

Signed-off-by: Daniel Kan <dan@nyansa.com>
---
Enable RSS even if rxq=1 based on comment from Maxime Leroy.

 app/test-pmd/testpmd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b11eb2e..42e1fdb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1546,6 +1546,9 @@ init_port_config(void)
 		if (nb_rxq > 0) {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
+			if (rss_hf != 0) {
+				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+			}
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;
-- 
1.7.9.5

[dpdk-dev] [PATCH v3] app/testpmd: fix RSS rx by setting mq_mode

[Daniel Kan]

The mq_mode was not set when rxq is > 0; it's defaulted to ETH_MQ_RX_NONE. 
As a result, RSS remains inactive. The fix is to set mq_mode to ETH_MQ_RX_RSS 
when hf is non-zero.

This bug was introduced by commit 243db2ddee3094a2cb39fdd4b17e26df4e7735e1
igb/ixgbe: ETH_MQ_RX_NONE should disable RSS

Signed-off-by: Daniel Kan <dan@nyansa.com>
---
Enable RSS even if rxq=1 based on comment from Maxime Leroy.

 app/test-pmd/testpmd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b11eb2e..42e1fdb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1546,6 +1546,9 @@ init_port_config(void)
 		if (nb_rxq > 0) {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
+			if (rss_hf != 0) {
+				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+			}
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix RSS rx by setting mq_mode

[Maxime Leroy]

> The mq_mode was not set when rxq is > 0; it's defaulted to ETH_MQ_RX_NONE.
> As a result, RSS remains inactive. The fix is to set mq_mode to ETH_MQ_RX_RSS
> when hf is non-zero.
>
> This bug was introduced by commit 243db2ddee3094a2cb39fdd4b17e26df4e7735e1
> igb/ixgbe: ETH_MQ_RX_NONE should disable RSS
>
> Signed-off-by: Daniel Kan <dan@nyansa.com>

Acked-by: Maxime Leroy <maxime.leroy@6wind.com>

Thanks.

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix RSS rx by setting mq_mode

[Thomas Monjalon]

> > The mq_mode was not set when rxq is > 0; it's defaulted to
> > ETH_MQ_RX_NONE. As a result, RSS remains inactive. The fix is to set
> > mq_mode to ETH_MQ_RX_RSS when hf is non-zero.
> > 
> > This bug was introduced by commit
> > 243db2ddee3094a2cb39fdd4b17e26df4e7735e1 igb/ixgbe: ETH_MQ_RX_NONE
> > should disable RSS
> > 
> > Signed-off-by: Daniel Kan <dan@nyansa.com>
> 
> Acked-by: Maxime Leroy <maxime.leroy@6wind.com>

Applied.
Thanks for the patch and the reworks.

-- 
Thomas