diff mbox series

[v2] app/testpmd: fix dynamic config error

Message ID 20201223085152.20866-1-stevex.yang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] app/testpmd: fix dynamic config error |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Steve Yang Dec. 23, 2020, 8:51 a.m. UTC 
  The offloads of 'tx/rx_conf' didn't keep up with the corresponding
offloads of 'dev_conf', it would cause the configuration invalid.

For example:
Configuring 'max-pkt-len' would change 'rx_offloads' in dev_conf while
rx_conf.offloads of each queue still kept the old value.
It would cause the failure of offloads check in 'rte_eth_rx_queue_setup'.

This patch applied tx/rx offloads configuration for each queue
once it changed.

Fixes: 5e91aeef218c ("app/testpmd: fix offload flags after port config")

Signed-off-by: Steve Yang <stevex.yang@intel.com>
---
v2:
 * moved the update logic to 'rxtx_port_config';
 * added the 'tx_conf' part;
 * optimized the 'default' assignment;
---
 app/test-pmd/testpmd.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Li, Xiaoyun Dec. 23, 2020, 9 a.m. UTC | #1 
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

> -----Original Message-----
> From: Steve Yang <stevex.yang@intel.com>
> Sent: Wednesday, December 23, 2020 16:52
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>
> Subject: [PATCH v2] app/testpmd: fix dynamic config error
> 
> The offloads of 'tx/rx_conf' didn't keep up with the corresponding offloads of
> 'dev_conf', it would cause the configuration invalid.
> 
> For example:
> Configuring 'max-pkt-len' would change 'rx_offloads' in dev_conf while
> rx_conf.offloads of each queue still kept the old value.
> It would cause the failure of offloads check in 'rte_eth_rx_queue_setup'.
> 
> This patch applied tx/rx offloads configuration for each queue once it changed.
> 
> Fixes: 5e91aeef218c ("app/testpmd: fix offload flags after port config")
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
> v2:
>  * moved the update logic to 'rxtx_port_config';
>  * added the 'tx_conf' part;
>  * optimized the 'default' assignment;
> ---
>  app/test-pmd/testpmd.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 33a060dffd..6ee28e3797 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3288,9 +3288,11 @@ rxtx_port_config(struct rte_port *port)
> 
>  	for (qid = 0; qid < nb_rxq; qid++) {
>  		offloads = port->rx_conf[qid].offloads;
> -		port->rx_conf[qid] = port->dev_info.default_rxconf;
> -		if (offloads != 0)
> -			port->rx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.rxmode.offloads)
> +			port->rx_conf[qid].offloads =
> +				port->dev_conf.rxmode.offloads;
> +		if (!offloads)
> +			port->rx_conf[qid] = port->dev_info.default_rxconf;
> 
>  		/* Check if any Rx parameters have been passed */
>  		if (rx_pthresh != RTE_PMD_PARAM_UNSET) @@ -3313,9
> +3315,11 @@ rxtx_port_config(struct rte_port *port)
> 
>  	for (qid = 0; qid < nb_txq; qid++) {
>  		offloads = port->tx_conf[qid].offloads;
> -		port->tx_conf[qid] = port->dev_info.default_txconf;
> -		if (offloads != 0)
> -			port->tx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.txmode.offloads)
> +			port->tx_conf[qid].offloads =
> +				port->dev_conf.txmode.offloads;
> +		if (!offloads)
> +			port->tx_conf[qid] = port->dev_info.default_txconf;
> 
>  		/* Check if any Tx parameters have been passed */
>  		if (tx_pthresh != RTE_PMD_PARAM_UNSET)
> --
> 2.17.1
Chen, BoX C Jan. 13, 2021, 8:13 a.m. UTC | #2 
Tested-by:  Chen, BoX C <BoX.C.Chen@intel.com>

Regards,
Chen Bo

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Steve Yang
> Sent: December 23, 2020 16:52
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix dynamic config error
> 
> The offloads of 'tx/rx_conf' didn't keep up with the corresponding offloads
> of 'dev_conf', it would cause the configuration invalid.
> 
> For example:
> Configuring 'max-pkt-len' would change 'rx_offloads' in dev_conf while
> rx_conf.offloads of each queue still kept the old value.
> It would cause the failure of offloads check in 'rte_eth_rx_queue_setup'.
> 
> This patch applied tx/rx offloads configuration for each queue once it
> changed.
> 
> Fixes: 5e91aeef218c ("app/testpmd: fix offload flags after port config")
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
> v2:
>  * moved the update logic to 'rxtx_port_config';
>  * added the 'tx_conf' part;
>  * optimized the 'default' assignment;
> ---
>  app/test-pmd/testpmd.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 33a060dffd..6ee28e3797 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3288,9 +3288,11 @@ rxtx_port_config(struct rte_port *port)
> 
>  	for (qid = 0; qid < nb_rxq; qid++) {
>  		offloads = port->rx_conf[qid].offloads;
> -		port->rx_conf[qid] = port->dev_info.default_rxconf;
> -		if (offloads != 0)
> -			port->rx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.rxmode.offloads)
> +			port->rx_conf[qid].offloads =
> +				port->dev_conf.rxmode.offloads;
> +		if (!offloads)
> +			port->rx_conf[qid] = port->dev_info.default_rxconf;
> 
>  		/* Check if any Rx parameters have been passed */
>  		if (rx_pthresh != RTE_PMD_PARAM_UNSET) @@ -3313,9
> +3315,11 @@ rxtx_port_config(struct rte_port *port)
> 
>  	for (qid = 0; qid < nb_txq; qid++) {
>  		offloads = port->tx_conf[qid].offloads;
> -		port->tx_conf[qid] = port->dev_info.default_txconf;
> -		if (offloads != 0)
> -			port->tx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.txmode.offloads)
> +			port->tx_conf[qid].offloads =
> +				port->dev_conf.txmode.offloads;
> +		if (!offloads)
> +			port->tx_conf[qid] = port->dev_info.default_txconf;
> 
>  		/* Check if any Tx parameters have been passed */
>  		if (tx_pthresh != RTE_PMD_PARAM_UNSET)
> --
> 2.17.1
Ferruh Yigit Jan. 19, 2021, 3:44 p.m. UTC | #3 
On 12/23/2020 8:51 AM, Steve Yang wrote:
> The offloads of 'tx/rx_conf' didn't keep up with the corresponding
> offloads of 'dev_conf', it would cause the configuration invalid.
> 
> For example:
> Configuring 'max-pkt-len' would change 'rx_offloads' in dev_conf while
> rx_conf.offloads of each queue still kept the old value.
> It would cause the failure of offloads check in 'rte_eth_rx_queue_setup'.
> 

