On Fri, Jun 1, 2018 at 6:49 AM, Jakub Sitnicki <j...@redhat.com> wrote:
>
> On Thu, 31 May 2018 10:14:50 -0700
> Han Zhou <zhou...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to