Besides the unrealistic case of an overflow, stats->n_packets can become
less than ukey->stats.n_packets only when incorrect statistics were
read. For that reason, the condition that the code employed is a
workaround to avoid large jumps in statistics simply by assuming no
packets were read. However, if this happens while the revalidator is
under heavy load, the workaround has an unintended side effect where
should_revalidate returns false because the metric it calculates is
based on a bogus value.

Therefore, this patch makes it explicit that the workaround has been
triggered and consequently skipping should_revalidate assuming it should
have returned true. Note also that the workaround is only triggered
if stats->n_packets is _strictly_ less than ukey->stats.n_packets. Not
receiving any packets is a valid scenario where should_revalidate should
not be skipped.

Signed-off-by: Balazs Nemeth <[email protected]>
---
 ofproto/ofproto-dpif-upcall.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ad9635496..f0078e3c1 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2327,6 +2327,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key 
*ukey,
     OVS_REQUIRES(ukey->mutex)
 {
     bool need_revalidate = ukey->reval_seq != reval_seq;
+    bool stats_workaround;
     enum reval_result result = UKEY_DELETE;
     struct dpif_flow_stats push;
 
@@ -2334,7 +2335,8 @@ 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_workaround = stats->n_packets < ukey->stats.n_packets;
+    push.n_packets = (!stats_workaround
                       ? stats->n_packets - ukey->stats.n_packets
                       : 0);
     push.n_bytes = (stats->n_bytes > ukey->stats.n_bytes
@@ -2342,7 +2344,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key 
*ukey,
                     : 0);
 
     if (need_revalidate) {
-        if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
+        if (stats_workaround ||
+            should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
             if (!ukey->xcache) {
                 ukey->xcache = xlate_cache_new();
             } else {
-- 
2.37.3

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to