DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] updates for testpmd
@ 2020-09-08  2:16 Wei Hu (Xavier)
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-08  2:16 UTC (permalink / raw)
  To: dev; +Cc: xavier.huwei, lihuisong, ferruh.yigit

This series are minor updates for testpmd application.

Huisong Li (2):
  app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  app/testpmd: retain original FDIR mode when configuring DCB

 app/test-pmd/testpmd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.9.5


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

* [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-08  2:16 [dpdk-dev] [PATCH 0/2] updates for testpmd Wei Hu (Xavier)
@ 2020-09-08  2:16 ` Wei Hu (Xavier)
  2020-09-22 16:21   ` Ferruh Yigit
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB Wei Hu (Xavier)
  2020-09-23  7:51 ` [dpdk-dev] [PATCH v2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
  2 siblings, 1 reply; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-08  2:16 UTC (permalink / raw)
  To: dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger
  Cc: xavier.huwei, lihuisong, ferruh.yigit

From: Huisong Li <lihuisong@huawei.com>

Currently, when starting testpmd application without '--disable-rss' and
the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS flag
is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
(dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
flag of rxmode.mq_mode in PMD drivers.

However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
in testpmd application, this will cause the inconsistent problem that
rx_offloads is different for testpmd and PMD drivers.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 app/test-pmd/testpmd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7842c3b..73543bb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3356,11 +3356,13 @@ init_port_config(void)
 		}
 
 		if (port->dcb_flag == 0) {
-			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
+			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
 				port->dev_conf.rxmode.mq_mode =
 					(enum rte_eth_rx_mq_mode)
 						(rx_mq_mode & ETH_MQ_RX_RSS);
-			else
+				port->dev_conf.rxmode.offloads |=
+							DEV_RX_OFFLOAD_RSS_HASH;
+			} else
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
 
-- 
2.9.5


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

* [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB
  2020-09-08  2:16 [dpdk-dev] [PATCH 0/2] updates for testpmd Wei Hu (Xavier)
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
@ 2020-09-08  2:16 ` Wei Hu (Xavier)
  2020-09-22  5:15   ` Ajit Khaparde
  2020-09-23  7:51 ` [dpdk-dev] [PATCH v2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
  2 siblings, 1 reply; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-08  2:16 UTC (permalink / raw)
  To: dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger, Jijiang Liu,
	Jingjing Wu, Helin Zhang
  Cc: xavier.huwei, lihuisong, ferruh.yigit

From: Huisong Li <lihuisong@huawei.com>

Sometimes, we have to start testpmd application with --pkt-filter-mode to
test flow table feature. When using 'port config 0 dcb vt off 4 pfc on'
command, the dcb information are configured in init_port_dcb_config
function by get_eth_dcb_conf function, And then rte_eth_dev_configure API
function will be called to re-configure PMD driver.

The values of rxmode and txmode in rte_eth_conf struct used to configure
PMD driver, come from the current values maintained in testpmd. However,
the fdir_conf.mode in rte_eth_conf struct is not set, which may cause that
some PMD driver cannot test the flow table feature when multiple TCs are
enabled.

Fixes: 1a572499beb6 ("app/testpmd: setup DCB forwarding based on traffic class")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 app/test-pmd/testpmd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 73543bb..19bf972 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3521,6 +3521,7 @@ init_port_dcb_config(portid_t pid,
 
 	port_conf.rxmode = rte_port->dev_conf.rxmode;
 	port_conf.txmode = rte_port->dev_conf.txmode;
+	port_conf.fdir_conf.mode = rte_port->dev_conf.fdir_conf.mode;
 
 	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
 	retval = get_eth_dcb_conf(pid, &port_conf, dcb_mode, num_tcs, pfc_en);
-- 
2.9.5


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

* Re: [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB Wei Hu (Xavier)
@ 2020-09-22  5:15   ` Ajit Khaparde
  2020-09-23  7:14     ` Wei Hu (Xavier)
  0 siblings, 1 reply; 11+ messages in thread
From: Ajit Khaparde @ 2020-09-22  5:15 UTC (permalink / raw)
  To: Wei Hu (Xavier)
  Cc: dpdk-dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger,
	Jijiang Liu, Jingjing Wu, Helin Zhang, Wei Hu (Xavier, lihuisong,
	Ferruh Yigit

On Mon, Sep 7, 2020 at 7:16 PM Wei Hu (Xavier)
<huwei013@chinasoftinc.com> wrote:
>
> From: Huisong Li <lihuisong@huawei.com>
>
> Sometimes, we have to start testpmd application with --pkt-filter-mode to
> test flow table feature. When using 'port config 0 dcb vt off 4 pfc on'
> command, the dcb information are configured in init_port_dcb_config
> function by get_eth_dcb_conf function, And then rte_eth_dev_configure API
> function will be called to re-configure PMD driver.
>
> The values of rxmode and txmode in rte_eth_conf struct used to configure
> PMD driver, come from the current values maintained in testpmd. However,
> the fdir_conf.mode in rte_eth_conf struct is not set, which may cause that
> some PMD driver cannot test the flow table feature when multiple TCs are
> enabled.
>
> Fixes: 1a572499beb6 ("app/testpmd: setup DCB forwarding based on traffic class")
> Cc: stable@dpdk.org
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>  app/test-pmd/testpmd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 73543bb..19bf972 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3521,6 +3521,7 @@ init_port_dcb_config(portid_t pid,
>
>         port_conf.rxmode = rte_port->dev_conf.rxmode;
>         port_conf.txmode = rte_port->dev_conf.txmode;
> +       port_conf.fdir_conf.mode = rte_port->dev_conf.fdir_conf.mode;
This is probably not necessary. FDIR is due for deprecation [1].
https://doc.dpdk.org/guides/rel_notes/deprecation.html

>
>         /*set configuration of DCB in vt mode and DCB in non-vt mode*/
>         retval = get_eth_dcb_conf(pid, &port_conf, dcb_mode, num_tcs, pfc_en);
> --
> 2.9.5
>

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
@ 2020-09-22 16:21   ` Ferruh Yigit
  2020-09-23  7:04     ` Wei Hu (Xavier)
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-09-22 16:21 UTC (permalink / raw)
  To: Wei Hu (Xavier), dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger
  Cc: xavier.huwei, lihuisong, Pavan Nikhilesh, Andrew Rybchenko

On 9/8/2020 3:16 AM, Wei Hu (Xavier) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> Currently, when starting testpmd application without '--disable-rss' and
> the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS flag
> is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
> DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
> (dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
> flag of rxmode.mq_mode in PMD drivers.
> 
> However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
> in testpmd application, this will cause the inconsistent problem that
> rx_offloads is different for testpmd and PMD drivers.

Yes for DEV_RX_OFFLOAD_RSS_HASH, application rx_offload config and PMD 
one diverges, for *some* PMDs.

This is done to have the backward compatibility of the PMD behavior.

The PMDs that would like to write the provide the calculated hash value 
back, overwrites the offload config to enable 'DEV_RX_OFFLOAD_RSS_HASH'. 
And does more work than user requested, this shouldn't have any side affect.
Some doesn't provide the hash unless user explicitly requests it.

Applications shouldn't set it blindly, at least first check if PMD 
supports it but even that case, unless it is needed I think HASH offload 
shouldn't be requested by default.

> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>   app/test-pmd/testpmd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 7842c3b..73543bb 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3356,11 +3356,13 @@ init_port_config(void)
>   		}
>   
>   		if (port->dcb_flag == 0) {
> -			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> +			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
>   				port->dev_conf.rxmode.mq_mode =
>   					(enum rte_eth_rx_mq_mode)
>   						(rx_mq_mode & ETH_MQ_RX_RSS);
> -			else
> +				port->dev_conf.rxmode.offloads |=
> +							DEV_RX_OFFLOAD_RSS_HASH;
> +			} else
>   				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>   		}
>   
> 


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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-22 16:21   ` Ferruh Yigit
@ 2020-09-23  7:04     ` Wei Hu (Xavier)
  2020-09-23  9:35       ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-23  7:04 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger, xavier.huwei,
	lihuisong, Pavan Nikhilesh, Andrew Rybchenko

Hi, Ferruh Yigit

On 2020/9/23 0:21, Ferruh Yigit wrote:
> On 9/8/2020 3:16 AM, Wei Hu (Xavier) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Currently, when starting testpmd application without '--disable-rss' and
>> the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS flag
>> is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
>> DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
>> (dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
>> flag of rxmode.mq_mode in PMD drivers.
>>
>> However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
>> in testpmd application, this will cause the inconsistent problem that
>> rx_offloads is different for testpmd and PMD drivers.
> 
> Yes for DEV_RX_OFFLOAD_RSS_HASH, application rx_offload config and PMD 
> one diverges, for *some* PMDs.
> 
> This is done to have the backward compatibility of the PMD behavior.
> 
> The PMDs that would like to write the provide the calculated hash value 
> back, overwrites the offload config to enable 'DEV_RX_OFFLOAD_RSS_HASH'. 
> And does more work than user requested, this shouldn't have any side 
> affect.
> Some doesn't provide the hash unless user explicitly requests it.
> 
> Applications shouldn't set it blindly, at least first check if PMD 
> supports it but even that case, unless it is needed I think HASH offload 
> shouldn't be requested by default.

OK, we are going to do modification to add check if PMD support 
DEV_RX_OFFLOAD_RSS_HASH before setting it as below:

  @@ -3356,11 +3356,13 @@ init_port_config(void)
            }
            if (port->dcb_flag == 0) {
               if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
                    port->dev_conf.rxmode.mq_mode =
                        (enum rte_eth_rx_mq_mode)
                            (rx_mq_mode & ETH_MQ_RX_RSS);
  		  if (port->dev_info.rx_offload_capa &
		      DEV_RX_OFFLOAD_RSS_HASH)
  			port->dev_conf.rxmode.offloads |=
						DEV_RX_OFFLOAD_RSS_HASH;
              } else
                    port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
            }

Thanks, Xavier
> 
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>   app/test-pmd/testpmd.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 7842c3b..73543bb 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3356,11 +3356,13 @@ init_port_config(void)
>>           }
>>           if (port->dcb_flag == 0) {
>> -            if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>> +            if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
>>                   port->dev_conf.rxmode.mq_mode =
>>                       (enum rte_eth_rx_mq_mode)
>>                           (rx_mq_mode & ETH_MQ_RX_RSS);
>> -            else
>> +                port->dev_conf.rxmode.offloads |=
>> +                            DEV_RX_OFFLOAD_RSS_HASH;
>> +            } else
>>                   port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>>           }
>>


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

* Re: [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB
  2020-09-22  5:15   ` Ajit Khaparde
@ 2020-09-23  7:14     ` Wei Hu (Xavier)
  2020-09-24  2:41       ` Wei Hu (Xavier)
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-23  7:14 UTC (permalink / raw)
  To: Ajit Khaparde
  Cc: dpdk-dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger,
	Jijiang Liu, Jingjing Wu, Helin Zhang, Wei Hu (Xavier, lihuisong,
	Ferruh Yigit

Hi, Ajit Khaparde

On 2020/9/22 13:15, Ajit Khaparde wrote:
> On Mon, Sep 7, 2020 at 7:16 PM Wei Hu (Xavier)
> <huwei013@chinasoftinc.com> wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Sometimes, we have to start testpmd application with --pkt-filter-mode to
>> test flow table feature. When using 'port config 0 dcb vt off 4 pfc on'
>> command, the dcb information are configured in init_port_dcb_config
>> function by get_eth_dcb_conf function, And then rte_eth_dev_configure API
>> function will be called to re-configure PMD driver.
>>
>> The values of rxmode and txmode in rte_eth_conf struct used to configure
>> PMD driver, come from the current values maintained in testpmd. However,
>> the fdir_conf.mode in rte_eth_conf struct is not set, which may cause that
>> some PMD driver cannot test the flow table feature when multiple TCs are
>> enabled.
>>
>> Fixes: 1a572499beb6 ("app/testpmd: setup DCB forwarding based on traffic class")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>   app/test-pmd/testpmd.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 73543bb..19bf972 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3521,6 +3521,7 @@ init_port_dcb_config(portid_t pid,
>>
>>          port_conf.rxmode = rte_port->dev_conf.rxmode;
>>          port_conf.txmode = rte_port->dev_conf.txmode;
>> +       port_conf.fdir_conf.mode = rte_port->dev_conf.fdir_conf.mode;
> This is probably not necessary. FDIR is due for deprecation [1].
> https://doc.dpdk.org/guides/rel_notes/deprecation.html

Thanks your for pointing out it, this patch is really necessary.


Thanks, Xavier

>>          /*set configuration of DCB in vt mode and DCB in non-vt mode*/
>>          retval = get_eth_dcb_conf(pid, &port_conf, dcb_mode, num_tcs, pfc_en);
>> --
>> 2.9.5
>>

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

* [dpdk-dev] [PATCH v2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-08  2:16 [dpdk-dev] [PATCH 0/2] updates for testpmd Wei Hu (Xavier)
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
  2020-09-08  2:16 ` [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB Wei Hu (Xavier)
@ 2020-09-23  7:51 ` Wei Hu (Xavier)
  2 siblings, 0 replies; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-23  7:51 UTC (permalink / raw)
  To: ferruh.yigit, wenzhuo.lu, beilei.xing, bernard.iremonger
  Cc: xavier.huwei, lihuisong, dev

From: Huisong Li <lihuisong@huawei.com>

Currently, when starting testpmd application without '--disable-rss' and
the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS flag
is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
(dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
flag of rxmode.mq_mode in PMD drivers.

However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
in testpmd application, this will cause the inconsistent problem that
rx_offloads is different for testpmd and PMD drivers.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
v1 -> v2: add check if PMD support DEV_RX_OFFLOAD_RSS_HASH before
	  setting it.
---
 app/test-pmd/testpmd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe6450c..a36db76 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3353,11 +3353,15 @@ init_port_config(void)
 		}
 
 		if (port->dcb_flag == 0) {
-			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
+			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
 				port->dev_conf.rxmode.mq_mode =
 					(enum rte_eth_rx_mq_mode)
 						(rx_mq_mode & ETH_MQ_RX_RSS);
-			else
+				if (port->dev_info.rx_offload_capa &
+				    DEV_RX_OFFLOAD_RSS_HASH)
+					port->dev_conf.rxmode.offloads |=
+							DEV_RX_OFFLOAD_RSS_HASH;
+			} else
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
 
-- 
2.9.5


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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-23  7:04     ` Wei Hu (Xavier)
@ 2020-09-23  9:35       ` Ferruh Yigit
  2020-09-28 12:27         ` Wei Hu (Xavier)
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-09-23  9:35 UTC (permalink / raw)
  To: Wei Hu (Xavier)
  Cc: dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger, xavier.huwei,
	lihuisong, Pavan Nikhilesh, Andrew Rybchenko, jerinj

On 9/23/2020 8:04 AM, Wei Hu (Xavier) wrote:
> Hi, Ferruh Yigit
> 
> On 2020/9/23 0:21, Ferruh Yigit wrote:
>> On 9/8/2020 3:16 AM, Wei Hu (Xavier) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Currently, when starting testpmd application without '--disable-rss' and
>>> the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS flag
>>> is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
>>> DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
>>> (dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
>>> flag of rxmode.mq_mode in PMD drivers.
>>>
>>> However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
>>> in testpmd application, this will cause the inconsistent problem that
>>> rx_offloads is different for testpmd and PMD drivers.
>>
>> Yes for DEV_RX_OFFLOAD_RSS_HASH, application rx_offload config and PMD 
>> one diverges, for *some* PMDs.
>>
>> This is done to have the backward compatibility of the PMD behavior.
>>
>> The PMDs that would like to write the provide the calculated hash 
>> value back, overwrites the offload config to enable 
>> 'DEV_RX_OFFLOAD_RSS_HASH'. And does more work than user requested, 
>> this shouldn't have any side affect.
>> Some doesn't provide the hash unless user explicitly requests it.
>>
>> Applications shouldn't set it blindly, at least first check if PMD 
>> supports it but even that case, unless it is needed I think HASH 
>> offload shouldn't be requested by default.
> 
> OK, we are going to do modification to add check if PMD support 
> DEV_RX_OFFLOAD_RSS_HASH before setting it as below:
> 
>   @@ -3356,11 +3356,13 @@ init_port_config(void)
>             }
>             if (port->dcb_flag == 0) {
>                if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
>                     port->dev_conf.rxmode.mq_mode =
>                         (enum rte_eth_rx_mq_mode)
>                             (rx_mq_mode & ETH_MQ_RX_RSS);
>             if (port->dev_info.rx_offload_capa &
>                DEV_RX_OFFLOAD_RSS_HASH)
>               port->dev_conf.rxmode.offloads |=
>                          DEV_RX_OFFLOAD_RSS_HASH;
>               } else
>                     port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>             }
> 

Hi Xavier,

The capability check is correct thing to do, but even in that case I am 
not sure if we should set the config by default.

The reason to extract the 'DEV_RX_OFFLOAD_RSS_HASH' offload is to gain 
performance for some NICs, enabling it by default defeats the purpose.

The offload should be enabled when application needs the provided hash 
value.

I can see you are trying to remove the divergence between PMD and 
application config, this can be fixed when all PMDs take this user 
offload request into account instead of overwriting, but for some PMDs 
it is cheaper to provide the hash value instead of checks, so this 
divergence is not easy to address.


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

* Re: [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB
  2020-09-23  7:14     ` Wei Hu (Xavier)
@ 2020-09-24  2:41       ` Wei Hu (Xavier)
  0 siblings, 0 replies; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-24  2:41 UTC (permalink / raw)
  To: Ajit Khaparde
  Cc: dpdk-dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger,
	Jijiang Liu, Jingjing Wu, Helin Zhang, Wei Hu (Xavier, lihuisong,
	Ferruh Yigit

Hi, Ajit Khaparde

On 2020/9/23 15:14, Wei Hu (Xavier) wrote:
> Hi, Ajit Khaparde
>
> On 2020/9/22 13:15, Ajit Khaparde wrote:
>> On Mon, Sep 7, 2020 at 7:16 PM Wei Hu (Xavier)
>> <huwei013@chinasoftinc.com> wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Sometimes, we have to start testpmd application with 
>>> --pkt-filter-mode to
>>> test flow table feature. When using 'port config 0 dcb vt off 4 pfc on'
>>> command, the dcb information are configured in init_port_dcb_config
>>> function by get_eth_dcb_conf function, And then 
>>> rte_eth_dev_configure API
>>> function will be called to re-configure PMD driver.
>>>
>>> The values of rxmode and txmode in rte_eth_conf struct used to 
>>> configure
>>> PMD driver, come from the current values maintained in testpmd. 
>>> However,
>>> the fdir_conf.mode in rte_eth_conf struct is not set, which may 
>>> cause that
>>> some PMD driver cannot test the flow table feature when multiple TCs 
>>> are
>>> enabled.
>>>
>>> Fixes: 1a572499beb6 ("app/testpmd: setup DCB forwarding based on 
>>> traffic class")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>> ---
>>>   app/test-pmd/testpmd.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index 73543bb..19bf972 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -3521,6 +3521,7 @@ init_port_dcb_config(portid_t pid,
>>>
>>>          port_conf.rxmode = rte_port->dev_conf.rxmode;
>>>          port_conf.txmode = rte_port->dev_conf.txmode;
>>> +       port_conf.fdir_conf.mode = rte_port->dev_conf.fdir_conf.mode;
>> This is probably not necessary. FDIR is due for deprecation [1].
>> https://doc.dpdk.org/guides/rel_notes/deprecation.html
>
> Thanks your for pointing out it, this patch is really necessary.
>
Sorry, it is a typo.

This patch is not really necessary.


Thanks, Xavier

>
> Thanks, Xavier
>
>>>          /*set configuration of DCB in vt mode and DCB in non-vt mode*/
>>>          retval = get_eth_dcb_conf(pid, &port_conf, dcb_mode, 
>>> num_tcs, pfc_en);
>>> -- 
>>> 2.9.5
>>>

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS
  2020-09-23  9:35       ` Ferruh Yigit
@ 2020-09-28 12:27         ` Wei Hu (Xavier)
  0 siblings, 0 replies; 11+ messages in thread
From: Wei Hu (Xavier) @ 2020-09-28 12:27 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: dev, Wenzhuo Lu, Beilei Xing, Bernard Iremonger, xavier.huwei,
	lihuisong, Pavan Nikhilesh, Andrew Rybchenko, jerinj

Hi, Ferruh Yigit

On 2020/9/23 17:35, Ferruh Yigit wrote:
> On 9/23/2020 8:04 AM, Wei Hu (Xavier) wrote:
>> Hi, Ferruh Yigit
>>
>> On 2020/9/23 0:21, Ferruh Yigit wrote:
>>> On 9/8/2020 3:16 AM, Wei Hu (Xavier) wrote:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> Currently, when starting testpmd application without 
>>>> '--disable-rss' and
>>>> the number of Rx queue configured is greater than 1, ETH_MQ_RX_RSS 
>>>> flag
>>>> is set in port->dev_conf.rxmode.mq_mode in testpmd application, and
>>>> DEV_RX_OFFLOAD_RSS_HASH flag is set in rx_offloads
>>>> (dev->data->dev_conf.rxmode.offloads) according to the ETH_MQ_RX_RSS
>>>> flag of rxmode.mq_mode in PMD drivers.
>>>>
>>>> However, DEV_RX_OFFLOAD_RSS_HASH is not set to rx_offloads maintained
>>>> in testpmd application, this will cause the inconsistent problem that
>>>> rx_offloads is different for testpmd and PMD drivers.
>>>
>>> Yes for DEV_RX_OFFLOAD_RSS_HASH, application rx_offload config and 
>>> PMD one diverges, for *some* PMDs.
>>>
>>> This is done to have the backward compatibility of the PMD behavior.
>>>
>>> The PMDs that would like to write the provide the calculated hash 
>>> value back, overwrites the offload config to enable 
>>> 'DEV_RX_OFFLOAD_RSS_HASH'. And does more work than user requested, 
>>> this shouldn't have any side affect.
>>> Some doesn't provide the hash unless user explicitly requests it.
>>>
>>> Applications shouldn't set it blindly, at least first check if PMD 
>>> supports it but even that case, unless it is needed I think HASH 
>>> offload shouldn't be requested by default.
>>
>> OK, we are going to do modification to add check if PMD support 
>> DEV_RX_OFFLOAD_RSS_HASH before setting it as below:
>>
>>   @@ -3356,11 +3356,13 @@ init_port_config(void)
>>             }
>>             if (port->dcb_flag == 0) {
>>                if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
>>                     port->dev_conf.rxmode.mq_mode =
>>                         (enum rte_eth_rx_mq_mode)
>>                             (rx_mq_mode & ETH_MQ_RX_RSS);
>>             if (port->dev_info.rx_offload_capa &
>>                DEV_RX_OFFLOAD_RSS_HASH)
>>               port->dev_conf.rxmode.offloads |=
>>                          DEV_RX_OFFLOAD_RSS_HASH;
>>               } else
>>                     port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>>             }
>>
>
> Hi Xavier,
>
> The capability check is correct thing to do, but even in that case I 
> am not sure if we should set the config by default.
>
> The reason to extract the 'DEV_RX_OFFLOAD_RSS_HASH' offload is to gain 
> performance for some NICs, enabling it by default defeats the purpose.
>
> The offload should be enabled when application needs the provided hash 
> value.
>
> I can see you are trying to remove the divergence between PMD and 
> application config, this can be fixed when all PMDs take this user 
> offload request into account instead of overwriting, but for some PMDs 
> it is cheaper to provide the hash value instead of checks, so this 
> divergence is not easy to address.
>
OK, please ignore this patch and the V2 patch.

http://patches.dpdk.org/patch/78502/

Thanks.


Regards

Xavier


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

end of thread, other threads:[~2020-09-28 12:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08  2:16 [dpdk-dev] [PATCH 0/2] updates for testpmd Wei Hu (Xavier)
2020-09-08  2:16 ` [dpdk-dev] [PATCH 1/2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)
2020-09-22 16:21   ` Ferruh Yigit
2020-09-23  7:04     ` Wei Hu (Xavier)
2020-09-23  9:35       ` Ferruh Yigit
2020-09-28 12:27         ` Wei Hu (Xavier)
2020-09-08  2:16 ` [dpdk-dev] [PATCH 2/2] app/testpmd: retain original FDIR mode when configuring DCB Wei Hu (Xavier)
2020-09-22  5:15   ` Ajit Khaparde
2020-09-23  7:14     ` Wei Hu (Xavier)
2020-09-24  2:41       ` Wei Hu (Xavier)
2020-09-23  7:51 ` [dpdk-dev] [PATCH v2] app/testpmd: update Rx RSS HASH offload when setting MQ RSS Wei Hu (Xavier)

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git