DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev]  [PATCH V1] testpmd: add eeprom/module eeprom display
@ 2020-09-10  6:00 David Liu
  2020-09-10 11:47 ` Ferruh Yigit
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: David Liu @ 2020-09-10  6:00 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Add module EEPROM/EEPROM dump command
  "show port <port_id> (module_eeprom|eeprom)"
Commands will dump the content of the
EEPROM/module EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  50 ++++++++
 app/test-pmd/config.c                       | 120 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 179 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..56b99aa86 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port port_id (module_eeprom|eeprom)\n"
+			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7594,6 +7597,51 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	uint16_t portnum;
+	cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_showeeprom_result *res = parsed_result;
+
+	if (!strcmp(res->type, "eeprom"))
+		port_eeprom_display(res->portnum);
+	else if (!strcmp(res->type, "module_eeprom"))
+		port_module_eeprom_display(res->portnum);
+	else
+		printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+	.f = cmd_showeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom|eeprom",
+	.tokens = {
+		(void *)&cmd_showeeprom_show,
+		(void *)&cmd_showeeprom_port,
+		(void *)&cmd_showeeprom_portnum,
+		(void *)&cmd_showeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19373,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+	(cmdline_parse_inst_t *)&cmd_showeeprom,
+	(cmdline_parse_inst_t *)&cmd_showmoduleeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..61e21ac26 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,7 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
 
 #include "testpmd.h"
 
@@ -710,6 +711,125 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+	
+	if(len_eeprom < 0) {
+		switch (len_eeprom) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+		       	printf("device is removed\n");
+			break;
+	       	default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+	
+	char buf[len_eeprom];
+	
+	einfo.offset = 0;
+	einfo.length = len_eeprom;
+	einfo.data = buf;
+	
+	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+		       	printf("device is removed\n");
+			break;
+	       	default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+}
+
+void
+port_module_eeprom_displao(portid_t port_id)
+{
+	struct rte_eth_dev_module_info minfo;
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+	       	print_valid_ports();
+		return;
+	}
+
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+		       	printf("device is removed\n");
+			break;
+	       	default:
+			printf("Unable to get module EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	char buf[minfo.eeprom_len];
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+	
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+		       	printf("device is removed\n");
+			break;
+	       	default:
+			printf("Unable to get module EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..6a2e1c0f2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe
 
+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-10  6:00 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
@ 2020-09-10 11:47 ` Ferruh Yigit
  2020-09-10 18:48   ` David Liu
  2020-09-10 20:00 ` [dpdk-dev] [PATCH V1] " David Liu
  2020-09-10 20:12 ` [dpdk-dev] [PATCH V1 2/2] Fix coding style issues David Liu
  2 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-09-10 11:47 UTC (permalink / raw)
  To: David Liu; +Cc: dev, lylavoie, thomas

On 9/10/2020 7:00 AM, David Liu wrote:
> Add module EEPROM/EEPROM dump command
>   "show port <port_id> (module_eeprom|eeprom)"
> Commands will dump the content of the
> EEPROM/module EEPROM for the selected port.


Hi David,

When sending a new version, can you please increase the verstion tag in the
title, V1 -> V2 -> ... -> vN

Also sending new patch as reply to previos one keep all versions in same email
thread and helps reviewers also people who later checks from archives for a feature.

For both above you can find more details in the contribution guide, please check
https://doc.dpdk.org/guides/contributing/patches.html#sending-patches


And there are a few errors below that this patch shouldn't be compiling
successfully, you can verify the build error from lab reports :)

<...>

>  /* *** SHOW QUEUE INFO *** */
>  struct cmd_showqueue_result {
>  	cmdline_fixed_string_t show;
> @@ -19325,6 +19373,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_load_from_file,
>  	(cmdline_parse_inst_t *)&cmd_showport,
>  	(cmdline_parse_inst_t *)&cmd_showqueue,
> +	(cmdline_parse_inst_t *)&cmd_showeeprom,
> +	(cmdline_parse_inst_t *)&cmd_showmoduleeeprom,

This shouldn't compile because 'cmd_showmoduleeeprom' no more exists ...

<...>

> +
> +void
> +port_module_eeprom_displao(portid_t port_id)
> +{

There is a typo in the function name.

> +	struct rte_eth_dev_module_info minfo;
> +	struct rte_dev_eeprom_info einfo;
> +	int ret;
> +	
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +	       	print_valid_ports();
> +		return;
> +	}
> +
> +
> +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
> +	if (ret != 0) {
> +		switch (ret) {
> +		case -ENODEV:
> +			printf("port index %d invalid\n", port_id);
> +			break;
> +		case -ENOTSUP:
> +			printf("operation not supported by device\n");
> +			break;
> +		case -EIO:
> +		       	printf("device is removed\n");
> +			break;
> +	       	default:
> +			printf("Unable to get module EEPROM: %d\n", len_eeprom);

I guess this is copy/paste error 'len_eeprom' is not defined in this function.
There is one more occurance below.

<...>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-10 11:47 ` Ferruh Yigit
@ 2020-09-10 18:48   ` David Liu
  2020-09-14 15:11     ` [dpdk-dev] [PATCH V2] " David Liu
  0 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-10 18:48 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

On Thu, Sep 10, 2020 at 7:47 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 9/10/2020 7:00 AM, David Liu wrote:
> > Add module EEPROM/EEPROM dump command
> >   "show port <port_id> (module_eeprom|eeprom)"
> > Commands will dump the content of the
> > EEPROM/module EEPROM for the selected port.
>
>
> Hi David,
>
> When sending a new version, can you please increase the verstion tag in the
> title, V1 -> V2 -> ... -> vN
>
> Also sending new patch as reply to previos one keep all versions in same
> email
> thread and helps reviewers also people who later checks from archives for
> a feature.
>
> For both above you can find more details in the contribution guide, please
> check
> https://doc.dpdk.org/guides/contributing/patches.html#sending-patches
>
>
> And there are a few errors below that this patch shouldn't be compiling
> successfully, you can verify the build error from lab reports :)
>

Thank you so much, I will change to that format.


