[ovs-dev] [PATCH v3 1/6] dpif-netdev: Rename pmd-maxsleep config option.

2023-07-10 Thread Kevin Traynor 
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.

2023-07-13 Thread Ilya Maximets 
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.

2023-07-14 Thread Kevin Traynor 

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