[ovs-dev] [PATCH] ofproto-dpif: GOTO_TABLE recursion removal.

Jarno Rajahalme jarno.rajahalme at nsn.com
Fri Mar 8 13:50:12 UTC 2013


Remove recursion from GOTO_TABLE processing in do_xlate_actions().
This allows packet processing pipelines built with goto table be
longer and not interact with each other via the resubmit recursion limit.

Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
---
 ofproto/ofproto-dpif.c |  112 +++++++++++++++++++++++++++++++-----------------
 tests/ofproto-dpif.at  |   14 ++++++
 2 files changed, 87 insertions(+), 39 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 89f5bf4..f720095 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5816,61 +5816,73 @@ compose_output_action(struct action_xlate_ctx *ctx, uint16_t ofp_port)
 }
 
 static void
+tag_the_flow(struct action_xlate_ctx *ctx, struct rule_dpif *rule)
+{
+    struct ofproto_dpif *ofproto = ctx->ofproto;
+    uint8_t table_id = ctx->table_id;
+
+    if (table_id > 0 && table_id < N_TABLES) {
+        struct table_dpif *table = &ofproto->tables[table_id];
+        if (table->other_table) {
+            ctx->tags |= (rule && rule->tag
+                          ? rule->tag
+                          : rule_calculate_tag(&ctx->flow,
+                                               &table->other_table->mask,
+                                               table->basis));
+        }
+    }
+}
+
+/* Common rule processing in one place to avoid duplicating code. */
+static struct rule_dpif *
+ctx_rule_hooks(struct action_xlate_ctx *ctx, struct rule_dpif *rule,
+               bool may_packet_in)
+{
+    if (ctx->resubmit_hook) {
+        ctx->resubmit_hook(ctx, rule);
+    }
+    if (rule == NULL && may_packet_in) {
+        /* XXX
+         * check if table configuration flags
+         * OFPTC_TABLE_MISS_CONTROLLER, default.
+         * OFPTC_TABLE_MISS_CONTINUE,
+         * OFPTC_TABLE_MISS_DROP
+         * When OF1.0, OFPTC_TABLE_MISS_CONTINUE is used. What to do?
+         */
+        rule = rule_dpif_miss_rule(ctx->ofproto, &ctx->flow);
+    }
+    if (rule && ctx->resubmit_stats) {
+        rule_credit_stats(rule, ctx->resubmit_stats);
+    }
+    return rule;
+}
+
+static void
 xlate_table_action(struct action_xlate_ctx *ctx,
                    uint16_t in_port, uint8_t table_id, bool may_packet_in)
 {
     if (ctx->recurse < MAX_RESUBMIT_RECURSION) {
-        struct ofproto_dpif *ofproto = ctx->ofproto;
         struct rule_dpif *rule;
-        uint16_t old_in_port;
-        uint8_t old_table_id;
+        uint16_t old_in_port = ctx->flow.in_port;
+        uint8_t old_table_id = ctx->table_id;
 
-        old_table_id = ctx->table_id;
         ctx->table_id = table_id;
 
         /* Look up a flow with 'in_port' as the input port. */
-        old_in_port = ctx->flow.in_port;
         ctx->flow.in_port = in_port;
-        rule = rule_dpif_lookup__(ofproto, &ctx->flow, table_id);
-
-        /* Tag the flow. */
-        if (table_id > 0 && table_id < N_TABLES) {
-            struct table_dpif *table = &ofproto->tables[table_id];
-            if (table->other_table) {
-                ctx->tags |= (rule && rule->tag
-                              ? rule->tag
-                              : rule_calculate_tag(&ctx->flow,
-                                                   &table->other_table->mask,
-                                                   table->basis));
-            }
-        }
+        rule = rule_dpif_lookup__(ctx->ofproto, &ctx->flow, table_id);
+
+        tag_the_flow(ctx, rule);
 
         /* Restore the original input port.  Otherwise OFPP_NORMAL and
          * OFPP_IN_PORT will have surprising behavior. */
         ctx->flow.in_port = old_in_port;
 
-        if (ctx->resubmit_hook) {
-            ctx->resubmit_hook(ctx, rule);
-        }
-
-        if (rule == NULL && may_packet_in) {
-            /* XXX
-             * check if table configuration flags
-             * OFPTC_TABLE_MISS_CONTROLLER, default.
-             * OFPTC_TABLE_MISS_CONTINUE,
-             * OFPTC_TABLE_MISS_DROP
-             * When OF1.0, OFPTC_TABLE_MISS_CONTINUE is used. What to do?
-             */
-            rule = rule_dpif_miss_rule(ofproto, &ctx->flow);
-        }
+        rule = ctx_rule_hooks(ctx, rule, may_packet_in);
 
         if (rule) {
             struct rule_dpif *old_rule = ctx->rule;
 
-            if (ctx->resubmit_stats) {
-                rule_credit_stats(rule, ctx->resubmit_stats);
-            }
-
             ctx->recurse++;
             ctx->rule = rule;
             do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx);
@@ -6341,12 +6353,15 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 {
     bool was_evictable = true;
     const struct ofpact *a;
+    struct rule_dpif *orig_rule = ctx->rule;
 
     if (ctx->rule) {
         /* Don't let the rule we're working on get evicted underneath us. */
         was_evictable = ctx->rule->up.evictable;
         ctx->rule->up.evictable = false;
     }
+
+ do_xlate_actions_again:
     OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
         struct ofpact_controller *controller;
         const struct ofpact_metadata *metadata;
@@ -6536,17 +6551,36 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_GOTO_TABLE: {
-            /* XXX remove recursion */
-            /* It is assumed that goto-table is last action */
+            /* It is assumed that goto-table is the last action. */
             struct ofpact_goto_table *ogt = ofpact_get_GOTO_TABLE(a);
+            struct rule_dpif *rule;
+
             ovs_assert(ctx->table_id < ogt->table_id);
-            xlate_table_action(ctx, ctx->flow.in_port, ogt->table_id, true);
+
+            ctx->table_id = ogt->table_id;
+
+            /* Look up a flow from the new table. */
+            rule = rule_dpif_lookup__(ctx->ofproto, &ctx->flow, ctx->table_id);
+
+            tag_the_flow(ctx, rule);
+
+            rule = ctx_rule_hooks(ctx, rule, true);
+
+            if (rule) {
+                ctx->rule = rule;
+
+                /* Tail recursion removal. */
+                ofpacts = rule->up.ofpacts;
+                ofpacts_len = rule->up.ofpacts_len;
+                goto do_xlate_actions_again;
+            }
             break;
         }
         }
     }
 
 out:
+    ctx->rule = orig_rule;
     if (ctx->rule) {
         ctx->rule->up.evictable = was_evictable;
     }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 7915792..dcde4ea 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -20,6 +20,20 @@ AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - goto table])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11])
+echo "table=0 in_port=1 actions=output(10),goto_table(1)" > flows.txt
+for i in `seq 1 252`; do echo "table=$i actions=goto_table($(($i+1)))"; done >> flows.txt
+echo "table=253 actions=output(11)" >> flows.txt
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 10,11
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - registers])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])
-- 
1.7.10.4




More information about the dev mailing list