[ovs-dev,v5,1/2] ofproto-dpif-upcall: Reset ukey's last stats value if the datapath changed.

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c9f7179c3..aed2c8fbb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9616,6 +9616,7 @@  dpif_netdev_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
 const struct dpif_class dpif_netdev_class = {
     "netdev",
     true,                       /* cleanup_required */
+    true,                       /* synced_dp_layers */
     dpif_netdev_init,
     dpif_netdev_enumerate,
     dpif_netdev_port_open_type,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 026b0daa8..80225e9e7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4515,6 +4515,7 @@  dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t level, uint32_t size)
 const struct dpif_class dpif_netlink_class = {
     "system",
     false,                      /* cleanup_required */
+    false,                      /* synced_dp_layers */
     NULL,                       /* init */
     dpif_netlink_enumerate,
     NULL,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 12477a24f..b8ead8a02 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -127,6 +127,14 @@  struct dpif_class {
      * datapaths that can not exist without it (e.g. netdev datapath).  */
     bool cleanup_required;
 
+    /* If 'true' the specific dpif implementation synchronizes the various
+     * datapath implementation layers, i.e., the dpif's layer in combination
+     * with the underlying netdev offload layers. For example, dpif-netlink
+     * does not sync its kernel flows with the tc ones, i.e., only one gets
+     * installed. On the other hand, dpif-netdev installs both flows,
+     * internally keeps track of both, and represents them as one. */
+    bool synced_dp_layers;
+
     /* Called when the dpif provider is registered, typically at program
      * startup.  Returning an error from this function will prevent any
      * datapath with this class from being created.
diff --git a/lib/dpif.c b/lib/dpif.c
index fe4db83fb..5ed8aef17 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -2109,3 +2109,8 @@  dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size)
            ? dpif->dpif_class->cache_set_size(dpif, level, size)
            : EOPNOTSUPP;
 }
+
+bool dpif_synced_dp_layers(struct dpif *dpif)
+{
+    return dpif->dpif_class->synced_dp_layers;
+}
diff --git a/lib/dpif.h b/lib/dpif.h
index 6cb4dae6d..129cbf6a1 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -939,6 +939,7 @@  int dpif_get_pmds_for_port(const struct dpif * dpif, odp_port_t port_no,
 char *dpif_get_dp_version(const struct dpif *);
 bool dpif_supports_tnl_push_pop(const struct dpif *);
 bool dpif_supports_explicit_drop_action(const struct dpif *);
+bool dpif_synced_dp_layers(struct dpif *);
 
 /* Log functions. */
 struct vlog_module;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 31ac02d11..e24d18802 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -48,17 +48,20 @@ 
 
 #define UPCALL_MAX_BATCH 64
 #define REVALIDATE_MAX_BATCH 50
+#define UINT64_THREE_QUARTERS (UINT64_MAX / 4 * 3)
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
 
 COVERAGE_DEFINE(dumped_duplicate_flow);
 COVERAGE_DEFINE(dumped_new_flow);
 COVERAGE_DEFINE(handler_duplicate_upcall);
-COVERAGE_DEFINE(upcall_ukey_contention);
-COVERAGE_DEFINE(upcall_ukey_replace);
 COVERAGE_DEFINE(revalidate_missed_dp_flow);
+COVERAGE_DEFINE(ukey_dp_change);
+COVERAGE_DEFINE(ukey_invalid_stat_reset);
 COVERAGE_DEFINE(upcall_flow_limit_hit);
 COVERAGE_DEFINE(upcall_flow_limit_kill);
+COVERAGE_DEFINE(upcall_ukey_contention);
+COVERAGE_DEFINE(upcall_ukey_replace);
 
 /* A thread that reads upcalls from dpif, forwards each upcall's packet,
  * and possibly sets up a kernel flow as a cache. */
@@ -288,6 +291,7 @@  struct udpif_key {
 
     struct ovs_mutex mutex;                   /* Guards the following. */
     struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
+    const char *dp_layer OVS_GUARDED;         /* Last known dp_layer. */
     long long int created OVS_GUARDED;        /* Estimate of creation time. */
     uint64_t dump_seq OVS_GUARDED;            /* Tracks udpif->dump_seq. */
     uint64_t reval_seq OVS_GUARDED;           /* Tracks udpif->reval_seq. */
@@ -1771,6 +1775,7 @@  ukey_create__(const struct nlattr *key, size_t key_len,
     ukey->created = ukey->flow_time = time_msec();
     memset(&ukey->stats, 0, sizeof ukey->stats);
     ukey->stats.used = used;
+    ukey->dp_layer = NULL;
     ukey->xcache = NULL;
 
     ukey->offloaded = false;
@@ -2356,6 +2361,13 @@  revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
                     ? stats->n_bytes - ukey->stats.n_bytes
                     : 0);
 
+    if (stats->n_packets < ukey->stats.n_packets &&
+        ukey->stats.n_packets < UINT64_THREE_QUARTERS) {
+        /* Report cases where the packet counter is lower than the previous
+         * instance, but exclude the potential wrapping of an uint64_t. */
+        COVERAGE_INC(ukey_invalid_stat_reset);
+    }
+
     if (need_revalidate) {
         if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
             if (!ukey->xcache) {
@@ -2469,6 +2481,15 @@  push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)
             push->tcp_flags = stats->tcp_flags | op->ukey->stats.tcp_flags;
             push->n_packets = stats->n_packets - op->ukey->stats.n_packets;
             push->n_bytes = stats->n_bytes - op->ukey->stats.n_bytes;
+
+            if (stats->n_packets < op->ukey->stats.n_packets &&
+                op->ukey->stats.n_packets < UINT64_THREE_QUARTERS) {
+                /* Report cases where the packet counter is lower than the
+                 * previous instance, but exclude the potential wrapping of an
+                 * uint64_t. */
+                COVERAGE_INC(ukey_invalid_stat_reset);
+            }
+
             ovs_mutex_unlock(&op->ukey->mutex);
         } else {
             push = stats;
@@ -2773,6 +2794,22 @@  revalidate(struct revalidator *revalidator)
                 continue;
             }
 
+            ukey->offloaded = f->attrs.offloaded;
+            if (!ukey->dp_layer
+                || (!dpif_synced_dp_layers(udpif->dpif)
+                    && strcmp(ukey->dp_layer, f->attrs.dp_layer))) {
+
+                if (ukey->dp_layer) {
+                    /* The dp_layer has changed this is probably due to an
+                     * earlier revalidate cycle moving it to/from hw offload.
+                     * In this case we should reset the ukey stored statistics,
+                     * as they are from the deleted DP flow. */
+                    COVERAGE_INC(ukey_dp_change);
+                    memset(&ukey->stats, 0, sizeof ukey->stats);
+                }
+                ukey->dp_layer = f->attrs.dp_layer;
+            }
+
             already_dumped = ukey->dump_seq == dump_seq;
             if (already_dumped) {
                 /* The flow has already been handled during this flow dump
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 16a4c1a00..f47ded081 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -680,3 +680,63 @@  OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(5),le(check_pkt_len(size=100,gt(5),
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([offloads - offload flow to none-offload])
+OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+add in_port=ovs-p0,actions=ovs-p1
+add in_port=ovs-p1,actions=ovs-p0
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl
+recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.0s, actions:3
+recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.0s, actions:2
+])
+
+# Here we use an output action with truncate, which will force a kernel flow.
+AT_DATA([flows2.txt], [dnl
+modify in_port=ovs-p0,actions=output(port=ovs-p1, max_len=128)
+modify in_port=ovs-p1,actions=output(port=ovs-p0, max_len=128)
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows2.txt])
+AT_CHECK([ovs-appctl revalidator/wait], [0])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=ovs | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl
+recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:980, used:0.0s, actions:trunc(128),3
+recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:980, used:0.0s, actions:trunc(128),2
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl revalidator/wait], [0])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl
+recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.0s, actions:3
+recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.0s, actions:2
+])
+
+AT_CHECK([ovs-appctl coverage/read-counter ukey_invalid_stat_reset], [0], [dnl
+0
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP