[ovs-dev] [PATCH V3 1/4] bfd: Implement BFD decay.

[Alex Wang]

On Wed, Aug 14, 2013 at 7:21 PM, Ethan Jackson <ethan at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130815/3f66b828/attachment-0003.html>