[dpdk-dev] [PATCH v1] testpmd: eeprom display

[dpdk-dev] [PATCH v1] testpmd: eeprom display

[Gaetan Rivet]

The interactive command

  show port eeprom <id>

will dump the content of the EEPROM for the selected port.
Dumping eeprom of all ports at once is not supported.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 app/test-pmd/cmdline.c |  9 +++++++--
 app/test-pmd/config.c  | 32 ++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a7c0e622a..0801e7e74 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -170,6 +170,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port eeprom (port_id)\n"
+			"    Display port EEPROM 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"
@@ -7202,6 +7205,8 @@ static void cmd_showport_parsed(void *parsed_result,
 		port_dcb_info_display(res->portnum);
 	else if (!strcmp(res->what, "cap"))
 		port_offload_cap_display(res->portnum);
+	else if (!strcmp(res->what, "eeprom"))
+		port_eeprom_display(res->portnum);
 }
 
 cmdline_parse_token_string_t cmd_showport_show =
@@ -7211,7 +7216,7 @@ cmdline_parse_token_string_t cmd_showport_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, port, "port");
 cmdline_parse_token_string_t cmd_showport_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
-				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap");
+				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap#eeprom");
 cmdline_parse_token_num_t cmd_showport_portnum =
 	TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, UINT16);
 
@@ -7219,7 +7224,7 @@ cmdline_parse_inst_t cmd_showport = {
 	.f = cmd_showport_parsed,
 	.data = NULL,
 	.help_str = "show|clear port "
-		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap "
+		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap|eeprom "
 		"<port_id>",
 	.tokens = {
 		(void *)&cmd_showport_show,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 14ccd6864..af1a7d37a 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 <cmdline_parse_etheraddr.h>
 
 #include "testpmd.h"
@@ -739,6 +740,37 @@ port_offload_cap_display(portid_t port_id)
 	}
 }
 
+void
+port_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 == (portid_t)RTE_PORT_ALL)
+		return;
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret) {
+		printf("Unable to get module info: %d\n", ret);
+		return;
+	}
+
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret) {
+		printf("Unable to get module EEPROM: %d\n", ret);
+		return;
+	}
+
+	printf("Port %hhu EEPROM:\n", port_id);
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+}
+
 int
 port_id_is_invalid(portid_t port_id, enum print_warning warning)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a1f661472..bf817bdcf 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -595,6 +595,7 @@ void nic_xstats_clear(portid_t port_id);
 void nic_stats_mapping_display(portid_t port_id);
 void port_infos_display(portid_t port_id);
 void port_offload_cap_display(portid_t port_id);
+void port_eeprom_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
 void tx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
 void fwd_lcores_config_display(void);
-- 
2.18.0

Re: [dpdk-dev] [PATCH v1] testpmd: eeprom display

[Gaëtan Rivet]

Hi,

I am looking at support on e1000/IGB ports, and I have used this
command to verify support. It works for fiber but not for copper.

I can see in the get_module_info callback that copper SFPs are
not supported. Same thing as the kernel driver.

Is it possible to use MDIO for dumping the SFP EEPROM?
Alternatively, is I2C available in MGII mode with copper?

Thanks,

