[ovs-dev] [PATCH v2] tests: Add PMD auto load balance unit tests.

Kevin Traynor ktraynor at redhat.com
Thu Jun 24 13:05:43 UTC 2021


On 23/06/2021 21:49, Ilya Maximets wrote:
> On 3/16/21 4:45 PM, Kevin Traynor wrote:
>> These tests focus on enabling/disabling and user parameters.
>>
>> Co-Authored-by: David Marchand <david.marchand at redhat.com>
>> Signed-off-by: David Marchand <david.marchand at redhat.com>
>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>>
>> ---
>> v2:
>> - Remove above max documented interval test
>> - Add David's code to combine param checks and add as co-author
>> ---
>>  tests/alb.at       | 218 +++++++++++++++++++++++++++++++++++++++++++++
>>  tests/automake.mk  |   1 +
>>  tests/testsuite.at |   1 +
>>  3 files changed, 220 insertions(+)
>>  create mode 100644 tests/alb.at
> 
> Hi, Kevin.  While testing these tests I noticed one thing:
> 
> get_log_line_num() returns current line and not the next one, so if log
> didn't change, several subsequent get_log_line_num + OVS_WAIT_UNTIL
> will succeed.  Meaning that it maybe unreliable to test for the same
> text in a log two times in a row with some command in-between, because
> command may return faster than logs printed to a file and the check
> will be performed with the previous line in a log.  Suggesting to
> increase the line number by one to avoid that.  I understand that you
> ported this part from the pmd.at, so we, probably, need to fix that
> there too in a separate change.
> 

Hi Ilya, that makes sense. Even if it was ok for the current tests due
to the particular logs, there is a danger that it would be reused again
and cause an issue then. I will update here and send a patch for pmd.at.

> Suggesting following incremental:
> 
> diff --git a/tests/alb.at b/tests/alb.at
> index 0ea1bbdd1..1331b742c 100644
> --- a/tests/alb.at
> +++ b/tests/alb.at
> @@ -3,7 +3,7 @@ AT_BANNER([PMD Auto Load Balance])
>  m4_divert_push([PREPARE_TESTS])
>  
>  get_log_line_num () {
> -    LINENUM=$(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])
> +    LINENUM=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1))
>  }
>  
>  m4_divert_pop([PREPARE_TESTS])
> @@ -21,7 +21,8 @@ m4_define([CHECK_ALB_PARAM], [
>          line_st="+0"
>      fi
>      OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "PMD auto load balance $1 set to"])
> -    AT_CHECK([tail -n $line_st ovs-vswitchd.log | sed -n "s#.*\(PMD auto load balance $1 set to.*\)#\1#p" | tail -1], [0], [dnl
> +    AT_CHECK([tail -n $line_st ovs-vswitchd.log dnl
> +                | sed -n "s#.*\(PMD auto load balance $1 set to.*\)#\1#p" | tail -1], [0], [dnl
>  PMD auto load balance $1 set to $2
>  ])
>  ])
> @@ -107,7 +108,9 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance
>  get_log_line_num
>  AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="false"])
>  OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
> +get_log_line_num
>  AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin])
> +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'roundrobin'"])
>  get_log_line_num
>  AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
>  OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
> ---
> 
> What do you think?
> The check around 'roundrobin' is just in case, to be sure that
> log actually updated.
> 

Yeah, good idea. In earlier cases it is changing between enable/disable,
but I also added similar check on assignment mode change, just to be
explicit that it is the reason.

thanks,
Kevin.

> Best regards, Ilya Maximets.
> 



More information about the dev mailing list