On Wed, Aug 14, 2013 at 7:21 PM, Ethan Jackson <et...@nicira.com> wrote:
> > - do not allow bfd to call bfd_may_decay when decay_min_rx > > is less than rmt_min_tx. > > Why? I don't think we should need this. If the rmt_min_tx is greater > than our min_rx, according to the bfd protocol we'll simply use the > rmt_min_tx as our rx interval (see bfd_rx_interval()). In other > words, I don't think correct from a feature design perspective to have > the min_rx value depend on the rmt_min_tx value. They're completely > independent. Thanks for the explanation. It makes sense, I'll drop the related changes. > > + diff = bfd_rx_packets(bfd) - bfd->rx_packets; > > + expect = (MAX(time_msec() - bfd->decay_last_detect_time, 2000) > > + / bfd->min_rx) + (2000 / bfd->cfg_min_rx + 1); > > All of this feels awfully fragile to me. Instead of estimating the > number of BFD packets we've received and subtracting it out. Why > don't we simply count the number of BFD packet's we've received in > bfd_process_packet()? I.E we ditch the bfd->decay_last_detect_time > field and instead every time bfd_process_packet() is called we > increment bfd->rx_packets. I like this idea. When seeing this, I remembered you once told me about this. Just forgot it over time. > Thus, bfd_rx_packets(bfd) - bfd->rx_packets won't include the bfd control > packets we've received. If that number is less than some threshold (giving > us some wiggle room), we go ahead and drop into decay. Note that it's > entirely possible that the diff will be negative in which case we drop into > decay. > However, still, there can be jitters in this "bfd_rx_packets(bfd) - bfd->rx_packets", since the stats at interface is updated asynchronously. For example, the control packets are counted by counter in "bfd_process_packet()", but not updated to the interface stats, at the time of calling "bfd_try_decay()". So it is possible that "bfd_rx_packets(bfd) - bfd->rx_packets" is smaller/greater than 0, even if there is no data traffic. As I said in comments, the maximum jitter of "bfd_rx_packets(bfd) - bfd->rx_packets" is approximately "(2000ms / bfd->cfg_min_rx + 1)", if there is only control packet. So the worst case is, we are receiving at min_rx=100ms, and we miss the update of entire 20 bfd control packets. And those 20 bfd control pkt counts goes to next call of "bfd_try_decay()". There are two solutions I can think of: 1. use "(2000 / bfd->cfg_min_rx + 10)" to compensate for the jitter. 2. if I can share the "push_timer" in ofproto-dpif.c with bfd.c then, I can tell exactly how much the jitter should be.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