Can you please give some details how can I reproduce the issue?

> This patch applied tx/rx offloads configuration for each queue
> once it changed.
> 
> Fixes: 5e91aeef218c ("app/testpmd: fix offload flags after port config")
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
> v2:
>   * moved the update logic to 'rxtx_port_config';
>   * added the 'tx_conf' part;
>   * optimized the 'default' assignment;
> ---
>   app/test-pmd/testpmd.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 33a060dffd..6ee28e3797 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3288,9 +3288,11 @@ rxtx_port_config(struct rte_port *port)
>   
>   	for (qid = 0; qid < nb_rxq; qid++) {
>   		offloads = port->rx_conf[qid].offloads;
> -		port->rx_conf[qid] = port->dev_info.default_rxconf;
> -		if (offloads != 0)
> -			port->rx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.rxmode.offloads)
> +			port->rx_conf[qid].offloads =
> +				port->dev_conf.rxmode.offloads;

Isn't 'rxmode.offloads' for the port, but the 'rx_conf[qid].offloads' for the 
queue, can this cause problem if the queue level offloads used?

> +		if (!offloads)
> +			port->rx_conf[qid] = port->dev_info.default_rxconf;

Can you please explain this, why the default config is used, only if the 
'offload' is zero?
The original code is always using the default config, but overwriting the 
'offloads' if needed.

>   
>   		/* Check if any Rx parameters have been passed */
>   		if (rx_pthresh != RTE_PMD_PARAM_UNSET)
> @@ -3313,9 +3315,11 @@ rxtx_port_config(struct rte_port *port)
>   
>   	for (qid = 0; qid < nb_txq; qid++) {
>   		offloads = port->tx_conf[qid].offloads;
> -		port->tx_conf[qid] = port->dev_info.default_txconf;
> -		if (offloads != 0)
> -			port->tx_conf[qid].offloads = offloads;
> +		if (offloads != port->dev_conf.txmode.offloads)
> +			port->tx_conf[qid].offloads =
> +				port->dev_conf.txmode.offloads;
> +		if (!offloads)
> +			port->tx_conf[qid] = port->dev_info.default_txconf;
>   
>   		/* Check if any Tx parameters have been passed */
>   		if (tx_pthresh != RTE_PMD_PARAM_UNSET)
>
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 33a060dffd..6ee28e3797 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3288,9 +3288,11 @@  rxtx_port_config(struct rte_port *port)
 
 	for (qid = 0; qid < nb_rxq; qid++) {
 		offloads = port->rx_conf[qid].offloads;
-		port->rx_conf[qid] = port->dev_info.default_rxconf;
-		if (offloads != 0)
-			port->rx_conf[qid].offloads = offloads;
+		if (offloads != port->dev_conf.rxmode.offloads)
+			port->rx_conf[qid].offloads =
+				port->dev_conf.rxmode.offloads;
+		if (!offloads)
+			port->rx_conf[qid] = port->dev_info.default_rxconf;
 
 		/* Check if any Rx parameters have been passed */
 		if (rx_pthresh != RTE_PMD_PARAM_UNSET)
@@ -3313,9 +3315,11 @@  rxtx_port_config(struct rte_port *port)
 
 	for (qid = 0; qid < nb_txq; qid++) {
 		offloads = port->tx_conf[qid].offloads;
-		port->tx_conf[qid] = port->dev_info.default_txconf;
-		if (offloads != 0)
-			port->tx_conf[qid].offloads = offloads;
+		if (offloads != port->dev_conf.txmode.offloads)
+			port->tx_conf[qid].offloads =
+				port->dev_conf.txmode.offloads;
+		if (!offloads)
+			port->tx_conf[qid] = port->dev_info.default_txconf;
 
 		/* Check if any Tx parameters have been passed */
 		if (tx_pthresh != RTE_PMD_PARAM_UNSET)