On Fri, 1 Jun 2018 10:50:07 -0700 Han Zhou <zhou...@gmail.com> wrote:
> On Fri, Jun 1, 2018 at 10:03 AM, Jakub Sitnicki <j...@redhat.com> wrote: > > > > On Fri, 1 Jun 2018 09:21:28 -0700 > > Han Zhou <zhou...@gmail.com> wrote: > > > > > 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. > > > > Apologies, I've missed your other comment. > > > > Yes, I saw what you're describing in with a PoC script I did before > > rewriting it in Autotest: > > > > > https://github.com/jsitnicki/tools/blob/master/net/ovn/example_count_lflow_run.txt > > > > I was considering making the EXPECT_HIT check less coarse but decided to > > keep it simple at first. It's certainly something I would like to > > improve. > > > > > 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. > > > > Good point, I did not think of that. Having the EXPECT_{HIT,NO_HIT} > > accept a batch of commands could be doable with some m4_foreach magic. > > I will need to give it a try. > > Shouldn't ";" separated commands as the "cmd" parameter just work? Great idea! I will try it out. The less m4 macros the better. ;-) > > > > > > 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? > > > > Yes, sure. Feel free to pull these patches into your series. I'm > > glad they will be of use. > > > > Meanwhile, I will try to add the two enhancements we're discussing > > here. > > > > That's great, thanks! > I just sent out v3 of my series with address set and port group incremental > processing: > https://patchwork.ozlabs.org/project/openvswitch/list/?series=48060 > I will wait for your enhancements and fold your patch in v4. OK, sounds good. Thanks, Jakub > > Thanks, > Han _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev