[2/3] app/testpmd: fix the initial value when setting PFC

Message ID 20200121114433.57543-3-huwei013@chinasoftinc.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fixes for testpmd application |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wei Hu (Xavier) Jan. 21, 2020, 11:44 a.m. UTC
  From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

Currently, the initial values of the local structure variable named
rx_tx_onoff_2_lfc_mode and rx_tx_onoff_2_pfc_mode are different in the
similar part of these two following functions:
	cmd_link_flow_ctrl_set_parsed
	cmd_priority_flow_ctrl_set_parsed
1) The code snippset in cmd_link_flow_ctrl_set_parsed function:
	static enum rte_eth_fc_mode rx_tx_onoff_2_lfc_mode[2][2] = {
	    {RTE_FC_NONE, RTE_FC_TX_PAUSE}, {RTE_FC_RX_PAUSE, RTE_FC_FULL}
	};

	if (!cmd || cmd == &cmd_link_flow_control_set_rx)
		rx_fc_en = (!strcmp(res->rx_lfc_mode, "on")) ? 1 : 0;
	if (!cmd || cmd == &cmd_link_flow_control_set_tx)
		tx_fc_en = (!strcmp(res->tx_lfc_mode, "on")) ? 1 : 0;

	fc_conf.mode = rx_tx_onoff_2_lfc_mode[rx_fc_en][tx_fc_en];
	<...>
	ret = rte_eth_dev_flow_ctrl_set(res->port_id, &fc_conf);
	<...>
2) The code snippset in cmd_priority_flow_ctrl_set_parsed function:
	static enum rte_eth_fc_mode rx_tx_onoff_2_pfc_mode[2][2] = {
	    {RTE_FC_NONE, RTE_FC_RX_PAUSE}, {RTE_FC_TX_PAUSE, RTE_FC_FULL}
	};

	rx_fc_enable = (!strncmp(res->rx_pfc_mode, "on",2)) ? 1 : 0;
	tx_fc_enable = (!strncmp(res->tx_pfc_mode, "on",2)) ? 1 : 0;
	pfc_conf.fc.mode =
		 rx_tx_onoff_2_pfc_mode[rx_fc_enable][tx_fc_enable];
	<...>
	ret = rte_eth_dev_priority_flow_ctrl_set(res->port_id, &pfc_conf);
	<...>
The initial value of rx_tx_onoff_2_pfc_mode is wrong, it should be the
same as rx_tx_onoff_2_lfc_mode.

Fixes: 9b53e542e9e1 ("app/testpmd: add priority flow control")
Cc: stable@dpdk.org

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

Comments

Ferruh Yigit Jan. 28, 2020, 11:21 a.m. UTC | #1
On 1/21/2020 11:44 AM, Wei Hu (Xavier) wrote:
> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
> 
> Currently, the initial values of the local structure variable named
> rx_tx_onoff_2_lfc_mode and rx_tx_onoff_2_pfc_mode are different in the
> similar part of these two following functions:
> 	cmd_link_flow_ctrl_set_parsed
> 	cmd_priority_flow_ctrl_set_parsed
> 1) The code snippset in cmd_link_flow_ctrl_set_parsed function:
> 	static enum rte_eth_fc_mode rx_tx_onoff_2_lfc_mode[2][2] = {
> 	    {RTE_FC_NONE, RTE_FC_TX_PAUSE}, {RTE_FC_RX_PAUSE, RTE_FC_FULL}
> 	};
> 
> 	if (!cmd || cmd == &cmd_link_flow_control_set_rx)
> 		rx_fc_en = (!strcmp(res->rx_lfc_mode, "on")) ? 1 : 0;
> 	if (!cmd || cmd == &cmd_link_flow_control_set_tx)
> 		tx_fc_en = (!strcmp(res->tx_lfc_mode, "on")) ? 1 : 0;
> 
> 	fc_conf.mode = rx_tx_onoff_2_lfc_mode[rx_fc_en][tx_fc_en];
> 	<...>
> 	ret = rte_eth_dev_flow_ctrl_set(res->port_id, &fc_conf);
> 	<...>
> 2) The code snippset in cmd_priority_flow_ctrl_set_parsed function:
> 	static enum rte_eth_fc_mode rx_tx_onoff_2_pfc_mode[2][2] = {
> 	    {RTE_FC_NONE, RTE_FC_RX_PAUSE}, {RTE_FC_TX_PAUSE, RTE_FC_FULL}
> 	};
> 
> 	rx_fc_enable = (!strncmp(res->rx_pfc_mode, "on",2)) ? 1 : 0;
> 	tx_fc_enable = (!strncmp(res->tx_pfc_mode, "on",2)) ? 1 : 0;
> 	pfc_conf.fc.mode =
> 		 rx_tx_onoff_2_pfc_mode[rx_fc_enable][tx_fc_enable];
> 	<...>
> 	ret = rte_eth_dev_priority_flow_ctrl_set(res->port_id, &pfc_conf);
> 	<...>
> The initial value of rx_tx_onoff_2_pfc_mode is wrong, it should be the
> same as rx_tx_onoff_2_lfc_mode.
> 
> Fixes: 9b53e542e9e1 ("app/testpmd: add priority flow control")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Xuan Li <lixuan47@hisilicon.com>

