Thanks Ethan for the review. I'll add 'in_decay' flag and set bfd->min_rx to decay_min_rx, when receiving the FINAL packet.
On Mon, Aug 5, 2013 at 3:35 PM, Ethan Jackson <et...@nicira.com> wrote: > On Mon, Aug 5, 2013 at 2:04 PM, Alex Wang <al...@nicira.com> wrote: > > When there is no incoming data traffic at the interface for a period, > > BFD decay allows the bfd session to increase the min_rx. This is > > helpful in that some interfaces usually idle for long time. And cpu > > consumption can be reduced by processing fewer bfd control > > "interaces usually" => "interfaces are usually" or "interfaces may be" > > "for long" => "for a long" > > There's one major issue with this patch. In several places, it > directly sets bfd->min_rx to either bfd->decay_min_rx or > bfd->cfg_min_rx. This is very much not allowed. We have to give the > remote BFD session a chance to react to the fact that we're changing > our min_rx before we actually change it, otherwise it won't be sending > control packets quickly enough and we'll have flaps. > > I think we should add a flag to in_decay which is set to true if decay > mode has triggered and false if not. This line of code in bfd_poll() > should change: > > bfd->poll_min_rx = bfd->min_rx == bfd->decay_min_rx > ? bfd->decay_min_rx : bfd->cfg_min_rx; > > to > > bfd->poll_min_rx = bfd->in_decay ? bfd->decay_min_rx > : bfd->cfg_min_rx > > Then whenever the configuration changes, or we move from in_decay = > false to in_decay = true, we can call bfd_poll() and know that the > transition to the new min_rx value will happen safely. I think that > would also simplify the patch quite a bit. > > Ethan >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
