[ovs-dev] ofproto: Fix for frequent invalidation of mega flows for push actions
diff mbox series

Message ID 1570610190-30806-1-git-send-email-vishal.deep.ajmera@ericsson.com
State New
Headers show
Series
  • [ovs-dev] ofproto: Fix for frequent invalidation of mega flows for push actions
Related show

Commit Message

Sriram Vatala via dev Oct. 9, 2019, 8:36 a.m. UTC
When a packet is processed by the slow path and the matching OpenFlow
rule has actions like push_mpls/set_field and push_vlan/set_field, the
ofproto layer un-wildcards the MPLS and VLAN match fields in the megaflow
entry that it plans to install. However, when the megaflow entry is
actually installed, all protocol match fields that are not present in the
packet are wildcarded. Thus, the wildcard bits in the installed megaflow
entry could be different from the bits originally generated by the ofproto
layer.

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.

This patch fixes the issue by wildcarding flow fields which are not present
in the incoming packet.

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
---
 ofproto/ofproto-dpif-xlate.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Ben Pfaff Oct. 9, 2019, 6:26 p.m. UTC | #1
On Wed, Oct 09, 2019 at 02:06:30PM +0530, Vishal Deep Ajmera via dev wrote:
> When a packet is processed by the slow path and the matching OpenFlow
> rule has actions like push_mpls/set_field and push_vlan/set_field, the
> ofproto layer un-wildcards the MPLS and VLAN match fields in the megaflow
> entry that it plans to install. However, when the megaflow entry is
> actually installed, all protocol match fields that are not present in the
> packet are wildcarded. Thus, the wildcard bits in the installed megaflow
> entry could be different from the bits originally generated by the ofproto
> layer.
> 
> 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.
> 
> This patch fixes the issue by wildcarding flow fields which are not present
> in the incoming packet.
> 
> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>

Thanks, applied to master.
Sriram Vatala via dev Oct. 10, 2019, 4:30 a.m. UTC | #2
> Thanks, applied to master.

Thanks Ben.

Warm Regards,
Vishal Ajmera

Patch
diff mbox series

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 44f856d..f92cb62 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7343,6 +7343,26 @@  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