[ovs-dev] [PATCH 14/26] ofproto-dpif-xlate: Factor wildcard processing out of xlate_actions().

[Ben Pfaff]

I think that this makes xlate_actions() easier to read.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif-xlate.c | 102 ++++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 44 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8acb908..03bca1b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4717,6 +4717,58 @@ too_many_output_actions(const struct ofpbuf *odp_actions OVS_UNUSED)
 #endif
 }
 
+static void
+xlate_wc_init(struct xlate_ctx *ctx)
+{
+    flow_wildcards_init_catchall(ctx->wc);
+
+    /* Some fields we consider to always be examined. */
+    memset(&ctx->wc->masks.in_port, 0xff, sizeof ctx->wc->masks.in_port);
+    memset(&ctx->wc->masks.dl_type, 0xff, sizeof ctx->wc->masks.dl_type);
+    if (is_ip_any(&ctx->xin->flow)) {
+        ctx->wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
+    }
+
+    if (ctx->xbridge->support.odp.recirc) {
+        /* Always exactly match recirc_id when datapath supports
+         * recirculation.  */
+        ctx->wc->masks.recirc_id = UINT32_MAX;
+    }
+
+    if (ctx->xbridge->netflow) {
+        netflow_mask_wc(&ctx->xin->flow, ctx->wc);
+    }
+
+    tnl_wc_init(&ctx->xin->flow, ctx->wc);
+}
+
+static void
+xlate_wc_finish(struct xlate_ctx *ctx)
+{
+    /* Clear the metadata and register wildcard masks, because we won't
+     * use non-header fields as part of the cache. */
+    flow_wildcards_clear_non_packet_fields(ctx->wc);
+
+    /* ICMPv4 and ICMPv6 have 8-bit "type" and "code" fields.  struct flow
+     * uses the low 8 bits of the 16-bit tp_src and tp_dst members to
+     * represent these fields.  The datapath interface, on the other hand,
+     * represents them with just 8 bits each.  This means that if the high
+     * 8 bits of the masks for these fields somehow become set, then they
+     * will get chopped off by a round trip through the datapath, and
+     * revalidation will spot that as an inconsistency and delete the flow.
+     * Avoid the problem here by making sure that only the low 8 bits of
+     * either field can be unwildcarded for ICMP.
+     */
+    if (is_icmpv4(&ctx->xin->flow) || is_icmpv6(&ctx->xin->flow)) {
+        ctx->wc->masks.tp_src &= htons(UINT8_MAX);
+        ctx->wc->masks.tp_dst &= htons(UINT8_MAX);
+    }
+    /* VLAN_TCI CFI bit must be matched if any of the TCI is matched. */
+    if (ctx->wc->masks.vlan_tci) {
+        ctx->wc->masks.vlan_tci |= htons(VLAN_CFI);
+    }
+}
+
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
  * 'xout'.
  * The caller must take responsibility for eventually freeing 'xout', with
@@ -4782,9 +4834,11 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     };
     memset(&ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
     ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
+    if (xin->wc) {
+        xlate_wc_init(&ctx);
+    }
 
     struct xport *in_port;
-    bool tnl_may_send;
 
     COVERAGE_INC(xlate_actions);
 
@@ -4809,26 +4863,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
      *   kernel does.  If we wish to maintain the original values an action
      *   needs to be generated. */
 
-    if (xin->wc) {
-        flow_wildcards_init_catchall(ctx.wc);
-        memset(&ctx.wc->masks.in_port, 0xff, sizeof ctx.wc->masks.in_port);
-        memset(&ctx.wc->masks.dl_type, 0xff, sizeof ctx.wc->masks.dl_type);
-        if (is_ip_any(flow)) {
-            ctx.wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
-        }
-        if (xbridge->support.odp.recirc) {
-            /* Always exactly match recirc_id when datapath supports
-             * recirculation.  */
-            ctx.wc->masks.recirc_id = UINT32_MAX;
-        }
-        if (xbridge->netflow) {
-            netflow_mask_wc(flow, ctx.wc);
-        }
-        tnl_wc_init(flow, xin->wc);
-    }
-
-    tnl_may_send = tnl_process_ecn(flow);
-
     /* The in_port of the original packet before recirculation. */
     in_port = get_ofp_port(xbridge, flow->in_port.ofp_port);
 
@@ -4968,7 +5002,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         }
         size_t sample_actions_len = ctx.odp_actions->size;
 
-        if (tnl_may_send && (!in_port || may_receive(in_port, &ctx))) {
+        if (tnl_process_ecn(flow)
+            && (!in_port || may_receive(in_port, &ctx))) {
             const struct ofpact *ofpacts;
             size_t ofpacts_len;
 
@@ -5079,28 +5114,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     }
 
     if (xin->wc) {
-        /* Clear the metadata and register wildcard masks, because we won't
-         * use non-header fields as part of the cache. */
-        flow_wildcards_clear_non_packet_fields(ctx.wc);
-
-        /* ICMPv4 and ICMPv6 have 8-bit "type" and "code" fields.  struct flow
-         * uses the low 8 bits of the 16-bit tp_src and tp_dst members to
-         * represent these fields.  The datapath interface, on the other hand,
-         * represents them with just 8 bits each.  This means that if the high
-         * 8 bits of the masks for these fields somehow become set, then they
-         * will get chopped off by a round trip through the datapath, and
-         * revalidation will spot that as an inconsistency and delete the flow.
-         * Avoid the problem here by making sure that only the low 8 bits of
-         * either field can be unwildcarded for ICMP.
-         */
-        if (is_icmpv4(flow) || is_icmpv6(flow)) {
-            ctx.wc->masks.tp_src &= htons(UINT8_MAX);
-            ctx.wc->masks.tp_dst &= htons(UINT8_MAX);
-        }
-        /* VLAN_TCI CFI bit must be matched if any of the TCI is matched. */
-        if (ctx.wc->masks.vlan_tci) {
-            ctx.wc->masks.vlan_tci |= htons(VLAN_CFI);
-        }
+        xlate_wc_finish(&ctx);
     }
 
 exit:
-- 
2.1.3