> <...>
>
> >  /* *** SHOW QUEUE INFO *** */
> >  struct cmd_showqueue_result {
> >       cmdline_fixed_string_t show;
> > @@ -19325,6 +19373,8 @@ cmdline_parse_ctx_t main_ctx[] = {
> >       (cmdline_parse_inst_t *)&cmd_load_from_file,
> >       (cmdline_parse_inst_t *)&cmd_showport,
> >       (cmdline_parse_inst_t *)&cmd_showqueue,
> > +     (cmdline_parse_inst_t *)&cmd_showeeprom,
> > +     (cmdline_parse_inst_t *)&cmd_showmoduleeeprom,
>
> This shouldn't compile because 'cmd_showmoduleeeprom' no more exists ...
>
> <...>
>
> > +
> > +void
> > +port_module_eeprom_displao(portid_t port_id)
> > +{
>
> There is a typo in the function name.
>
> > +     struct rte_eth_dev_module_info minfo;
> > +     struct rte_dev_eeprom_info einfo;
> > +     int ret;
> > +
> > +     if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> > +             print_valid_ports();
> > +             return;
> > +     }
> > +
> > +
> > +     ret = rte_eth_dev_get_module_info(port_id, &minfo);
> > +     if (ret != 0) {
> > +             switch (ret) {
> > +             case -ENODEV:
> > +                     printf("port index %d invalid\n", port_id);
> > +                     break;
> > +             case -ENOTSUP:
> > +                     printf("operation not supported by device\n");
> > +                     break;
> > +             case -EIO:
> > +                     printf("device is removed\n");
> > +                     break;
> > +             default:
> > +                     printf("Unable to get module EEPROM: %d\n",
> len_eeprom);
>
> I guess this is copy/paste error 'len_eeprom' is not defined in this
> function.
> There is one more occurance below.
>
>
I will send out a new patch with all the fixes and that can be compiled
hopefully.

<...>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev]  [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-10  6:00 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
  2020-09-10 11:47 ` Ferruh Yigit
@ 2020-09-10 20:00 ` " David Liu
  2020-09-15 15:42   ` [dpdk-dev] [PATCH] [PATCH V2] " David Liu
  2020-09-10 20:12 ` [dpdk-dev] [PATCH V1 2/2] Fix coding style issues David Liu
  2 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-10 20:00 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Add module EEPROM/EEPROM dump command
   "show port <port_id> (module_eeprom|eeprom)" 
Commands will dump the content of the EEPROM/module
EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  49 ++++++++
 app/test-pmd/config.c                       | 118 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 177 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..ba240c410 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port port_id (module_eeprom|eeprom)\n"
+			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7594,6 +7597,51 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	uint16_t portnum;
+	cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_showeeprom_result *res = parsed_result;
+
+	if (!strcmp(res->type, "eeprom"))
+		port_eeprom_display(res->portnum);
+	else if (!strcmp(res->type, "module_eeprom"))
+		port_module_eeprom_display(res->portnum);
+	else
+		printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+	.f = cmd_showeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom|eeprom",
+	.tokens = {
+		(void *)&cmd_showeeprom_show,
+		(void *)&cmd_showeeprom_port,
+		(void *)&cmd_showeeprom_portnum,
+		(void *)&cmd_showeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19373,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..5e3e9e211 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,9 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
+
 
 #include "testpmd.h"
 
@@ -710,6 +713,122 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+	if(len_eeprom < 0) {
+		switch (len_eeprom) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	char buf[len_eeprom];
+	einfo.offset = 0;
+	einfo.length = len_eeprom;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+	struct rte_eth_dev_module_info minfo;
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	char buf[minfo.eeprom_len];
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..6a2e1c0f2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe
 
+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev]  [PATCH V1 2/2] Fix coding style issues
  2020-09-10  6:00 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
  2020-09-10 11:47 ` Ferruh Yigit
  2020-09-10 20:00 ` [dpdk-dev] [PATCH V1] " David Liu
@ 2020-09-10 20:12 ` David Liu
  2 siblings, 0 replies; 22+ messages in thread
From: David Liu @ 2020-09-10 20:12 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/config.c | 1 +--
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 5e3e9e211..a34827707 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,7 +49,6 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
 
 #include <rte_hexdump.h>
 
@@ -724,7 +723,7 @@ port_eeprom_display(portid_t port_id)
 	}
 
 	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
-	if(len_eeprom < 0) {
+	if (len_eeprom < 0) {
 		switch (len_eeprom) {
 		case -ENODEV:
 			printf("port index %d invalid\n", port_id);
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-10 18:48   ` David Liu
@ 2020-09-14 15:11     ` " David Liu
  2020-09-14 16:48       ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-14 15:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

Add module EEPROM/EEPROM dump command
   "show port <port_id> (module_eeprom|eeprom)"
Commands will dump the content of the EEPROM/module
EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  49 ++++++++
 app/test-pmd/config.c                       | 118 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 177 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..ba240c410 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
  "show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap)
(port_id|all)\n"
  "    Display information for port_id, or all.\n\n"

+ "show port port_id (module_eeprom|eeprom)\n"
+ "    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
  "show port X rss reta (size) (mask0,mask1,...)\n"
  "    Display the rss redirection table entry indicated"
  " by masks on port X. size is used to indicate the"
@@ -7594,6 +7597,51 @@ cmdline_parse_inst_t cmd_showdevice = {
  NULL,
  },
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+ cmdline_fixed_string_t show;
+ cmdline_fixed_string_t port;
+ uint16_t portnum;
+ cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_showeeprom_result *res = parsed_result;
+
+ if (!strcmp(res->type, "eeprom"))
+ port_eeprom_display(res->portnum);
+ else if (!strcmp(res->type, "module_eeprom"))
+ port_module_eeprom_display(res->portnum);
+ else
+ printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+ TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+ TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+ TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
"module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+ .f = cmd_showeeprom_parsed,
+ .data = NULL,
+ .help_str = "show port <port_id> module_eeprom|eeprom",
+ .tokens = {
+ (void *)&cmd_showeeprom_show,
+ (void *)&cmd_showeeprom_port,
+ (void *)&cmd_showeeprom_portnum,
+ (void *)&cmd_showeeprom_type,
+ NULL,
+ },
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
  cmdline_fixed_string_t show;
@@ -19325,6 +19373,7 @@ cmdline_parse_ctx_t main_ctx[] = {
  (cmdline_parse_inst_t *)&cmd_load_from_file,
  (cmdline_parse_inst_t *)&cmd_showport,
  (cmdline_parse_inst_t *)&cmd_showqueue,
+ (cmdline_parse_inst_t *)&cmd_showeeprom,
  (cmdline_parse_inst_t *)&cmd_showportall,
  (cmdline_parse_inst_t *)&cmd_showdevice,
  (cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..5e3e9e211 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,9 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
+

 #include "testpmd.h"

@@ -710,6 +713,122 @@ port_summary_display(portid_t port_id)
  (unsigned int) link.link_speed);
 }

+void
+port_eeprom_display(portid_t port_id)
+{
+ struct rte_dev_eeprom_info einfo;
+ int ret;
+ if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+ print_valid_ports();
+ return;
+ }
+
+ int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+ if (len_eeprom < 0) {
+ switch (len_eeprom) {
+ case -ENODEV:
+ printf("port index %d invalid\n", port_id);
+ break;
+ case -ENOTSUP:
+ printf("operation not supported by device\n");
+ break;
+ case -EIO:
+ printf("device is removed\n");
+ break;
+ default:
+ printf("Unable to get EEPROM: %d\n", len_eeprom);
+ break;
+ }
+ return;
+ }
+
+ char buf[len_eeprom];
+ einfo.offset = 0;
+ einfo.length = len_eeprom;
+ einfo.data = buf;
+
+ ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+ if (ret != 0) {
+ switch (ret) {
+ case -ENODEV:
+ printf("port index %d invalid\n", port_id);
+ break;
+ case -ENOTSUP:
+ printf("operation not supported by device\n");
+ break;
+ case -EIO:
+ printf("device is removed\n");
+ break;
+ default:
+ printf("Unable to get EEPROM: %d\n", ret);
+ break;
+ }
+ return;
+ }
+ rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+ printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id,
len_eeprom);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+ struct rte_eth_dev_module_info minfo;
+ struct rte_dev_eeprom_info einfo;
+ int ret;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+ print_valid_ports();
+ return;
+ }
+
+
+ ret = rte_eth_dev_get_module_info(port_id, &minfo);
+ if (ret != 0) {
+ switch (ret) {
+ case -ENODEV:
+ printf("port index %d invalid\n", port_id);
+ break;
+ case -ENOTSUP:
+ printf("operation not supported by device\n");
+ break;
+ case -EIO:
+ printf("device is removed\n");
+ break;
+ default:
+ printf("Unable to get module EEPROM: %d\n", ret);
+ break;
+ }
+ return;
+ }
+
+ char buf[minfo.eeprom_len];
+ einfo.offset = 0;
+ einfo.length = minfo.eeprom_len;
+ einfo.data = buf;
+
+ ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+ if (ret != 0) {
+ switch (ret) {
+ case -ENODEV:
+ printf("port index %d invalid\n", port_id);
+ break;
+ case -ENOTSUP:
+ printf("operation not supported by device\n");
+ break;
+ case -EIO:
+ printf("device is removed\n");
+ break;
+ default:
+ printf("Unable to get module EEPROM: %d\n", ret);
+ break;
+ }
+ return;
+ }
+
+ rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+ printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id,
einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..6a2e1c0f2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe

+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~

-- 
2.17.1

On Thu, Sep 10, 2020 at 2:48 PM David Liu <dliu@iol.unh.edu> wrote:

>
> On Thu, Sep 10, 2020 at 7:47 AM Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
>
>> On 9/10/2020 7:00 AM, David Liu wrote:
>> > Add module EEPROM/EEPROM dump command
>> >   "show port <port_id> (module_eeprom|eeprom)"
>> > Commands will dump the content of the
>> > EEPROM/module EEPROM for the selected port.
>>
>>
>> Hi David,
>>
>> When sending a new version, can you please increase the verstion tag in
>> the
>> title, V1 -> V2 -> ... -> vN
>>
>> Also sending new patch as reply to previos one keep all versions in same
>> email
>> thread and helps reviewers also people who later checks from archives for
>> a feature.
>>
>> For both above you can find more details in the contribution guide,
>> please check
>> https://doc.dpdk.org/guides/contributing/patches.html#sending-patches
>>
>>
>> And there are a few errors below that this patch shouldn't be compiling
>> successfully, you can verify the build error from lab reports :)
>>
>
> Thank you so much, I will change to that format.
>
>
>> <...>
>>
>> >  /* *** SHOW QUEUE INFO *** */
>> >  struct cmd_showqueue_result {
>> >       cmdline_fixed_string_t show;
>> > @@ -19325,6 +19373,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>> >       (cmdline_parse_inst_t *)&cmd_load_from_file,
>> >       (cmdline_parse_inst_t *)&cmd_showport,
>> >       (cmdline_parse_inst_t *)&cmd_showqueue,
>> > +     (cmdline_parse_inst_t *)&cmd_showeeprom,
>> > +     (cmdline_parse_inst_t *)&cmd_showmoduleeeprom,
>>
>> This shouldn't compile because 'cmd_showmoduleeeprom' no more exists ...
>>
>> <...>
>>
>> > +
>> > +void
>> > +port_module_eeprom_displao(portid_t port_id)
>> > +{
>>
>> There is a typo in the function name.
>>
>> > +     struct rte_eth_dev_module_info minfo;
>> > +     struct rte_dev_eeprom_info einfo;
>> > +     int ret;
>> > +
>> > +     if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> > +             print_valid_ports();
>> > +             return;
>> > +     }
>> > +
>> > +
>> > +     ret = rte_eth_dev_get_module_info(port_id, &minfo);
>> > +     if (ret != 0) {
>> > +             switch (ret) {
>> > +             case -ENODEV:
>> > +                     printf("port index %d invalid\n", port_id);
>> > +                     break;
>> > +             case -ENOTSUP:
>> > +                     printf("operation not supported by device\n");
>> > +                     break;
>> > +             case -EIO:
>> > +                     printf("device is removed\n");
>> > +                     break;
>> > +             default:
>> > +                     printf("Unable to get module EEPROM: %d\n",
>> len_eeprom);
>>
>> I guess this is copy/paste error 'len_eeprom' is not defined in this
>> function.
>> There is one more occurance below.
>>
>>
> I will send out a new patch with all the fixes and that can be compiled
> hopefully.
>
> <...>
>>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-14 15:11     ` [dpdk-dev] [PATCH V2] " David Liu
@ 2020-09-14 16:48       ` Ferruh Yigit
  2020-09-15 14:22         ` David Liu
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-09-14 16:48 UTC (permalink / raw)
  To: David Liu; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

On 9/14/2020 4:11 PM, David Liu wrote:
> Add module EEPROM/EEPROM dump command
>    "show port <port_id> (module_eeprom|eeprom)"
> Commands will dump the content of the EEPROM/module
> EEPROM for the selected port.
> 
> Signed-off-by: David Liu <dliu@iol.unh.edu <mailto:dliu@iol.unh.edu>>

Hi David,

The formatting is wrong in the patch and it is not detected as patch by
patchwork, can you please send the patch via "git send-eamil"?


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-14 16:48       ` Ferruh Yigit
@ 2020-09-15 14:22         ` David Liu
  0 siblings, 0 replies; 22+ messages in thread
From: David Liu @ 2020-09-15 14:22 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

On Mon, Sep 14, 2020 at 12:48 PM Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 9/14/2020 4:11 PM, David Liu wrote:
> > Add module EEPROM/EEPROM dump command
> >    "show port <port_id> (module_eeprom|eeprom)"
> > Commands will dump the content of the EEPROM/module
> > EEPROM for the selected port.
> >
> > Signed-off-by: David Liu <dliu@iol.unh.edu <mailto:dliu@iol.unh.edu>>
>
> Hi David,
>
> The formatting is wrong in the patch and it is not detected as patch by
> patchwork, can you please send the patch via "git send-eamil"?
>
> Yes, I will send it in as path V2.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-10 20:00 ` [dpdk-dev] [PATCH V1] " David Liu
@ 2020-09-15 15:42   ` " David Liu
  2020-09-15 17:24     ` David Liu
  2020-09-15 17:27     ` David Liu
  0 siblings, 2 replies; 22+ messages in thread
From: David Liu @ 2020-09-15 15:42 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Add module EEPROM/EEPROM dump command
   "show port <port_id> (module_eeprom|eeprom)"
Commands will dump the content of the EEPROM/module
EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  49 ++++++++
 app/test-pmd/config.c                       | 118 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 176 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..ba240c410 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port port_id (module_eeprom|eeprom)\n"
+			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7594,6 +7597,51 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	uint16_t portnum;
+	cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_showeeprom_result *res = parsed_result;
+
+	if (!strcmp(res->type, "eeprom"))
+		port_eeprom_display(res->portnum);
+	else if (!strcmp(res->type, "module_eeprom"))
+		port_module_eeprom_display(res->portnum);
+	else
+		printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+	.f = cmd_showeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom|eeprom",
+	.tokens = {
+		(void *)&cmd_showeeprom_show,
+		(void *)&cmd_showeeprom_port,
+		(void *)&cmd_showeeprom_portnum,
+		(void *)&cmd_showeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19373,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..a34827707 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -50,6 +50,8 @@
 #endif
 #include <rte_gro.h>
 
+#include <rte_hexdump.h>
+
 #include "testpmd.h"
 
 static char *flowtype_to_str(uint16_t flow_type);
@@ -710,6 +712,122 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+	if (len_eeprom < 0) {
+		switch (len_eeprom) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	char buf[len_eeprom];
+	einfo.offset = 0;
+	einfo.length = len_eeprom;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+	struct rte_eth_dev_module_info minfo;
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	char buf[minfo.eeprom_len];
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..6a2e1c0f2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe
 
+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-15 15:42   ` [dpdk-dev] [PATCH] [PATCH V2] " David Liu
@ 2020-09-15 17:24     ` David Liu
  2020-09-15 17:27     ` David Liu
  1 sibling, 0 replies; 22+ messages in thread
From: David Liu @ 2020-09-15 17:24 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  49 ++++++++
 app/test-pmd/config.c                       | 118 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 176 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0a6ed85f3..8d38de197 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port port_id (module_eeprom|eeprom)\n"
+			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7682,6 +7685,51 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	uint16_t portnum;
+	cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_showeeprom_result *res = parsed_result;
+
+	if (!strcmp(res->type, "eeprom"))
+		port_eeprom_display(res->portnum);
+	else if (!strcmp(res->type, "module_eeprom"))
+		port_module_eeprom_display(res->portnum);
+	else
+		printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+	.f = cmd_showeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom|eeprom",
+	.tokens = {
+		(void *)&cmd_showeeprom_show,
+		(void *)&cmd_showeeprom_port,
+		(void *)&cmd_showeeprom_portnum,
+		(void *)&cmd_showeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19400,6 +19448,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee3324..6203d27e7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -50,6 +50,8 @@
 #endif
 #include <rte_gro.h>
 
+#include <rte_hexdump.h>
+
 #include "testpmd.h"
 
 #define ETHDEV_FWVERS_LEN 32
@@ -783,6 +785,122 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+	if (len_eeprom < 0) {
+		switch (len_eeprom) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	char buf[len_eeprom];
+	einfo.offset = 0;
+	einfo.length = len_eeprom;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+	struct rte_eth_dev_module_info minfo;
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	char buf[minfo.eeprom_len];
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 25a12b14f..40225bc64 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -697,6 +697,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 90cf252df..fba8428b7 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe
 
+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-15 15:42   ` [dpdk-dev] [PATCH] [PATCH V2] " David Liu
  2020-09-15 17:24     ` David Liu
@ 2020-09-15 17:27     ` David Liu
  2020-09-22  9:13       ` Phil Yang
  1 sibling, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-15 17:27 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, thomas, David Liu

Add module EEPROM/EEPROM dump command
   "show port <port_id> (module_eeprom|eeprom)"
Commands will dump the content of the EEPROM/module
EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  49 ++++++++
 app/test-pmd/config.c                       | 118 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 4 files changed, 176 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0a6ed85f3..8d38de197 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port port_id (module_eeprom|eeprom)\n"
+			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7682,6 +7685,51 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
+struct cmd_showeeprom_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t port;
+	uint16_t portnum;
+	cmdline_fixed_string_t type;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_showeeprom_result *res = parsed_result;
+
+	if (!strcmp(res->type, "eeprom"))
+		port_eeprom_display(res->portnum);
+	else if (!strcmp(res->type, "module_eeprom"))
+		port_module_eeprom_display(res->portnum);
+	else
+		printf("Unknown argument\n");
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "module_eeprom#eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+	.f = cmd_showeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom|eeprom",
+	.tokens = {
+		(void *)&cmd_showeeprom_show,
+		(void *)&cmd_showeeprom_port,
+		(void *)&cmd_showeeprom_portnum,
+		(void *)&cmd_showeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19400,6 +19448,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee3324..6203d27e7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -50,6 +50,8 @@
 #endif
 #include <rte_gro.h>
 
+#include <rte_hexdump.h>
+
 #include "testpmd.h"
 
 #define ETHDEV_FWVERS_LEN 32
@@ -783,6 +785,122 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+	if (len_eeprom < 0) {
+		switch (len_eeprom) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", len_eeprom);
+			break;
+		}
+		return;
+	}
+
+	char buf[len_eeprom];
+	einfo.offset = 0;
+	einfo.length = len_eeprom;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+	struct rte_eth_dev_module_info minfo;
+	struct rte_dev_eeprom_info einfo;
+	int ret;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		print_valid_ports();
+		return;
+	}
+
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	char buf[minfo.eeprom_len];
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret != 0) {
+		switch (ret) {
+		case -ENODEV:
+			printf("port index %d invalid\n", port_id);
+			break;
+		case -ENOTSUP:
+			printf("operation not supported by device\n");
+			break;
+		case -EIO:
+			printf("device is removed\n");
+			break;
+		default:
+			printf("Unable to get module EEPROM: %d\n", ret);
+			break;
+		}
+		return;
+	}
+
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 25a12b14f..40225bc64 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -697,6 +697,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 90cf252df..fba8428b7 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,13 @@ For example:
      nvgre
      vxlan-gpe
 
+show port (module_eeprom|eeprom)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM/EEPROM informatioon for port_id::
+
+   testpmd> show port (port_id) (module_eeprom|eeprom)
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-15 17:27     ` David Liu
@ 2020-09-22  9:13       ` Phil Yang
  2020-09-24 10:57         ` Phil Yang
  0 siblings, 1 reply; 22+ messages in thread
From: Phil Yang @ 2020-09-22  9:13 UTC (permalink / raw)
  To: David Liu, ferruh.yigit; +Cc: dev, lylavoie, thomas, nd, nd

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of David Liu
> Sent: Wednesday, September 16, 2020 1:28 AM
> To: ferruh.yigit@intel.com
> Cc: dev@dpdk.org; lylavoie@iol.unh.edu; thomas@monjalon.net; David Liu
> <dliu@iol.unh.edu>
> Subject: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module
> eeprom display
> 
> Add module EEPROM/EEPROM dump command
>    "show port <port_id> (module_eeprom|eeprom)"
> Commands will dump the content of the EEPROM/module
> EEPROM for the selected port.


Verified with some popular NICs, it can dump the same info as the Linux ethtool does.
This is supported in examples/ethtool as well.

Minor nitpick comments inlined.

Thanks,
Phil

> 
> Signed-off-by: David Liu <dliu@iol.unh.edu>
> ---
>  app/test-pmd/cmdline.c                      |  49 ++++++++
>  app/test-pmd/config.c                       | 118 ++++++++++++++++++++
>  app/test-pmd/testpmd.h                      |   2 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
>  4 files changed, 176 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0a6ed85f3..8d38de197 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void
> *parsed_result,
>  			"show port
> (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
>  			"    Display information for port_id, or all.\n\n"
> 
> +			"show port port_id (module_eeprom|eeprom)\n"
> +			"    Display the module EEPROM or EEPROM
> information for port_id.\n\n"
> +
>  			"show port X rss reta (size) (mask0,mask1,...)\n"
>  			"    Display the rss redirection table entry indicated"
>  			" by masks on port X. size is used to indicate the"
> @@ -7682,6 +7685,51 @@ cmdline_parse_inst_t cmd_showdevice = {
>  		NULL,
>  	},
>  };
> +
> +/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
> +struct cmd_showeeprom_result {
> +	cmdline_fixed_string_t show;
> +	cmdline_fixed_string_t port;
> +	uint16_t portnum;
> +	cmdline_fixed_string_t type;
> +};
> +
> +static void cmd_showeeprom_parsed(void *parsed_result,
> +		__rte_unused struct cmdline *cl,
> +		__rte_unused void *data)
> +{
> +	struct cmd_showeeprom_result *res = parsed_result;
> +
> +	if (!strcmp(res->type, "eeprom"))
> +		port_eeprom_display(res->portnum);
> +	else if (!strcmp(res->type, "module_eeprom"))
> +		port_module_eeprom_display(res->portnum);
> +	else
> +		printf("Unknown argument\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_showeeprom_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show,
> "show");
> +cmdline_parse_token_string_t cmd_showeeprom_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port,
> "port");
> +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> +	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result,
> portnum, UINT16);
> +cmdline_parse_token_string_t cmd_showeeprom_type =
> +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
> "module_eeprom#eeprom");
> +
> +cmdline_parse_inst_t cmd_showeeprom = {
> +	.f = cmd_showeeprom_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> module_eeprom|eeprom",
> +	.tokens = {
> +		(void *)&cmd_showeeprom_show,
> +		(void *)&cmd_showeeprom_port,
> +		(void *)&cmd_showeeprom_portnum,
> +		(void *)&cmd_showeeprom_type,
> +		NULL,
> +	},
> +};
> +
>  /* *** SHOW QUEUE INFO *** */
>  struct cmd_showqueue_result {
>  	cmdline_fixed_string_t show;
> @@ -19400,6 +19448,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_load_from_file,
>  	(cmdline_parse_inst_t *)&cmd_showport,
>  	(cmdline_parse_inst_t *)&cmd_showqueue,
> +	(cmdline_parse_inst_t *)&cmd_showeeprom,
>  	(cmdline_parse_inst_t *)&cmd_showportall,
>  	(cmdline_parse_inst_t *)&cmd_showdevice,
>  	(cmdline_parse_inst_t *)&cmd_showcfg,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 30bee3324..6203d27e7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -50,6 +50,8 @@
>  #endif
>  #include <rte_gro.h>
> 

Remove the blank line above the rte_hexdump.h include statement.

> +#include <rte_hexdump.h>
> +
>  #include "testpmd.h"
> 
>  #define ETHDEV_FWVERS_LEN 32
> @@ -783,6 +785,122 @@ port_summary_display(portid_t port_id)
>  		(unsigned int) link.link_speed);
>  }
> 
> +void
> +port_eeprom_display(portid_t port_id)
> +{
> +	struct rte_dev_eeprom_info einfo;
> +	int ret;
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		print_valid_ports();
> +		return;
> +	}
> +
> +	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
> +	if (len_eeprom < 0) {
> +		switch (len_eeprom) {
> +		case -ENODEV:
> +			printf("port index %d invalid\n", port_id);
> +			break;
> +		case -ENOTSUP:
> +			printf("operation not supported by device\n");
> +			break;
> +		case -EIO:
> +			printf("device is removed\n");
> +			break;
> +		default:
> +			printf("Unable to get EEPROM: %d\n", len_eeprom);
> +			break;
> +		}
> +		return;
> +	}
> +
> +	char buf[len_eeprom];


Is GCC-11 happy about this array parameters? [-Warray-parameter]
I didn't check.



> +	einfo.offset = 0;
> +	einfo.length = len_eeprom;
> +	einfo.data = buf;
> +
> +	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
> +	if (ret != 0) {
> +		switch (ret) {
> +		case -ENODEV:
> +			printf("port index %d invalid\n", port_id);
> +			break;
> +		case -ENOTSUP:
> +			printf("operation not supported by device\n");
> +			break;
> +		case -EIO:
> +			printf("device is removed\n");
> +			break;
> +		default:
> +			printf("Unable to get EEPROM: %d\n", ret);
> +			break;
> +		}
> +		return;
> +	}
> +	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> +	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id,
> len_eeprom);
> +}
> +
> +void
> +port_module_eeprom_display(portid_t port_id)
> +{
> +	struct rte_eth_dev_module_info minfo;
> +	struct rte_dev_eeprom_info einfo;
> +	int ret;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		print_valid_ports();
> +		return;
> +	}
> +
> +
> +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
> +	if (ret != 0) {
> +		switch (ret) {
> +		case -ENODEV:
> +			printf("port index %d invalid\n", port_id);
> +			break;
> +		case -ENOTSUP:
> +			printf("operation not supported by device\n");
> +			break;
> +		case -EIO:
> +			printf("device is removed\n");
> +			break;
> +		default:
> +			printf("Unable to get module EEPROM: %d\n", ret);
> +			break;
> +		}
> +		return;
> +	}
> +
> +	char buf[minfo.eeprom_len];

Is GCC-11 happy about this array parameters? [-Warray-parameter]
I didn't check.



> +	einfo.offset = 0;
> +	einfo.length = minfo.eeprom_len;
> +	einfo.data = buf;
> +
> +	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> +	if (ret != 0) {
> +		switch (ret) {
> +		case -ENODEV:
> +			printf("port index %d invalid\n", port_id);
> +			break;
> +		case -ENOTSUP:
> +			printf("operation not supported by device\n");
> +			break;
> +		case -EIO:
> +			printf("device is removed\n");
> +			break;
> +		default:
> +			printf("Unable to get module EEPROM: %d\n", ret);
> +			break;
> +		}
> +		return;
> +	}
> +
> +	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> +	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n",
> port_id, einfo.length);
> +}
> +
>  void
>  port_offload_cap_display(portid_t port_id)
>  {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 25a12b14f..40225bc64 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -697,6 +697,8 @@ void nic_stats_mapping_display(portid_t port_id);
>  void device_infos_display(const char *identifier);
>  void port_infos_display(portid_t port_id);
>  void port_summary_display(portid_t port_id);
> +void port_eeprom_display(portid_t port_id);
> +void port_module_eeprom_display(portid_t port_id);
>  void port_summary_header_display(void);
>  void port_offload_cap_display(portid_t port_id);
>  void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 90cf252df..fba8428b7 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -218,6 +218,13 @@ For example:
>       nvgre
>       vxlan-gpe
> 
> +show port (module_eeprom|eeprom)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Display the module EEPROM/EEPROM informatioon for port_id::
> +
> +   testpmd> show port (port_id) (module_eeprom|eeprom)
> +
>  show port rss reta
>  ~~~~~~~~~~~~~~~~~~
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-22  9:13       ` Phil Yang
@ 2020-09-24 10:57         ` Phil Yang
  2020-09-24 15:26           ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: Phil Yang @ 2020-09-24 10:57 UTC (permalink / raw)
  To: Phil Yang, David Liu, ferruh.yigit; +Cc: dev, lylavoie, thomas, nd, nd, nd


dev <dev-bounces@dpdk.org> On Behalf Of Phil Yang : writes:


> > Subject: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module
> > eeprom display
> >
> > Add module EEPROM/EEPROM dump command
> >    "show port <port_id> (module_eeprom|eeprom)"
> > Commands will dump the content of the EEPROM/module
> > EEPROM for the selected port.
> 
> 
> Verified with some popular NICs, it can dump the same info as the Linux
> ethtool does.
> This is supported in examples/ethtool as well.
> 
> Minor nitpick comments inlined.
> 
> Thanks,
> Phil
> 
> >
> > Signed-off-by: David Liu <dliu@iol.unh.edu>
> > ---
> >  app/test-pmd/cmdline.c                      |  49 ++++++++
> >  app/test-pmd/config.c                       | 118 ++++++++++++++++++++
> >  app/test-pmd/testpmd.h                      |   2 +
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
> >  4 files changed, 176 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index 0a6ed85f3..8d38de197 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -166,6 +166,9 @@ static void cmd_help_long_parsed(void
> > *parsed_result,
> >  			"show port
> > (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
> >  			"    Display information for port_id, or all.\n\n"
> >
> > +			"show port port_id (module_eeprom|eeprom)\n"
> > +			"    Display the module EEPROM or EEPROM
> > information for port_id.\n\n"
> > +
> >  			"show port X rss reta (size) (mask0,mask1,...)\n"
> >  			"    Display the rss redirection table entry indicated"
> >  			" by masks on port X. size is used to indicate the"
> > @@ -7682,6 +7685,51 @@ cmdline_parse_inst_t cmd_showdevice = {
> >  		NULL,
> >  	},
> >  };
> > +
> > +/* *** SHOW MODULE EEPROM/EEPROM port INFO *** */
> > +struct cmd_showeeprom_result {
> > +	cmdline_fixed_string_t show;
> > +	cmdline_fixed_string_t port;
> > +	uint16_t portnum;
> > +	cmdline_fixed_string_t type;
> > +};
> > +
> > +static void cmd_showeeprom_parsed(void *parsed_result,
> > +		__rte_unused struct cmdline *cl,
> > +		__rte_unused void *data)
> > +{
> > +	struct cmd_showeeprom_result *res = parsed_result;
> > +
> > +	if (!strcmp(res->type, "eeprom"))
> > +		port_eeprom_display(res->portnum);
> > +	else if (!strcmp(res->type, "module_eeprom"))
> > +		port_module_eeprom_display(res->portnum);
> > +	else
> > +		printf("Unknown argument\n");
> > +}
> > +
> > +cmdline_parse_token_string_t cmd_showeeprom_show =
> > +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show,
> > "show");
> > +cmdline_parse_token_string_t cmd_showeeprom_port =
> > +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port,
> > "port");
> > +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> > +	TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result,
> > portnum, UINT16);
> > +cmdline_parse_token_string_t cmd_showeeprom_type =
> > +	TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
> > "module_eeprom#eeprom");
> > +
> > +cmdline_parse_inst_t cmd_showeeprom = {
> > +	.f = cmd_showeeprom_parsed,
> > +	.data = NULL,
> > +	.help_str = "show port <port_id> module_eeprom|eeprom",
> > +	.tokens = {
> > +		(void *)&cmd_showeeprom_show,
> > +		(void *)&cmd_showeeprom_port,
> > +		(void *)&cmd_showeeprom_portnum,
> > +		(void *)&cmd_showeeprom_type,
> > +		NULL,
> > +	},
> > +};
> > +
> >  /* *** SHOW QUEUE INFO *** */
> >  struct cmd_showqueue_result {
> >  	cmdline_fixed_string_t show;
> > @@ -19400,6 +19448,7 @@ cmdline_parse_ctx_t main_ctx[] = {
> >  	(cmdline_parse_inst_t *)&cmd_load_from_file,
> >  	(cmdline_parse_inst_t *)&cmd_showport,
> >  	(cmdline_parse_inst_t *)&cmd_showqueue,
> > +	(cmdline_parse_inst_t *)&cmd_showeeprom,
> >  	(cmdline_parse_inst_t *)&cmd_showportall,
> >  	(cmdline_parse_inst_t *)&cmd_showdevice,
> >  	(cmdline_parse_inst_t *)&cmd_showcfg,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 30bee3324..6203d27e7 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -50,6 +50,8 @@
> >  #endif
> >  #include <rte_gro.h>
> >
> 
> Remove the blank line above the rte_hexdump.h include statement.
> 
> > +#include <rte_hexdump.h>
> > +
> >  #include "testpmd.h"
> >
> >  #define ETHDEV_FWVERS_LEN 32
> > @@ -783,6 +785,122 @@ port_summary_display(portid_t port_id)
> >  		(unsigned int) link.link_speed);
> >  }
> >
> > +void
> > +port_eeprom_display(portid_t port_id)
> > +{
> > +	struct rte_dev_eeprom_info einfo;
> > +	int ret;
> > +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> > +		print_valid_ports();
> > +		return;
> > +	}
> > +
> > +	int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
> > +	if (len_eeprom < 0) {
> > +		switch (len_eeprom) {
> > +		case -ENODEV:
> > +			printf("port index %d invalid\n", port_id);
> > +			break;
> > +		case -ENOTSUP:
> > +			printf("operation not supported by device\n");
> > +			break;
> > +		case -EIO:
> > +			printf("device is removed\n");
> > +			break;
> > +		default:
> > +			printf("Unable to get EEPROM: %d\n", len_eeprom);
> > +			break;
> > +		}
> > +		return;
> > +	}
> > +
> > +	char buf[len_eeprom];
> 
> 
> Is GCC-11 happy about this array parameters? [-Warray-parameter]
> I didn't check.

