[ovs-dev] [PATCH] ofproto: Fix for frequent invalidation of flows due to mismatch in mask bits

[Vishal Deep Ajmera]

The wildcard bits in the installed megaflow entry could be different from
the bits originally generated by the ofproto layer. Datapath implementation
wildcards those match fields which are not present in the incoming packet
before installing the flow.

When the revalidator thread validates a megaflow, it will first query the
ofproto layer to get the wildcard bits and then compare it against the
wildcard bits in the megaflow. If the bits are different the entry will be
removed. A subsequent packet will again result in the same megaflow entry
being installed only for it to be removed by the revalidator thread. This
cycle will continue and will significantly degrade performance.

An earlier patch fixing the issue for MPLS and VLAN was sent.
However similar problem now appears for IPv6 datapath flows.

This patch addresses the issue in a generic way.

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera at ericsson.com>
---
 ofproto/ofproto-dpif-upcall.c | 14 +++++++++++++-
 ofproto/ofproto-dpif-xlate.c  | 20 --------------------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 409286a..1371486 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2169,7 +2169,7 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey,
 {
     struct xlate_out *xoutp;
     struct netflow *netflow;
-    struct flow_wildcards dp_mask, wc;
+    struct flow_wildcards dp_mask, wc, wc_from_flow;
     enum reval_result result;
     struct reval_context ctx = {
         .odp_actions = odp_actions,
@@ -2215,6 +2215,18 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey,
         goto exit;
     }
 
+    /* Clear flow wildcard bits for fields which are not present
+     * in the original packet header. These wildcards may get set
+     * due to push/set_field actions. This results into frequent
+     * invalidation of datapath flows by revalidator thread. */
+
+    /* Create wc mask based on incoming packet. */
+    flow_wildcards_init_for_packet(&wc_from_flow,
+                                   &ctx.flow);
+
+    /* Clear mask fields in ctx which are not relevant for packet. */
+    flow_wildcards_and(ctx.wc, &wc_from_flow, ctx.wc);
+
     /* Do not modify if any bit is wildcarded by the installed datapath flow,
      * but not the newly revalidated wildcard mask (wc), i.e., if revalidation
      * tells that the datapath flow is now too generic and must be narrowed
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4407f9c..42fdb9f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7363,26 +7363,6 @@ xlate_wc_finish(struct xlate_ctx *ctx)
         ctx->wc->masks.tp_src = 0;
         ctx->wc->masks.tp_dst = 0;
     }
-
-    /* Clear flow wildcard bits for fields which are not present
-     * in the original packet header. These wildcards may get set
-     * due to push/set_field actions. This results into frequent
-     * invalidation of datapath flows by revalidator thread. */
-
-    /* Clear mpls label wc bits if original packet is non-mpls. */
-    if (!eth_type_mpls(ctx->xin->upcall_flow->dl_type)) {
-        for (i = 0; i < FLOW_MAX_MPLS_LABELS; i++) {
-            ctx->wc->masks.mpls_lse[i] = 0;
-        }
-    }
-    /* Clear vlan header wc bits if original packet does not have
-     * vlan header. */
-    for (i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
-        if (!eth_type_vlan(ctx->xin->upcall_flow->vlans[i].tpid)) {
-            ctx->wc->masks.vlans[i].tpid = 0;
-            ctx->wc->masks.vlans[i].tci = 0;
-        }
-    }
 }
 
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
-- 
1.9.1