DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: David Liu <dliu@iol.unh.edu>
Cc: dpdk-dev <dev@dpdk.org>, Lincoln Lavoie <lylavoie@iol.unh.edu>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH V1] testpmd: add eeprom/module eeprom display
Date: Thu, 3 Sep 2020 19:40:15 +0100	[thread overview]
Message-ID: <bdbc5745-7ca1-3d70-5181-3595eec1a909@intel.com> (raw)
In-Reply-To: <CAAuqQpQdyRPQL1psddrRO3fQD6iOiFnWZmGoebrRJpzrgWes4A@mail.gmail.com>

On 9/3/2020 5:40 PM, David Liu wrote:

> On Wed, Sep 2, 2020 at 6:00 AM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 9/1/2020 8:07 PM, David Liu wrote:
>     > Change display message.
>     > Add EEPROM dump command
>     >   "show port <port_id> eeprom"
>     > Add module EEPROM dump command
>     >      "show port <port_id> module_eeprom"
>     > Commands will dump the content of the
>     > EEPROM/module EEPROM for the selected port.
>     >
>     > Signed-off-by: David Liu <dliu@iol.unh.edu <mailto:dliu@iol.unh.edu>>
>     > ---
>     >  app/test-pmd/cmdline.c                      |  87 ++++++++++++++
>     >  app/test-pmd/config.c                       | 126 ++++++++++++++++++++
>     >  app/test-pmd/testpmd.h                      |   2 +
>     >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
>     >  4 files changed, 229 insertions(+)
>     >
>     > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>     > index a037a55c6..71c98dd96 100644
>     > --- a/app/test-pmd/cmdline.c
>     > +++ b/app/test-pmd/cmdline.c
>     > @@ -166,6 +166,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>     >                       "show port
>     (info|stats|summary|xstats|fdir|stat_qmap|dcb_tc|cap) (port_id|all)\n"
>     >                       "    Display information for port_id, or all.\n\n"
>     > 
>     > +                     "show port (port_id) eeprom \n"
>     > +                     "    Display the EEPROM infomation for given
>     port_id.\n\n"
>     > +
>     > +                     "show port (port_id) module_eeprom \n"
>     > +                     "    Display the port moudle EEPROM infomation for
>     given 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"
>     > @@ -7594,6 +7600,85 @@ cmdline_parse_inst_t cmd_showdevice = {
>     >               NULL,
>     >       },
>     >  };
>     > +
>     > +/* ** SHOW EEPROM INFO *** */
>     > +struct cmd_showeeprom_result {
>     > +       cmdline_fixed_string_t show;
>     > +       cmdline_fixed_string_t port;
>     > +       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;
>     > +
>     > +       port_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_port =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, port, "port");
>     > +cmdline_parse_token_num_t cmd_showeeprom_portnum =
>     > +       TOKEN_NUM_INITIALIZER(struct cmd_showeeprom_result, portnum, UINT16);
>     > +cmdline_parse_token_string_t cmd_showeeprom_type =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showeeprom_result, type,
>     "eeprom");
>     > +
>     > +cmdline_parse_inst_t cmd_showeeprom = {
>     > +       .f = cmd_showeeprom_parsed,
>     > +       .data = NULL,
>     > +       .help_str = "show port <port_id> eeprom",
>     > +       .tokens = {
>     > +               (void *)&cmd_showeeprom_show,
>     > +               (void *)&cmd_showeeprom_port,
>     > +               (void *)&cmd_showeeprom_portnum,
>     > +               (void *)&cmd_showeeprom_type,
>     > +               NULL,
>     > +       },
>     > +};
>     > +
>     > +/* ** SHOW MODULE EEPROM INFO *** */
>     > +struct cmd_showmoduleeeprom_result {
>     > +       cmdline_fixed_string_t show;
>     > +       cmdline_fixed_string_t port;
>     > +       cmdline_fixed_string_t type;
>     > +        uint16_t portnum;
>     > +};
>     > +
>     > +static void cmd_showmoduleeeprom_parsed(void *parsed_result,
>     > +               __rte_unused struct cmdline *cl,
>     > +               __rte_unused void *data)
>     > +{
>     > +       struct cmd_showmoduleeeprom_result *res = parsed_result;
>     > +
>     > +       port_module_eeprom_display(res->portnum);
>     > +}
>     > +
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_show =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, show,
>     "show");
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_port =
>     > +       TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, port,
>     "port");
>     > +cmdline_parse_token_num_t cmd_showmoduleeeprom_portnum =
>     > +     TOKEN_NUM_INITIALIZER(struct cmd_showmoduleeeprom_result, portnum,
>     UINT16);
>     > +cmdline_parse_token_string_t cmd_showmoduleeeprom_type =
>     > +     TOKEN_STRING_INITIALIZER(struct cmd_showmoduleeeprom_result, type,
>     "module_eeprom");
>     > +
>     > +cmdline_parse_inst_t cmd_showmoduleeeprom = {
>     > +     .f = cmd_showmoduleeeprom_parsed,
>     > +     .data = NULL,
>     > +     .help_str = "show port <port_id> module_eeprom",
>     > +     .tokens = {
>     > +             (void *)&cmd_showmoduleeeprom_show,
>     > +             (void *)&cmd_showmoduleeeprom_port,
>     > +             (void *)&cmd_showmoduleeeprom_portnum,
>     > +             (void *)&cmd_showmoduleeeprom_type,
>     > +             NULL,
>     > +     },
>     > +};> +
> 
>     Since both commands are simple and related, what do you think merging their
>     implementation? This reduces the clutter.
>     Please check '#' usage in the 'TOKEN_STRING_INITIALIZER', and "port
>     start|stop|close all" implementation can be sample.
> 
> Yes, that sounds good. What do you think of having the command to be "show port
> (port_id|all) (module_eeprom|eeprom)"?
>

(Please don't top post, bad for archiving, copy/pasted from top of the email)

+1 to "(module_eeprom|eeprom)" part.

Not sure about "(port_id|all)", since this will hexdump the eeprom, I assume the
output will be long and not sure if there will be need to dump all ports at once.

So I suggest start with only "port_id" and "all" support later if there is a
need, unless you already need it.

  reply	other threads:[~2020-09-03 18:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 19:07 David Liu
2020-09-02 10:00 ` Ferruh Yigit
2020-09-03 16:40   ` David Liu
2020-09-03 18:40     ` Ferruh Yigit [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-09-10  6:00 David Liu
2020-09-10 11:47 ` Ferruh Yigit
2020-09-10 18:48   ` David Liu
2020-09-10 20:00 ` David Liu
2020-09-01 18:56 David Liu
2020-08-26 19:00 David Liu
2020-08-26 22:46 ` Ferruh Yigit
2020-08-28 13:46   ` David Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bdbc5745-7ca1-3d70-5181-3595eec1a909@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=dliu@iol.unh.edu \
    --cc=lylavoie@iol.unh.edu \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).