On Tue, Sep 18, 2018 at 10:59:46AM +0200, Gaetan Rivet wrote:
> The interactive command
> 
>   show port eeprom <id>
> 
> will dump the content of the EEPROM for the selected port.
> Dumping eeprom of all ports at once is not supported.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  app/test-pmd/cmdline.c |  9 +++++++--
>  app/test-pmd/config.c  | 32 ++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h |  1 +
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index a7c0e622a..0801e7e74 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -170,6 +170,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"show port (info|stats|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
>  			"    Display information for port_id, or all.\n\n"
>  
> +			"show port eeprom (port_id)\n"
> +			"    Display port EEPROM 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"
> @@ -7202,6 +7205,8 @@ static void cmd_showport_parsed(void *parsed_result,
>  		port_dcb_info_display(res->portnum);
>  	else if (!strcmp(res->what, "cap"))
>  		port_offload_cap_display(res->portnum);
> +	else if (!strcmp(res->what, "eeprom"))
> +		port_eeprom_display(res->portnum);
>  }
>  
>  cmdline_parse_token_string_t cmd_showport_show =
> @@ -7211,7 +7216,7 @@ cmdline_parse_token_string_t cmd_showport_port =
>  	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, port, "port");
>  cmdline_parse_token_string_t cmd_showport_what =
>  	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
> -				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap");
> +				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap#eeprom");
>  cmdline_parse_token_num_t cmd_showport_portnum =
>  	TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, UINT16);
>  
> @@ -7219,7 +7224,7 @@ cmdline_parse_inst_t cmd_showport = {
>  	.f = cmd_showport_parsed,
>  	.data = NULL,
>  	.help_str = "show|clear port "
> -		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap "
> +		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap|eeprom "
>  		"<port_id>",
>  	.tokens = {
>  		(void *)&cmd_showport_show,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 14ccd6864..af1a7d37a 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 <cmdline_parse_etheraddr.h>
>  
>  #include "testpmd.h"
> @@ -739,6 +740,37 @@ port_offload_cap_display(portid_t port_id)
>  	}
>  }
>  
> +void
> +port_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 == (portid_t)RTE_PORT_ALL)
> +		return;
> +
> +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
> +	if (ret) {
> +		printf("Unable to get module info: %d\n", ret);
> +		return;
> +	}
> +
> +	einfo.offset = 0;
> +	einfo.length = minfo.eeprom_len;
> +	einfo.data = buf;
> +
> +	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> +	if (ret) {
> +		printf("Unable to get module EEPROM: %d\n", ret);
> +		return;
> +	}
> +
> +	printf("Port %hhu EEPROM:\n", port_id);
> +	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> +}
> +
>  int
>  port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index a1f661472..bf817bdcf 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -595,6 +595,7 @@ void nic_xstats_clear(portid_t port_id);
>  void nic_stats_mapping_display(portid_t port_id);
>  void port_infos_display(portid_t port_id);
>  void port_offload_cap_display(portid_t port_id);
> +void port_eeprom_display(portid_t port_id);
>  void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
>  void tx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
>  void fwd_lcores_config_display(void);
> -- 
> 2.18.0
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v1] testpmd: eeprom display

[Ferruh Yigit]

On 9/18/2018 9:59 AM, Gaetan Rivet wrote:
> The interactive command
> 
>   show port eeprom <id>
> 
> will dump the content of the EEPROM for the selected port.
> Dumping eeprom of all ports at once is not supported.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

<...>

> +void
> +port_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 == (portid_t)RTE_PORT_ALL)
> +		return;
> +
> +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
> +	if (ret) {
> +		printf("Unable to get module info: %d\n", ret);
> +		return;
> +	}
> +
> +	einfo.offset = 0;
> +	einfo.length = minfo.eeprom_len;
> +	einfo.data = buf;
> +
> +	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> +	if (ret) {
> +		printf("Unable to get module EEPROM: %d\n", ret);
> +		return;
> +	}
> +
> +	printf("Port %hhu EEPROM:\n", port_id);

Causing build error [1], there are various formatting used for printing port_id
[2], do we need this %hhu accuracy, I am for %u since port_id is an unsigned
value result should be same.

[1]
        printf("Port %hhu EEPROM:\n", port_id);
                     ~~~~             ^~~~~~~
                     %hu

[2]
%d, %u, %PRIu8 [wrong], %PRIu16

Re: [dpdk-dev] [PATCH v1] testpmd: eeprom display

[Gaëtan Rivet]

