[v1] testpmd: eeprom display

  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(-)
  

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
>
  

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
  

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.
  

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.