[v3,2/7] app/procinfo: add firmware version dump

Message ID 20220921142655.16427-3-liudongdong3@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series app/procinfo: add some extended features |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Dongdong Liu Sept. 21, 2022, 2:26 p.m. UTC
  Add support for dump ethdev firmware version.

The command is like:
dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- --firmware-version

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
  

Comments

Pattan, Reshma Sept. 23, 2022, 9:22 a.m. UTC | #1
> -----Original Message-----
> From: Dongdong Liu <liudongdong3@huawei.com>
> Subject: [PATCH v3 2/7] app/procinfo: add firmware version dump
> 
> Add support for dump ethdev firmware version.
> 
> The command is like:
> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- --firmware-version
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>

> +		if (rte_eth_dev_fw_version_get(i, fw_version,
> +					       ETHDEV_FWVERS_LEN) == 0)
> +			printf("Ethdev port %u firmware version: %s\n", i,
> +				fw_version);
> +		else
> +			printf("Ethdev port %u firmware version: %s\n", i,
> +				"not available");

Small question, do you want to show generic msg "Not available" ?  instead of different printing messages  based on what is returned from the " rte_eth_dev_fw_version_get" . 


Acked-by: Reshma Pattan <reshma.pattan@intel.com>
  
Dongdong Liu Sept. 24, 2022, 8:03 a.m. UTC | #2
Hi Reshma

Many thanks for your review.
On 2022/9/23 17:22, Pattan, Reshma wrote:
>
>
>> -----Original Message-----
>> From: Dongdong Liu <liudongdong3@huawei.com>
>> Subject: [PATCH v3 2/7] app/procinfo: add firmware version dump
>>
>> Add support for dump ethdev firmware version.
>>
>> The command is like:
>> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- --firmware-version
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>
>> +		if (rte_eth_dev_fw_version_get(i, fw_version,
>> +					       ETHDEV_FWVERS_LEN) == 0)
>> +			printf("Ethdev port %u firmware version: %s\n", i,
>> +				fw_version);
>> +		else
>> +			printf("Ethdev port %u firmware version: %s\n", i,
>> +				"not available");
>
> Small question, do you want to show generic msg "Not available" ?  instead of different printing messages  based on what is returned from the " rte_eth_dev_fw_version_get" .
Tt is ok to show generic msg "Not available".

Thanks,
Dongdong
>
>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>
>
>
> .
>
  

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 3f6d011049..ab4fee1138 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -45,6 +45,8 @@ 
 #define MAX_LONG_OPT_SZ 64
 #define MAX_STRING_LEN 256
 
+#define ETHDEV_FWVERS_LEN 32
+
 #define STATS_BDR_FMT "========================================"
 #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
 	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
@@ -105,6 +107,8 @@  static uint32_t enable_dump_regs;
 static char *dump_regs_file_prefix;
 /* Enable show DPDK version. */
 static uint32_t enable_shw_version;
+/* Enable show ethdev firmware version. */
+static uint32_t enable_shw_fw_version;
 
 /**< display usage */
 static void
@@ -134,6 +138,7 @@  proc_info_usage(const char *prgname)
 		"  --show-ring[=name]: to display ring information\n"
 		"  --show-mempool[=name]: to display mempool information\n"
 		"  --version: to display DPDK version\n"
+		"  --firmware-version: to display ethdev firmware version\n"
 		"  --iter-mempool=name: iterate mempool elements to display content\n"
 		"  --dump-regs=file-prefix: dump registers to file with the file-prefix\n",
 		prgname);
@@ -247,6 +252,7 @@  proc_info_parse_args(int argc, char **argv)
 		{"iter-mempool", required_argument, NULL, 0},
 		{"dump-regs", required_argument, NULL, 0},
 		{"version", 0, NULL, 0},
+		{"firmware-version", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -321,6 +327,9 @@  proc_info_parse_args(int argc, char **argv)
 			} else if (!strncmp(long_option[option_index].name,
 					"version", MAX_LONG_OPT_SZ))
 				enable_shw_version = 1;
+			else if (!strncmp(long_option[option_index].name,
+					"firmware-version", MAX_LONG_OPT_SZ))
+				enable_shw_fw_version = 1;
 			break;
 		case 1:
 			/* Print xstat single value given by name*/
@@ -1491,6 +1500,30 @@  show_version(void)
 	printf("DPDK version: %s\n", rte_version());
 }
 
+static void
+show_firmware_version(void)
+{
+	char fw_version[ETHDEV_FWVERS_LEN];
+	uint16_t i;
+
+	snprintf(bdr_str, MAX_STRING_LEN, " show - firmware version ");
+	STATS_BDR_STR(10, bdr_str);
+
+	RTE_ETH_FOREACH_DEV(i) {
+		/* Skip if port is not in mask */
+		if ((enabled_port_mask & (1ul << i)) == 0)
+			continue;
+
+		if (rte_eth_dev_fw_version_get(i, fw_version,
+					       ETHDEV_FWVERS_LEN) == 0)
+			printf("Ethdev port %u firmware version: %s\n", i,
+				fw_version);
+		else
+			printf("Ethdev port %u firmware version: %s\n", i,
+				"not available");
+	}
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1606,6 +1639,8 @@  main(int argc, char **argv)
 		dump_regs(dump_regs_file_prefix);
 	if (enable_shw_version)
 		show_version();
+	if (enable_shw_fw_version)
+		show_firmware_version();
 
 	RTE_ETH_FOREACH_DEV(i)
 		rte_eth_dev_close(i);