On 18/05/2022 10:11, Burakov, Anatoly wrote:
On 19-Apr-22 12:25 PM, Kevin Laatz wrote:
Add CLI options to l3fwd_power to utilize the new power APIs introduced in
this patchset. These CLI options allow the user to configure the
heuritstics made available through the new API via the l3fwd_power
application options.

Signed-off-by: Kevin Laatz <kevin.la...@intel.com>

---
v2: add doc update for l3fwd-power
---

This is a bit confusing. There are other scaling modes in l3fwd-power, but the --scale-freq-min only applies to the PMD power management mode.

.../sample_app_ug/l3_forward_power_man.rst    |  8 ++
  examples/l3fwd-power/main.c                   | 75 ++++++++++++++++++-
  2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/doc/guides/sample_app_ug/l3_forward_power_man.rst b/doc/guides/sample_app_ug/l3_forward_power_man.rst
index 2e350c45f1..b26f11a220 100644
--- a/doc/guides/sample_app_ug/l3_forward_power_man.rst
+++ b/doc/guides/sample_app_ug/l3_forward_power_man.rst
@@ -109,6 +109,14 @@ where,
    *   --pmd-mgmt: PMD power management mode.
  +*   --max-empty-polls : Number of empty polls to wait before entering sleep state.
+
+*   --pause-duration: Set the duration of the pause callback (microseconds).
+
+*   --scale-freq-min: Set minimum frequency for scaling.
+
+*   --scale-freq-max: Set maximum frequency for scaling.
+

Maybe make a note that these only apply to --pmd-mgmt mode?

Fair point, I'll add a note.



  See :doc:`l3_forward` for details.
  The L3fwd-power example reuses the L3fwd command line options.
  diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 20e5b59af9..ec2a71f02f 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -265,6 +265,10 @@ static struct rte_eth_conf port_conf = {
  };
    static uint32_t max_pkt_len;
+static uint32_t max_empty_polls;
+static uint32_t pause_duration;
+static uint32_t scale_freq_min;
+static uint32_t scale_freq_max;
    static struct rte_mempool * pktmbuf_pool[NB_SOCKETS];
  @@ -1626,10 +1630,32 @@ print_usage(const char *prgname)
          " empty polls, full polls, and core busyness to telemetry\n"
          " --interrupt-only: enable interrupt-only mode\n"
          " --pmd-mgmt MODE: enable PMD power management mode. "
-        "Currently supported modes: baseline, monitor, pause, scale\n",
+        "Currently supported modes: baseline, monitor, pause, scale\n"
+        "  --max-empty-polls MAX_EMPTY_POLLS: number of empty polls to"
+        " wait before entering sleep state\n"
+        "  --pause_duration DURATION: set the duration, in microseconds,"
+        " of the pause callback\n"
+        "  --scale-freq-min FREQ_MIN: set minimum frequency for scaling"
+        " mode for all application lcores\n"
+        "  --scale_freq_max FREQ_MAX: set maximum frequency for scaling"
+        " mode for all application lcores\n",

It would be useful to note which units are used here :) This is increments in 100MHz, correct?

Ack



          prgname);
  }
  +static int
