[ovs-dev] [PATCH V2 2/4] bfd: Implements forwarding_if_rx

[Ethan Jackson]

I think this would be a bit cleaner if we ditched the has_rx flag.
Instead, each time we go through the run loop, we can check if we've
received any packets, if we have, we reset to rx_detect_time to now +
bfd_min_rx * mult.  Then in bfd_forwarding() when forwarding_if_rx is
enabled, we only have to check if the rx_detect_time has passed or
not.

> +    if (diff < 0) {
> +        VLOG_WARN("rx_packets count is smaller than last time.");
> +    }
> +    bfd->rx_packets = rx_packets;
> +    bfd->has_rx = (diff > 0);
> +    incr = bfd_rx_interval(bfd) * bfd->mult;
> +    bfd->rx_detect_time = (incr > 2000 ? incr : 2000) + time_msec();

This would be easier to read as MAX(incr, 2000).  Also it deserves a
comment explaining why we're setting 2000 as the minimum
rx_detect_time.  That number seems awfully high to me at any rate, I'd
feel better if we set it to 1000, and made (in a separate patch) made
ofproto-dpif pull stats from the datapath at least once every 800ms so
we have time to grab our stats and update the bfd module.

Ethan
X-CudaMail-Whitelist-To: dev at openvswitch.org