push.n_{packets,bytes} contains the difference in the number of packets
and bytes. Even if the counters are uint64_t, they can and do still
overflow. In particular, this happens with high throughput traffic while
running for a long enough time.
The old code used to just set n_{packets,bytes} to 0. Consequently,
should_revalidate would return false (assuming the dump took longer
than ofproto_max_revalidator / 2) since the metric would be incorrectly
calculated to be too large.
The module arithmatic behind a simple subtraction will take care of the
overflow case. For that reason, no conditional is needed.
---
ofproto/ofproto-dpif-upcall.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ad9635496..f6584af11 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2334,12 +2334,10 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
push.used = stats->used;
push.tcp_flags = stats->tcp_flags;
- push.n_packets = (stats->n_packets > ukey->stats.n_packets
- ? stats->n_packets - ukey->stats.n_packets
- : 0);
- push.n_bytes = (stats->n_bytes > ukey->stats.n_bytes
- ? stats->n_bytes - ukey->stats.n_bytes
- : 0);
+ /* In case lhs of the subtraction overflowed since the last check,
+ the subtraction is still correct */
+ push.n_packets = stats->n_packets - ukey->stats.n_packets;
+ push.n_bytes = stats->n_bytes - ukey->stats.n_bytes;
if (need_revalidate) {
if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
--
2.37.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
