[ovs-dev] [PATCH v2 2/2] ovn: Test for full logical flow processing in ovn-controller

Han Zhou zhouhan at gmail.com
Fri Jun 1 16:21:28 UTC 2018


On Fri, Jun 1, 2018 at 6:49 AM, Jakub Sitnicki <jkbs at redhat.com> wrote:
>
> On Thu, 31 May 2018 10:14:50 -0700
> Han Zhou <zhouhan at gmail.com> wrote:
>
> > > +# vec_sub VEC_A VEC_B
> > > +#
> > > +# Subtracts two vectors:
> > > +#
> > > +#     VEC_A = [a1, a2, ...]
> > > +#     VEC_B = [b1, b2, ...]
> > > +#     OUT = [(a1 - b1), (a2 - b2), ...]
> > > +#
> > > +# VEC_A and VEC_B must be lists of values separated by a character
from
> > $IFS.
> > > +vec_sub() {
> > > +    local a b i j
> > > +
> > > +    i=0
> > > +    for a in $1; do
> > > +        j=0
> > > +        for b in $2; do
> > > +            if test $i -eq $j; then
> > > +                expr $a - $b
> > > +                break
> > > +            fi
> > > +            j=`expr $j + 1`
> > > +        done
> > > +        i=`expr $i + 1`
> > > +    done
> >
> > The loop is O(n^2) while ideally it can be O(n). I think it is not a big
> > deal since it is testing, and I don't have better idea how to achieve
O(n)
> > while keeping the current convenient input parameters. Just to confirm
it
> > is not supposed to be used in scalability testing environment to
calculate
> > counters for thousands of sandboxes, right?
>
> Oh, no, I would not be brave enough to go testing 1000's of sandboxes
> with shell scripts. For that scale I would rewrite the whole test to
> Python probably.
>
> This is the best I could come up with, w/o going into Bash arrays. It
> is intended only for the scale we test at in the automated test suite.
> So just a few sandboxes as most.
>
Agree. It is totally fine for this test suite.

How about my other comment:
> > > +    # Bind port $lp
> > +    OVN_CONTROLLER_EXPECT_HIT(

> > For port binding, we should expect recompute on the HV where the port
is bound to, and expect no recompute on all other HVs.

> > +        [hv1 hv2], [lflow_run],
> > +        [as $hv ovs-vsctl add-port br-int $vif -- set Interface $vif
external-ids:iface-id=$lp]
> > +    )

For example, after binding on hv1, we should expect lflow_run counter
increased on hv1, but not on hv2. We want to ensure incremental
port-binding processing is taking effect on hv2. OVN_CONTROLLER_EXPECT_HIT
checks the total count, but doesn't tell the differences for each HV. I
think we can split this test into two.

And I just thought about another problem in this port-binding test. It is
better to combine with the wait-until ... up="true" test, and do a
ovn-nbctl --wait=hv sync, so that we are sure the ovn-controller already
processed the port-binding. Otherwise, the test could easily fail due to
timing issue.

BTW, can I fold your patch into my incremental patch series so that I can
use it and add more test cases on top of it? I am not sure about the
practice in this community when two or more people are working on patches
that depends on each other. Can I just keep everything in your patch
(including Signed-off-by) and also add a comment that the patch is from
you, so that when the committer (most likely blp) will commit with the
correct Author in github?

Thanks,
Han


More information about the dev mailing list