No worries.
Checked with gcc version 11.0.0 20200920 (experimental)

Reviewed-by: Phil Yang <phil.yang@arm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module eeprom display
  2020-09-24 10:57         ` Phil Yang
@ 2020-09-24 15:26           ` Ferruh Yigit
  0 siblings, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2020-09-24 15:26 UTC (permalink / raw)
  To: Phil Yang, David Liu; +Cc: dev, lylavoie, thomas, nd

On 9/24/2020 11:57 AM, Phil Yang wrote:
> 
> dev <dev-bounces@dpdk.org> On Behalf Of Phil Yang : writes:
> 
> 
>>> Subject: [dpdk-dev] [PATCH] [PATCH V2] testpmd: add eeprom/module
>>> eeprom display
>>>
>>> Add module EEPROM/EEPROM dump command
>>>     "show port <port_id> (module_eeprom|eeprom)"
>>> Commands will dump the content of the EEPROM/module
>>> EEPROM for the selected port.
>>
>>
>> Verified with some popular NICs, it can dump the same info as the Linux
>> ethtool does.
>> This is supported in examples/ethtool as well.
>>
>> Minor nitpick comments inlined.
>>
>> Thanks,
>> Phil
>>
>>>
>>> Signed-off-by: David Liu <dliu@iol.unh.edu>

