> On 2024/9/12 14:54, Chaoyong He wrote:
> > From: James Hershaw <james.hers...@corigine.com>
> >
> > Add command to change the state of a controllable LED on an ethdev
> > port to on/off. This is for the purpose of identifying which physical
> > port is associated with an ethdev.
> >
> > usage:
> > testpmd> set port <port-id> led <on/off>
> >
> > Signed-off-by: James Hershaw <james.hers...@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong...@corigine.com>
> > ---
> > app/test-pmd/cmdline.c | 61 +++++++++++++++++++++
> > app/test-pmd/config.c | 19 +++++++
> > app/test-pmd/testpmd.h | 1 +
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
> > 4 files changed, 88 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > ebc5a9a361..3898c125c2 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -518,6 +518,10 @@ static void cmd_help_long_parsed(void
> *parsed_result,
> > " value (value) offset (offset)/n"
> > " Set the device eeprom for certain port.\n\n"
> >
> > + "set port (port_id) led (on|off)\n"
> > + " set a controllable LED associated with a
> > certain\n"
> > + " port on or off.\n\n"
> > +
> > "set port (port_id) uta (mac_address|all) (on|off)\n"
> > " Add/Remove a or all unicast hash filter(s)"
> > "from port X.\n\n"
> > @@ -7589,6 +7593,62 @@ static cmdline_parse_inst_t cmd_seteeprom = {
> > (void *)&cmd_seteeprom_magic,
> > (void *)&cmd_seteeprom_value,
> > (void *)&cmd_seteeprom_token_str,
> > + NULL,
> > + },
> > +};
> > +
> > +/* *** SET THE LED FOR CERTAIN PORT TO ON/OFF *** */ struct
> > +cmd_dev_led_result {
> > + cmdline_fixed_string_t set;
> > + cmdline_fixed_string_t port;
> > + portid_t port_id;
> > + cmdline_fixed_string_t led;
> > + cmdline_fixed_string_t led_state;
>
> led_state -> state, led_ is redundant.
>
> > +};
> > +
> > +static void cmd_set_dev_led_parsed(void *parsed_result,
> > + __rte_unused struct cmdline *cl,
> > + __rte_unused void *data) {
> > + struct cmd_dev_led_result *res = parsed_result;
> > +
> > + if (test_done == 0) {
> > + fprintf(stderr, "Please stop forwarding first\n");
> > + return;
> > + }
>
> From my understanding, there is a priority:
> 1\ if led on, then both led are on
> 2\ if led off, then led was lighted by current link and active flow.
>
> So there is no need to stop active flow when execute this command.
>
> > +
> > + if (strcmp(res->led, "led") != 0)
> > + return;
> > +
> > + if (strcmp(res->led_state, "on") == 0)
> > + set_dev_led(res->port_id, true);
> > + else if (strcmp(res->led_state, "off") == 0)
> > + set_dev_led(res->port_id, false);
> > + else
> > + fprintf(stderr, "Invalid option for LED
> > +state\n"); }
> > +
> > +static cmdline_parse_token_string_t cmd_dev_led_set =
> > + TOKEN_STRING_INITIALIZER(struct cmd_dev_led_result, set, "set");
> > +static cmdline_parse_token_string_t cmd_dev_led_port =
> > + TOKEN_STRING_INITIALIZER(struct cmd_dev_led_result, port,
> > +"port"); static cmdline_parse_token_num_t cmd_dev_led_port_id =
> > + TOKEN_NUM_INITIALIZER(struct cmd_dev_led_result, port_id,
> > +RTE_UINT16); static cmdline_parse_token_string_t cmd_dev_led =
> > + TOKEN_STRING_INITIALIZER(struct cmd_dev_led_result, led, "led");
> > +static cmdline_parse_token_string_t cmd_dev_led_state =
> > + TOKEN_STRING_INITIALIZER(struct cmd_dev_led_result, led_state,
> > +NULL);
>
> Suggest use tool parse capa, use TOKEN_STRING_INITIALIZER(struct
> cmd_dev_led_result, led_state, "on|off");
>
> then could only judge whether is on, like:
>
> if (strcmp(res->led_state, "on") == 0)
> set_dev_led(res->port_id, true);
> else
> set_dev_led(res->port_id, false);
>
>
> > +
> > +static cmdline_parse_inst_t cmd_set_dev_led = {
> > + .f = cmd_set_dev_led_parsed,
> > + .data = NULL,
> > + .help_str = "set port <port_id> led <on/off>",
> > + .tokens = {
> > + (void *)&cmd_dev_led_set,
> > + (void *)&cmd_dev_led_port,
> > + (void *)&cmd_dev_led_port_id,
> > + (void *)&cmd_dev_led,
> > + (void *)&cmd_dev_led_state,
> > NULL,
> > },
> > };
> > @@ -13332,6 +13392,7 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
> > &cmd_stop,
> > &cmd_mac_addr,
> > &cmd_set_fwd_eth_peer,
> > + &cmd_set_dev_led,
> > &cmd_set_qmap,
> > &cmd_set_xstats_hide_zero,
> > &cmd_set_record_core_cycles,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > fb1a1485e2..9673f9f207 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -5385,6 +5385,25 @@ set_fwd_eth_peer(portid_t port_id, char
> *peer_addr)
> > peer_eth_addrs[port_id] = new_peer_addr; }
> >
> > +void
> > +set_dev_led(portid_t port_id, bool active) {
> > + int ret;
> > +
> > + if (!rte_eth_dev_is_valid_port(port_id)) {
> > + fprintf(stderr, "Error: Invalid port number %i\n",
> > + port_id);
>
> %i -> %u
>
> > + return;
> > + }
> > +
> > + if (active)
> > + ret = rte_eth_led_on(port_id);
> > + else
> > + ret = rte_eth_led_off(port_id);
> > +
> > + if (ret < 0)
> > + fprintf(stderr, "Error: Unable to change LED state for
> > + port %i\n", port_id);
>
> %i -> %u
>
> > +}
> > +
> > int
> > set_fwd_lcores_list(unsigned int *lcorelist, unsigned int nb_lc) {
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 1292d1a0a7..aa1f0e9dd1 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -944,6 +944,7 @@ int init_fwd_streams(void); void
> > update_fwd_ports(portid_t new_pid);
> >
> > void set_fwd_eth_peer(portid_t port_id, char *peer_addr);
> > +void set_dev_led(portid_t port_id, bool active);
> >
> > void port_mtu_set(portid_t port_id, uint16_t mtu); int
> > port_action_handle_create(portid_t port_id, uint32_t id, bool
> > indirect_list, diff --git
> > a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 3aa214ef9f..c033b07aa7 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -1370,6 +1370,13 @@ Value should be given in the form of a hex-as-
> string, with no leading ``0x``.
> > The offset field here is optional, if not specified then the offset
> > will default to 0.
> >
> > +set port led
> > +~~~~~~~~~~~~
> > +
> > +Set a controllable LED associated with a certain port on or off::
> > +
> > + testpmd> set port (port_id) led (on|off)
> > +
> > set port-uta
> > ~~~~~~~~~~~~
> >
Thanks for review, we'll send a new version patch and do as your advice.
