[dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

This patch supports the query of the link flow control parameter
on a port.

The command format is as follows:
show port <port_id> flow_ctrl

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
 2 files changed, 90 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..9fa85c4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"show port (port_id) fec_mode"
 			"	Show fec mode of a port.\n\n"
+
+			"show port <port_id> flow_ctrl"
+			"	Show flow control info of a port.\n\n"
 		);
 	}
 
@@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
 	},
 };
 
+/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
+struct cmd_link_flow_ctrl_show {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	portid_t port_id;
+	cmdline_fixed_string_t flow_ctrl;
+};
+
+cmdline_parse_token_string_t cmd_lfc_show_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				show, "show");
+cmdline_parse_token_string_t cmd_lfc_show_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port, "port");
+cmdline_parse_token_num_t cmd_lfc_show_portid =
+	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port_id, RTE_UINT16);
+cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				flow_ctrl, "flow_ctrl");
+
+static void
+cmd_link_flow_ctrl_show_parsed(void *parsed_result,
+			      __rte_unused struct cmdline *cl,
+			      __rte_unused void *data)
+{
+	struct cmd_link_flow_ctrl_show *res = parsed_result;
+	static const char *info_border = "*********************";
+	struct rte_eth_fc_conf fc_conf;
+	bool rx_fc_en = false;
+	bool tx_fc_en = false;
+	int ret;
+
+	if (!rte_eth_dev_is_valid_port(res->port_id)) {
+		printf("Invalid port id %u\n", res->port_id);
+		return;
+	}
+
+	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
+	if (ret != 0) {
+		printf("Failed to get current flow ctrl information: err = %d\n",
+		       ret);
+		return;
+	}
+
+	if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+		rx_fc_en = true;
+	if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+		tx_fc_en = true;
+
+	printf("\n%s Flow control infos for port %-2d %s\n",
+		info_border, res->port_id, info_border);
+	printf("FC mode:\n");
+	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
+	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
+	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
+	printf("pause_time: 0x%x\n", fc_conf.pause_time);
+	printf("high_water: 0x%x\n", fc_conf.high_water);
+	printf("low_water: 0x%x\n", fc_conf.low_water);
+	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+	printf("mac ctrl frame fwd: %s\n",
+		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+	printf("\n%s**************   End  ***********%s\n",
+		info_border, info_border);
+}
+
+cmdline_parse_inst_t cmd_link_flow_control_show = {
+	.f = cmd_link_flow_ctrl_show_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> flow_ctrl",
+	.tokens = {
+		(void *)&cmd_lfc_show_show,
+		(void *)&cmd_lfc_show_port,
+		(void *)&cmd_lfc_show_portid,
+		(void *)&cmd_lfc_show_flow_ctrl,
+		NULL,
+	},
+};
+
 /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
 struct cmd_link_flow_ctrl_set_result {
 	cmdline_fixed_string_t set;
@@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
+	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
 	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
 	(cmdline_parse_inst_t *)&cmd_config_dcb,
 	(cmdline_parse_inst_t *)&cmd_read_reg,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index e3bfed5..3fca011 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1526,6 +1526,13 @@ Where:
 
 * ``autoneg``: Change the auto-negotiation parameter.
 
+show flow ctrl
+~~~~~~~~~~~~~~
+
+show the link flow control parameter on a port::
+
+   testpmd> show port <port_id> flow_ctrl
+
 set pfc_ctrl rx
 ~~~~~~~~~~~~~~~
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Thursday, April 15, 2021 14:47
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
> 
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>  2 files changed, 90 insertions(+)
> 
<snip>
> +	printf("\n%s Flow control infos for port %-2d %s\n",
> +		info_border, res->port_id, info_border);
> +	printf("FC mode:\n");
> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");

"fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.

> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
> +	printf("high_water: 0x%x\n", fc_conf.high_water);
> +	printf("low_water: 0x%x\n", fc_conf.low_water);
> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> +	printf("mac ctrl frame fwd: %s\n",

Follow others' format will be better like "Send Xon".
"Forward MAC control frames:"

> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
> +	printf("\n%s**************   End  ***********%s\n",
> +		info_border, info_border);
> +}
> +
<snip>
> 
> --
> 2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

Hi,

在 2021/4/19 10:53, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Thursday, April 15, 2021 14:47
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
>> Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
>>
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> This patch supports the query of the link flow control parameter on a port.
>>
>> The command format is as follows:
>> show port <port_id> flow_ctrl
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>   2 files changed, 90 insertions(+)
>>
> <snip>
>> +	printf("\n%s Flow control infos for port %-2d %s\n",
>> +		info_border, res->port_id, info_border);
>> +	printf("FC mode:\n");
>> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
> 
> "fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.Got it.
>> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
>> +	printf("high_water: 0x%x\n", fc_conf.high_water);
>> +	printf("low_water: 0x%x\n", fc_conf.low_water);
>> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>> +	printf("mac ctrl frame fwd: %s\n",
> 
> Follow others' format will be better like "Send Xon".
> "Forward MAC control frames:"
> I don not catch  your meaning. Is that right?:
change the statement
"
+	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+	printf("mac ctrl frame fwd: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" : 
"Off")
"
to

"
+	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+	printf("Forward MAC control frames: %s\n",fc_conf.mac_ctrl_frame_fwd ? 
"On" : "Off")
"

>> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
>> +	printf("\n%s**************   End  ***********%s\n",
>> +		info_border, info_border);
>> +}
>> +
> <snip>
>>
>> --
>> 2.7.4
> 
> .
> 

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Monday, April 19, 2021 14:41
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH] app/testpmd: support the query of link flow ctrl info
> 
> Hi,
> 
> 在 2021/4/19 10:53, Li, Xiaoyun 写道:
> > Hi
> >
> >> -----Original Message-----
> >> From: Min Hu (Connor) <humin29@huawei.com>
> >> Sent: Thursday, April 15, 2021 14:47
> >> To: dev@dpdk.org
> >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
> >> Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
> >>
> >> From: Huisong Li <lihuisong@huawei.com>
> >>
> >> This patch supports the query of the link flow control parameter on a port.
> >>
> >> The command format is as follows:
> >> show port <port_id> flow_ctrl
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> ---
> >>   app/test-pmd/cmdline.c                      | 83
> +++++++++++++++++++++++++++++
> >>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
> >>   2 files changed, 90 insertions(+)
> >>
> > <snip>
> >> +	printf("\n%s Flow control infos for port %-2d %s\n",
> >> +		info_border, res->port_id, info_border);
> >> +	printf("FC mode:\n");
> >> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
> >> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
> >> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
> >
> > "fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.Got it.
> >> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
> >> +	printf("high_water: 0x%x\n", fc_conf.high_water);
> >> +	printf("low_water: 0x%x\n", fc_conf.low_water);
> >> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> >> +	printf("mac ctrl frame fwd: %s\n",
> >
> > Follow others' format will be better like "Send Xon".
> > "Forward MAC control frames:"
> > I don not catch  your meaning. Is that right?:
> change the statement

I mean change the statement as: 
printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
printf("Forward MAC control frames: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");

Keep the same format.

> "
> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> +	printf("mac ctrl frame fwd: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" :
> "Off")
> "
> to
> 
> "
> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> +	printf("Forward MAC control
> frames: %s\n",fc_conf.mac_ctrl_frame_fwd ?
> "On" : "Off")
> "
> 
> >> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
> >> +	printf("\n%s**************   End  ***********%s\n",
> >> +		info_border, info_border);
> >> +}
> >> +
> > <snip>
> >>
> >> --
> >> 2.7.4
> >
> > .
> >

[dpdk-dev] [PATCH v2] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

This patch supports the query of the link flow control parameter
on a port.

The command format is as follows:
show port <port_id> flow_ctrl

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
 2 files changed, 90 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 31884c5..06a6503 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"show port (port_id) fec_mode"
 			"	Show fec mode of a port.\n\n"
+
+			"show port <port_id> flow_ctrl"
+			"	Show flow control info of a port.\n\n"
 		);
 	}
 
