Thanks for the review,
Just talked with Shih-Hao, right now, nvp expose:
"tunnel_rx_state" using "bfd: local_state"
"tunnel_tx_state" using "bfd: local_state && remote_state"
The "forwarding" flag is not imported and used by nvp.
So, based on your comment, it seems to me that there may be the
need to import this flag. Since it is possible that both bfd: local_state
and remote_state are down, when forwarding_if_rx is used.
Thanks,
On Wed, Nov 13, 2013 at 6:22 PM, Ethan Jackson <et...@nicira.com> wrote:
> At a high level, I don't think this patch is tracking the right thing.
> We don't actually care how many times the bfd state changes. What we
> actually care about is how many times the result of bfd_forwarding()
> changes. That's because bfd_forwarding() is the function which
> actually changes how traffic bounces around.
>
> > diff --git a/lib/bfd.c b/lib/bfd.c
> > index 740f4fc..f34b895 100644
> > --- a/lib/bfd.c
> > +++ b/lib/bfd.c
> > @@ -206,6 +206,8 @@ struct bfd {
> > /* detect interval. */
> > uint64_t decay_rx_packets; /* Packets received by 'netdev'. */
> > long long int decay_detect_time; /* Decay detection time. */
> > +
> > + uint64_t flap_count; /* Counts the flapping. */
>
> This comment could be better. Perhaps "Counts bfd forwarding flaps"?
>
> > + flap_count = xasprintf("%"PRIu64, bfd->flap_count);
> > + smap_add(smap, "flap_count", flap_count);
> > + free(flap_count);
>
> Could you use smap_add_format() instead?
>
> > + /* If there is a flap, increments the counter. */
> > + if (bfd->state == STATE_DOWN || state == STATE_DOWN) {
>
> Ethan
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
