Np, i'll adjust accordingly and send a v2 soon,
with the conjunction test.
On Tue, Aug 20, 2013 at 3:53 PM, Ethan Jackson <et...@nicira.com> wrote:
> > I don't understand. I have used a separate decay_rx_packets to record the
> > rx_packets stats for bfd decay. So, rx_packets is not shared.
>
> hmm I think you're right. I must have messed up rebasing.
>
> Ethan
>
> >
> > But I'll definitely add an unit test running the two together.
> >
> >
> >>
> >> > + uint64_t decay_rx_ctrl; /* Count bfd packets received
> within
> >> > decay */
> >> > /* detect interval. */
> >> > + uint64_t decay_rx_packets; /* Packets received by 'netdev'. */
> >>
> >> decay_rx_packets is unused. Also, if changing the type of
> >> decay_rx_ctl is necessary, it should probably be in a separate patch.
> >>
> >
> >
> > This is a typo. It is not necessary to change the type of decay_rx_ctl.
> > Also, I think I use the decay_rx_packets in the bfd_try_decay() and
> > bfd_decay_update().
> >
> >
> >>
> >>
> >> > @@ -753,6 +776,7 @@ bfd_set_netdev(struct bfd *bfd, const struct
> netdev
> >> > *netdev)
> >> > if (bfd->decay_min_rx) {
> >> > bfd_decay_update(bfd);
> >> > }
> >> > + bfd->rx_packets = bfd_rx_packets(bfd);
> >>
> >> we should probably reset the detect time here as well.
> >>
> >>
> >> > + if (diff < 0) {
> >> > + VLOG_WARN("rx_packets count is smaller than last time.");
> >>
> >> It's probably worth rate limiting this log message. I suspect if it
> >> happens it might happen frequently.
> >>
> >
> >
> > I'll adjust accordingly. Thanks,
> >
> >
> >>
> >>
> >> I think the forwarding field in the database bfd_status column is far
> >> more important than that in the appctl output which is mostly for
> >> developers to debug things. I'd rewrite this and the commit message
> >> to mention that instead. Also both here and in the comment message, I
> >> would talk about "interfaces" instead of "tunnels". BFD can be run
> >> on any kind of interface, so there's no reason to restrict it in the
> >> documentation.
> >>
> >
> > Thanks a lot, I agree (especially after I know how it will affect the
> bundle
> > ports). I'll redo them accordingly.
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