@@ -6899,6 +6902,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
 	},
 };
 
+/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
+struct cmd_link_flow_ctrl_show {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	portid_t port_id;
+	cmdline_fixed_string_t flow_ctrl;
+};
+
+cmdline_parse_token_string_t cmd_lfc_show_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				show, "show");
+cmdline_parse_token_string_t cmd_lfc_show_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port, "port");
+cmdline_parse_token_num_t cmd_lfc_show_portid =
+	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port_id, RTE_UINT16);
+cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				flow_ctrl, "flow_ctrl");
+
+static void
+cmd_link_flow_ctrl_show_parsed(void *parsed_result,
+			      __rte_unused struct cmdline *cl,
+			      __rte_unused void *data)
+{
+	struct cmd_link_flow_ctrl_show *res = parsed_result;
+	static const char *info_border = "*********************";
+	struct rte_eth_fc_conf fc_conf;
+	bool rx_fc_en = false;
+	bool tx_fc_en = false;
+	int ret;
+
+	if (!rte_eth_dev_is_valid_port(res->port_id)) {
+		printf("Invalid port id %u\n", res->port_id);
+		return;
+	}
+
+	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
+	if (ret != 0) {
+		printf("Failed to get current flow ctrl information: err = %d\n",
+		       ret);
+		return;
+	}
+
+	if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+		rx_fc_en = true;
+	if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+		tx_fc_en = true;
+
+	printf("\n%s Flow control infos for port %-2d %s\n",
+		info_border, res->port_id, info_border);
+	printf("FC mode:\n");
+	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
+	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
+	printf("FC autoneg status: %s\n", fc_conf.autoneg ? "On" : "Off");
+	printf("pause_time: 0x%x\n", fc_conf.pause_time);
+	printf("high_water: 0x%x\n", fc_conf.high_water);
+	printf("low_water: 0x%x\n", fc_conf.low_water);
+	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+	printf("Forward MAC control frames: %s\n",
+		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+	printf("\n%s**************   End  ***********%s\n",
+		info_border, info_border);
+}
+
+cmdline_parse_inst_t cmd_link_flow_control_show = {
+	.f = cmd_link_flow_ctrl_show_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> flow_ctrl",
+	.tokens = {
+		(void *)&cmd_lfc_show_show,
+		(void *)&cmd_lfc_show_port,
+		(void *)&cmd_lfc_show_portid,
+		(void *)&cmd_lfc_show_flow_ctrl,
+		NULL,
+	},
+};
+
 /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
 struct cmd_link_flow_ctrl_set_result {
 	cmdline_fixed_string_t set;
@@ -17037,6 +17119,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
+	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
 	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
 	(cmdline_parse_inst_t *)&cmd_config_dcb,
 	(cmdline_parse_inst_t *)&cmd_read_reg,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 9854b70..a7dbaf2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1524,6 +1524,13 @@ Where:
 
 * ``autoneg``: Change the auto-negotiation parameter.
 
+show flow ctrl
+~~~~~~~~~~~~~~
+
+show the link flow control parameter on a port::
+
+   testpmd> show port <port_id> flow_ctrl
+
 set pfc_ctrl rx
 ~~~~~~~~~~~~~~~
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

Hi, fixed in v2, thanks

在 2021/4/19 16:04, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Monday, April 19, 2021 14:41
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: Re: [PATCH] app/testpmd: support the query of link flow ctrl info
>>
>> Hi,
>>
>> 在 2021/4/19 10:53, Li, Xiaoyun 写道:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Min Hu (Connor) <humin29@huawei.com>
>>>> Sent: Thursday, April 15, 2021 14:47
>>>> To: dev@dpdk.org
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
>>>> Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
>>>>
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> This patch supports the query of the link flow control parameter on a port.
>>>>
>>>> The command format is as follows:
>>>> show port <port_id> flow_ctrl
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> ---
>>>>    app/test-pmd/cmdline.c                      | 83
>> +++++++++++++++++++++++++++++
>>>>    doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>>>    2 files changed, 90 insertions(+)
>>>>
>>> <snip>
>>>> +	printf("\n%s Flow control infos for port %-2d %s\n",
>>>> +		info_border, res->port_id, info_border);
>>>> +	printf("FC mode:\n");
>>>> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>>>> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>>>> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
>>>
>>> "fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.Got it.
>>>> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
>>>> +	printf("high_water: 0x%x\n", fc_conf.high_water);
>>>> +	printf("low_water: 0x%x\n", fc_conf.low_water);
>>>> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>>>> +	printf("mac ctrl frame fwd: %s\n",
>>>
>>> Follow others' format will be better like "Send Xon".
>>> "Forward MAC control frames:"
>>> I don not catch  your meaning. Is that right?:
>> change the statement
> 
> I mean change the statement as:
> printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> printf("Forward MAC control frames: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
> 
> Keep the same format.
> 
>> "
>> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>> +	printf("mac ctrl frame fwd: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" :
>> "Off")
>> "
>> to
>>
>> "
>> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>> +	printf("Forward MAC control
>> frames: %s\n",fc_conf.mac_ctrl_frame_fwd ?
>> "On" : "Off")
>> "
>>
>>>> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
>>>> +	printf("\n%s**************   End  ***********%s\n",
>>>> +		info_border, info_border);
>>>> +}
>>>> +
>>> <snip>
>>>>
>>>> --
>>>> 2.7.4
>>>
>>> .
>>>
> .
> 

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Kevin Traynor]

On 15/04/2021 07:46, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter
> on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Hi Connor,

Just a few minor comments,

thanks,
Kevin.

> ---
>  app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 5bf1497..9fa85c4 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>  
>  			"show port (port_id) fec_mode"
>  			"	Show fec mode of a port.\n\n"
> +
> +			"show port <port_id> flow_ctrl"
> +			"	Show flow control info of a port.\n\n"
>  		);
>  	}
>  
> @@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
>  	},
>  };
>  
> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
> +struct cmd_link_flow_ctrl_show {
> +	cmdline_fixed_string_t show;
> +	cmdline_fixed_string_t port;
> +	portid_t port_id;
> +	cmdline_fixed_string_t flow_ctrl;
> +};
> +
> +cmdline_parse_token_string_t cmd_lfc_show_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				show, "show");
> +cmdline_parse_token_string_t cmd_lfc_show_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				port, "port");
> +cmdline_parse_token_num_t cmd_lfc_show_portid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				port_id, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				flow_ctrl, "flow_ctrl");
> +
> +static void
> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
> +			      __rte_unused struct cmdline *cl,
> +			      __rte_unused void *data)
> +{
> +	struct cmd_link_flow_ctrl_show *res = parsed_result;
> +	static const char *info_border = "*********************";
> +	struct rte_eth_fc_conf fc_conf;
> +	bool rx_fc_en = false;
> +	bool tx_fc_en = false;
> +	int ret;
> +
> +	if (!rte_eth_dev_is_valid_port(res->port_id)) {

This ^ is already checked in rte_eth_dev_flow_ctrl_get() below. That
said, it's not doing any harm here and you would need to add check for
-ENODEV below if you wanted to tell the user that the port is invalid
so either way is ok IMHO.

> +		printf("Invalid port id %u\n", res->port_id);
> +		return;
> +	}
> +
> +	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
> +	if (ret != 0) {
> +		printf("Failed to get current flow ctrl information: err = %d\n",
> +		       ret);
> +		return;
> +	}
> +
> +	if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))