<...>

> 
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> 

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

(empty line above include fixed while merging.)
Applied to dpdk-next-net/main, thanks.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-03 16:40   ` David Liu
@ 2020-09-03 18:40     ` Ferruh Yigit
  0 siblings, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2020-09-03 18:40 UTC (permalink / raw)
  To: David Liu; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

On 9/3/2020 5:40 PM, David Liu wrote:

> On Wed, Sep 2, 2020 at 6:00 AM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 9/1/2020 8:07 PM, David Liu wrote:
>     > Change display message.
>     > Add EEPROM dump command
>     >   "show port <port_id> eeprom"
>     > Add module EEPROM dump command
>     >      "show port <port_id> module_eeprom"
>     > Commands will dump the content of the
>     > EEPROM/module EEPROM for the selected port.
>     >
>     > Signed-off-by: David Liu <dliu@iol.unh.edu <mailto:dliu@iol.unh.edu>>
>     > ---
>     >  app/test-pmd/cmdline.c                      |  87 ++++++++++++++
>     >  app/test-pmd/config.c                       | 126 ++++++++++++++++++++
>     >  app/test-pmd/testpmd.h                      |   2 +
>     >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
>     >  4 files changed, 229 insertions(+)
>     >
>     > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>     > index a037a55c6..71c98dd96 100644
>     > --- a/app/test-pmd/cmdline.c
>     > +++ b/app/test-pmd/cmdline.c
>     > @@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>     >                       "show port
>     (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
>     >                       "    Display information for port_id, or all.\n\n"
>     > 
>     > +                     "show port (port_id) eeprom \n"
>     > +                     "    Display the EEPROM infomation for given
>     port_id.\n\n"
>     > +
>     > +                     "show port (port_id) module_eeprom \n"
>     > +                     "    Display the port moudle EEPROM infomation for
>     given port_id.\n\n"
>     > +
>     >                       "show port X rss reta (size) (mask0,mask1,...)\n"
>     >                       "    Display the rss redirection table entry indicated"
>     >                       " by masks on port X. size is used to indicate the"
>     > @@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
>     >               NULL,
>     >       },
>     >  };
>     > +
>     > +/* ** SHOW EEPROM INFO *** */
>     > +struct cmd_showeeprom_result {
>     > +       cmdline_fixed_string_t show;
>     > +       cmdline_fixed_string_t port;
>     > +       cmdline_fixed_string_t type;
>     > +        uint16_t portnum;
>     > +};
>     > +
>     > +static void cmd_showeeprom_parsed(void *parsed_result,
>     > +               __rte_unused struct cmdline *cl,
>     > +               __rte_unused void *data)
>     > +{
>     > +       struct cmd_showeeprom_result *res = parsed_result;
>     > +
>     > +       port_eeprom_display(res->portnum);
>     > +}
>     > +
>     > +cmdline_parse_token_string_t cmd_showeeprom_show =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
>     > +cmdline_parse_token_string_t cmd_showeeprom_port =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
>     > +cmdline_parse_token_num_t cmd_showeeprom_portnum =
>     > +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
>     > +cmdline_parse_token_string_t cmd_showeeprom_type =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
>     "eeprom");
>     > +
>     > +cmdline_parse_inst_t cmd_showeeprom = {
>     > +       .f = cmd_showeeprom_parsed,
>     > +       .data = NULL,
>     > +       .help_str = "show port <port_id> eeprom",
>     > +       .tokens = {
>     > +               (void *)&cmd_showeeprom_show,
>     > +               (void *)&cmd_showeeprom_port,
>     > +               (void *)&cmd_showeeprom_portnum,
>     > +               (void *)&cmd_showeeprom_type,
>     > +               NULL,
>     > +       },
>     > +};
>     > +
>     > +/* ** SHOW MODULE EEPROM INFO *** */
>     > +struct cmd_showmoduleeeprom_result {
>     > +       cmdline_fixed_string_t show;
>     > +       cmdline_fixed_string_t port;
>     > +       cmdline_fixed_string_t type;
>     > +        uint16_t portnum;
>     > +};
>     > +
>     > +static void cmd_showmoduleeeprom_parsed(void *parsed_result,
>     > +               __rte_unused struct cmdline *cl,
>     > +               __rte_unused void *data)
>     > +{
>     > +       struct cmd_showmoduleeeprom_result *res = parsed_result;
>     > +
>     > +       port_module_eeprom_display(res->portnum);
>     > +}
>     > +
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, show,
>     "show");
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, port,
>     "port");
>     > +cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
>     > +     TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum,
>     UINT16);
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
>     > +     TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type,
>     "module_eeprom");
>     > +
>     > +cmdline_parse_inst_t cmd_showmoduleeeprom = {
>     > +     .f = cmd_showmoduleeeprom_parsed,
>     > +     .data = NULL,
>     > +     .help_str = "show port <port_id> module_eeprom",
>     > +     .tokens = {
>     > +             (void *)&cmd_showmoduleeeprom_show,
>     > +             (void *)&cmd_showmoduleeeprom_port,
>     > +             (void *)&cmd_showmoduleeeprom_portnum,
>     > +             (void *)&cmd_showmoduleeeprom_type,
>     > +             NULL,
>     > +     },
>     > +};> +
> 
>     Since both commands are simple and related, what do you think merging their
>     implementation? This reduces the clutter.
>     Please check '#' usage in the 'TOKEN_STRING_INITIALIZER', and "port
>     start|stop|close all" implementation can be sample.
> 
> Yes, that sounds good. What do you think of having the command to be "show port
> (port_id|all) (module_eeprom|eeprom)"?
>

