[ovs-dev] [PATCH v3 1/6] dpif-netdev: Rename pmd-maxsleep config option.
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. Use of other_config:pmd-maxsleep will result in a warning and it's value will be ignored. Signed-off-by: Kevin Traynor Reviewed-by: David Marchand --- Documentation/topics/dpdk/pmd.rst | 2 +- NEWS | 1 + lib/dpif-netdev.c | 7 ++- tests/pmd.at | 12 ++-- vswitchd/vswitch.xml | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) 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 6a990c921..6c0b09e0a 100644 --- a/NEWS +++ b/NEWS @@ -45,4 +45,5 @@ Post-v3.1.0 interfaces that support it. See the 'status' column in the 'interface' table to check the status. + * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'. - 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 ab493f9d4..9df481dd6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -4983,5 +4983,10 @@ 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); +if (smap_get_ullong(other_config, "pmd-maxsleep", 0)) { +VLOG_WARN("pmd-maxsleep is not supported. " + "Please use pmd-sleep-max instead."); +} + +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="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."]) @@ -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 1 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.
Re: [ovs-dev] [PATCH v3 1/6] dpif-netdev: Rename pmd-maxsleep config option.
On 7/10/23 16:54, 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. > > Use of other_config:pmd-maxsleep will result in a warning > and it's value will be ignored. > > Signed-off-by: Kevin Traynor > Reviewed-by: David Marchand > --- > Documentation/topics/dpdk/pmd.rst | 2 +- > NEWS | 1 + > lib/dpif-netdev.c | 7 ++- > tests/pmd.at | 12 ++-- > vswitchd/vswitch.xml | 2 +- > 5 files changed, 15 insertions(+), 9 deletions(-) > > 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 6a990c921..6c0b09e0a 100644 > --- a/NEWS > +++ b/NEWS > @@ -45,4 +45,5 @@ Post-v3.1.0 > interfaces that support it. See the 'status' column in the > 'interface' > table to check the status. > + * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'. Nit: Might be better to not write NEWS in imperative form. It's not a git log, we're just saying what changed, not asking users to do something. I know that some entries are written this way, but I don't think that's a good thing to have. > - 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 ab493f9d4..9df481dd6 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -4983,5 +4983,10 @@ 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); > +if (smap_get_ullong(other_config, "pmd-maxsleep", 0)) { > +VLOG_WARN("pmd-maxsleep is not supported. " > + "Please use pmd-sleep-max instead."); > +} > + > +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="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."]) > @@ -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 1 usecs."]) > OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based > sleeps are enabled."]
Re: [ovs-dev] [PATCH v3 1/6] dpif-netdev: Rename pmd-maxsleep config option.
On 13/07/2023 21:48, Ilya Maximets wrote: On 7/10/23 16:54, 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. Use of other_config:pmd-maxsleep will result in a warning and it's value will be ignored. Signed-off-by: Kevin Traynor Reviewed-by: David Marchand --- Documentation/topics/dpdk/pmd.rst | 2 +- NEWS | 1 + lib/dpif-netdev.c | 7 ++- tests/pmd.at | 12 ++-- vswitchd/vswitch.xml | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) 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 6a990c921..6c0b09e0a 100644 --- a/NEWS +++ b/NEWS @@ -45,4 +45,5 @@ Post-v3.1.0 interfaces that support it. See the 'status' column in the 'interface' table to check the status. + * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'. Nit: Might be better to not write NEWS in imperative form. It's not a git log, we're just saying what changed, not asking users to do something. I know that some entries are written this way, but I don't think that's a good thing to have. I get your point - I rewrote it for next version as - 'pmd-maxsleep' other_config was renamed to 'pmd-sleep-max'. - 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 ab493f9d4..9df481dd6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -4983,5 +4983,10 @@ 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); +if (smap_get_ullong(other_config, "pmd-maxsleep", 0)) { +VLOG_WARN("pmd-maxsleep is not supported. " + "Please use pmd-sleep-max instead."); +} + +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="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."]) @@ -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 1 usecs."]) OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based sleeps are enabled."]) @@ -1290,10