diff mbox series

[ovs-dev,2/6] pmd.at: Add macro for checking pmd sleep max time and state.

Message ID 20230614133644.1755282-3-ktraynor@redhat.com
State Superseded
Headers show
Series PMD load based sleep updates and per-pmd config. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Kevin Traynor June 14, 2023, 1:36 p.m. UTC
This is just cosmetic. There is no change to the tests.

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 tests/pmd.at | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

Comments

David Marchand June 14, 2023, 3:08 p.m. UTC | #1
On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> This is just cosmetic. There is no change to the tests.

Hum, except one issue in CHECK_DP_SLEEP_MAX, I would tend to agree :-).

>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
>  tests/pmd.at | 39 ++++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 374ad7217..64d8f6e2b 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -61,4 +61,20 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
>  ])
>
> +dnl CHECK_DP_SLEEP_MAX([max_sleep], [enabled], [+line])
> +dnl
> +dnl Checks correct pmd load based sleep is set for the datapath.
> +dnl Checking starts from line number 'line' in ovs-vswithd.log .
> +m4_define([CHECK_DP_SLEEP_MAX], [
> +    SLEEP_TIME="PMD max sleep request is $1 usecs."
> +    SLEEP_STATE="PMD load based sleeps are $2."
> +    line_st=$4

$3 ?


> +    if [[ -z "$line_st" ]]
> +    then
> +        line_st="+0"
> +    fi
> +    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_TIME"])
> +    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_STATE"])
> +])
> +
>  m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id \)[[0-9]]*:/\1<cleared>\2<cleared>:/"])
>  m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
> @@ -1256,46 +1272,39 @@ OVS_VSWITCHD_STOP
>  AT_CLEANUP
>
> -dnl Check default state
>  AT_SETUP([PMD - pmd sleep])
>  OVS_VSWITCHD_START
>
>  dnl Check default
> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD max sleep request is 0 usecs."])
> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load based sleeps are disabled."])
> +CHECK_DP_SLEEP_MAX([0], [disabled], [])
>
>  dnl Check low value max sleep
>  get_log_next_line_num

Not directly related to this patch.
I did not realise before when those tests were added, but it is
surprising get_log_next_line_num (from tests/alb.at) is visible from
these tests defined in pmd.at.
I would move this helper to a common file instead.


The rest lgtm.
Kevin Traynor June 20, 2023, 4:58 p.m. UTC | #2
On 14/06/2023 16:08, David Marchand wrote:
> On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>>
>> This is just cosmetic. There is no change to the tests.
> 
> Hum, except one issue in CHECK_DP_SLEEP_MAX, I would tend to agree :-).
> 
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>> ---
>>   tests/pmd.at | 39 ++++++++++++++++++++++++---------------
>>   1 file changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/pmd.at b/tests/pmd.at
>> index 374ad7217..64d8f6e2b 100644
>> --- a/tests/pmd.at
>> +++ b/tests/pmd.at
>> @@ -61,4 +61,20 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
>>   ])
>>
>> +dnl CHECK_DP_SLEEP_MAX([max_sleep], [enabled], [+line])
>> +dnl
>> +dnl Checks correct pmd load based sleep is set for the datapath.
>> +dnl Checking starts from line number 'line' in ovs-vswithd.log .
>> +m4_define([CHECK_DP_SLEEP_MAX], [
>> +    SLEEP_TIME="PMD max sleep request is $1 usecs."
>> +    SLEEP_STATE="PMD load based sleeps are $2."
>> +    line_st=$4
> 
> $3 ?
> 

ah yes :/ I missed changing from the one I based it on. I also fixed a 
test in 6/6 because of this.

> 
>> +    if [[ -z "$line_st" ]]
>> +    then
>> +        line_st="+0"
>> +    fi
>> +    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_TIME"])
>> +    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_STATE"])
>> +])
>> +
>>   m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id \)[[0-9]]*:/\1<cleared>\2<cleared>:/"])
>>   m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
>> @@ -1256,46 +1272,39 @@ OVS_VSWITCHD_STOP
>>   AT_CLEANUP
>>
>> -dnl Check default state
>>   AT_SETUP([PMD - pmd sleep])
>>   OVS_VSWITCHD_START
>>
>>   dnl Check default
>> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD max sleep request is 0 usecs."])
>> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load based sleeps are disabled."])
>> +CHECK_DP_SLEEP_MAX([0], [disabled], [])
>>
>>   dnl Check low value max sleep
>>   get_log_next_line_num
> 
> Not directly related to this patch.
> I did not realise before when those tests were added, but it is
> surprising get_log_next_line_num (from tests/alb.at) is visible from
> these tests defined in pmd.at.
> I would move this helper to a common file instead.
> 

ok. I put it near check_logs() in ofproto-macros.at. I can move if you 
think there's a better place. I'll send that as a separate patch outside 
this series as the patches here are not dependent on it for 
functionality or rebases.

> 
> The rest lgtm.
> 
>
diff mbox series

Patch

diff --git a/tests/pmd.at b/tests/pmd.at
index 374ad7217..64d8f6e2b 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -61,4 +61,20 @@  m4_define([CHECK_PMD_THREADS_CREATED], [
 ])
 
+dnl CHECK_DP_SLEEP_MAX([max_sleep], [enabled], [+line])
+dnl
+dnl Checks correct pmd load based sleep is set for the datapath.
+dnl Checking starts from line number 'line' in ovs-vswithd.log .
+m4_define([CHECK_DP_SLEEP_MAX], [
+    SLEEP_TIME="PMD max sleep request is $1 usecs."
+    SLEEP_STATE="PMD load based sleeps are $2."
+    line_st=$4
+    if [[ -z "$line_st" ]]
+    then
+        line_st="+0"
+    fi
+    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_TIME"])
+    OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_STATE"])
+])
+
 m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id \)[[0-9]]*:/\1<cleared>\2<cleared>:/"])
 m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
@@ -1256,46 +1272,39 @@  OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-dnl Check default state
 AT_SETUP([PMD - pmd sleep])
 OVS_VSWITCHD_START
 
 dnl Check default
-OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD max sleep request is 0 usecs."])
-OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load based sleeps are disabled."])
+CHECK_DP_SLEEP_MAX([0], [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"])
-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."])
+CHECK_DP_SLEEP_MAX([1], [enabled], [+$LINENUM])
 
 dnl Check high value max sleep
 get_log_next_line_num
 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."])
+CHECK_DP_SLEEP_MAX([10000], [enabled], [+$LINENUM])
 
 dnl Check setting max sleep to zero
 get_log_next_line_num
 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."])
+CHECK_DP_SLEEP_MAX([0], [disabled], [+$LINENUM])
 
 dnl Check above high value max sleep
 get_log_next_line_num
 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."])
+CHECK_DP_SLEEP_MAX([10000], [enabled], [+$LINENUM])
 
 dnl Check rounding
 get_log_next_line_num
 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."])
+CHECK_DP_SLEEP_MAX([490], [enabled], [+$LINENUM])
+
 dnl Check rounding
 get_log_next_line_num
 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."])
+CHECK_DP_SLEEP_MAX([499], [enabled], [+$LINENUM])
 
 OVS_VSWITCHD_STOP