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 9581CA04B1; Thu, 27 Aug 2020 00:46:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A14192AB; Thu, 27 Aug 2020 00:46:24 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 050C9A3 for ; Thu, 27 Aug 2020 00:46:21 +0200 (CEST) IronPort-SDR: KyyKAmHGXnB3nqCjxnZ+z353P/MHeFfkWPFaSJ4aQdOgQgp6h8ZjH7ZqsA4rHSkmGhAG1WSIuD /VchZgDKwMfQ== X-IronPort-AV: E=McAfee;i="6000,8403,9725"; a="144070400" X-IronPort-AV: E=Sophos;i="5.76,357,1592895600"; d="scan'208";a="144070400" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Aug 2020 15:46:21 -0700 IronPort-SDR: X6bZfd8Cfd9pmaw78hiiojKupqcFJb5mPae4KQxIbTQAZAVZnX7PetCfKGZ/uPEKXFGnynAtHc jwlRbK9YtB/A== X-IronPort-AV: E=Sophos;i="5.76,357,1592895600"; d="scan'208";a="474949518" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.213.201.139]) ([10.213.201.139]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Aug 2020 15:46:19 -0700 To: David Liu , dev@dpdk.org Cc: lylavoie@iol.unh.edu, tmonjalon@nvidia.com References: <20200826190019.29612-1-dliu@iol.unh.edu> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJsBBMBCgBWAhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEABQkKqZZ8FiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl6ha3sXGHZrczovL2tl eXMub3BlbnBncC5vcmcACgkQ+TPrQ98TYR8uLA//QwltuFliUWe60xwmu9sY38c1DXvX67wk UryQ1WijVdIoj4H8cf/s2KtyIBjc89R254KMEfJDao/LrXqJ69KyGKXFhFPlF3VmFLsN4XiT PSfxkx8s6kHVaB3O183p4xAqnnl/ql8nJ5ph9HuwdL8CyO5/7dC/MjZ/mc4NGq5O9zk3YRGO lvdZAp5HW9VKW4iynvy7rl3tKyEqaAE62MbGyfJDH3C/nV/4+mPc8Av5rRH2hV+DBQourwuC ci6noiDP6GCNQqTh1FHYvXaN4GPMHD9DX6LtT8Fc5mL/V9i9kEVikPohlI0WJqhE+vQHFzR2 1q5nznE+pweYsBi3LXIMYpmha9oJh03dJOdKAEhkfBr6n8BWkWQMMiwfdzg20JX0o7a/iF8H 4dshBs+dXdIKzPfJhMjHxLDFNPNH8zRQkB02JceY9ESEah3wAbzTwz+e/9qQ5OyDTQjKkVOo cxC2U7CqeNt0JZi0tmuzIWrfxjAUulVhBmnceqyMOzGpSCQIkvalb6+eXsC9V1DZ4zsHZ2Mx Hi+7pCksdraXUhKdg5bOVCt8XFmx1MX4AoV3GWy6mZ4eMMvJN2hjXcrreQgG25BdCdcxKgqp e9cMbCtF+RZax8U6LkAWueJJ1QXrav1Jk5SnG8/5xANQoBQKGz+yFiWcgEs9Tpxth15o2v59 gXK5Ag0EV9ZMvgEQAKc0Db17xNqtSwEvmfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ES YpV8QWj0xK4YM0dLxnDU2IYxjEshSB1TqAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4Ai bPtrHuIXWQOBECcVZTTOdZYGAzaYzxiAONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxD UQljeNvKYt1lZE/gAUUxNLWsYyTT+22/vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/ 3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35piVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVj sM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQI3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdc q9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYHfVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH7 1PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZqw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFB VOQOxCvwRG2QCgcJ/UTn5vlivul+cThi6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI 8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJlRr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYC GwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNhHwUCXqFrngUJCKxSYAAKCRD5M+tD3xNhH3YWD/9b cUiWaHJasX+OpiuZ1Li5GG3m9aw4lR/k2lET0UPRer2Jy1JsL+uqzdkxGvPqzFTBXgx/6Byz EMa2mt6R9BCyR286s3lxVS5Bgr5JGB3EkpPcoJT3A7QOYMV95jBiiJTy78Qdzi5LrIu4tW6H o0MWUjpjdbR01cnj6EagKrDx9kAsqQTfvz4ff5JIFyKSKEHQMaz1YGHyCWhsTwqONhs0G7V2 0taQS1bGiaWND0dIBJ/u0pU998XZhmMzn765H+/MqXsyDXwoHv1rcaX/kcZIcN3sLUVcbdxA WHXOktGTQemQfEpCNuf2jeeJlp8sHmAQmV3dLS1R49h0q7hH4qOPEIvXjQebJGs5W7s2vxbA 5u5nLujmMkkfg1XHsds0u7Zdp2n200VC4GQf8vsUp6CSMgjedHeF9zKv1W4lYXpHp576ZV7T GgsEsvveAE1xvHnpV9d7ZehPuZfYlP4qgo2iutA1c0AXZLn5LPcDBgZ+KQZTzm05RU1gkx7n gL9CdTzVrYFy7Y5R+TrE9HFUnsaXaGsJwOB/emByGPQEKrupz8CZFi9pkqPuAPwjN6Wonokv ChAewHXPUadcJmCTj78Oeg9uXR6yjpxyFjx3vdijQIYgi5TEGpeTQBymLANOYxYWYOjXk+ae dYuOYKR9nbPv+2zK9pwwQ2NXbUBystaGyQ== Message-ID: <9f48e895-c90d-7cce-7431-ba1a85944162@intel.com> Date: Wed, 26 Aug 2020 23:46:15 +0100 MIME-Version: 1.0 In-Reply-To: <20200826190019.29612-1-dliu@iol.unh.edu> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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. > + .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? > + > + 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); >