| Message ID | 20230614133644.1755282-4-ktraynor@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PMD load based sleep updates and per-pmd config. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktraynor@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 <ktraynor@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.
On 14/06/2023 16:49, David Marchand wrote: > On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktraynor@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 <ktraynor@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. > Will add to v2. I placed it after the other dpif_netdev_pmd_info() related functions. > >> 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. > Yes, I can do that if I remove the initialization. It will work now, but not being able to initialize feels like it makes the code a bit more brittle as that nuance could be missed if a default initialization was ever was needed in future (I don't have something in mind). What do you think? I can go with your preference. > >> >> 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. > > That's true at this patch stage, but it changes with the next patch. In that case it would also be checking about pmd core ids/numas etc so i didn't think it made sense to add to the macro.
On 20/06/2023 18:02, Kevin Traynor wrote: > On 14/06/2023 16:49, David Marchand wrote: >> On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktraynor@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 <ktraynor@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. >> > > Will add to v2. I placed it after the other dpif_netdev_pmd_info() > related functions. > >> >>> 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. >> > > Yes, I can do that if I remove the initialization. It will work now, but > not being able to initialize feels like it makes the code a bit more > brittle as that nuance could be missed if a default initialization was > ever was needed in future (I don't have something in mind). > > What do you think? I can go with your preference. > Discussed with David offline. Working in practice, but it's not fully safe to rely that default_max_sleep will keep the read value after the loop if we move the declaration. So will stick with current declaration. >> >>> >>> 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. >> >> > > That's true at this patch stage, but it changes with the next patch. In > that case it would also be checking about pmd core ids/numas etc so i > didn't think it made sense to add to the macro. >
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"); + } +} + 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; 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. +]) + 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
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 <ktraynor@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(-)