(Please don't top post, bad for archiving, copy/pasted from top of the email)

+1 to "(module_eeprom|eeprom)" part.

Not sure about "(port_id|all)", since this will hexdump the eeprom, I assume the
output will be long and not sure if there will be need to dump all ports at once.

So I suggest start with only "port_id" and "all" support later if there is a
need, unless you already need it.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-02 10:00 ` Ferruh Yigit
@ 2020-09-03 16:40   ` David Liu
  2020-09-03 18:40     ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-03 16:40 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, Lincoln Lavoie, Thomas Monjalon

Yes, that sounds good. What do you think of having the command to be "show
port (port_id|all) (module_eeprom|eeprom)"?

On Wed, Sep 2, 2020 at 6:00 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 9/1/2020 8:07 PM, David Liu wrote:
> > Change display message.
> > Add EEPROM dump command
> >   "show port <port_id> eeprom"
> > Add module EEPROM dump command
> >      "show port <port_id> module_eeprom"
> > Commands will dump the content of the
> > EEPROM/module EEPROM for the selected port.
> >
> > Signed-off-by: David Liu <dliu@iol.unh.edu>
> > ---
> >  app/test-pmd/cmdline.c                      |  87 ++++++++++++++
> >  app/test-pmd/config.c                       | 126 ++++++++++++++++++++
> >  app/test-pmd/testpmd.h                      |   2 +
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
> >  4 files changed, 229 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index a037a55c6..71c98dd96 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void
> *parsed_result,
> >                       "show port
> (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
> >                       "    Display information for port_id, or all.\n\n"
> >
> > +                     "show port (port_id) eeprom \n"
> > +                     "    Display the EEPROM infomation for given
> port_id.\n\n"
> > +
> > +                     "show port (port_id) module_eeprom \n"
> > +                     "    Display the port moudle EEPROM infomation for
> given port_id.\n\n"
> > +
> >                       "show port X rss reta (size) (mask0,mask1,...)\n"
> >                       "    Display the rss redirection table entry
> indicated"
> >                       " by masks on port X. size is used to indicate the"
> > @@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
> >               NULL,
> >       },
> >  };
> > +
> > +/* ** SHOW EEPROM INFO *** */
> > +struct cmd_showeeprom_result {
> > +       cmdline_fixed_string_t show;
> > +       cmdline_fixed_string_t port;
> > +       cmdline_fixed_string_t type;
> > +        uint16_t portnum;
> > +};
> > +
> > +static void cmd_showeeprom_parsed(void *parsed_result,
> > +               __rte_unused struct cmdline *cl,
> > +               __rte_unused void *data)
> > +{
> > +       struct cmd_showeeprom_result *res = parsed_result;
> > +
> > +       port_eeprom_display(res->portnum);
> > +}
> > +
> > +cmdline_parse_token_string_t cmd_showeeprom_show =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show,
> "show");
> > +cmdline_parse_token_string_t cmd_showeeprom_port =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port,
> "port");
> > +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> > +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum,
> UINT16);
> > +cmdline_parse_token_string_t cmd_showeeprom_type =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
> "eeprom");
> > +
> > +cmdline_parse_inst_t cmd_showeeprom = {
> > +       .f = cmd_showeeprom_parsed,
> > +       .data = NULL,
> > +       .help_str = "show port <port_id> eeprom",
> > +       .tokens = {
> > +               (void *)&cmd_showeeprom_show,
> > +               (void *)&cmd_showeeprom_port,
> > +               (void *)&cmd_showeeprom_portnum,
> > +               (void *)&cmd_showeeprom_type,
> > +               NULL,
> > +       },
> > +};
> > +
> > +/* ** SHOW MODULE EEPROM INFO *** */
> > +struct cmd_showmoduleeeprom_result {
> > +       cmdline_fixed_string_t show;
> > +       cmdline_fixed_string_t port;
> > +       cmdline_fixed_string_t type;
> > +        uint16_t portnum;
> > +};
> > +
> > +static void cmd_showmoduleeeprom_parsed(void *parsed_result,
> > +               __rte_unused struct cmdline *cl,
> > +               __rte_unused void *data)
> > +{
> > +       struct cmd_showmoduleeeprom_result *res = parsed_result;
> > +
> > +       port_module_eeprom_display(res->portnum);
> > +}
> > +
> > +cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result,
> show, "show");
> > +cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result,
> port, "port");
> > +cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
> > +     TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum,
> UINT16);
> > +cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
> > +     TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type,
> "module_eeprom");
> > +
> > +cmdline_parse_inst_t cmd_showmoduleeeprom = {
> > +     .f = cmd_showmoduleeeprom_parsed,
> > +     .data = NULL,
> > +     .help_str = "show port <port_id> module_eeprom",
> > +     .tokens = {
> > +             (void *)&cmd_showmoduleeeprom_show,
> > +             (void *)&cmd_showmoduleeeprom_port,
> > +             (void *)&cmd_showmoduleeeprom_portnum,
> > +             (void *)&cmd_showmoduleeeprom_type,
> > +             NULL,
> > +     },
> > +};> +
>
> Since both commands are simple and related, what do you think merging their
> implementation? This reduces the clutter.
> Please check '#' usage in the 'TOKEN_STRING_INITIALIZER', and "port
> start|stop|close all" implementation can be sample.
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-09-01 19:07 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
@ 2020-09-02 10:00 ` Ferruh Yigit
  2020-09-03 16:40   ` David Liu
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-09-02 10:00 UTC (permalink / raw)
  To: David Liu; +Cc: dev, lylavoie, tmonjalon

On 9/1/2020 8:07 PM, David Liu wrote:
> Change display message.
> Add EEPROM dump command
>   "show port <port_id> eeprom"
> Add module EEPROM dump command
>      "show port <port_id> module_eeprom"
> Commands will dump the content of the 
> EEPROM/module EEPROM for the selected port.
> 
> Signed-off-by: David Liu <dliu@iol.unh.edu>
> ---
>  app/test-pmd/cmdline.c                      |  87 ++++++++++++++
>  app/test-pmd/config.c                       | 126 ++++++++++++++++++++
>  app/test-pmd/testpmd.h                      |   2 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
>  4 files changed, 229 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index a037a55c6..71c98dd96 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
>  			"    Display information for port_id, or all.\n\n"
>  
> +			"show port (port_id) eeprom \n"
> +			"    Display the EEPROM infomation for given port_id.\n\n"
> +
> +			"show port (port_id) module_eeprom \n"
> +			"    Display the port moudle EEPROM infomation for given port_id.\n\n"
> +
>  			"show port X rss reta (size) (mask0,mask1,...)\n"
>  			"    Display the rss redirection table entry indicated"
>  			" by masks on port X. size is used to indicate the"
> @@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
>  		NULL,
>  	},
>  };
> +
> +/* ** SHOW EEPROM INFO *** */
> +struct cmd_showeeprom_result {
> +       cmdline_fixed_string_t show;
> +       cmdline_fixed_string_t port;
> +       cmdline_fixed_string_t type;
> +        uint16_t portnum;
> +};
> +
> +static void cmd_showeeprom_parsed(void *parsed_result,
> +               __rte_unused struct cmdline *cl,
> +               __rte_unused void *data)
> +{
> +       struct cmd_showeeprom_result *res = parsed_result;
> +
> +       port_eeprom_display(res->portnum);
> +}
> +
> +cmdline_parse_token_string_t cmd_showeeprom_show =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
> +cmdline_parse_token_string_t cmd_showeeprom_port =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
> +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
> +cmdline_parse_token_string_t cmd_showeeprom_type =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "eeprom");
> +
> +cmdline_parse_inst_t cmd_showeeprom = {
> +       .f = cmd_showeeprom_parsed,
> +       .data = NULL,
> +       .help_str = "show port <port_id> eeprom",
> +       .tokens = {
> +               (void *)&cmd_showeeprom_show,
> +               (void *)&cmd_showeeprom_port,
> +               (void *)&cmd_showeeprom_portnum,
> +               (void *)&cmd_showeeprom_type,
> +               NULL,
> +       },
> +};
> +
> +/* ** SHOW MODULE EEPROM INFO *** */
> +struct cmd_showmoduleeeprom_result {
> +       cmdline_fixed_string_t show;
> +       cmdline_fixed_string_t port;
> +       cmdline_fixed_string_t type;
> +        uint16_t portnum;
> +};
> +
> +static void cmd_showmoduleeeprom_parsed(void *parsed_result,
> +               __rte_unused struct cmdline *cl,
> +               __rte_unused void *data)
> +{
> +       struct cmd_showmoduleeeprom_result *res = parsed_result;
> +
> +       port_module_eeprom_display(res->portnum);
> +}
> +
> +cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, show, "show");
> +cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, port, "port");
> +cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
> +	TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum, UINT16);
> +cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
> +	TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type, "module_eeprom");
> +
> +cmdline_parse_inst_t cmd_showmoduleeeprom = {
> +	.f = cmd_showmoduleeeprom_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> module_eeprom",
> +	.tokens = {
> +		(void *)&cmd_showmoduleeeprom_show,
> +		(void *)&cmd_showmoduleeeprom_port,
> +		(void *)&cmd_showmoduleeeprom_portnum,
> +		(void *)&cmd_showmoduleeeprom_type,
> +		NULL,
> +	},
> +};> +