You can remove unnecessary brackets, i.e.
if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)


> +		rx_fc_en = true;
> +	if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))

same comment on brackets

> +		tx_fc_en = true;
> +
> +	printf("\n%s Flow control infos for port %-2d %s\n",

s/infos/info/

> +		info_border, res->port_id, info_border);
> +	printf("FC mode:\n");

It's better not to introduce a new acronym (even if it's a bit obvious)

> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
> +	printf("high_water: 0x%x\n", fc_conf.high_water);
> +	printf("low_water: 0x%x\n", fc_conf.low_water);
> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
> +	printf("mac ctrl frame fwd: %s\n",
> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");

There's a mix of struct member names and descriptions here. I think it's
better to stick to one or the other.

i.e. currently

testpmd> show port 0 flow_ctrl

********************* Flow control infos for port 0  *********************
FC mode:
   Rx: Off
   Tx: Off
FC autoneg status: On
pause_time: 0x680
high_water: 0x80
low_water: 0x40
Send Xon: On
Forward MAC control frames: Off


Suggestion:

********************* Flow control info for port 0  *********************
Rx mode: off
Tx mode: off
Autoneg: on
Pause time: 0x680
High water: 0x80
Low water: 0x40
Send XON: on
Forward MAC control frames: off
***********************************   End  ********************************

As ever, display is subjective, so please take as suggestion, you/others
may have different view.


> +	printf("\n%s**************   End  ***********%s\n",
> +		info_border, info_border);
> +}
> +
> +cmdline_parse_inst_t cmd_link_flow_control_show = {
> +	.f = cmd_link_flow_ctrl_show_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> flow_ctrl",
> +	.tokens = {
> +		(void *)&cmd_lfc_show_show,
> +		(void *)&cmd_lfc_show_port,
> +		(void *)&cmd_lfc_show_portid,
> +		(void *)&cmd_lfc_show_flow_ctrl,
> +		NULL,
> +	},
> +};
> +
>  /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>  struct cmd_link_flow_ctrl_set_result {
>  	cmdline_fixed_string_t set;
> @@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
> +	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>  	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>  	(cmdline_parse_inst_t *)&cmd_config_dcb,
>  	(cmdline_parse_inst_t *)&cmd_read_reg,
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index e3bfed5..3fca011 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -1526,6 +1526,13 @@ Where:
>  
>  * ``autoneg``: Change the auto-negotiation parameter.
>  
> +show flow ctrl
> +~~~~~~~~~~~~~~
> +
> +show the link flow control parameter on a port::
> +
> +   testpmd> show port <port_id> flow_ctrl
> +
>  set pfc_ctrl rx
>  ~~~~~~~~~~~~~~~
>  
> 

Re: [dpdk-dev] [PATCH v2] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/19/2021 9:28 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter
> on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: support the query of link flow ctrl info

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Monday, April 19, 2021 16:28
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v2] app/testpmd: support the query of link flow ctrl info
> 
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>  2 files changed, 90 insertions(+)
> 
> 2.7.4

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/20/2021 2:34 AM, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Monday, April 19, 2021 16:28
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
>> Subject: [PATCH v2] app/testpmd: support the query of link flow ctrl info
>>
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> This patch supports the query of the link flow control parameter on a port.
>>
>> The command format is as follows:
>> show port <port_id> flow_ctrl
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>   2 files changed, 90 insertions(+)
>>
>> 2.7.4
> 
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> 

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

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Huisong Li]

Hi Kevin,

Thank you for your review.

This patchset has been applied to dpdk-next-net/main. I will send a new 
patch to fix it.

Your suggestion is better.  I intend to use the following format:

********************* Flow control info for port 0 *********************
FC mode:
     Rx pause: off
     Tx pause: off
Autoneg: on
Pause time: 0x680
High waterline: 0x80
Low waterline: 0x40
Send XON: on
Forward MAC control frames: off
*********************************** End ********************************


