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 >> later. >> >> 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], [ >> line_st=$3 >> if [[ -z "$line_st" ]] >> then >> 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 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_VSWITCHD_START >>> +OVS_WAIT_UNTIL([grep "PMD auto load balance is disabled" >>> +ovs-vswitchd.log]) >>> + >>> +# Check default >>> +CHECK_INTERVAL_PARAM([1], []) >> >> Here, it becomes: >> CHECK_ALB_PARAM([interval], [1 mins], []) >> >> etc.. >> >> [snip] >> >>> +# 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: https://github.com/openvswitch/ovs/blob/master/vswitchd/vswitch.xml#L677-L679 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. Kevin. > The rest looks to good to me! > > <snip> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