Since both commands are simple and related, what do you think merging their
implementation? This reduces the clutter.
Please check '#' usage in the 'TOKEN_STRING_INITIALIZER', and "port
start|stop|close all" implementation can be sample.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev]  [PATCH V1] testpmd: add eeprom/module eeprom display
@ 2020-09-01 19:07 David Liu
  2020-09-02 10:00 ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-09-01 19:07 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, tmonjalon, David Liu

Change display message.
Add EEPROM dump command
  "show port <port_id> eeprom"
Add module EEPROM dump command
     "show port <port_id> module_eeprom"
Commands will dump the content of the 
EEPROM/module EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  87 ++++++++++++++
 app/test-pmd/config.c                       | 126 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
 4 files changed, 229 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..71c98dd96 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port (port_id) eeprom \n"
+			"    Display the EEPROM infomation for given port_id.\n\n"
+
+			"show port (port_id) module_eeprom \n"
+			"    Display the port moudle EEPROM infomation for given port_id.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* ** SHOW EEPROM INFO *** */
+struct cmd_showeeprom_result {
+       cmdline_fixed_string_t show;
+       cmdline_fixed_string_t port;
+       cmdline_fixed_string_t type;
+        uint16_t portnum;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+               __rte_unused struct cmdline *cl,
+               __rte_unused void *data)
+{
+       struct cmd_showeeprom_result *res = parsed_result;
+
+       port_eeprom_display(res->portnum);
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+       .f = cmd_showeeprom_parsed,
+       .data = NULL,
+       .help_str = "show port <port_id> eeprom",
+       .tokens = {
+               (void *)&cmd_showeeprom_show,
+               (void *)&cmd_showeeprom_port,
+               (void *)&cmd_showeeprom_portnum,
+               (void *)&cmd_showeeprom_type,
+               NULL,
+       },
+};
+
+/* ** SHOW MODULE EEPROM INFO *** */
+struct cmd_showmoduleeeprom_result {
+       cmdline_fixed_string_t show;
+       cmdline_fixed_string_t port;
+       cmdline_fixed_string_t type;
+        uint16_t portnum;
+};
+
+static void cmd_showmoduleeeprom_parsed(void *parsed_result,
+               __rte_unused struct cmdline *cl,
+               __rte_unused void *data)
+{
+       struct cmd_showmoduleeeprom_result *res = parsed_result;
+
+       port_module_eeprom_display(res->portnum);
+}
+
+cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
+       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
+       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type, "module_eeprom");
+
+cmdline_parse_inst_t cmd_showmoduleeeprom = {
+	.f = cmd_showmoduleeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom",
+	.tokens = {
+		(void *)&cmd_showmoduleeeprom_show,
+		(void *)&cmd_showmoduleeeprom_port,
+		(void *)&cmd_showmoduleeeprom_portnum,
+		(void *)&cmd_showmoduleeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19410,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+        (cmdline_parse_inst_t *)&cmd_showeeprom,
+	(cmdline_parse_inst_t *)&cmd_showmoduleeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..ea026a8eb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,7 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
 
 #include "testpmd.h"
 
@@ -710,6 +711,131 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+       struct rte_dev_eeprom_info einfo;
+       int ret;
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+
+       if(len_eeprom < 0) {
+               switch (len_eeprom) {
+               case -ENODEV:
+		       printf("port index %d invalid\n", port_id);
+		       break;
+	       case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+		       printf("device is removed\n");
+		       break;
+               default:
+                       printf("Unable to get module EEPROM: %d\n", ret);
+                       break;
+               }
+	       return;
+       }
+
+       char buf[len_eeprom];
+       printf("\nPort: %d\nModule EEPROM:\n", port_id);
+
+       einfo.offset = 0;
+       einfo.length = len_eeprom;
+       einfo.data = buf;
+
+       ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+		       printf("port index %d invalid\n", port_id);
+		       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+		       break;
+               case -EIO:
+		       printf("device is removed\n");
+		       break;
+	       default:
+		       printf("Unable to get module EEPROM: %d\n", ret);
+		       break;
+	       }
+	       return;
+       }
+       
+       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+       printf("Finish -- Total EEPROM length: %i bytes\n", len_eeprom);
+}
+
+void
+port_module_eeprom_displao(portid_t port_id)
+{
+       struct rte_eth_dev_module_info minfo;
+       struct rte_dev_eeprom_info einfo;
+       int ret;
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       ret = rte_eth_dev_get_module_info(port_id, &minfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module info: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       if (minfo.eeprom_len < 0){
+               printf("invalid module eeprom length");
+               return;
+       }
+
+       char buf[minfo.eeprom_len];
+       einfo.offset = 0;
+       einfo.length = minfo.eeprom_len;
+       einfo.data = buf;
+
+       ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module EEPROM: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       printf("\nPort: %d\nEEPROM:\n", port_id);
+       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..f3e2211ab 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,20 @@ For example:
      nvgre
      vxlan-gpe
 
+show port eeprom
+~~~~~~~~~~~~~~~~
+
+Display the EEPROM informatioon for given port_id::
+
+   testpmd> show port (port_id) eeprom
+
+show port module eeprom
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM informatioon for given port_id::
+
+   testpmd> show port (port_id) module_eeprom
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev]  [PATCH V1] testpmd: add eeprom/module eeprom display
@ 2020-09-01 18:56 David Liu
  0 siblings, 0 replies; 22+ messages in thread
From: David Liu @ 2020-09-01 18:56 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, lylavoie, tmonjalon, David Liu

Add EEPROM dump command
  "show port <port_id> eeprom"
Add module EEPROM dump command
     "show port <port_id> module_eeprom"
Commands will dump the content of the 
EEPROM/module EEPROM for the selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c                      |  87 ++++++++++++++
 app/test-pmd/config.c                       | 126 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |   2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
 4 files changed, 229 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..71c98dd96 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port (port_id) eeprom \n"
+			"    Display the EEPROM infomation for port x.\n\n"
+
+			"show port (port_id) module_eeprom \n"
+			"    Display the port moudle EEPROM infomation for port x.\n\n"
+
 			"show port X rss reta (size) (mask0,mask1,...)\n"
 			"    Display the rss redirection table entry indicated"
 			" by masks on port X. size is used to indicate the"
@@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* ** SHOW EEPROM INFO *** */
+struct cmd_showeeprom_result {
+       cmdline_fixed_string_t show;
+       cmdline_fixed_string_t port;
+       cmdline_fixed_string_t type;
+        uint16_t portnum;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+               __rte_unused struct cmdline *cl,
+               __rte_unused void *data)
+{
+       struct cmd_showeeprom_result *res = parsed_result;
+
+       port_eeprom_display(res->portnum);
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_port =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showeeprom_type =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "eeprom");
+
+cmdline_parse_inst_t cmd_showeeprom = {
+       .f = cmd_showeeprom_parsed,
+       .data = NULL,
+       .help_str = "show port <port_id> eeprom",
+       .tokens = {
+               (void *)&cmd_showeeprom_show,
+               (void *)&cmd_showeeprom_port,
+               (void *)&cmd_showeeprom_portnum,
+               (void *)&cmd_showeeprom_type,
+               NULL,
+       },
+};
+
+/* ** SHOW module EEPROM INFO *** */
+struct cmd_showmoduleeeprom_result {
+       cmdline_fixed_string_t show;
+       cmdline_fixed_string_t port;
+       cmdline_fixed_string_t type;
+        uint16_t portnum;
+};
+
+static void cmd_showmoduleeeprom_parsed(void *parsed_result,
+               __rte_unused struct cmdline *cl,
+               __rte_unused void *data)
+{
+       struct cmd_showmoduleeeprom_result *res = parsed_result;
+
+       port_module_eeprom_display(res->portnum);
+}
+
+cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
+       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
+       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, port, "port");
+cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum, UINT16);
+cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type, "module_eeprom");
+
+cmdline_parse_inst_t cmd_showmoduleeeprom = {
+	.f = cmd_showmoduleeeprom_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> module_eeprom",
+	.tokens = {
+		(void *)&cmd_showmoduleeeprom_show,
+		(void *)&cmd_showmoduleeeprom_port,
+		(void *)&cmd_showmoduleeeprom_portnum,
+		(void *)&cmd_showmoduleeeprom_type,
+		NULL,
+	},
+};
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19410,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+        (cmdline_parse_inst_t *)&cmd_showeeprom,
+	(cmdline_parse_inst_t *)&cmd_showmoduleeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..ea026a8eb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,7 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
 
 #include "testpmd.h"
 
