[ovs-dev] [PATCH] BFD: Unit tests for BFD. Add unit tests to verify basic functionality in BFD.

Ethan Jackson ethan at nicira.com
Tue Jun 25 21:15:25 UTC 2013


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 <blp at nicira.com> wrote:
> On Tue, Jun 25, 2013 at 12:55:30PM -0700, Pavithra Ramesh wrote:
>> Signed-off-by: Pavithra Ramesh <paramesh at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list