Thanks for writing these up. They look pretty good, thorough and easy to follow. Comments below.
The first test fails on my system. Does it pass on yours against master? I don't think we need to bother setting the datapath-id and the hwaddr on all the bridges at creation time. I wouldn't bother specifying the supported openflow protocols either. I think Ben's right about ignoring output. I think we could pretty easily write a macro which makes it easy to check the entire bfd/show output, without having to grep around for things. Find me and we'll discuss it offline. I think we could fill the first test out a bit. Something like: 1) check p1 and p0 are up 2) set bfd:enable=false on p1 3) check p1 is disabled, and p0 is down 4) set bfd:enable=true 5) check p1 and p0 are up 6) del-br br0 7) check p0 is missing and p1 is down 8) del-br br1 9) check p1 is missing The final line of the comment for Verify tunnel down detection has a redundant space before the word 'that'. How about we combine the "Verify tunnel down detection" and the "Verify one-side tunnel down detection" tests into one? They share the same setup code. After we check that dropping all flows has the behavior we want, we can bring the session back up and then drop traffic in one direction only. Ethan On Tue, Jun 25, 2013 at 12:59 PM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Jun 25, 2013 at 12:55:30PM -0700, Pavithra Ramesh wrote: >> Signed-off-by: Pavithra Ramesh <param...@vmware.com> > > This isn't a full review, but: there are tons of [ignore]s in this > patch. It's best not to ignore output. Please try to remove as many > as you can. > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
