Re: [ovs-dev] [PATCH v3 3/6] dpif-netdev: Add pmd-sleep-show command.

2023-07-14 Thread Kevin Traynor

On 13/07/2023 22:14, Ilya Maximets wrote:

On 7/10/23 16:54, Kevin Traynor wrote:

Max requested sleep time and status for a PMD thread
is logged at start up or when changed, but it can be
convenient to have a command to dump this information
explicitly.

It is envisaged that this will be expanded when future
additions are added.

Signed-off-by: Kevin Traynor 
Reviewed-by: David Marchand 
---
  Documentation/topics/dpdk/pmd.rst |  4 
  NEWS  |  2 ++
  lib/dpif-netdev.c | 40 +++
  tests/pmd.at  | 29 ++
  4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index b261e9254..bedd42194 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -349,4 +349,8 @@ time not processing packets will be determined by the sleep 
and processor
  wake-up times and should be tested with each system configuration.
  
+The max sleep time that a PMD thread may request can be shown with::


I'm not sure why, but this sentence seems strange to me.
How about we change it to something like:

"The current configuration of the PMD load based sleeping can be shown with"

?



That works and it's less overloaded :-)


+
+$ ovs-appctl dpif-netdev/pmd-sleep-show
+
  Sleep time statistics for 10 secs can be seen with::
  
diff --git a/NEWS b/NEWS

index 6c0b09e0a..dd3d3aa91 100644
--- a/NEWS
+++ b/NEWS
@@ -46,4 +46,6 @@ Post-v3.1.0
 table to check the status.
   * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
+ * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
+   max sleep request setting of PMD thread cores.


Same NEWS comment as for patch #1.



done


 - Linux TC offload:
   * Add support for offloading VXLAN tunnels with the GBP extensions.
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9df481dd6..522253c82 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -702,4 +702,5 @@ enum pmd_info_type {
  PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
  PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
+PMD_INFO_MAX_SLEEP_SHOW,   /* Show max sleep performance details. */


s/performance/configuration/ ?



That's more accurate, done.


  };
  
@@ -1007,4 +1008,18 @@ sorted_poll_thread_list(struct dp_netdev *dp,

  }
  
+static void

+pmd_max_sleep_show(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
+   uint64_t default_max_sleep)
+{
+if (pmd->core_id != NON_PMD_CORE_ID) {
+ds_put_format(reply,
+  "PMD thread core %3u NUMA %2d: "
+  "Max sleep request set to",
+  pmd->core_id, pmd->numa_id);
+ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
+ds_put_cstr(reply, "\n");
+}
+}
+
  static void
  dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
@@ -1442,5 +1457,6 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
  unsigned long long max_secs = (PMD_INTERVAL_LEN * PMD_INTERVAL_MAX)
/ INTERVAL_USEC_TO_SEC;
-bool first_show_rxq = true;
+bool first_pmd = true;


Maybe rename this to 'show_header' ?



done


+uint64_t default_max_sleep = 0;


I know, it's a mess, but maybe swap above lines to at least keep
some sort of the reverse x-mass tree in a new code?



done

  
  ovs_mutex_lock(_netdev_mutex);

