On 4/12/2021 3:21 AM, Li, Xiaoyun wrote:
-----Original Message-----
From: Yu, DapengX <dapengx...@intel.com>
Sent: Friday, April 9, 2021 18:29
To: Li, Xiaoyun <xiaoyun...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>;
Zhang, Qi Z <qi.z.zh...@intel.com>
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx offload
reconfig cmd
-----Original Message-----
From: Li, Xiaoyun <xiaoyun...@intel.com>
Sent: Friday, April 9, 2021 3:50 PM
To: Yu, DapengX <dapengx...@intel.com>; Yigit, Ferruh
<ferruh.yi...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx
offload reconfig cmd
-----Original Message-----
From: Yu, DapengX <dapengx...@intel.com>
Sent: Friday, April 9, 2021 13:25
To: Yigit, Ferruh <ferruh.yi...@intel.com>; Li, Xiaoyun
<xiaoyun...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx
offload reconfig cmd
-----Original Message-----
From: Yigit, Ferruh <ferruh.yi...@intel.com>
Sent: Thursday, April 8, 2021 11:42 PM
To: Yu, DapengX <dapengx...@intel.com>; Li, Xiaoyun
<xiaoyun...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and
Tx offload reconfig cmd
On 4/1/2021 9:28 AM, dapengx...@intel.com wrote:
From: Dapeng Yu <dapengx...@intel.com>
Configure per queue rx offloading and per queue tx offloading
command shouldn't trigger the rte_eth_dev_configure() to
reconfigure
device.
The patch sets the queue reconfiguration flag only, and does not
set the device reconfiguration flag. Therefore after port is
restarted,
rte_eth_dev_configure() will not be called again.
Just to clarify the impact, was calling 'rte_eth_dev_configure()'
causing any problem, is this fixing any issue?
Or is this patch an optimization to eliminate an unnecessary call?
This patch does fix an issue, and it also eliminates an unnecessary call.
The issue is:
per-queue configuration, for example: port 0 rxq 0 rx_offload
jumbo_frame off triggers the per-device configuration change: the
RSS key is reconfigured and changes after rte_eth_dev_configure() is
called on ICE PMD driver, that cause a test case failure.
Hmmm. I agree on the following. It doesn't need dev_configure. That's
why I give you ack.
But your issue. Shouldn't you fix the driver? The vsi->rss_key[] just
updated itself as a random value when dev_conf doesn't contain one.
But your case is dev_conf doesn't contain new rss key, but
vsi->rss_key is not null. In this case, it should keep the same not get a new
random key.
In current implementation, dev->data->dev_conf.rx_adv_conf.rss_conf in PMD
does not hold the rss_key value if user does not set it in the input parameter
of
rte_eth_dev_configure().
Even if PMD generate one automatically, it will not be saved in dev->data-
dev_conf.rx_adv_conf.rss_conf.
When user want to get rss_key, it will be retrieved on the fly from hardware,
but
not from any variable in PMD.
So PMD (ice and i40e) think only rss_key which is set by user via
rte_eth_dev_configure() can be reused when port is reconfigured.
If it is not present, PMD will generate one by itself anyway even if it is
present in
vsi->rss_key.
I don’t think this behavior is wrong, so did not fix ice PMD.
If you want to recover the rss key. The driver should store the key in
vsi->rss_key also in ice_rss_hash_update(). Then when user not config rss_key in
rss_conf and vs->rss_key is not 0, you should set the original value not a random
value to hw.
+1
Otherwise, you are assuming the behavior rss to different queues is right since
user want random key. It's not an issue.
Any scenario, it's not testpmd issue. Please withdraw your patch.
There is an unnecessary call in original implementation because both
cmd_config_per_queue_rx_offload_parsed() and
cmd_config_per_queue_tx_offload_parsed()
does not update the "port->dev_conf" which hold the port
configuration, therefore there is no need to call rte_eth_dev_configure().