+parse_int(const char *opt)
+{
+    char *end = NULL;
+    unsigned long val;
+
+    /* parse integer string */
+    val = strtoul(opt, &end, 10);
+    if ((opt[0] == '\0') || (end == NULL) || (*end != '\0'))
+        return -1;
+
+    return val;
+}
+
  static int parse_max_pkt_len(const char *pktlen)
  {
      char *end = NULL;
@@ -1803,6 +1829,10 @@ parse_ep_config(const char *q_arg)
  #define CMD_LINE_OPT_TELEMETRY "telemetry"
  #define CMD_LINE_OPT_PMD_MGMT "pmd-mgmt"
  #define CMD_LINE_OPT_MAX_PKT_LEN "max-pkt-len"
+#define CMD_LINE_OPT_MAX_EMPTY_POLLS "max-empty-poll"
+#define CMD_LINE_OPT_PAUSE_DURATION "pause-duration"
+#define CMD_LINE_OPT_SCALE_FREQ_MIN "scale-freq-min"
+#define CMD_LINE_OPT_SCALE_FREQ_MAX "scale-freq-max"
    /* Parse the argument given in the command line of the application */
  static int
@@ -1812,6 +1842,7 @@ parse_args(int argc, char **argv)
      char **argvopt;
      int option_index;
      uint32_t limit;
+    int i;
      char *prgname = argv[0];
      static struct option lgopts[] = {
          {"config", 1, 0, 0},
@@ -1825,6 +1856,10 @@ parse_args(int argc, char **argv)
          {CMD_LINE_OPT_TELEMETRY, 0, 0, 0},
          {CMD_LINE_OPT_INTERRUPT_ONLY, 0, 0, 0},
          {CMD_LINE_OPT_PMD_MGMT, 1, 0, 0},
+        {CMD_LINE_OPT_MAX_EMPTY_POLLS, 1, 0, 0},
+        {CMD_LINE_OPT_PAUSE_DURATION, 1, 0, 0},
+        {CMD_LINE_OPT_SCALE_FREQ_MIN, 1, 0, 0},
+        {CMD_LINE_OPT_SCALE_FREQ_MAX, 1, 0, 0},
          {NULL, 0, 0, 0}
      };
  @@ -1975,6 +2010,44 @@ parse_args(int argc, char **argv)
                  parse_ptype = 1;
              }
  +            if (!strncmp(lgopts[option_index].name,
+                    CMD_LINE_OPT_MAX_EMPTY_POLLS,
+                    sizeof(CMD_LINE_OPT_MAX_EMPTY_POLLS))) {
+                printf("Maximum empty polls configured\n");
+                max_empty_polls = parse_int(optarg);
+ rte_power_pmd_mgmt_set_emptypoll_max(max_empty_polls);

If you have global variables anyway, why are you setting this here? :) You could call these API's under PMD mgmt mode init only, and this would also give you a chance of checking return values of any of these API's - for example, AFAIR set_pause_duration() can return an error if the value supplied is invalid (0).

Plus, I still feel uneasy about having DPDK API calls right inside arg parsing code.

I'll look into this and move the API calls out of the parsing for v3.

Thanks for reviewing!



+            }
+
+            if (!strncmp(lgopts[option_index].name,
+                    CMD_LINE_OPT_PAUSE_DURATION,
+                    sizeof(CMD_LINE_OPT_PAUSE_DURATION))) {
+                printf("Pause duration configured\n");
+                pause_duration = parse_int(optarg);
+ rte_power_pmd_mgmt_set_pause_duration(pause_duration);
+            }
+
+            if (!strncmp(lgopts[option_index].name,
+                    CMD_LINE_OPT_SCALE_FREQ_MIN,
+                    sizeof(CMD_LINE_OPT_SCALE_FREQ_MIN))) {
+                printf("Scaling frequency minimum configured\n");
+                scale_freq_min = parse_int(optarg);
+                for (i = 0; i < RTE_MAX_LCORE; i++)
+                    if (rte_lcore_is_enabled(i))
+ rte_power_pmd_mgmt_set_scaling_freq_min(i,
+                                scale_freq_min);
+            }
+
+            if (!strncmp(lgopts[option_index].name,
+                    CMD_LINE_OPT_SCALE_FREQ_MAX,
+                    sizeof(CMD_LINE_OPT_SCALE_FREQ_MAX))) {
+                printf("Scaling frequency maximum configured\n");
+                scale_freq_max = parse_int(optarg);
+                for (i = 0; i < RTE_MAX_LCORE; i++)
+                    if (rte_lcore_is_enabled(i))
+ rte_power_pmd_mgmt_set_scaling_freq_max(i,
+                                scale_freq_max);
+            }
+
              break;
            default:


Reply via email to