@@ -1490,5 +1506,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
  }
  if (type == PMD_INFO_SHOW_RXQ) {
-if (first_show_rxq) {
+if (first_pmd) {
  if (!secs || secs > max_secs) {
  secs = max_secs;
@@ -1499,5 +1515,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
  ds_put_format(, "Displaying last %u seconds "
"pmd usage %%\n", secs);
-first_show_rxq = false;
+first_pmd = false;
  }
  pmd_info_show_rxq(, pmd, secs);
@@ -1508,4 +1524,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
argc, const char *argv[],
  } else if (type == PMD_INFO_PERF_SHOW) {
  pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
+} else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
+if (first_pmd) {
+atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
+ds_put_format(, "PMD max sleep request is %"PRIu64" "
+  "usecs.", default_max_sleep);
+ds_put_cstr(, "\n");
+ds_put_format(, "PMD load based sleeps are %s.",
+  default_max_sleep ? "enabled" : "disabled");
+ds_put_cstr(, "\n");

Re: [ovs-dev] [PATCH v3 3/6] dpif-netdev: Add pmd-sleep-show command.

2023-07-13 Thread Ilya Maximets
On 7/10/23 16:54, Kevin Traynor wrote:
> Max requested sleep time and status for a PMD thread
> is logged at start up or when changed, but it can be
> convenient to have a command to dump this information
> explicitly.
> 
> It is envisaged that this will be expanded when future
> additions are added.
> 
> Signed-off-by: Kevin Traynor 
> Reviewed-by: David Marchand 
> ---
>  Documentation/topics/dpdk/pmd.rst |  4 
>  NEWS  |  2 ++
>  lib/dpif-netdev.c | 40 +++
>  tests/pmd.at  | 29 ++
>  4 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/pmd.rst 
> b/Documentation/topics/dpdk/pmd.rst
> index b261e9254..bedd42194 100644
> --- a/Documentation/topics/dpdk/pmd.rst
> +++ b/Documentation/topics/dpdk/pmd.rst
> @@ -349,4 +349,8 @@ time not processing packets will be determined by the 
> sleep and processor
>  wake-up times and should be tested with each system configuration.
>  
> +The max sleep time that a PMD thread may request can be shown with::

I'm not sure why, but this sentence seems strange to me.
How about we change it to something like:

"The current configuration of the PMD load based sleeping can be shown with"

?

> +
> +$ ovs-appctl dpif-netdev/pmd-sleep-show
> +
>  Sleep time statistics for 10 secs can be seen with::
>  
> diff --git a/NEWS b/NEWS
> index 6c0b09e0a..dd3d3aa91 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -46,4 +46,6 @@ Post-v3.1.0
> table to check the status.
>   * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
> + * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
> +   max sleep request setting of PMD thread cores.

Same NEWS comment as for patch #1.

> - Linux TC offload:
>   * Add support for offloading VXLAN tunnels with the GBP extensions.
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9df481dd6..522253c82 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -702,4 +702,5 @@ enum pmd_info_type {
>  PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
>  PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
> +PMD_INFO_MAX_SLEEP_SHOW,   /* Show max sleep performance details. */

s/performance/configuration/ ?

>  };
>  
> @@ -1007,4 +1008,18 @@ sorted_poll_thread_list(struct dp_netdev *dp,
>  }
>  
> +static void
> +pmd_max_sleep_show(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
> +   uint64_t default_max_sleep)
> +{
> +if (pmd->core_id != NON_PMD_CORE_ID) {
> +ds_put_format(reply,
> +  "PMD thread core %3u NUMA %2d: "
> +  "Max sleep request set to",
> +  pmd->core_id, pmd->numa_id);
> +ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
> +ds_put_cstr(reply, "\n");
> +}
> +}
> +
>  static void
>  dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
> @@ -1442,5 +1457,6 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  unsigned long long max_secs = (PMD_INTERVAL_LEN * PMD_INTERVAL_MAX)
>/ INTERVAL_USEC_TO_SEC;
> -bool first_show_rxq = true;
> +bool first_pmd = true;

Maybe rename this to 'show_header' ?

> +uint64_t default_max_sleep = 0;

I know, it's a mess, but maybe swap above lines to at least keep
some sort of the reverse x-mass tree in a new code?

>  
>  ovs_mutex_lock(_netdev_mutex);
> @@ -1490,5 +1506,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  }
>  if (type == PMD_INFO_SHOW_RXQ) {
> -if (first_show_rxq) {
> +if (first_pmd) {
>  if (!secs || secs > max_secs) {
>  secs = max_secs;
> @@ -1499,5 +1515,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  ds_put_format(, "Displaying last %u seconds "
>"pmd usage %%\n", secs);
> -first_show_rxq = false;
> +first_pmd = false;
>  }
>  pmd_info_show_rxq(, pmd, secs);
> @@ -1508,4 +1524,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  } else if (type == PMD_INFO_PERF_SHOW) {
>  pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
> +} else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
> +if (first_pmd) {
> +atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
> +ds_put_format(, "PMD max sleep request is %"PRIu64" "
> +  "usecs.", default_max_sleep);
> +ds_put_cstr(, "\n");
> +ds_put_format(, "PMD load based sleeps are %s.",
> +  default_max_sleep ? "enabled" : "disabled");
> +   

[ovs-dev] [PATCH v3 3/6] dpif-netdev: Add pmd-sleep-show command.

2023-07-10 Thread Kevin Traynor
Max requested sleep time and status for a PMD thread
is logged at start up or when changed, but it can be
convenient to have a command to dump this information
explicitly.

It is envisaged that this will be expanded when future
additions are added.

Signed-off-by: Kevin Traynor 
Reviewed-by: David Marchand 
---
 Documentation/topics/dpdk/pmd.rst |  4 
 NEWS  |  2 ++
 lib/dpif-netdev.c | 40 +++
 tests/pmd.at  | 29 ++
 4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index b261e9254..bedd42194 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -349,4 +349,8 @@ time not processing packets will be determined by the sleep 
and processor
 wake-up times and should be tested with each system configuration.
 
+The max sleep time that a PMD thread may request can be shown with::
+
+$ ovs-appctl dpif-netdev/pmd-sleep-show
+
 Sleep time statistics for 10 secs can be seen with::
 
diff --git a/NEWS b/NEWS
index 6c0b09e0a..dd3d3aa91 100644
--- a/NEWS
+++ b/NEWS
@@ -46,4 +46,6 @@ Post-v3.1.0
table to check the status.
  * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
+ * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
+   max sleep request setting of PMD thread cores.
- Linux TC offload:
  * Add support for offloading VXLAN tunnels with the GBP extensions.
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9df481dd6..522253c82 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -702,4 +702,5 @@ enum pmd_info_type {
 PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
 PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
+PMD_INFO_MAX_SLEEP_SHOW,   /* Show max sleep performance details. */
 };
 
@@ -1007,4 +1008,18 @@ sorted_poll_thread_list(struct dp_netdev *dp,
 }
 
+static void
+pmd_max_sleep_show(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
+   uint64_t default_max_sleep)
+{
+if (pmd->core_id != NON_PMD_CORE_ID) {
+ds_put_format(reply,
+  "PMD thread core %3u NUMA %2d: "
+  "Max sleep request set to",
+  pmd->core_id, pmd->numa_id);
+ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
+ds_put_cstr(reply, "\n");
+}
+}
+
 static void
 dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
@@ -1442,5 +1457,6 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 unsigned long long max_secs = (PMD_INTERVAL_LEN * PMD_INTERVAL_MAX)
   / INTERVAL_USEC_TO_SEC;
-bool first_show_rxq = true;
+bool first_pmd = true;
+uint64_t default_max_sleep = 0;
 
 ovs_mutex_lock(_netdev_mutex);
@@ -1490,5 +1506,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 }
 if (type == PMD_INFO_SHOW_RXQ) {
-if (first_show_rxq) {
+if (first_pmd) {
 if (!secs || secs > max_secs) {
 secs = max_secs;
@@ -1499,5 +1515,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 ds_put_format(, "Displaying last %u seconds "
   "pmd usage %%\n", secs);
-first_show_rxq = false;
+first_pmd = false;
 }
 pmd_info_show_rxq(, pmd, secs);
@@ -1508,4 +1524,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
argc, const char *argv[],
 } else if (type == PMD_INFO_PERF_SHOW) {
 pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
+} else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
+if (first_pmd) {
+atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
+ds_put_format(, "PMD max sleep request is %"PRIu64" "
+  "usecs.", default_max_sleep);
+ds_put_cstr(, "\n");
+ds_put_format(, "PMD load based sleeps are %s.",
+  default_max_sleep ? "enabled" : "disabled");
+ds_put_cstr(, "\n");
+first_pmd = false;
+}
+pmd_max_sleep_show(, pmd, default_max_sleep);
 }
 }
@@ -1608,5 +1636,6 @@ dpif_netdev_init(void)
 static enum pmd_info_type show_aux = PMD_INFO_SHOW_STATS,
   clear_aux = PMD_INFO_CLEAR_STATS,
-  poll_aux = PMD_INFO_SHOW_RXQ;
+  poll_aux = PMD_INFO_SHOW_RXQ,
+  sleep_aux = PMD_INFO_MAX_SLEEP_SHOW;
 
 unixctl_command_register("dpif-netdev/pmd-stats-show", "[-pmd core] [dp]",
@@ -1620,4 +1649,7 @@ dpif_netdev_init(void)