[v2] examples/ethtool: adds promiscuous mode functionality

  ethtool did not have promiscuous mode functioality previously
which is needed for viewing broadcast and multicast packets.
This patch allows user to turn on/off promiscuous mode on
each port through command line.

Signed-off-by: Muhammad Jawad Hussain <jawad.hussain@emumba.com>
---
 doc/guides/sample_app_ug/ethtool.rst  |  1 +
 examples/ethtool/ethtool-app/ethapp.c | 79 ++++++++++++++++++++++++++-
 examples/ethtool/lib/rte_ethtool.c    | 24 ++++++++
 examples/ethtool/lib/rte_ethtool.h    |  2 +
 4 files changed, 104 insertions(+), 2 deletions(-)
  

Hi,
The following test is failing on my patch and its is not related to my
changes, can u please re-run it.
test:
ci/iol-x86_64-unit-testing

Thanks

Regards,
Jawad


On Thu, Aug 11, 2022 at 2:45 PM Muhammad Jawad Hussain <
jawad.hussain@emumba.com> wrote:

> ethtool did not have promiscuous mode functioality previously
> which is needed for viewing broadcast and multicast packets.
> This patch allows user to turn on/off promiscuous mode on
> each port through command line.
>
> Signed-off-by: Muhammad Jawad Hussain <jawad.hussain@emumba.com>
> ---
>  doc/guides/sample_app_ug/ethtool.rst  |  1 +
>  examples/ethtool/ethtool-app/ethapp.c | 79 ++++++++++++++++++++++++++-
>  examples/ethtool/lib/rte_ethtool.c    | 24 ++++++++
>  examples/ethtool/lib/rte_ethtool.h    |  2 +
>  4 files changed, 104 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/ethtool.rst
> b/doc/guides/sample_app_ug/ethtool.rst
> index 159e9e0639..6edd9940b8 100644
> --- a/doc/guides/sample_app_ug/ethtool.rst
> +++ b/doc/guides/sample_app_ug/ethtool.rst
> @@ -54,6 +54,7 @@ they do as follows:
>  * ``regs``: Dump port register(s) to file
>  * ``ringparam``: Get/set ring parameters
>  * ``rxmode``: Toggle port Rx mode
> +* ``set promisc``: Enable/Disable promiscuous mode on ports
>  * ``stop``: Stop port
>  * ``validate``: Check that given MAC address is valid unicast address
>  * ``vlan``: Add/remove VLAN id
> diff --git a/examples/ethtool/ethtool-app/ethapp.c
> b/examples/ethtool/ethtool-app/ethapp.c
> index 78e86534e8..f89e4c4cf0 100644
> --- a/examples/ethtool/ethtool-app/ethapp.c
> +++ b/examples/ethtool/ethtool-app/ethapp.c
> @@ -13,8 +13,16 @@
>  #include "ethapp.h"
>
>  #define EEPROM_DUMP_CHUNKSIZE 1024
> -
> -
> +typedef uint16_t portid_t;
> +
> +/* *** PROMISC_MODE *** */
> +struct cmd_set_promisc_mode_result {
> +       cmdline_fixed_string_t set;
> +       cmdline_fixed_string_t promisc;
> +       cmdline_fixed_string_t port_all; /* valid if "allports" argument
> == 1 */
> +       uint16_t port_num;               /* valid if "allports" argument
> == 0 */
> +       cmdline_fixed_string_t mode;
> +};
>  struct pcmd_get_params {
>         cmdline_fixed_string_t cmd;
>  };
> @@ -133,6 +141,22 @@ cmdline_parse_token_string_t pcmd_vlan_token_mode =
>  cmdline_parse_token_num_t pcmd_vlan_token_vid =
>         TOKEN_NUM_INITIALIZER(struct pcmd_vlan_params, vid, RTE_UINT16);
>
> +/* promisc mode */
> +
> +cmdline_parse_token_string_t cmd_setpromisc_set =
> +       TOKEN_STRING_INITIALIZER(struct cmd_set_promisc_mode_result, set,
> "set");
> +cmdline_parse_token_string_t cmd_setpromisc_promisc =
> +       TOKEN_STRING_INITIALIZER(struct cmd_set_promisc_mode_result,
> promisc,
> +                                "promisc");
> +cmdline_parse_token_string_t cmd_setpromisc_portall =
> +       TOKEN_STRING_INITIALIZER(struct cmd_set_promisc_mode_result,
> port_all,
> +                                "all");
> +cmdline_parse_token_num_t cmd_setpromisc_portnum =
> +       TOKEN_NUM_INITIALIZER(struct cmd_set_promisc_mode_result, port_num,
> +                             RTE_UINT16);
> +cmdline_parse_token_string_t cmd_setpromisc_mode =
> +       TOKEN_STRING_INITIALIZER(struct cmd_set_promisc_mode_result, mode,
> +                                "on#off");
>
>  static void
>  pcmd_quit_callback(__rte_unused void *ptr_params,
> @@ -142,6 +166,30 @@ pcmd_quit_callback(__rte_unused void *ptr_params,
>         cmdline_quit(ctx);
>  }
>
> +static void pcmd_set_promisc_mode_parsed(void *ptr_params,
> +                                       __rte_unused struct cmdline *ctx,
> +                                       void *allports)
> +{
> +       struct cmd_set_promisc_mode_result *res = ptr_params;
> +       int enable;
> +       portid_t i;
> +       if (!strcmp(res->mode, "on"))
> +               enable = 1;
> +       else
> +               enable = 0;
> +
> +       /* all ports */
> +       if (allports) {
> +               RTE_ETH_FOREACH_DEV(i)
> +                       eth_set_promisc_mode(i, enable);
> +       } else {
> +               eth_set_promisc_mode(res->port_num, enable);
> +       }
> +       if (enable)
> +               printf("Promisc mode Enabled\n");
> +       else
> +               printf("Promisc mode Disabled\n");
> +}
>
>  static void
>  pcmd_drvinfo_callback(__rte_unused void *ptr_params,
> @@ -869,6 +917,31 @@ cmdline_parse_inst_t pcmd_vlan = {
>         },
>  };
>
> +cmdline_parse_inst_t cmd_set_promisc_mode_all = {
> +       .f = pcmd_set_promisc_mode_parsed,
> +       .data = (void *)1,
> +       .help_str = "set promisc all <on|off>\n     Set promisc mode for
> all ports",
> +       .tokens = {
> +               (void *)&cmd_setpromisc_set,
> +               (void *)&cmd_setpromisc_promisc,
> +               (void *)&cmd_setpromisc_portall,
> +               (void *)&cmd_setpromisc_mode,
> +               NULL,
> +       },
> +};
> +
> +cmdline_parse_inst_t cmd_set_promisc_mode_one = {
> +       .f = pcmd_set_promisc_mode_parsed,
> +       .data = (void *)0,
> +       .help_str = "set promisc <port_id> <on|off>\n     Set promisc mode
> on port_id",
> +       .tokens = {
> +               (void *)&cmd_setpromisc_set,
> +               (void *)&cmd_setpromisc_promisc,
> +               (void *)&cmd_setpromisc_portnum,
> +               (void *)&cmd_setpromisc_mode,
> +               NULL,
> +       },
> +};
>
>  cmdline_parse_ctx_t list_prompt_commands[] = {
>         (cmdline_parse_inst_t *)&pcmd_drvinfo,
> @@ -886,6 +959,8 @@ cmdline_parse_ctx_t list_prompt_commands[] = {
>         (cmdline_parse_inst_t *)&pcmd_ringparam,
>         (cmdline_parse_inst_t *)&pcmd_ringparam_set,
>         (cmdline_parse_inst_t *)&pcmd_rxmode,
> +       (cmdline_parse_inst_t *)&cmd_set_promisc_mode_one,
> +       (cmdline_parse_inst_t *)&cmd_set_promisc_mode_all,
>         (cmdline_parse_inst_t *)&pcmd_stop,
>         (cmdline_parse_inst_t *)&pcmd_validate,
>         (cmdline_parse_inst_t *)&pcmd_vlan,
> diff --git a/examples/ethtool/lib/rte_ethtool.c
> b/examples/ethtool/lib/rte_ethtool.c
> index ffaad96498..2fb47471cb 100644
> --- a/examples/ethtool/lib/rte_ethtool.c
> +++ b/examples/ethtool/lib/rte_ethtool.c
> @@ -18,6 +18,30 @@
>  #define PKTPOOL_CACHE 32
>
>
> +int
> +eth_set_promisc_mode(uint16_t port, int enable)
> +{
> +       int ret;
> +
> +
> +       if (enable)
> +               ret = rte_eth_promiscuous_enable(port);
> +       else
> +               ret = rte_eth_promiscuous_disable(port);
> +
> +       if (ret != 0) {
> +               fprintf(stderr,
> +                       "Error during %s promiscuous mode for port %u:
> %s\n",
> +                       enable ? "enabling" : "disabling",
> +                       port, rte_strerror(-ret));
> +                       return 0;
> +       } else
> +               return 1;
> +}
> +
> +
> +
> +
>  int
>  rte_ethtool_get_drvinfo(uint16_t port_id, struct ethtool_drvinfo *drvinfo)
>  {
> diff --git a/examples/ethtool/lib/rte_ethtool.h
> b/examples/ethtool/lib/rte_ethtool.h
> index d27e0102b1..2b19907b4d 100644
> --- a/examples/ethtool/lib/rte_ethtool.h
> +++ b/examples/ethtool/lib/rte_ethtool.h
> @@ -408,6 +408,8 @@ int rte_ethtool_get_ringparam(uint16_t port_id,
>  int rte_ethtool_set_ringparam(uint16_t port_id,
>         struct ethtool_ringparam *ring_param);
>
> +int
> +eth_set_promisc_mode(uint16_t port, int enable);
>
>  #ifdef __cplusplus
>  }
> --
> 2.32.0
>
>
  

On Thu, 11 Aug 2022 14:45:08 +0500
Muhammad Jawad Hussain <jawad.hussain@emumba.com> wrote:

>  
>  
> +int
> +eth_set_promisc_mode(uint16_t port, int enable)
> +{
> +	int ret;
> +
> +
> +	if (enable)
> +		ret = rte_eth_promiscuous_enable(port);
> +	else
> +		ret = rte_eth_promiscuous_disable(port);
> +
> +	if (ret != 0) {
> +		fprintf(stderr,
> +			"Error during %s promiscuous mode for port %u: %s\n",
> +			enable ? "enabling" : "disabling",
> +			port, rte_strerror(-ret));
> +			return 0;
> +	} else
> +		return 1;
> +}
> +
> +
> +
> +


Some minor comments:
  1. The naming convention in ethtool should be followed.
     This should be named: rte_ethtool_set_promisc_mode

  2. Indentation is wrong for the "return 0" part

  3. Lots of useless extra blank lines, one will do fine.

Overall, the idea is fine, but almost no one cares about the ethtool example.
That is why no one reviewed or acked it.