[ovs-dev] [PATCH 2/2] ofproto-dpif-xlate: Clarify recirculation in do_xlate_actions().

Jarno Rajahalme jarno at ovn.org
Fri Jan 29 23:38:12 UTC 2016


Handle implicit recirculation explicitly for each action type, so that
it is easier to follow what is happening.

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 164 +++++++++++++++++++++++++------------------
 1 file changed, 97 insertions(+), 67 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1edc1b0..54ee73f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -354,12 +354,6 @@ ctx_trigger_recirculation(struct xlate_ctx *ctx)
     ctx->recirc_action_offset = ctx->action_set.size;
 }
 
-static bool
-ctx_first_recirculation_action(const struct xlate_ctx *ctx)
-{
-    return ctx->recirc_action_offset == ctx->action_set.size;
-}
-
 static inline bool
 exit_recirculates(const struct xlate_ctx *ctx)
 {
@@ -4247,16 +4241,6 @@ recirc_unroll_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
     }
 }
 
-#define CHECK_MPLS_RECIRCULATION()      \
-    if (ctx->was_mpls) {                \
-        ctx_trigger_recirculation(ctx); \
-        break;                          \
-    }
-#define CHECK_MPLS_RECIRCULATION_IF(COND) \
-    if (COND) {                           \
-        CHECK_MPLS_RECIRCULATION();       \
-    }
-
 static void
 put_ct_mark(const struct flow *flow, struct flow *base_flow,
             struct ofpbuf *odp_actions, struct flow_wildcards *wc)
@@ -4413,12 +4397,20 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
 }
 
 static void
+xlate_implicit_recirculation(const struct ofpact *ofpacts, size_t ofpacts_len,
+                             struct xlate_ctx *ctx)
+{
+    ctx_trigger_recirculation(ctx);
+    recirc_unroll_actions(ofpacts, ofpacts_len, ctx);
+}
+
+static void
 do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
                  struct xlate_ctx *ctx)
 {
     struct flow_wildcards *wc = ctx->wc;
     struct flow *flow = &ctx->xin->flow;
-    const struct ofpact *a;
+    const struct ofpact *a, * const end = ofpact_end(ofpacts, ofpacts_len);
 
     if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
         tnl_neigh_snoop(flow, wc, ctx->xbridge->name);
@@ -4426,6 +4418,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
     /* dl_type already in the mask, not set below. */
 
     OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+        size_t rem_len = (uint8_t *)end - (uint8_t *)a;
         struct ofpact_controller *controller;
         const struct ofpact_metadata *metadata;
         const struct ofpact_set_field *set_field;
@@ -4436,13 +4429,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         }
 
         if (ctx->exit) {
-            /* Check if need to store the remaining actions for later
-             * execution. */
             if (exit_recirculates(ctx)) {
-                recirc_unroll_actions(a, OFPACT_ALIGN(ofpacts_len -
-                                                      ((uint8_t *)a -
-                                                       (uint8_t *)ofpacts)),
-                                      ctx);
+                /* Remaining actions must be executed after recirculation. */
+                recirc_unroll_actions(a, rem_len, ctx);
             }
             break;
         }
@@ -4451,6 +4440,10 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         case OFPACT_OUTPUT:
             xlate_output_action(ctx, ofpact_get_OUTPUT(a)->port,
                                 ofpact_get_OUTPUT(a)->max_len, true);
+            if (exit_recirculates(ctx)) {
+                /* OUTPUT needs recirculation before it can be translated. */
+                recirc_unroll_actions(a, rem_len, ctx);
+            }
             break;
 
         case OFPACT_GROUP:
