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

Alex Wang alexw at nicira.com
Tue Aug 20 22:55:46 UTC 2013


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 <ethan at 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.
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130820/7c876901/attachment-0004.html>


More information about the dev mailing list