> 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