On Fri, Sep 21, 2018 at 04:41:10PM +0100, Ferruh Yigit wrote:
> On 9/18/2018 9:59 AM, Gaetan Rivet wrote:
> > The interactive command
> > 
> >   show port eeprom <id>
> > 
> > will dump the content of the EEPROM for the selected port.
> > Dumping eeprom of all ports at once is not supported.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> 
> <...>
> 
> > +void
> > +port_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 == (portid_t)RTE_PORT_ALL)
> > +		return;
> > +
> > +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
> > +	if (ret) {
> > +		printf("Unable to get module info: %d\n", ret);
> > +		return;
> > +	}
> > +
> > +	einfo.offset = 0;
> > +	einfo.length = minfo.eeprom_len;
> > +	einfo.data = buf;
> > +
> > +	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> > +	if (ret) {
> > +		printf("Unable to get module EEPROM: %d\n", ret);
> > +		return;
> > +	}
> > +
> > +	printf("Port %hhu EEPROM:\n", port_id);
> 
> Causing build error [1], there are various formatting used for printing port_id
> [2], do we need this %hhu accuracy, I am for %u since port_id is an unsigned
> value result should be same.
> 
> [1]
>         printf("Port %hhu EEPROM:\n", port_id);
>                      ~~~~             ^~~~~~~
>                      %hu
> 
> [2]
> %d, %u, %PRIu8 [wrong], %PRIu16

You're right, no need for %hhu.
I'd prefer myself using PRIu8 only by principle, but I think consistency
is better, and testpmd uses %u more often.

On another note, I think this command was misnamed anyway.

> show port sfp_eeprom 0

is more correct, because we won't get the actual port EEPROM.
I will send a v2, thanks for reading Ferruh.

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v1] testpmd: eeprom display

[Ferruh Yigit]

On 9/21/2018 5:13 PM, Gaëtan Rivet wrote:
> On Fri, Sep 21, 2018 at 04:41:10PM +0100, Ferruh Yigit wrote:
>> On 9/18/2018 9:59 AM, Gaetan Rivet wrote:
>>> The interactive command
>>>
>>>   show port eeprom <id>
>>>
>>> will dump the content of the EEPROM for the selected port.
>>> Dumping eeprom of all ports at once is not supported.
>>>
>>> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>>
>> <...>
>>
>>> +void
>>> +port_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 == (portid_t)RTE_PORT_ALL)
>>> +		return;
>>> +
>>> +	ret = rte_eth_dev_get_module_info(port_id, &minfo);
>>> +	if (ret) {
>>> +		printf("Unable to get module info: %d\n", ret);
>>> +		return;
>>> +	}
>>> +
>>> +	einfo.offset = 0;
>>> +	einfo.length = minfo.eeprom_len;
>>> +	einfo.data = buf;
>>> +
>>> +	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
>>> +	if (ret) {
>>> +		printf("Unable to get module EEPROM: %d\n", ret);
>>> +		return;
>>> +	}
>>> +
>>> +	printf("Port %hhu EEPROM:\n", port_id);
>>
>> Causing build error [1], there are various formatting used for printing port_id
>> [2], do we need this %hhu accuracy, I am for %u since port_id is an unsigned
>> value result should be same.
>>
>> [1]
>>         printf("Port %hhu EEPROM:\n", port_id);
>>                      ~~~~             ^~~~~~~
>>                      %hu
>>
>> [2]
>> %d, %u, %PRIu8 [wrong], %PRIu16
> 
> You're right, no need for %hhu.
> I'd prefer myself using PRIu8 only by principle, but I think consistency
> is better, and testpmd uses %u more often.
> 
> On another note, I think this command was misnamed anyway.
> 
>> show port sfp_eeprom 0
> 
> is more correct, because we won't get the actual port EEPROM.
> I will send a v2, thanks for reading Ferruh.

Ok, thanks.

[dpdk-dev] [PATCH v2] testpmd: sfp eeprom display

[Gaetan Rivet]

The interactive command

  show port sfp_eeprom <id>

will dump the content of the SFP EEPROM for the selected port.
Dumping SFP eeprom of all ports at once is not supported.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 app/test-pmd/cmdline.c |  9 +++++++--
 app/test-pmd/config.c  | 32 ++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0c5399dc4..cc3dd1373 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -170,6 +170,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port sfp_eeprom (port_id)\n"