Not tested but looks reasonable from the code, it would be nice if someone can
test it.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
  
Ferruh Yigit Feb. 4, 2020, 6:25 p.m. UTC | #2
On 1/28/2020 11:21 AM, Ferruh Yigit wrote:
> On 1/21/2020 11:44 AM, Wei Hu (Xavier) wrote:
>> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
>>
>> Currently, the initial values of the local structure variable named
>> rx_tx_onoff_2_lfc_mode and rx_tx_onoff_2_pfc_mode are different in the
>> similar part of these two following functions:
>> 	cmd_link_flow_ctrl_set_parsed
>> 	cmd_priority_flow_ctrl_set_parsed
>> 1) The code snippset in cmd_link_flow_ctrl_set_parsed function:
>> 	static enum rte_eth_fc_mode rx_tx_onoff_2_lfc_mode[2][2] = {
>> 	    {RTE_FC_NONE, RTE_FC_TX_PAUSE}, {RTE_FC_RX_PAUSE, RTE_FC_FULL}
>> 	};
>>
>> 	if (!cmd || cmd == &cmd_link_flow_control_set_rx)
>> 		rx_fc_en = (!strcmp(res->rx_lfc_mode, "on")) ? 1 : 0;
>> 	if (!cmd || cmd == &cmd_link_flow_control_set_tx)
>> 		tx_fc_en = (!strcmp(res->tx_lfc_mode, "on")) ? 1 : 0;
>>
>> 	fc_conf.mode = rx_tx_onoff_2_lfc_mode[rx_fc_en][tx_fc_en];
>> 	<...>
>> 	ret = rte_eth_dev_flow_ctrl_set(res->port_id, &fc_conf);
>> 	<...>
>> 2) The code snippset in cmd_priority_flow_ctrl_set_parsed function:
>> 	static enum rte_eth_fc_mode rx_tx_onoff_2_pfc_mode[2][2] = {
>> 	    {RTE_FC_NONE, RTE_FC_RX_PAUSE}, {RTE_FC_TX_PAUSE, RTE_FC_FULL}
>> 	};
>>
>> 	rx_fc_enable = (!strncmp(res->rx_pfc_mode, "on",2)) ? 1 : 0;
>> 	tx_fc_enable = (!strncmp(res->tx_pfc_mode, "on",2)) ? 1 : 0;
>> 	pfc_conf.fc.mode =
>> 		 rx_tx_onoff_2_pfc_mode[rx_fc_enable][tx_fc_enable];
>> 	<...>
>> 	ret = rte_eth_dev_priority_flow_ctrl_set(res->port_id, &pfc_conf);
>> 	<...>
>> The initial value of rx_tx_onoff_2_pfc_mode is wrong, it should be the
>> same as rx_tx_onoff_2_lfc_mode.
>>
>> Fixes: 9b53e542e9e1 ("app/testpmd: add priority flow control")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Signed-off-by: Xuan Li <lixuan47@hisilicon.com>
> 
> Not tested but looks reasonable from the code, it would be nice if someone can
> test it.
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index dab22bc4d..a09cb87e1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7087,7 +7087,7 @@  cmd_priority_flow_ctrl_set_parsed(void *parsed_result,
 	 * the RTE_FC_RX_PAUSE, Respond to the pause frame at the Tx side.
 	 */
 	static enum rte_eth_fc_mode rx_tx_onoff_2_pfc_mode[2][2] = {
-			{RTE_FC_NONE, RTE_FC_RX_PAUSE}, {RTE_FC_TX_PAUSE, RTE_FC_FULL}
+		{RTE_FC_NONE, RTE_FC_TX_PAUSE}, {RTE_FC_RX_PAUSE, RTE_FC_FULL}
 	};
 
 	rx_fc_enable = (!strncmp(res->rx_pfc_mode, "on",2)) ? 1 : 0;