| Message ID | 20230614133644.1755282-2-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 02:36:39PM +0100, Kevin Traynor wrote: > other_config:pmd-maxsleep is a config option to allow > PMD thread cores to sleep under low or no load conditions. > > Rename it to 'pmd-sleep-max' to allow a more structured > name and so that additional options or command can follow > the 'pmd-sleep-xyz' pattern. > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Hi Kevin, are there any backwards compatibility considerations for this change?
On Thu, Jun 29, 2023 at 4:30 PM Simon Horman <simon.horman@corigine.com> wrote: > > On Wed, Jun 14, 2023 at 02:36:39PM +0100, Kevin Traynor wrote: > > other_config:pmd-maxsleep is a config option to allow > > PMD thread cores to sleep under low or no load conditions. > > > > Rename it to 'pmd-sleep-max' to allow a more structured > > name and so that additional options or command can follow > > the 'pmd-sleep-xyz' pattern. > > > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > > Hi Kevin, > > are there any backwards compatibility considerations for this change? The PMD sleep feature is pretty recent (well, it was introduced in the previous release), and marked as experimental. https://docs.openvswitch.org/en/latest/topics/dpdk/pmd/#pmd-load-based-sleeping-experimental I don't have a strong opinion for or against this change, but I think it is ok to update the param name now for the reason Kevin gave in the commitlog.
On 29/06/2023 15:30, Simon Horman wrote: > On Wed, Jun 14, 2023 at 02:36:39PM +0100, Kevin Traynor wrote: >> other_config:pmd-maxsleep is a config option to allow >> PMD thread cores to sleep under low or no load conditions. >> >> Rename it to 'pmd-sleep-max' to allow a more structured >> name and so that additional options or command can follow >> the 'pmd-sleep-xyz' pattern. >> >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > > Hi Kevin, > > are there any backwards compatibility considerations for this change? > Hi Simon, It does mean the previous config option would not work anymore, yes. However, that is an experimental option for a new feature in OVS 3.1, so I don't think in practice it will be widely used. Also, with it being experimental it carries the health risk that it may be changed. I considered leaving it as a type of alias, but the later patches extend the functionality to using a string and per core, so having both might confusion for the user too. For example, a user might set both strings with competing values etc. or think the new one is only for per core etc. e.g. pmd-maxsleep=50 (general default sleep is 50) and pmd-sleep-max=12:200 (general default sleep is 0). A clearer way might be to add a warning if the user sets pmd-maxsleep just for the chance that anyone starts using with 3.1 and hops to 3.2+ and doesn't notice the change. I've added the below to v3. We could remove it sometime in the future when we think OVS 3.1 has become a bit old for users to start trying experimental features with it. What do you think? + if (smap_get_ullong(other_config, "pmd-maxsleep", 0)) { + VLOG_WARN("pmd-maxsleep is not supported. " + "Please use pmd-sleep-max instead."); + } ovs-vsctl set Open_vSwitch . other_config:pmd-maxsleep=200 2023-07-10T10:55:21Z|00108|dpif_netdev|WARN|pmd-maxsleep is not supported. Please use pmd-sleep-max instead.
diff --git a/Documentation/topics/dpdk/pmd.rst b/Documentation/topics/dpdk/pmd.rst index e70986d16..b261e9254 100644 --- a/Documentation/topics/dpdk/pmd.rst +++ b/Documentation/topics/dpdk/pmd.rst @@ -335,5 +335,5 @@ This can be enabled by setting the max requested sleep time (in microseconds) for a PMD thread:: - $ ovs-vsctl set open_vswitch . other_config:pmd-maxsleep=50 + $ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50 With a non-zero max value a PMD may request to sleep by an incrementing amount diff --git a/NEWS b/NEWS index cfd466663..1ccc6f948 100644 --- a/NEWS +++ b/NEWS @@ -37,4 +37,6 @@ Post-v3.1.0 - SRv6 Tunnel Protocol * Added support for userspace datapath (only). + - Userspace datapath: + * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'. diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 70b953ae6..94c8ca943 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -4983,5 +4983,5 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config) set_pmd_auto_lb(dp, autolb_state, log_autolb); - pmd_max_sleep = smap_get_ullong(other_config, "pmd-maxsleep", 0); + pmd_max_sleep = smap_get_ullong(other_config, "pmd-sleep-max", 0); pmd_max_sleep = MIN(PMD_RCU_QUIESCE_INTERVAL, pmd_max_sleep); atomic_read_relaxed(&dp->pmd_max_sleep, &cur_pmd_max_sleep); diff --git a/tests/pmd.at b/tests/pmd.at index 48f3d432d..374ad7217 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -1266,5 +1266,5 @@ OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "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-maxsleep="1"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 1 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) @@ -1272,5 +1272,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps dnl Check high value max sleep get_log_next_line_num -AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="10000"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10000"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 10000 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) @@ -1278,5 +1278,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps dnl Check setting max sleep to zero get_log_next_line_num -AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="0"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="0"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 0 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are disabled."]) @@ -1284,5 +1284,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps dnl Check above high value max sleep get_log_next_line_num -AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="10001"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10001"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 10000 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) @@ -1290,10 +1290,10 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps dnl Check rounding get_log_next_line_num -AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="490"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="490"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 490 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) dnl Check rounding get_log_next_line_num -AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="499"]) +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="499"]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep request is 499 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 59c404bbb..fe5f89154 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -803,5 +803,5 @@ </p> </column> - <column name="other_config" key="pmd-maxsleep" + <column name="other_config" key="pmd-sleep-max" type='{"type": "integer", "minInteger": 0, "maxInteger": 10000}'>
other_config:pmd-maxsleep is a config option to allow PMD thread cores to sleep under low or no load conditions. Rename it to 'pmd-sleep-max' to allow a more structured name and so that additional options or command can follow the 'pmd-sleep-xyz' pattern. Signed-off-by: Kevin Traynor <ktraynor@redhat.com> --- Documentation/topics/dpdk/pmd.rst | 2 +- NEWS | 2 ++ lib/dpif-netdev.c | 2 +- tests/pmd.at | 12 ++++++------ vswitchd/vswitch.xml | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-)