@@ -4515,7 +4508,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_IPV4_SRC:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (flow->dl_type == htons(ETH_TYPE_IP)) {
                 memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
                 flow->nw_src = ofpact_get_SET_IPV4_SRC(a)->ipv4;
@@ -4523,7 +4518,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_IPV4_DST:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (flow->dl_type == htons(ETH_TYPE_IP)) {
                 memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
                 flow->nw_dst = ofpact_get_SET_IPV4_DST(a)->ipv4;
@@ -4531,7 +4528,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_IP_DSCP:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (is_ip_any(flow)) {
                 wc->masks.nw_tos |= IP_DSCP_MASK;
                 flow->nw_tos &= ~IP_DSCP_MASK;
@@ -4540,7 +4539,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_IP_ECN:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (is_ip_any(flow)) {
                 wc->masks.nw_tos |= IP_ECN_MASK;
                 flow->nw_tos &= ~IP_ECN_MASK;
@@ -4549,7 +4550,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_IP_TTL:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (is_ip_any(flow)) {
                 wc->masks.nw_ttl = 0xff;
                 flow->nw_ttl = ofpact_get_SET_IP_TTL(a)->ttl;
@@ -4557,7 +4560,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_L4_SRC_PORT:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (is_ip_any(flow) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) {
                 memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
                 memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
@@ -4566,7 +4571,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_SET_L4_DST_PORT:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (is_ip_any(flow) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) {
                 memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
                 memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
@@ -4594,11 +4601,10 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
              *       the resubmit to the post-recirculation actions.
              */
             if (ctx->was_mpls) {
-                ctx_trigger_recirculation(ctx);
-                break;
+                return xlate_implicit_recirculation(a, rem_len, ctx);
             }
             xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a));
-            continue;
+            break;
 
         case OFPACT_SET_TUNNEL:
             flow->tunnel.tun_id = htonll(ofpact_get_SET_TUNNEL(a)->tun_id);
@@ -4617,15 +4623,19 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_REG_MOVE:
-            CHECK_MPLS_RECIRCULATION_IF(
-                mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->dst.field) ||
-                mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->src.field));
+            if (ctx->was_mpls &&
+                (mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->dst.field) ||
+                 mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->src.field))) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             nxm_execute_reg_move(ofpact_get_REG_MOVE(a), flow, wc);
             break;
 
         case OFPACT_SET_FIELD:
-            CHECK_MPLS_RECIRCULATION_IF(
-                mf_is_l3_or_higher(ofpact_get_SET_FIELD(a)->field));
+            if (ctx->was_mpls &&
+                mf_is_l3_or_higher(ofpact_get_SET_FIELD(a)->field)) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             set_field = ofpact_get_SET_FIELD(a);
             mf = set_field->field;
 
@@ -4651,15 +4661,19 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_STACK_PUSH:
-            CHECK_MPLS_RECIRCULATION_IF(
-                mf_is_l3_or_higher(ofpact_get_STACK_PUSH(a)->subfield.field));
+            if (ctx->was_mpls &&
+                mf_is_l3_or_higher(ofpact_get_STACK_PUSH(a)->subfield.field)) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             nxm_execute_stack_push(ofpact_get_STACK_PUSH(a), flow, wc,
                                    &ctx->stack);
             break;
 
         case OFPACT_STACK_POP:
-            CHECK_MPLS_RECIRCULATION_IF(
-                mf_is_l3_or_higher(ofpact_get_STACK_POP(a)->subfield.field));
+            if (ctx->was_mpls &&
+                mf_is_l3_or_higher(ofpact_get_STACK_POP(a)->subfield.field)) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             nxm_execute_stack_pop(ofpact_get_STACK_POP(a), flow, wc,
                                   &ctx->stack);
             break;
@@ -4670,43 +4684,57 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
              * action converted it to IP. In this case recirculating should
              * reveal the IP TTL which is used as the basis for a new MPLS
              * LSE. */
-            CHECK_MPLS_RECIRCULATION_IF(
-                !flow_count_mpls_labels(flow, wc)
+            if (ctx->was_mpls
+                && !flow_count_mpls_labels(flow, wc)
                 && flow->nw_ttl == 0
-                && is_ip_any(flow));
+                && is_ip_any(flow)) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_mpls_push_action(ctx, ofpact_get_PUSH_MPLS(a));
             break;
 
         case OFPACT_POP_MPLS:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_mpls_pop_action(ctx, ofpact_get_POP_MPLS(a)->ethertype);
             break;
 
         case OFPACT_SET_MPLS_LABEL:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_set_mpls_label_action(
                 ctx, ofpact_get_SET_MPLS_LABEL(a)->label);
             break;
 
         case OFPACT_SET_MPLS_TC:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_set_mpls_tc_action(ctx, ofpact_get_SET_MPLS_TC(a)->tc);
             break;
 
         case OFPACT_SET_MPLS_TTL:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_set_mpls_ttl_action(ctx, ofpact_get_SET_MPLS_TTL(a)->ttl);
             break;
 
         case OFPACT_DEC_MPLS_TTL:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             if (compose_dec_mpls_ttl_action(ctx)) {
                 return;
             }
             break;
 
         case OFPACT_DEC_TTL:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             wc->masks.nw_ttl = 0xff;
             if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) {
                 return;
@@ -4718,12 +4746,16 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_MULTIPATH:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             multipath_execute(ofpact_get_MULTIPATH(a), flow, wc);
             break;
 
         case OFPACT_BUNDLE:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             xlate_bundle_action(ctx, ofpact_get_BUNDLE(a));
             break;
 
@@ -4732,7 +4764,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_LEARN:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             xlate_learn_action(ctx, ofpact_get_LEARN(a));
             break;
 
@@ -4759,7 +4793,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
         }
         case OFPACT_FIN_TIMEOUT:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
             xlate_fin_timeout(ctx, ofpact_get_FIN_TIMEOUT(a));
             break;
@@ -4789,6 +4825,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
             ovs_assert(ctx->table_id < ogt->table_id);
 
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
                                ogt->table_id, true, true);
             break;
@@ -4799,7 +4838,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_CT:
-            CHECK_MPLS_RECIRCULATION();
+            if (ctx->was_mpls) {
+                return xlate_implicit_recirculation(a, rem_len, ctx);
+            }
             compose_conntrack_action(ctx, ofpact_get_CT(a));
             break;
 
@@ -4810,17 +4851,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
         case OFPACT_DEBUG_RECIRC:
             ctx_trigger_recirculation(ctx);
-            a = ofpact_next(a);
-            break;
-        }
-
-        /* Check if need to store this and the remaining actions for later
-         * execution. */
-        if (!ctx->error && ctx->exit && ctx_first_recirculation_action(ctx)) {
-            recirc_unroll_actions(a, OFPACT_ALIGN(ofpacts_len -
-                                                  ((uint8_t *)a -
-                                                   (uint8_t *)ofpacts)),
-                                  ctx);
             break;
         }
     }
-- 
2.1.4




More information about the dev mailing list