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 <ktray...@redhat.com>
> Reviewed-by: David Marchand <david.march...@redhat.com>
> ---
> 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(&dp_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(&reply, "Displaying last %u seconds "
> "pmd usage %%\n", secs);
> - first_show_rxq = false;
> + first_pmd = false;
> }
> pmd_info_show_rxq(&reply, 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(&reply, pmd, (struct pmd_perf_params *)aux);
> + } else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
> + if (first_pmd) {
> + atomic_read_relaxed(&dp->pmd_max_sleep, &default_max_sleep);
> + ds_put_format(&reply, "PMD max sleep request is %"PRIu64" "
> + "usecs.", default_max_sleep);
> + ds_put_cstr(&reply, "\n");
> + ds_put_format(&reply, "PMD load based sleeps are %s.",
> + default_max_sleep ? "enabled" : "disabled");
> + ds_put_cstr(&reply, "\n");
> + first_pmd = false;
> + }
> + pmd_max_sleep_show(&reply, 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)
> 0, 5, dpif_netdev_pmd_info,
> (void *)&poll_aux);
> + unixctl_command_register("dpif-netdev/pmd-sleep-show", "[-pmd core]
> [dp]",
> + 0, 3, dpif_netdev_pmd_info,
> + (void *)&sleep_aux);
> unixctl_command_register("dpif-netdev/pmd-perf-show",
> "[-nh] [-it iter-history-len]"
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 4dd775bd3..df578608c 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -1278,8 +1278,17 @@ dnl Check default
> CHECK_DP_SLEEP_MAX([0], [disabled], [])
>
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 0 usecs.
> +PMD load based sleeps are disabled.
The output of this command looks busy. And it's getting harder
to read in the later patches. I'll comment on that further down
the series.
> +])
> +
> dnl Check low value max sleep
> get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"])
> CHECK_DP_SLEEP_MAX([1], [enabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 1 usecs.
> +PMD load based sleeps are enabled.
> +])
>
> dnl Check high value max sleep
> @@ -1287,4 +1296,8 @@ get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10000"])
> CHECK_DP_SLEEP_MAX([10000], [enabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 10000 usecs.
> +PMD load based sleeps are enabled.
> +])
>
> dnl Check setting max sleep to zero
> @@ -1292,4 +1305,8 @@ get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="0"])
> CHECK_DP_SLEEP_MAX([0], [disabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 0 usecs.
> +PMD load based sleeps are disabled.
> +])
>
> dnl Check above high value max sleep
> @@ -1297,4 +1314,8 @@ get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10001"])
> CHECK_DP_SLEEP_MAX([10000], [enabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 10000 usecs.
> +PMD load based sleeps are enabled.
> +])
>
> dnl Check rounding
> @@ -1302,4 +1323,8 @@ get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="490"])
> CHECK_DP_SLEEP_MAX([490], [enabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 490 usecs.
> +PMD load based sleeps are enabled.
> +])
>
> dnl Check rounding
> @@ -1307,4 +1332,8 @@ get_log_next_line_num
> AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="499"])
> CHECK_DP_SLEEP_MAX([499], [enabled], [+$LINENUM])
> +AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
> +PMD max sleep request is 499 usecs.
> +PMD load based sleeps are enabled.
> +])
>
> OVS_VSWITCHD_STOP
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