在 2021/4/19 23:30, Kevin Traynor 写道:
> On 15/04/2021 07:46, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> This patch supports the query of the link flow control parameter
>> on a port.
>>
>> The command format is as follows:
>> show port <port_id> flow_ctrl
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Hi Connor,
>
> Just a few minor comments,
>
> thanks,
> Kevin.
>
>> ---
>>   app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>   2 files changed, 90 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 5bf1497..9fa85c4 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>   
>>   			"show port (port_id) fec_mode"
>>   			"	Show fec mode of a port.\n\n"
>> +
>> +			"show port <port_id> flow_ctrl"
>> +			"	Show flow control info of a port.\n\n"
>>   		);
>>   	}
>>   
>> @@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
>>   	},
>>   };
>>   
>> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
>> +struct cmd_link_flow_ctrl_show {
>> +	cmdline_fixed_string_t show;
>> +	cmdline_fixed_string_t port;
>> +	portid_t port_id;
>> +	cmdline_fixed_string_t flow_ctrl;
>> +};
>> +
>> +cmdline_parse_token_string_t cmd_lfc_show_show =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>> +				show, "show");
>> +cmdline_parse_token_string_t cmd_lfc_show_port =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>> +				port, "port");
>> +cmdline_parse_token_num_t cmd_lfc_show_portid =
>> +	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
>> +				port_id, RTE_UINT16);
>> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>> +				flow_ctrl, "flow_ctrl");
>> +
>> +static void
>> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
>> +			      __rte_unused struct cmdline *cl,
>> +			      __rte_unused void *data)
>> +{
>> +	struct cmd_link_flow_ctrl_show *res = parsed_result;
>> +	static const char *info_border = "*********************";
>> +	struct rte_eth_fc_conf fc_conf;
>> +	bool rx_fc_en = false;
>> +	bool tx_fc_en = false;
>> +	int ret;
>> +
>> +	if (!rte_eth_dev_is_valid_port(res->port_id)) {
> This ^ is already checked in rte_eth_dev_flow_ctrl_get() below. That
> said, it's not doing any harm here and you would need to add check for
> -ENODEV below if you wanted to tell the user that the port is invalid
> so either way is ok IMHO.
>
>> +		printf("Invalid port id %u\n", res->port_id);
>> +		return;
>> +	}
>> +
>> +	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
>> +	if (ret != 0) {
>> +		printf("Failed to get current flow ctrl information: err = %d\n",
>> +		       ret);
>> +		return;
>> +	}
>> +
>> +	if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
> You can remove unnecessary brackets, i.e.
> if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
>
>
>> +		rx_fc_en = true;
>> +	if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
> same comment on brackets
>
>> +		tx_fc_en = true;
>> +
>> +	printf("\n%s Flow control infos for port %-2d %s\n",
> s/infos/info/
>
>> +		info_border, res->port_id, info_border);
>> +	printf("FC mode:\n");
> It's better not to introduce a new acronym (even if it's a bit obvious)
>
>> +	printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>> +	printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>> +	printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
>> +	printf("pause_time: 0x%x\n", fc_conf.pause_time);
>> +	printf("high_water: 0x%x\n", fc_conf.high_water);
>> +	printf("low_water: 0x%x\n", fc_conf.low_water);
>> +	printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>> +	printf("mac ctrl frame fwd: %s\n",
>> +		fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
> There's a mix of struct member names and descriptions here. I think it's
> better to stick to one or the other.
>
> i.e. currently
>
> testpmd> show port 0 flow_ctrl
>
> ********************* Flow control infos for port 0  *********************
> FC mode:
>     Rx: Off
>     Tx: Off
> FC autoneg status: On
> pause_time: 0x680
> high_water: 0x80
> low_water: 0x40
> Send Xon: On
> Forward MAC control frames: Off
>
>
> Suggestion:
>
> ********************* Flow control info for port 0  *********************
> Rx mode: off
> Tx mode: off
> Autoneg: on
> Pause time: 0x680
> High water: 0x80
> Low water: 0x40
> Send XON: on
> Forward MAC control frames: off
> ***********************************   End  ********************************
>
> As ever, display is subjective, so please take as suggestion, you/others
> may have different view.
>
>
>> +	printf("\n%s**************   End  ***********%s\n",
>> +		info_border, info_border);
>> +}
>> +
>> +cmdline_parse_inst_t cmd_link_flow_control_show = {
>> +	.f = cmd_link_flow_ctrl_show_parsed,
>> +	.data = NULL,
>> +	.help_str = "show port <port_id> flow_ctrl",
>> +	.tokens = {
>> +		(void *)&cmd_lfc_show_show,
>> +		(void *)&cmd_lfc_show_port,
>> +		(void *)&cmd_lfc_show_portid,
>> +		(void *)&cmd_lfc_show_flow_ctrl,
>> +		NULL,
>> +	},
>> +};
>> +
>>   /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>>   struct cmd_link_flow_ctrl_set_result {
>>   	cmdline_fixed_string_t set;
>> @@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>>   	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>>   	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>>   	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
>> +	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>>   	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>>   	(cmdline_parse_inst_t *)&cmd_config_dcb,
>>   	(cmdline_parse_inst_t *)&cmd_read_reg,
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index e3bfed5..3fca011 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -1526,6 +1526,13 @@ Where:
>>   
>>   * ``autoneg``: Change the auto-negotiation parameter.
>>   
>> +show flow ctrl
>> +~~~~~~~~~~~~~~
>> +
>> +show the link flow control parameter on a port::
>> +
>> +   testpmd> show port <port_id> flow_ctrl
>> +
>>   set pfc_ctrl rx
>>   ~~~~~~~~~~~~~~~
>>   
>>
> .

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/21/2021 7:18 AM, Huisong Li wrote:
> Hi Kevin,
> 
> Thank you for your review.
> 
> This patchset has been applied to dpdk-next-net/main. I will send a new patch to 
> fix it.
> 
> Your suggestion is better.  I intend to use the following format:
> 
> ********************* Flow control info for port 0 *********************
> FC mode:
>      Rx pause: off
>      Tx pause: off
> Autoneg: on
> Pause time: 0x680
> High waterline: 0x80
> Low waterline: 0x40
> Send XON: on
> Forward MAC control frames: off
> *********************************** End ********************************
> 

I missed the outstanding comments from Kevin, I will drop the patch from 
next-net, we can proceed with next version.

> 
> 在 2021/4/19 23:30, Kevin Traynor 写道:
>> On 15/04/2021 07:46, Min Hu (Connor) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> This patch supports the query of the link flow control parameter
>>> on a port.
>>>
>>> The command format is as follows:
>>> show port <port_id> flow_ctrl
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Hi Connor,
>>
>> Just a few minor comments,
>>
>> thanks,
>> Kevin.
>>
>>> ---
>>>   app/test-pmd/cmdline.c                      | 83 +++++++++++++++++++++++++++++
>>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>>   2 files changed, 90 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index 5bf1497..9fa85c4 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>               "show port (port_id) fec_mode"
>>>               "    Show fec mode of a port.\n\n"
>>> +
>>> +            "show port <port_id> flow_ctrl"
>>> +            "    Show flow control info of a port.\n\n"
>>>           );
>>>       }
>>> @@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
>>>       },
>>>   };
>>> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
>>> +struct cmd_link_flow_ctrl_show {
>>> +    cmdline_fixed_string_t show;
>>> +    cmdline_fixed_string_t port;
>>> +    portid_t port_id;
>>> +    cmdline_fixed_string_t flow_ctrl;
>>> +};
>>> +
>>> +cmdline_parse_token_string_t cmd_lfc_show_show =
>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>> +                show, "show");
>>> +cmdline_parse_token_string_t cmd_lfc_show_port =
>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>> +                port, "port");
>>> +cmdline_parse_token_num_t cmd_lfc_show_portid =
>>> +    TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>> +                port_id, RTE_UINT16);
>>> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>> +                flow_ctrl, "flow_ctrl");
>>> +
>>> +static void
>>> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
>>> +                  __rte_unused struct cmdline *cl,
>>> +                  __rte_unused void *data)
>>> +{
>>> +    struct cmd_link_flow_ctrl_show *res = parsed_result;
>>> +    static const char *info_border = "*********************";
>>> +    struct rte_eth_fc_conf fc_conf;
>>> +    bool rx_fc_en = false;
>>> +    bool tx_fc_en = false;
>>> +    int ret;
>>> +
>>> +    if (!rte_eth_dev_is_valid_port(res->port_id)) {
>> This ^ is already checked in rte_eth_dev_flow_ctrl_get() below. That
>> said, it's not doing any harm here and you would need to add check for
>> -ENODEV below if you wanted to tell the user that the port is invalid
>> so either way is ok IMHO.
>>
>>> +        printf("Invalid port id %u\n", res->port_id);
>>> +        return;
>>> +    }
>>> +
>>> +    ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
>>> +    if (ret != 0) {
>>> +        printf("Failed to get current flow ctrl information: err = %d\n",
>>> +               ret);
>>> +        return;
>>> +    }
>>> +
>>> +    if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
>> You can remove unnecessary brackets, i.e.
>> if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
>>
>>
>>> +        rx_fc_en = true;
>>> +    if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
>> same comment on brackets
>>
>>> +        tx_fc_en = true;
>>> +
>>> +    printf("\n%s Flow control infos for port %-2d %s\n",
>> s/infos/info/
>>
>>> +        info_border, res->port_id, info_border);
>>> +    printf("FC mode:\n");
>> It's better not to introduce a new acronym (even if it's a bit obvious)
>>
>>> +    printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>>> +    printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>>> +    printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
>>> +    printf("pause_time: 0x%x\n", fc_conf.pause_time);
>>> +    printf("high_water: 0x%x\n", fc_conf.high_water);
>>> +    printf("low_water: 0x%x\n", fc_conf.low_water);
>>> +    printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>>> +    printf("mac ctrl frame fwd: %s\n",
>>> +        fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
>> There's a mix of struct member names and descriptions here. I think it's
>> better to stick to one or the other.
>>
>> i.e. currently
>>
>> testpmd> show port 0 flow_ctrl
>>
>> ********************* Flow control infos for port 0  *********************
>> FC mode:
>>     Rx: Off
>>     Tx: Off
>> FC autoneg status: On
>> pause_time: 0x680
>> high_water: 0x80
>> low_water: 0x40
>> Send Xon: On
>> Forward MAC control frames: Off
>>
>>
>> Suggestion:
>>
>> ********************* Flow control info for port 0  *********************
>> Rx mode: off
>> Tx mode: off
>> Autoneg: on
>> Pause time: 0x680
>> High water: 0x80
>> Low water: 0x40
>> Send XON: on
>> Forward MAC control frames: off
>> ***********************************   End  ********************************
>>
>> As ever, display is subjective, so please take as suggestion, you/others
>> may have different view.
>>
>>
>>> +    printf("\n%s**************   End  ***********%s\n",
>>> +        info_border, info_border);
>>> +}
>>> +
>>> +cmdline_parse_inst_t cmd_link_flow_control_show = {
>>> +    .f = cmd_link_flow_ctrl_show_parsed,
>>> +    .data = NULL,
>>> +    .help_str = "show port <port_id> flow_ctrl",
>>> +    .tokens = {
>>> +        (void *)&cmd_lfc_show_show,
>>> +        (void *)&cmd_lfc_show_port,
>>> +        (void *)&cmd_lfc_show_portid,
>>> +        (void *)&cmd_lfc_show_flow_ctrl,
>>> +        NULL,
>>> +    },
>>> +};
>>> +
>>>   /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>>>   struct cmd_link_flow_ctrl_set_result {
>>>       cmdline_fixed_string_t set;
>>> @@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
>>> +    (cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>>>       (cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>>>       (cmdline_parse_inst_t *)&cmd_config_dcb,
>>>       (cmdline_parse_inst_t *)&cmd_read_reg,
>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> index e3bfed5..3fca011 100644
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> @@ -1526,6 +1526,13 @@ Where:
>>>   * ``autoneg``: Change the auto-negotiation parameter.
>>> +show flow ctrl
>>> +~~~~~~~~~~~~~~
>>> +
>>> +show the link flow control parameter on a port::
>>> +
>>> +   testpmd> show port <port_id> flow_ctrl
>>> +
>>>   set pfc_ctrl rx
>>>   ~~~~~~~~~~~~~~~
>>>
>> .

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/21/2021 8:56 AM, Ferruh Yigit wrote:
> On 4/21/2021 7:18 AM, Huisong Li wrote:
>> Hi Kevin,
>>
>> Thank you for your review.
>>
>> This patchset has been applied to dpdk-next-net/main. I will send a new patch 
>> to fix it.
>>
>> Your suggestion is better.  I intend to use the following format:
>>
>> ********************* Flow control info for port 0 *********************
>> FC mode:
>>      Rx pause: off
>>      Tx pause: off
>> Autoneg: on
>> Pause time: 0x680
>> High waterline: 0x80
>> Low waterline: 0x40
>> Send XON: on
>> Forward MAC control frames: off
>> *********************************** End ********************************
>>
> 
> I missed the outstanding comments from Kevin, I will drop the patch from 
> next-net, we can proceed with next version.
> 

v2 dropped from tree, patchwork status updated. Please send v3.

>>
>> 在 2021/4/19 23:30, Kevin Traynor 写道:
>>> On 15/04/2021 07:46, Min Hu (Connor) wrote:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> This patch supports the query of the link flow control parameter
>>>> on a port.
>>>>
>>>> The command format is as follows:
>>>> show port <port_id> flow_ctrl
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Hi Connor,
>>>
>>> Just a few minor comments,
>>>
>>> thanks,
>>> Kevin.
>>>
>>>> ---
>>>>   app/test-pmd/cmdline.c                      | 83 
>>>> +++++++++++++++++++++++++++++
>>>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>>>   2 files changed, 90 insertions(+)
>>>>
>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>> index 5bf1497..9fa85c4 100644
>>>> --- a/app/test-pmd/cmdline.c
>>>> +++ b/app/test-pmd/cmdline.c
>>>> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>>               "show port (port_id) fec_mode"
>>>>               "    Show fec mode of a port.\n\n"
>>>> +
>>>> +            "show port <port_id> flow_ctrl"
>>>> +            "    Show flow control info of a port.\n\n"
>>>>           );
>>>>       }
>>>> @@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
>>>>       },
>>>>   };
>>>> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
>>>> +struct cmd_link_flow_ctrl_show {
>>>> +    cmdline_fixed_string_t show;
>>>> +    cmdline_fixed_string_t port;
>>>> +    portid_t port_id;
>>>> +    cmdline_fixed_string_t flow_ctrl;
>>>> +};
>>>> +
>>>> +cmdline_parse_token_string_t cmd_lfc_show_show =
>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>> +                show, "show");
>>>> +cmdline_parse_token_string_t cmd_lfc_show_port =
>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>> +                port, "port");
>>>> +cmdline_parse_token_num_t cmd_lfc_show_portid =
>>>> +    TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>> +                port_id, RTE_UINT16);
>>>> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>> +                flow_ctrl, "flow_ctrl");
>>>> +
>>>> +static void
>>>> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
>>>> +                  __rte_unused struct cmdline *cl,
>>>> +                  __rte_unused void *data)
>>>> +{
>>>> +    struct cmd_link_flow_ctrl_show *res = parsed_result;
>>>> +    static const char *info_border = "*********************";
>>>> +    struct rte_eth_fc_conf fc_conf;
>>>> +    bool rx_fc_en = false;
>>>> +    bool tx_fc_en = false;
>>>> +    int ret;
>>>> +
>>>> +    if (!rte_eth_dev_is_valid_port(res->port_id)) {
>>> This ^ is already checked in rte_eth_dev_flow_ctrl_get() below. That
>>> said, it's not doing any harm here and you would need to add check for
>>> -ENODEV below if you wanted to tell the user that the port is invalid
>>> so either way is ok IMHO.
>>>
>>>> +        printf("Invalid port id %u\n", res->port_id);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
>>>> +    if (ret != 0) {
>>>> +        printf("Failed to get current flow ctrl information: err = %d\n",
>>>> +               ret);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
>>> You can remove unnecessary brackets, i.e.
>>> if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
>>>
>>>
>>>> +        rx_fc_en = true;
>>>> +    if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
>>> same comment on brackets
>>>
>>>> +        tx_fc_en = true;
>>>> +
>>>> +    printf("\n%s Flow control infos for port %-2d %s\n",
>>> s/infos/info/
>>>
>>>> +        info_border, res->port_id, info_border);
>>>> +    printf("FC mode:\n");
>>> It's better not to introduce a new acronym (even if it's a bit obvious)
>>>
>>>> +    printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>>>> +    printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>>>> +    printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
>>>> +    printf("pause_time: 0x%x\n", fc_conf.pause_time);
>>>> +    printf("high_water: 0x%x\n", fc_conf.high_water);
>>>> +    printf("low_water: 0x%x\n", fc_conf.low_water);
>>>> +    printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>>>> +    printf("mac ctrl frame fwd: %s\n",
>>>> +        fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
>>> There's a mix of struct member names and descriptions here. I think it's
>>> better to stick to one or the other.
>>>
>>> i.e. currently
>>>
>>> testpmd> show port 0 flow_ctrl
>>>
>>> ********************* Flow control infos for port 0  *********************
>>> FC mode:
>>>     Rx: Off
>>>     Tx: Off
>>> FC autoneg status: On
>>> pause_time: 0x680
>>> high_water: 0x80
>>> low_water: 0x40
>>> Send Xon: On
>>> Forward MAC control frames: Off
>>>
>>>
>>> Suggestion:
>>>
>>> ********************* Flow control info for port 0  *********************
>>> Rx mode: off
>>> Tx mode: off
>>> Autoneg: on
>>> Pause time: 0x680
>>> High water: 0x80
>>> Low water: 0x40
>>> Send XON: on
>>> Forward MAC control frames: off
>>> ***********************************   End  ********************************
>>>
>>> As ever, display is subjective, so please take as suggestion, you/others
>>> may have different view.
>>>
>>>
>>>> +    printf("\n%s**************   End  ***********%s\n",
>>>> +        info_border, info_border);
>>>> +}
>>>> +
>>>> +cmdline_parse_inst_t cmd_link_flow_control_show = {
>>>> +    .f = cmd_link_flow_ctrl_show_parsed,
>>>> +    .data = NULL,
>>>> +    .help_str = "show port <port_id> flow_ctrl",
>>>> +    .tokens = {
>>>> +        (void *)&cmd_lfc_show_show,
>>>> +        (void *)&cmd_lfc_show_port,
>>>> +        (void *)&cmd_lfc_show_portid,
>>>> +        (void *)&cmd_lfc_show_flow_ctrl,
>>>> +        NULL,
>>>> +    },
>>>> +};
>>>> +
>>>>   /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>>>>   struct cmd_link_flow_ctrl_set_result {
>>>>       cmdline_fixed_string_t set;
>>>> @@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
>>>> +    (cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>>>>       (cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>>>>       (cmdline_parse_inst_t *)&cmd_config_dcb,
>>>>       (cmdline_parse_inst_t *)&cmd_read_reg,
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
>>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index e3bfed5..3fca011 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -1526,6 +1526,13 @@ Where:
>>>>   * ``autoneg``: Change the auto-negotiation parameter.
>>>> +show flow ctrl
>>>> +~~~~~~~~~~~~~~
>>>> +
>>>> +show the link flow control parameter on a port::
>>>> +
>>>> +   testpmd> show port <port_id> flow_ctrl
>>>> +
>>>>   set pfc_ctrl rx
>>>>   ~~~~~~~~~~~~~~~
>>>>
>>> .
> 

[dpdk-dev] [PATCH v3] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

This patch supports the query of the link flow control parameter
on a port.

The command format is as follows:
show port <port_id> flow_ctrl

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v3:
* fixed the print format of link flow ctrl, and removes redundant
port number verification.

v2:
* fixed logging.
---
 app/test-pmd/cmdline.c                      | 78 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
 2 files changed, 85 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d282c7c..08da2b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"show port (port_id) fec_mode"
 			"	Show fec mode of a port.\n\n"
+
+			"show port <port_id> flow_ctrl"
+			"	Show flow control info of a port.\n\n"
 		);
 	}
 
@@ -6863,6 +6866,80 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
 	},
 };
 
+/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
+struct cmd_link_flow_ctrl_show {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	portid_t port_id;
+	cmdline_fixed_string_t flow_ctrl;
+};
+
+cmdline_parse_token_string_t cmd_lfc_show_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				show, "show");
+cmdline_parse_token_string_t cmd_lfc_show_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port, "port");
+cmdline_parse_token_num_t cmd_lfc_show_portid =
+	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				port_id, RTE_UINT16);
+cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
+	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+				flow_ctrl, "flow_ctrl");
+
+static void
+cmd_link_flow_ctrl_show_parsed(void *parsed_result,
+			      __rte_unused struct cmdline *cl,
+			      __rte_unused void *data)
+{
+	struct cmd_link_flow_ctrl_show *res = parsed_result;
+	static const char *info_border = "*********************";
+	struct rte_eth_fc_conf fc_conf;
+	bool rx_fc_en = false;
+	bool tx_fc_en = false;
+	int ret;
+
+	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
+	if (ret != 0) {
+		printf("Failed to get current flow ctrl information: err = %d\n",
+		       ret);
+		return;
+	}
+
+	if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
+		rx_fc_en = true;
+	if (fc_conf.mode == RTE_FC_TX_PAUSE || fc_conf.mode == RTE_FC_FULL)
+		tx_fc_en = true;
+
+	printf("\n%s Flow control infos for port %-2d %s\n",
+		info_border, res->port_id, info_border);
+	printf("FC mode:\n");
+	printf("   Rx pause: %s\n", rx_fc_en ? "on" : "off");
+	printf("   Tx pause: %s\n", tx_fc_en ? "on" : "off");
+	printf("Autoneg: %s\n", fc_conf.autoneg ? "on" : "off");
+	printf("Pause time: 0x%x\n", fc_conf.pause_time);
+	printf("High waterline: 0x%x\n", fc_conf.high_water);
+	printf("Low waterline: 0x%x\n", fc_conf.low_water);
+	printf("Send XON: %s\n", fc_conf.send_xon ? "on" : "off");
+	printf("Forward MAC control frames: %s\n",
+		fc_conf.mac_ctrl_frame_fwd ? "on" : "off");
+	printf("\n%s**************   End  ***********%s\n",
+		info_border, info_border);
+}
+
+cmdline_parse_inst_t cmd_link_flow_control_show = {
+	.f = cmd_link_flow_ctrl_show_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> flow_ctrl",
+	.tokens = {
+		(void *)&cmd_lfc_show_show,
+		(void *)&cmd_lfc_show_port,
+		(void *)&cmd_lfc_show_portid,
+		(void *)&cmd_lfc_show_flow_ctrl,
+		NULL,
+	},
+};
+
 /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
 struct cmd_link_flow_ctrl_set_result {
 	cmdline_fixed_string_t set;
@@ -17354,6 +17431,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
+	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
 	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
 	(cmdline_parse_inst_t *)&cmd_config_dcb,
 	(cmdline_parse_inst_t *)&cmd_read_reg,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index c0cbc40..5dd98c2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1524,6 +1524,13 @@ Where:
 
 * ``autoneg``: Change the auto-negotiation parameter.
 
+show flow ctrl
+~~~~~~~~~~~~~~
+
+show the link flow control parameter on a port::
+
+   testpmd> show port <port_id> flow_ctrl
+
 set pfc_ctrl rx
 ~~~~~~~~~~~~~~~
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info

[Min Hu (Connor)]

在 2021/4/21 16:05, Ferruh Yigit 写道:
> On 4/21/2021 8:56 AM, Ferruh Yigit wrote:
>> On 4/21/2021 7:18 AM, Huisong Li wrote:
>>> Hi Kevin,
>>>
>>> Thank you for your review.
>>>
>>> This patchset has been applied to dpdk-next-net/main. I will send a 
>>> new patch to fix it.
>>>
>>> Your suggestion is better.  I intend to use the following format:
>>>
>>> ********************* Flow control info for port 0 *********************
>>> FC mode:
>>>      Rx pause: off
>>>      Tx pause: off
>>> Autoneg: on
>>> Pause time: 0x680
>>> High waterline: 0x80
>>> Low waterline: 0x40
>>> Send XON: on
>>> Forward MAC control frames: off
>>> *********************************** End ********************************
>>>
>>
>> I missed the outstanding comments from Kevin, I will drop the patch 
>> from next-net, we can proceed with next version.
>>
> 
> v2 dropped from tree, patchwork status updated. Please send v3.
> 
Hi, v3 has been sent, thanks.
>>>
>>> 在 2021/4/19 23:30, Kevin Traynor 写道:
>>>> On 15/04/2021 07:46, Min Hu (Connor) wrote:
>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>
>>>>> This patch supports the query of the link flow control parameter
>>>>> on a port.
>>>>>
>>>>> The command format is as follows:
>>>>> show port <port_id> flow_ctrl
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> Hi Connor,
>>>>
>>>> Just a few minor comments,
>>>>
>>>> thanks,
>>>> Kevin.
>>>>
>>>>> ---
>>>>>   app/test-pmd/cmdline.c                      | 83 
>>>>> +++++++++++++++++++++++++++++
>>>>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>>>>>   2 files changed, 90 insertions(+)
>>>>>
>>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>>> index 5bf1497..9fa85c4 100644
>>>>> --- a/app/test-pmd/cmdline.c
>>>>> +++ b/app/test-pmd/cmdline.c
>>>>> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void 
>>>>> *parsed_result,
>>>>>               "show port (port_id) fec_mode"
>>>>>               "    Show fec mode of a port.\n\n"
>>>>> +
>>>>> +            "show port <port_id> flow_ctrl"
>>>>> +            "    Show flow control info of a port.\n\n"
>>>>>           );
>>>>>       }
>>>>> @@ -6863,6 +6866,85 @@ cmdline_parse_inst_t 
>>>>> cmd_set_allmulti_mode_one = {
>>>>>       },
>>>>>   };
>>>>> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
>>>>> +struct cmd_link_flow_ctrl_show {
>>>>> +    cmdline_fixed_string_t show;
>>>>> +    cmdline_fixed_string_t port;
>>>>> +    portid_t port_id;
>>>>> +    cmdline_fixed_string_t flow_ctrl;
>>>>> +};
>>>>> +
>>>>> +cmdline_parse_token_string_t cmd_lfc_show_show =
>>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>>> +                show, "show");
>>>>> +cmdline_parse_token_string_t cmd_lfc_show_port =
>>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>>> +                port, "port");
>>>>> +cmdline_parse_token_num_t cmd_lfc_show_portid =
>>>>> +    TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>>> +                port_id, RTE_UINT16);
>>>>> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
>>>>> +    TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
>>>>> +                flow_ctrl, "flow_ctrl");
>>>>> +
>>>>> +static void
>>>>> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
>>>>> +                  __rte_unused struct cmdline *cl,
>>>>> +                  __rte_unused void *data)
>>>>> +{
>>>>> +    struct cmd_link_flow_ctrl_show *res = parsed_result;
>>>>> +    static const char *info_border = "*********************";
>>>>> +    struct rte_eth_fc_conf fc_conf;
>>>>> +    bool rx_fc_en = false;
>>>>> +    bool tx_fc_en = false;
>>>>> +    int ret;
>>>>> +
>>>>> +    if (!rte_eth_dev_is_valid_port(res->port_id)) {
>>>> This ^ is already checked in rte_eth_dev_flow_ctrl_get() below. That
>>>> said, it's not doing any harm here and you would need to add check for
>>>> -ENODEV below if you wanted to tell the user that the port is invalid
>>>> so either way is ok IMHO.
>>>>
>>>>> +        printf("Invalid port id %u\n", res->port_id);
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
>>>>> +    if (ret != 0) {
>>>>> +        printf("Failed to get current flow ctrl information: err = 
>>>>> %d\n",
>>>>> +               ret);
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == 
>>>>> RTE_FC_FULL))
>>>> You can remove unnecessary brackets, i.e.
>>>> if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
>>>>
>>>>
>>>>> +        rx_fc_en = true;
>>>>> +    if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == 
>>>>> RTE_FC_FULL))
>>>> same comment on brackets
>>>>
>>>>> +        tx_fc_en = true;
>>>>> +
>>>>> +    printf("\n%s Flow control infos for port %-2d %s\n",
>>>> s/infos/info/
>>>>
>>>>> +        info_border, res->port_id, info_border);
>>>>> +    printf("FC mode:\n");
>>>> It's better not to introduce a new acronym (even if it's a bit obvious)
>>>>
>>>>> +    printf("   Rx: %s\n", rx_fc_en ? "On" : "Off");
>>>>> +    printf("   Tx: %s\n", tx_fc_en ? "On" : "Off");
>>>>> +    printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" 
>>>>> : "Off");
>>>>> +    printf("pause_time: 0x%x\n", fc_conf.pause_time);
>>>>> +    printf("high_water: 0x%x\n", fc_conf.high_water);
>>>>> +    printf("low_water: 0x%x\n", fc_conf.low_water);
>>>>> +    printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
>>>>> +    printf("mac ctrl frame fwd: %s\n",
>>>>> +        fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
>>>> There's a mix of struct member names and descriptions here. I think 
>>>> it's
>>>> better to stick to one or the other.
>>>>
>>>> i.e. currently
>>>>
>>>> testpmd> show port 0 flow_ctrl
>>>>
>>>> ********************* Flow control infos for port 0  
>>>> *********************
>>>> FC mode:
>>>>     Rx: Off
>>>>     Tx: Off
>>>> FC autoneg status: On
>>>> pause_time: 0x680
>>>> high_water: 0x80
>>>> low_water: 0x40
>>>> Send Xon: On
>>>> Forward MAC control frames: Off
>>>>
>>>>
>>>> Suggestion:
>>>>
>>>> ********************* Flow control info for port 0  
>>>> *********************
>>>> Rx mode: off
>>>> Tx mode: off
>>>> Autoneg: on
>>>> Pause time: 0x680
>>>> High water: 0x80
>>>> Low water: 0x40
>>>> Send XON: on
>>>> Forward MAC control frames: off
>>>> ***********************************   End  
>>>> ********************************
>>>>
>>>> As ever, display is subjective, so please take as suggestion, 
>>>> you/others
>>>> may have different view.
>>>>
>>>>
>>>>> +    printf("\n%s**************   End  ***********%s\n",
>>>>> +        info_border, info_border);
>>>>> +}
>>>>> +
>>>>> +cmdline_parse_inst_t cmd_link_flow_control_show = {
>>>>> +    .f = cmd_link_flow_ctrl_show_parsed,
>>>>> +    .data = NULL,
>>>>> +    .help_str = "show port <port_id> flow_ctrl",
>>>>> +    .tokens = {
>>>>> +        (void *)&cmd_lfc_show_show,
>>>>> +        (void *)&cmd_lfc_show_port,
>>>>> +        (void *)&cmd_lfc_show_portid,
>>>>> +        (void *)&cmd_lfc_show_flow_ctrl,
>>>>> +        NULL,
>>>>> +    },
>>>>> +};
>>>>> +
>>>>>   /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>>>>>   struct cmd_link_flow_ctrl_set_result {
>>>>>       cmdline_fixed_string_t set;
>>>>> @@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>>>>>       (cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
>>>>> +    (cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>>>>>       (cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>>>>>       (cmdline_parse_inst_t *)&cmd_config_dcb,
>>>>>       (cmdline_parse_inst_t *)&cmd_read_reg,
>>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
>>>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> index e3bfed5..3fca011 100644
>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> @@ -1526,6 +1526,13 @@ Where:
>>>>>   * ``autoneg``: Change the auto-negotiation parameter.
>>>>> +show flow ctrl
>>>>> +~~~~~~~~~~~~~~
>>>>> +
>>>>> +show the link flow control parameter on a port::
>>>>> +
>>>>> +   testpmd> show port <port_id> flow_ctrl
>>>>> +
>>>>>   set pfc_ctrl rx
>>>>>   ~~~~~~~~~~~~~~~
>>>>>
>>>> .
>>
> 
> .

Re: [dpdk-dev] [PATCH v3] app/testpmd: support the query of link flow ctrl info

[Kevin Traynor]

On 21/04/2021 10:41, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter
> on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 

Thanks Connor,
Acked-by: Kevin Traynor <ktraynor@redhat.com>

> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v3:
> * fixed the print format of link flow ctrl, and removes redundant
> port number verification.
> 
> v2:
> * fixed logging.
> ---
>  app/test-pmd/cmdline.c                      | 78 +++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index d282c7c..08da2b1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>  
>  			"show port (port_id) fec_mode"
>  			"	Show fec mode of a port.\n\n"
> +
> +			"show port <port_id> flow_ctrl"
> +			"	Show flow control info of a port.\n\n"
>  		);
>  	}
>  
> @@ -6863,6 +6866,80 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
>  	},
>  };
>  
> +/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
> +struct cmd_link_flow_ctrl_show {
> +	cmdline_fixed_string_t show;
> +	cmdline_fixed_string_t port;
> +	portid_t port_id;
> +	cmdline_fixed_string_t flow_ctrl;
> +};
> +
> +cmdline_parse_token_string_t cmd_lfc_show_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				show, "show");
> +cmdline_parse_token_string_t cmd_lfc_show_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				port, "port");
> +cmdline_parse_token_num_t cmd_lfc_show_portid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				port_id, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
> +	TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
> +				flow_ctrl, "flow_ctrl");
> +
> +static void
> +cmd_link_flow_ctrl_show_parsed(void *parsed_result,
> +			      __rte_unused struct cmdline *cl,
> +			      __rte_unused void *data)
> +{
> +	struct cmd_link_flow_ctrl_show *res = parsed_result;
> +	static const char *info_border = "*********************";
> +	struct rte_eth_fc_conf fc_conf;
> +	bool rx_fc_en = false;
> +	bool tx_fc_en = false;
> +	int ret;
> +
> +	ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
> +	if (ret != 0) {
> +		printf("Failed to get current flow ctrl information: err = %d\n",
> +		       ret);
> +		return;
> +	}
> +
> +	if (fc_conf.mode == RTE_FC_RX_PAUSE || fc_conf.mode == RTE_FC_FULL)
> +		rx_fc_en = true;
> +	if (fc_conf.mode == RTE_FC_TX_PAUSE || fc_conf.mode == RTE_FC_FULL)
> +		tx_fc_en = true;
> +
> +	printf("\n%s Flow control infos for port %-2d %s\n",
> +		info_border, res->port_id, info_border);
> +	printf("FC mode:\n");
> +	printf("   Rx pause: %s\n", rx_fc_en ? "on" : "off");
> +	printf("   Tx pause: %s\n", tx_fc_en ? "on" : "off");
> +	printf("Autoneg: %s\n", fc_conf.autoneg ? "on" : "off");
> +	printf("Pause time: 0x%x\n", fc_conf.pause_time);
> +	printf("High waterline: 0x%x\n", fc_conf.high_water);
> +	printf("Low waterline: 0x%x\n", fc_conf.low_water);
> +	printf("Send XON: %s\n", fc_conf.send_xon ? "on" : "off");
> +	printf("Forward MAC control frames: %s\n",
> +		fc_conf.mac_ctrl_frame_fwd ? "on" : "off");
> +	printf("\n%s**************   End  ***********%s\n",
> +		info_border, info_border);
> +}
> +
> +cmdline_parse_inst_t cmd_link_flow_control_show = {
> +	.f = cmd_link_flow_ctrl_show_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> flow_ctrl",
> +	.tokens = {
> +		(void *)&cmd_lfc_show_show,
> +		(void *)&cmd_lfc_show_port,
> +		(void *)&cmd_lfc_show_portid,
> +		(void *)&cmd_lfc_show_flow_ctrl,
> +		NULL,
> +	},
> +};
> +
>  /* *** SETUP ETHERNET LINK FLOW CONTROL *** */
>  struct cmd_link_flow_ctrl_set_result {
>  	cmdline_fixed_string_t set;
> @@ -17354,6 +17431,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
>  	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
> +	(cmdline_parse_inst_t *)&cmd_link_flow_control_show,
>  	(cmdline_parse_inst_t *)&cmd_priority_flow_control_set,
>  	(cmdline_parse_inst_t *)&cmd_config_dcb,
>  	(cmdline_parse_inst_t *)&cmd_read_reg,
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index c0cbc40..5dd98c2 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -1524,6 +1524,13 @@ Where:
>  
>  * ``autoneg``: Change the auto-negotiation parameter.
>  
> +show flow ctrl
> +~~~~~~~~~~~~~~
> +
> +show the link flow control parameter on a port::
> +
> +   testpmd> show port <port_id> flow_ctrl
> +
>  set pfc_ctrl rx
>  ~~~~~~~~~~~~~~~
>  
> 

Re: [dpdk-dev] [PATCH v3] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/21/2021 12:12 PM, Kevin Traynor wrote:
> On 21/04/2021 10:41, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> This patch supports the query of the link flow control parameter
>> on a port.
>>
>> The command format is as follows:
>> show port <port_id> flow_ctrl
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
 > Thanks Connor,
 > Acked-by: Kevin Traynor <ktraynor@redhat.com>
 >

Carrying Xiaoyun's ack from previous version:
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

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


>> @@ -1524,6 +1524,13 @@ Where:
>>   
>>   * ``autoneg``: Change the auto-negotiation parameter.
>>   
>> +show flow ctrl
>> +~~~~~~~~~~~~~~
>> +

Expanded to "show flow control" while merging.

Re: [dpdk-dev] [PATCH v3] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/21/2021 12:36 PM, Ferruh Yigit wrote:
> On 4/21/2021 12:12 PM, Kevin Traynor wrote:
>> On 21/04/2021 10:41, Min Hu (Connor) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> This patch supports the query of the link flow control parameter
>>> on a port.
>>>
>>> The command format is as follows:
>>> show port <port_id> flow_ctrl
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>
>> Thanks Connor,
>> Acked-by: Kevin Traynor <ktraynor@redhat.com>
>>
> 
> Carrying Xiaoyun's ack from previous version:
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> Applied to dpdk-next-net/main, thanks.
> 
> 
>>> @@ -1524,6 +1524,13 @@ Where:
>>>   * ``autoneg``: Change the auto-negotiation parameter.
>>> +show flow ctrl
>>> +~~~~~~~~~~~~~~
>>> +
> 
> Expanded to "show flow control" while merging.
> 
> 
> 

Ahh, it seems the release notes update is missing, adding following while merging:

  diff --git a/doc/guides/rel_notes/release_21_05.rst 
b/doc/guides/rel_notes/release_21_05.rst
  index b13c76124169..b36c120e5e1d 100644
  --- a/doc/guides/rel_notes/release_21_05.rst
  +++ b/doc/guides/rel_notes/release_21_05.rst
  @@ -233,6 +233,8 @@ New Features
       item matching.
     * Added command to cleanup a Tx queue's mbuf on a port.
       ``port cleanup (port_id) txq (queue_id) (free_cnt)``
  +  * Added command to show link flow control info.
  +    ``show port (port_id) flow_ctrl``

   * **Updated ipsec-secgw sample application.**

Re: [dpdk-dev] [PATCH v3] app/testpmd: support the query of link flow ctrl info

[Ferruh Yigit]

On 4/21/2021 10:41 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patch supports the query of the link flow control parameter
> on a port.
> 
> The command format is as follows:
> show port <port_id> flow_ctrl
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

<...>

> @@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>   
>   			"show port (port_id) fec_mode"
>   			"	Show fec mode of a port.\n\n"
> +
> +			"show port <port_id> flow_ctrl"

Fixing syntax as following, to be consistent, while merging:

show port (port_id) flow_ctrl