@@ -710,6 +711,131 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+       struct rte_dev_eeprom_info einfo;
+       int ret;
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
+
+       if(len_eeprom < 0) {
+               switch (len_eeprom) {
+               case -ENODEV:
+		       printf("port index %d invalid\n", port_id);
+		       break;
+	       case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+		       printf("device is removed\n");
+		       break;
+               default:
+                       printf("Unable to get module EEPROM: %d\n", ret);
+                       break;
+               }
+	       return;
+       }
+
+       char buf[len_eeprom];
+       printf("\nPort: %d\nModule EEPROM:\n", port_id);
+
+       einfo.offset = 0;
+       einfo.length = len_eeprom;
+       einfo.data = buf;
+
+       ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+		       printf("port index %d invalid\n", port_id);
+		       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+		       break;
+               case -EIO:
+		       printf("device is removed\n");
+		       break;
+	       default:
+		       printf("Unable to get module EEPROM: %d\n", ret);
+		       break;
+	       }
+	       return;
+       }
+       
+       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+       printf("Finish -- Total EEPROM length: %i bytes\n", len_eeprom);
+}
+
+void
+port_module_eeprom_displao(portid_t port_id)
+{
+       struct rte_eth_dev_module_info minfo;
+       struct rte_dev_eeprom_info einfo;
+       int ret;
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       ret = rte_eth_dev_get_module_info(port_id, &minfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module info: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       if (minfo.eeprom_len < 0){
+               printf("invalid module eeprom length");
+               return;
+       }
+
+       char buf[minfo.eeprom_len];
+       einfo.offset = 0;
+       einfo.length = minfo.eeprom_len;
+       einfo.data = buf;
+
+       ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module EEPROM: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       printf("\nPort: %d\nEEPROM:\n", port_id);
+       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5103ff925..f3e2211ab 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -218,6 +218,20 @@ For example:
      nvgre
      vxlan-gpe
 
+show port eeprom
+~~~~~~~~~~~~~~~~
+
+Display the EEPROM informatioon for port X::
+
+   testpmd> show port (port_id) eeprom
+
+show port module eeprom
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the module EEPROM informatioon for port X::
+
+   testpmd> show port (port_id) module_eeprom
+
 show port rss reta
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-08-26 22:46 ` Ferruh Yigit
@ 2020-08-28 13:46   ` David Liu
  0 siblings, 0 replies; 22+ messages in thread
From: David Liu @ 2020-08-28 13:46 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, Lincoln Lavoie, tmonjalon

z

On Wed, Aug 26, 2020 at 6:46 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/26/2020 8:00 PM, David Liu wrote:
> > Commands will dump the content of the EEPROM/module EEPROM for the
> > selected port.
> >
> > Signed-off-by: David Liu <dliu@iol.unh.edu>
> > ---
> >  app/test-pmd/cmdline.c |  41 +++++++++++++++
> >  app/test-pmd/config.c  | 112 +++++++++++++++++++++++++++++++++++++++++
> >  app/test-pmd/testpmd.h |   2 +
> >  3 files changed, 155 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index a037a55c6..84943a889 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -7594,6 +7594,46 @@ cmdline_parse_inst_t cmd_showdevice = {
> >               NULL,
> >       },
> >  };
> > +
> > +/* ** SHOW EEPROM INFO *** */
> > +struct cmd_showeeprom_result {
> > +       cmdline_fixed_string_t show;
> > +       cmdline_fixed_string_t type;
> > +        uint16_t portnum;
> > +};
> > +
> > +static void cmd_showeeprom_parsed(void *parsed_result,
> > +               __rte_unused struct cmdline *cl,
> > +               __rte_unused void *data)
> > +{
> > +       struct cmd_showeeprom_result *res = parsed_result;
> > +
> > +       if (!strcmp(res->type, "eeprom"))
> > +               port_eeprom_display(res->portnum);
> > +       else if (!strcmp(res->type, "module_eeprom"))
> > +               port_module_eeprom_display(res->portnum);
> > +}
> > +
> > +cmdline_parse_token_string_t cmd_showeeprom_show =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show,
> "show");
> > +cmdline_parse_token_string_t cmd_showeeprom_type =
> > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
> "eeprom#module_eeprom");
> > +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> > +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum,
> UINT16);
> > +
> > +cmdline_parse_inst_t cmd_showeeprom = {
> > +       .f = cmd_showeeprom_parsed,
> > +       .data = NULL,
> > +       .help_str = "show eeprom|module_eeprom <port_id>",
>
> There is more common syntax like:
> "show port X <port_id>|all"
> or
> "show port <port_id> Y"
>
> Keeping command syntax consistent helps using testpmd. What do you think
> selecting one of above two?
>
> Also can you please update 'cmd_help_long_parsed()' (in same file) and
> documentation (testpmd_funcs.rst) for new command.
>
> Sure, I can use that syntax above and update the documentation.


> > +       .tokens = {
> > +               (void *)&cmd_showeeprom_show,
> > +               (void *)&cmd_showeeprom_type,
> > +               (void *)&cmd_showeeprom_portnum,
> > +               NULL,
> > +       },
> > +};
> > +
> > +
> >  /* *** SHOW QUEUE INFO *** */
> >  struct cmd_showqueue_result {
> >       cmdline_fixed_string_t show;
> > @@ -19325,6 +19365,7 @@ cmdline_parse_ctx_t main_ctx[] = {
> >       (cmdline_parse_inst_t *)&cmd_load_from_file,
> >       (cmdline_parse_inst_t *)&cmd_showport,
> >       (cmdline_parse_inst_t *)&cmd_showqueue,
> > +        (cmdline_parse_inst_t *)&cmd_showeeprom,
> >       (cmdline_parse_inst_t *)&cmd_showportall,
> >       (cmdline_parse_inst_t *)&cmd_showdevice,
> >       (cmdline_parse_inst_t *)&cmd_showcfg,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 8cf84ccd3..79fd81a65 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -49,6 +49,7 @@
> >  #include <rte_pmd_bnxt.h>
> >  #endif
> >  #include <rte_gro.h>
> > +#include <rte_hexdump.h>
> >
> >  #include "testpmd.h"
> >
> > @@ -710,6 +711,117 @@ port_summary_display(portid_t port_id)
> >               (unsigned int) link.link_speed);
> >  }
> >
> > +void
> > +port_eeprom_display(portid_t port_id)
> > +{
> > +       struct rte_dev_eeprom_info einfo;
> > +       int ret;
> > +
> > +       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> > +               print_valid_ports();
> > +               return;
> > +       }
> > +
> > +       int len_eeprom = rte_eth_dev_get_eeprom_length(port_id);
> > +       char buf[len_eeprom];
>
> If 'len_eeprom' can be negative what will be the 'buf' size?
>
> Thank you for the catch. I will include a negative length error checking,

> > +
> > +       if (len_eeprom > 0) {
>
> To reduce the code indentation, you can eliminate this if block by
> returning on
> error cases. (like as done in below 'port_module_eeprom_display()')
>
> > +               printf("\nPort: %d\nModule EEPROM:\n", port_id);
> > +
> > +               einfo.offset = 0;
> > +               einfo.length = len_eeprom;
> > +               einfo.data = buf;
> > +
> > +               ret = rte_eth_dev_get_eeprom(port_id, &einfo);
> > +               if (ret != 0) {
> > +                       switch (ret) {
> > +                       case -ENODEV:
> > +                               printf("port index %d invalid\n",
> port_id);
> > +                               break;
> > +                       case -ENOTSUP:
> > +                               printf("operation not supported by
> device\n");
> > +                               break;
> > +                       case -EIO:
> > +                               printf("device Iis removed\n");
>
> there is a typo before 'is' (and in same messages below)
>
> > +                               break;
> > +                       default:
> > +                               printf("Unable to get module EEPROM:
> %d\n", ret);
> > +                               break;
> > +                       }
> > +                       return;
> > +               }
> > +
> > +               rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> > +               printf("Finish -- Total EEPROM length: %i bytes\n",
> len_eeprom);
> > +
> > +       } else if (len_eeprom == 0)
> > +               printf("Port %d: Device does not have EEPROM\n",
> port_id);
> > +       else if (len_eeprom == -ENOTSUP)
> > +               printf("Port %d: Operation not supported\n", port_id);
> > +       else
> > +               printf("Port %d: Error getting EEPROM\n", port_id);
> > +}
> > +
> > +void
> > +port_module_eeprom_display(portid_t port_id)
> > +{
> > +       struct rte_eth_dev_module_info minfo;
> > +       struct rte_dev_eeprom_info einfo;
> > +       char buf[1024];
>
> Size can exceed the '1024'
>
> > +       int ret;
> > +
> > +
> > +       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> > +               print_valid_ports();
> > +               return;
> > +       }
> > +
> > +       ret = rte_eth_dev_get_module_info(port_id, &minfo);
> > +       if (ret != 0) {
> > +               switch (ret) {
> > +               case -ENODEV:
> > +                       printf("port index %d invalid\n", port_id);
> > +                       break;
> > +               case -ENOTSUP:
> > +                       printf("operation not supported by device\n");
> > +                       break;
> > +               case -EIO:
> > +                       printf("device Iis removed\n");
> > +                       break;
> > +               default:
> > +                       printf("Unable to get module info: %d\n", ret);
> > +                       break;
> > +               }
> > +               return;
> > +       }
> > +
> > +       einfo.offset = 0;
> > +       einfo.length = minfo.eeprom_len;
> > +       einfo.data = buf;
> > +
> > +       ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> > +       if (ret != 0) {
> > +               switch (ret) {
> > +               case -ENODEV:
> > +                       printf("port index %d invalid\n", port_id);
> > +                       break;
> > +               case -ENOTSUP:
> > +                       printf("operation not supported by device\n");
> > +                       break;
> > +               case -EIO:
> > +                       printf("device Iis removed\n");
> > +                       break;
> > +               default:
> > +                       printf("Unable to get module EEPROM: %d\n", ret);
> > +                       break;
> > +               }
> > +               return;
> > +       }
> > +
> > +       printf("\nPort: %d\nEEPROM:\n", port_id);
> > +       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> > +}
> > +
> >  void
> >  port_offload_cap_display(portid_t port_id)
> >  {
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> > index 7a7c73f79..e3a0d17e5 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
> >  void device_infos_display(const char *identifier);
> >  void port_infos_display(portid_t port_id);
> >  void port_summary_display(portid_t port_id);
> > +void port_eeprom_display(portid_t port_id);
> > +void port_module_eeprom_display(portid_t port_id);
> >  void port_summary_header_display(void);
> >  void port_offload_cap_display(portid_t port_id);
> >  void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
> >
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
  2020-08-26 19:00 David Liu
@ 2020-08-26 22:46 ` Ferruh Yigit
  2020-08-28 13:46   ` David Liu
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2020-08-26 22:46 UTC (permalink / raw)
  To: David Liu, dev; +Cc: lylavoie, tmonjalon

On 8/26/2020 8:00 PM, David Liu wrote:
> Commands will dump the content of the EEPROM/module EEPROM for the
> selected port.
> 
> Signed-off-by: David Liu <dliu@iol.unh.edu>
> ---
>  app/test-pmd/cmdline.c |  41 +++++++++++++++
>  app/test-pmd/config.c  | 112 +++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h |   2 +
>  3 files changed, 155 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index a037a55c6..84943a889 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -7594,6 +7594,46 @@ cmdline_parse_inst_t cmd_showdevice = {
>  		NULL,
>  	},
>  };
> +
> +/* ** SHOW EEPROM INFO *** */
> +struct cmd_showeeprom_result {
> +       cmdline_fixed_string_t show;
> +       cmdline_fixed_string_t type;
> +        uint16_t portnum;
> +};
> +
> +static void cmd_showeeprom_parsed(void *parsed_result,
> +               __rte_unused struct cmdline *cl,
> +               __rte_unused void *data)
> +{
> +       struct cmd_showeeprom_result *res = parsed_result;
> +
> +       if (!strcmp(res->type, "eeprom"))
> +               port_eeprom_display(res->portnum);
> +       else if (!strcmp(res->type, "module_eeprom"))
> +               port_module_eeprom_display(res->portnum);
> +}
> +
> +cmdline_parse_token_string_t cmd_showeeprom_show =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
> +cmdline_parse_token_string_t cmd_showeeprom_type =
> +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "eeprom#module_eeprom");
> +cmdline_parse_token_num_t cmd_showeeprom_portnum =
> +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
> +
> +cmdline_parse_inst_t cmd_showeeprom = {
> +       .f = cmd_showeeprom_parsed,
> +       .data = NULL,
> +       .help_str = "show eeprom|module_eeprom <port_id>",

There is more common syntax like:
"show port X <port_id>|all"
or
"show port <port_id> Y"

Keeping command syntax consistent helps using testpmd. What do you think
selecting one of above two?

Also can you please update 'cmd_help_long_parsed()' (in same file) and
documentation (testpmd_funcs.rst) for new command.

> +       .tokens = {
> +               (void *)&cmd_showeeprom_show,
> +               (void *)&cmd_showeeprom_type,
> +               (void *)&cmd_showeeprom_portnum,
> +               NULL,
> +       },
> +};
> +
> +
>  /* *** SHOW QUEUE INFO *** */
>  struct cmd_showqueue_result {
>  	cmdline_fixed_string_t show;
> @@ -19325,6 +19365,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_load_from_file,
>  	(cmdline_parse_inst_t *)&cmd_showport,
>  	(cmdline_parse_inst_t *)&cmd_showqueue,
> +        (cmdline_parse_inst_t *)&cmd_showeeprom,
>  	(cmdline_parse_inst_t *)&cmd_showportall,
>  	(cmdline_parse_inst_t *)&cmd_showdevice,
>  	(cmdline_parse_inst_t *)&cmd_showcfg,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 8cf84ccd3..79fd81a65 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -49,6 +49,7 @@
>  #include <rte_pmd_bnxt.h>
>  #endif
>  #include <rte_gro.h>
> +#include <rte_hexdump.h>
>  
>  #include "testpmd.h"
>  
> @@ -710,6 +711,117 @@ port_summary_display(portid_t port_id)
>  		(unsigned int) link.link_speed);
>  }
>  
> +void
> +port_eeprom_display(portid_t port_id)
> +{
> +       struct rte_dev_eeprom_info einfo;
> +       int ret;
> +
> +       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +               print_valid_ports();
> +               return;
> +       }
> +
> +       int len_eeprom = rte_eth_dev_get_eeprom_length(port_id); 
> +       char buf[len_eeprom];