+			"    Display SFP EEPROM 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"
@@ -7237,6 +7240,8 @@ static void cmd_showport_parsed(void *parsed_result,
 		port_dcb_info_display(res->portnum);
 	else if (!strcmp(res->what, "cap"))
 		port_offload_cap_display(res->portnum);
+	else if (!strcmp(res->what, "sfp_eeprom"))
+		port_sfp_eeprom_display(res->portnum);
 }
 
 cmdline_parse_token_string_t cmd_showport_show =
@@ -7246,7 +7251,7 @@ cmdline_parse_token_string_t cmd_showport_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, port, "port");
 cmdline_parse_token_string_t cmd_showport_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
-				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap");
+				 "info#stats#xstats#fdir#stat_qmap#dcb_tc#cap#sfp_eeprom");
 cmdline_parse_token_num_t cmd_showport_portnum =
 	TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, UINT16);
 
@@ -7254,7 +7259,7 @@ cmdline_parse_inst_t cmd_showport = {
 	.f = cmd_showport_parsed,
 	.data = NULL,
 	.help_str = "show|clear port "
-		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap "
+		"info|stats|xstats|fdir|stat_qmap|dcb_tc|cap|sfp_eeprom "
 		"<port_id>",
 	.tokens = {
 		(void *)&cmd_showport_show,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 794aa5268..24c4bbcad 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 <cmdline_parse_etheraddr.h>
 
 #include "testpmd.h"
@@ -757,6 +758,37 @@ port_offload_cap_display(portid_t port_id)
 	}
 }
 
+void
+port_sfp_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 == (portid_t)RTE_PORT_ALL)
+		return;
+
+	ret = rte_eth_dev_get_module_info(port_id, &minfo);
+	if (ret) {
+		printf("Unable to get module info: %d\n", ret);
+		return;
+	}
+
+	einfo.offset = 0;
+	einfo.length = minfo.eeprom_len;
+	einfo.data = buf;
+
+	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
+	if (ret) {
+		printf("Unable to get module EEPROM: %d\n", ret);
+		return;
+	}
+
+	printf("Port %u SFP EEPROM:\n", port_id);
+	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
+}
+
 int
 port_id_is_invalid(portid_t port_id, enum print_warning warning)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a1f661472..edfee29bf 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -595,6 +595,7 @@ void nic_xstats_clear(portid_t port_id);
 void nic_stats_mapping_display(portid_t port_id);
 void port_infos_display(portid_t port_id);
 void port_offload_cap_display(portid_t port_id);
+void port_sfp_eeprom_display(portid_t port_id);
 void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
 void tx_queue_infos_display(portid_t port_idi, uint16_t queue_id);
 void fwd_lcores_config_display(void);
-- 
2.19.0

Re: [dpdk-dev] [PATCH v2] testpmd: sfp eeprom display

[Iremonger, Bernard]

Hi Gaetan

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaetan Rivet
> Sent: Friday, October 5, 2018 10:59 PM
> To: dev@dpdk.org
> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: [dpdk-dev] [PATCH v2] testpmd: sfp eeprom display

./devtools/check-git-log.sh 
Wrong headline format:
        myscripts: add setup_dpdk.sh
Wrong headline label:
        testpmd: sfp eeprom display
Wrong headline lowercase:
        testpmd: sfp eeprom display

> 
> The interactive command
> 
>   show port sfp_eeprom <id>
> 
> will dump the content of the SFP EEPROM for the selected port.
> Dumping SFP eeprom of all ports at once is not supported.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  app/test-pmd/cmdline.c |  9 +++++++--
>  app/test-pmd/config.c  | 32 ++++++++++++++++++++++++++++++++  app/test-
> pmd/testpmd.h |  1 +

There should also be an entry in the following file for this command:
dpdk/doc/guides/testpmd_app_ug/testpmd_funcs.rst

Should there be an entry in the release notes to announce this new testpmd command?


>  3 files changed, 40 insertions(+), 2 deletions(-)
> 

<snip>

Regards,

Bernard.