From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 77A63A04B1; Fri, 28 Aug 2020 15:47:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7498F1C1C9; Fri, 28 Aug 2020 15:47:18 +0200 (CEST) Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by dpdk.org (Postfix) with ESMTP id 1DB291C1B4 for ; Fri, 28 Aug 2020 15:47:17 +0200 (CEST) Received: by mail-lj1-f195.google.com with SMTP id m22so1381267ljj.5 for ; Fri, 28 Aug 2020 06:47:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=a+21OjUyEx+fdx8DFfeq1J97Ef9FrM3qvNeA2FDBqaE=; b=U8FCqe1NOqBsTWjnrPucduFrD9F4rBBJUErSpmMuRErHn8coxYuPaS4v4T/dexCg6z B45qmlu3NPg/jsqSbS4JybBOGgvYfW8ixaOjLaaoQ6y6ZbIX8TEzjHzPtHoB32Rl4YKA 3ZwHH60HrJs0UZNNtvlDWbikgWHq5I8X9MKbQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=a+21OjUyEx+fdx8DFfeq1J97Ef9FrM3qvNeA2FDBqaE=; b=HrGF+6GR5yi+zZpBRmqFHAbUOb2rGpWhLuzZy8XuvUdYjpOzWFNMGY3sZJG6mjDDHq fYksHmVYkGG9bNNQi8HS0TLWwWizIOJRGjgZYAnQljC+rED6jJT7OzUKKHxuphnM20Am AzzXvBwdNsHuqrtDoFL8PnrIOX222zgG+7GcK0BPxuWGa00KdT1VyZYiB0c22cMdBk5H 5QimwfTwsp2VQ4U4B5/2qnklAOLvzZ9bMiVfwyXqfZN2y1ofIkO889TpZa0iu9gWv1Ah Bu6ENHzGhTCy36EzcHbyYwNieQ0EyXaF6Zlztsx/qwdZJuWLJ5QNc3zSXOffSejnLega EfXw== X-Gm-Message-State: AOAM5318QH5DAg29T5G6tpKNeJaRsa4bwBRH82924GAhLImH3GCAmUqw pKZ/S++krWnDzM7KuQtidFvyfGCV2p0L+Gc58YQfNA== X-Google-Smtp-Source: ABdhPJx8RDdWv4DdcIwNDJSaxpHskOHGU8Rq1A+YKa/1h0QbuJ0HRO9QRruhDrlmePPcw/oj6gMsgJPBTGLmc2eJJ1o= X-Received: by 2002:a2e:8689:: with SMTP id l9mr985425lji.467.1598622436209; Fri, 28 Aug 2020 06:47:16 -0700 (PDT) MIME-Version: 1.0 References: <20200826190019.29612-1-dliu@iol.unh.edu> <9f48e895-c90d-7cce-7431-ba1a85944162@intel.com> In-Reply-To: <9f48e895-c90d-7cce-7431-ba1a85944162@intel.com> From: David Liu Date: Fri, 28 Aug 2020 09:46:40 -0400 Message-ID: To: Ferruh Yigit Cc: dpdk-dev , Lincoln Lavoie , tmonjalon@nvidia.com Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" z On Wed, Aug 26, 2020 at 6:46 PM Ferruh Yigit 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 > > --- > > 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 ", > > There is more common syntax like: > "show port X |all" > or > "show port 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 > > #endif > > #include > > +#include > > > > #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); > > > >