push_dp_ops only handles delete ops errors but ignores the modify
ops results. It's better to handle all the dp operation errors in
a consistent way.

We observe in the production environment that sometimes a megaflow
with wrong actions keep staying in datapath. The coverage command shows
revalidators have dumped several times, however the correct
actions are not set. This implies that the ukey's action does not
equal to the meagaflow's, i.e. revalidators think the underlying
megaflow's actions are correct however they are not.

We also check the megaflow using the ofproto/trace command, and the
actions are not matched with the ones in the actual magaflow. By
performing a revalidator/purge command, the right actions are set.

This patch prevents the inconsistency by considering modify failure
in revalidators.

Signed-off-by: Peng He <[email protected]>
---
 ofproto/ofproto-dpif-upcall.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 7ad728adf..7ea2a30f5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2416,23 +2416,26 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, 
size_t n_ops)
 
     for (i = 0; i < n_ops; i++) {
         struct ukey_op *op = &ops[i];
-        struct dpif_flow_stats *push, *stats, push_buf;
-
-        stats = op->dop.flow_del.stats;
-        push = &push_buf;
-
-        if (op->dop.type != DPIF_OP_FLOW_DEL) {
-            /* Only deleted flows need their stats pushed. */
-            continue;
-        }
 
         if (op->dop.error) {
-            /* flow_del error, 'stats' is unusable. */
             if (op->ukey) {
                 ovs_mutex_lock(&op->ukey->mutex);
                 transition_ukey(op->ukey, UKEY_EVICTED);
                 ovs_mutex_unlock(&op->ukey->mutex);
             }
+            /* if it's a flow_del error, 'stats' is unusable, it's ok
+             * to discard the stats.
+             */
+            continue;
+        }
+
+        struct dpif_flow_stats *push, *stats, push_buf;
+
+        stats = op->dop.flow_del.stats;
+        push = &push_buf;
+
+        if (op->dop.type != DPIF_OP_FLOW_DEL) {
+            /* Only deleted flows need their stats pushed. */
             continue;
         }
 
-- 
2.25.1

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

Reply via email to