[V1] testpmd: add eeprom/module eeprom display
Checks
Commit Message
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(+)
Comments
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);
>
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);
> >
>
>
@@ -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,
@@ -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)
{
@@ -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);