If 'len_eeprom' can be negative what will be the 'buf' size?

> +
> +       if (len_eeprom > 0) {

To reduce the code indentation, you can eliminate this if block by returning on
error cases. (like as done in below 'port_module_eeprom_display()')

> +               printf("\nPort: %d\nModule EEPROM:\n", port_id);
> +
> +               einfo.offset = 0;
> +               einfo.length = len_eeprom;
> +               einfo.data = buf;
> +
> +               ret = rte_eth_dev_get_eeprom(port_id, &einfo);
> +               if (ret != 0) {
> +                       switch (ret) {
> +                       case -ENODEV:
> +                               printf("port index %d invalid\n", port_id);
> +                               break;
> +                       case -ENOTSUP:
> +                               printf("operation not supported by device\n");
> +                               break;
> +                       case -EIO:
> +                               printf("device Iis removed\n");

there is a typo before 'is' (and in same messages below)

> +                               break;
> +                       default:
> +                               printf("Unable to get module EEPROM: %d\n", ret);
> +                               break;
> +                       }
> +                       return;
> +               }
> +
> +               rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> +               printf("Finish -- Total EEPROM length: %i bytes\n", len_eeprom);
> +
> +       } else if (len_eeprom == 0)
> +               printf("Port %d: Device does not have EEPROM\n", port_id);
> +       else if (len_eeprom == -ENOTSUP)
> +               printf("Port %d: Operation not supported\n", port_id);
> +       else
> +               printf("Port %d: Error getting EEPROM\n", port_id);
> +}
> +
> +void
> +port_module_eeprom_display(portid_t port_id)
> +{
> +       struct rte_eth_dev_module_info minfo;
> +       struct rte_dev_eeprom_info einfo;
> +       char buf[1024];

Size can exceed the '1024'

> +       int ret;
> +
> +
> +       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +               print_valid_ports();
> +               return;
> +       }
> +
> +       ret = rte_eth_dev_get_module_info(port_id, &minfo);
> +       if (ret != 0) {
> +               switch (ret) {
> +               case -ENODEV:
> +                       printf("port index %d invalid\n", port_id);
> +                       break;
> +               case -ENOTSUP:
> +                       printf("operation not supported by device\n");
> +                       break;
> +               case -EIO:
> +                       printf("device Iis removed\n");
> +                       break;
> +               default:
> +                       printf("Unable to get module info: %d\n", ret);
> +                       break;
> +               }
> +               return;
> +       }
> +
> +       einfo.offset = 0;
> +       einfo.length = minfo.eeprom_len;
> +       einfo.data = buf;
> +
> +       ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> +       if (ret != 0) {
> +               switch (ret) {
> +               case -ENODEV:
> +                       printf("port index %d invalid\n", port_id);
> +                       break;
> +               case -ENOTSUP:
> +                       printf("operation not supported by device\n");
> +                       break;
> +               case -EIO:
> +                       printf("device Iis removed\n");
> +                       break;
> +               default:
> +                       printf("Unable to get module EEPROM: %d\n", ret);
> +                       break;
> +               }
> +               return;
> +       }
> +
> +       printf("\nPort: %d\nEEPROM:\n", port_id);
> +       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> +}
> +
>  void
>  port_offload_cap_display(portid_t port_id)
>  {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 7a7c73f79..e3a0d17e5 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
>  void device_infos_display(const char *identifier);
>  void port_infos_display(portid_t port_id);
>  void port_summary_display(portid_t port_id);
> +void port_eeprom_display(portid_t port_id);
> +void port_module_eeprom_display(portid_t port_id);
>  void port_summary_header_display(void);
>  void port_offload_cap_display(portid_t port_id);
>  void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev]  [PATCH V1] testpmd: add eeprom/module eeprom display
@ 2020-08-26 19:00 David Liu
  2020-08-26 22:46 ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: David Liu @ 2020-08-26 19:00 UTC (permalink / raw)
  To: dev; +Cc: lylavoie, dliu, tmonjalon

Commands will dump the content of the EEPROM/module EEPROM for the
selected port.

Signed-off-by: David Liu <dliu@iol.unh.edu>
---
 app/test-pmd/cmdline.c |  41 +++++++++++++++
 app/test-pmd/config.c  | 112 +++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |   2 +
 3 files changed, 155 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a037a55c6..84943a889 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7594,6 +7594,46 @@ cmdline_parse_inst_t cmd_showdevice = {
 		NULL,
 	},
 };
+
+/* ** SHOW EEPROM INFO *** */
+struct cmd_showeeprom_result {
+       cmdline_fixed_string_t show;
+       cmdline_fixed_string_t type;
+        uint16_t portnum;
+};
+
+static void cmd_showeeprom_parsed(void *parsed_result,
+               __rte_unused struct cmdline *cl,
+               __rte_unused void *data)
+{
+       struct cmd_showeeprom_result *res = parsed_result;
+
+       if (!strcmp(res->type, "eeprom"))
+               port_eeprom_display(res->portnum);
+       else if (!strcmp(res->type, "module_eeprom"))
+               port_module_eeprom_display(res->portnum);
+}
+
+cmdline_parse_token_string_t cmd_showeeprom_show =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, show, "show");
+cmdline_parse_token_string_t cmd_showeeprom_type =
+       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type, "eeprom#module_eeprom");
+cmdline_parse_token_num_t cmd_showeeprom_portnum =
+       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
+
+cmdline_parse_inst_t cmd_showeeprom = {
+       .f = cmd_showeeprom_parsed,
+       .data = NULL,
+       .help_str = "show eeprom|module_eeprom <port_id>",
+       .tokens = {
+               (void *)&cmd_showeeprom_show,
+               (void *)&cmd_showeeprom_type,
+               (void *)&cmd_showeeprom_portnum,
+               NULL,
+       },
+};
+
+
 /* *** SHOW QUEUE INFO *** */
 struct cmd_showqueue_result {
 	cmdline_fixed_string_t show;
@@ -19325,6 +19365,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
+        (cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 8cf84ccd3..79fd81a65 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -49,6 +49,7 @@
 #include <rte_pmd_bnxt.h>
 #endif
 #include <rte_gro.h>
+#include <rte_hexdump.h>
 
 #include "testpmd.h"
 
@@ -710,6 +711,117 @@ port_summary_display(portid_t port_id)
 		(unsigned int) link.link_speed);
 }
 
+void
+port_eeprom_display(portid_t port_id)
+{
+       struct rte_dev_eeprom_info einfo;
+       int ret;
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       int len_eeprom = rte_eth_dev_get_eeprom_length(port_id); 
+       char buf[len_eeprom];
+
+       if (len_eeprom > 0) {
+               printf("\nPort: %d\nModule EEPROM:\n", port_id);
+
+               einfo.offset = 0;
+               einfo.length = len_eeprom;
+               einfo.data = buf;
+
+               ret = rte_eth_dev_get_eeprom(port_id, &einfo);
+               if (ret != 0) {
+                       switch (ret) {
+                       case -ENODEV:
+                               printf("port index %d invalid\n", port_id);
+                               break;
+                       case -ENOTSUP:
+                               printf("operation not supported by device\n");
+                               break;
+                       case -EIO:
+                               printf("device Iis removed\n");
+                               break;
+                       default:
+                               printf("Unable to get module EEPROM: %d\n", ret);
+                               break;
+                       }
+                       return;
+               }
+
+               rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+               printf("Finish -- Total EEPROM length: %i bytes\n", len_eeprom);
+
+       } else if (len_eeprom == 0)
+               printf("Port %d: Device does not have EEPROM\n", port_id);
+       else if (len_eeprom == -ENOTSUP)
+               printf("Port %d: Operation not supported\n", port_id);
+       else
+               printf("Port %d: Error getting EEPROM\n", port_id);
+}
+
+void
+port_module_eeprom_display(portid_t port_id)
+{
+       struct rte_eth_dev_module_info minfo;
+       struct rte_dev_eeprom_info einfo;
+       char buf[1024];
+       int ret;
+
+
+       if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+               print_valid_ports();
+               return;
+       }
+
+       ret = rte_eth_dev_get_module_info(port_id, &minfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module info: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       einfo.offset = 0;
+       einfo.length = minfo.eeprom_len;
+       einfo.data = buf;
+
+       ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+       if (ret != 0) {
+               switch (ret) {
+               case -ENODEV:
+                       printf("port index %d invalid\n", port_id);
+                       break;
+               case -ENOTSUP:
+                       printf("operation not supported by device\n");
+                       break;
+               case -EIO:
+                       printf("device Iis removed\n");
+                       break;
+               default:
+                       printf("Unable to get module EEPROM: %d\n", ret);
+                       break;
+               }
+               return;
+       }
+
+       printf("\nPort: %d\nEEPROM:\n", port_id);
+       rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+}
+
 void
 port_offload_cap_display(portid_t port_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7a7c73f79..e3a0d17e5 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -714,6 +714,8 @@ void nic_stats_mapping_display(portid_t port_id);
 void device_infos_display(const char *identifier);
 void port_infos_display(portid_t port_id);
 void port_summary_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
+void port_module_eeprom_display(portid_t port_id);
 void port_summary_header_display(void);
 void port_offload_cap_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10  6:00 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
2020-09-10 11:47 ` Ferruh Yigit
2020-09-10 18:48   ` David Liu
2020-09-14 15:11     ` [dpdk-dev] [PATCH V2] " David Liu
2020-09-14 16:48       ` Ferruh Yigit
2020-09-15 14:22         ` David Liu
2020-09-10 20:00 ` [dpdk-dev] [PATCH V1] " David Liu
2020-09-15 15:42   ` [dpdk-dev] [PATCH] [PATCH V2] " David Liu
2020-09-15 17:24     ` David Liu
2020-09-15 17:27     ` David Liu
2020-09-22  9:13       ` Phil Yang
2020-09-24 10:57         ` Phil Yang
2020-09-24 15:26           ` Ferruh Yigit
2020-09-10 20:12 ` [dpdk-dev] [PATCH V1 2/2] Fix coding style issues David Liu
  -- strict thread matches above, loose matches on Subject: below --
2020-09-01 19:07 [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display David Liu
2020-09-02 10:00 ` Ferruh Yigit
2020-09-03 16:40   ` David Liu
2020-09-03 18:40     ` Ferruh Yigit
2020-09-01 18:56 David Liu
2020-08-26 19:00 David Liu
2020-08-26 22:46 ` Ferruh Yigit
2020-08-28 13:46   ` David Liu

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox