[ovs-dev] [PATCH 1/1] tests: Add PMD auto load balance unit tests.
ktraynor at redhat.com
Fri Mar 12 18:20:14 UTC 2021
On 12/03/2021 13:21, Pai G, Sunil wrote:
> Hey Kevin,
> Tested the patch , works fine for me too :)
>> The ALB log messages follow a common format, so I'd suggest combining
>> those 3 helpers as a single.
>> That is with the hope we will conform to this format for new parameters
>> dnl CHECK_ALB_PARAM([param], [value], [+line]) dnl dnl Waits for ALB logs
>> for 'param' in logs and checks if value matches dnl 'value'. Checking starts
>> from line number 'line' in ovs-vswithd.log.
>> m4_define([CHECK_ALB_PARAM], [
>> if [[ -z "$line_st" ]]
>> 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], , [dnl PMD auto load balance $1 set
>> to $2
> I agree with David on this one. It might be good to create a helper like CHECK_LOAD_PARAM.
>>> +AT_SETUP([ALB - interval param])
>>> +OVS_WAIT_UNTIL([grep "PMD auto load balance is disabled"
>>> +# Check default
>>> +CHECK_INTERVAL_PARAM(, )
>> Here, it becomes:
>> CHECK_ALB_PARAM([interval], [1 mins], )
>>> +# TODO 20000 is documented as max value but it seems arbitary #
>>> +Setting to 20001 works, should it be checked and rejected?
>>> +# For now add a dummy test above the max value that accepts it
>> Afaiu, from ovsdb pov, other_config is a simple map of strings with no
>> constraint on the content of the strings.
>> I understand the constraints expressed for sub fields like pmd-auto-lb-XX
>> parameters in vswitchd/vswitchd.xml as only for documentation.
>> I don't see the benefit of checking values 20000 or 20001.
> I couldn't find the doc where the max value is mentioned.
> Would you mind pointing it ?
> Or its calculated somehow ?
Thanks Sunil (and David). The max is mentioned here:
We probably need to think a bit more about the units and max for this
parameter in general, but that's what's currently documented.
> The rest looks to good to me!
More information about the dev