On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktray...@redhat.com> 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>
> ---
> 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 1ccc6f948..32b9e8167 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -39,4 +39,6 @@ Post-v3.1.0
> - Userspace datapath:
> * 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.
>
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 94c8ca943..dadf17b70 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. */
> };
>
> @@ -884,4 +885,18 @@ sorted_poll_list(struct dp_netdev_pmd_thread *pmd,
> struct rxq_poll **list,
> }
>
> +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");
> + }
> +}
> +
Nit: I would move this new function later, and let pmd_info_show_rxq()
and sorted_poll_list() close to each other.
> static void
> pmd_info_show_rxq(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
> @@ -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;
You can move this new variable in the only block that cares about it.
>
> 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 64d8f6e2b..a158d0753 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.
> +])
> +
Functionnally, we are checking the same thing than CHECK_DP_SLEEP_MAX().
I would check the pmd-sleep-show command output in a single helper.
--
David Marchand
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
