Thanks Ethan, for the review,
On Mon, Aug 5, 2013 at 3:58 PM, Ethan Jackson <et...@nicira.com> wrote: > Thanks for writing this up, it's pretty close. > > In bfd_check_rx() the assertion makes me uncomfortable. I'd prefer we > warn and reset bfd->rx_packets instead. Im worried that we could hit > the assertion transiently during a configuration change. > I see, I'll fix this. I'd prefer we don't use the bfd->detect_time for this. It has a very > specific meaning which I'd rather not overload. Let's add a new timer > bfd->rx_detect_time used specifically for this feature? Could the bfd->state <= DOWN && bfd->forwarding_if_rx check be moved > into the bfd_check_rx() function? > > Actually come to think of it, we shouldn't need to check if bfd->state > <=DOWN at all. Consider a tunnel which is up and happily receiving > data traffic. If we suddenly have congestion and lose our BFD > packets, we'd have to transition into a DOWN state before this > forwarding_if_rx feature would kick in. > This makes sense. I'll adjust accordingly,
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
