ofproto-dpif: Introduce "internal flows" for handling flow table misses.
authorBen Pfaff <blp@nicira.com>
Sat, 5 May 2012 17:55:30 +0000 (10:55 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 9 May 2012 19:58:54 +0000 (12:58 -0700)
The ofproto-dpif implementation of "facet"s requires a facet to be
associated with an OpenFlow rule.  Until now, this meant that packets
that miss in the OpenFlow table (and thus didn't have OpenFlow rules)
couldn't be set up as facets and thus couldn't be installed in the
kernel.  This commit changes that, by introducing "internal" OpenFlow
rules to associate with such packets.

Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto/ofproto-dpif.c
tests/ofproto.at

index 5290afb973b28333bc0f36dea040a534eb8c93f5..fad88a92f325cea7015e508bb9abcd17a2a3b385 100644 (file)
@@ -57,7 +57,6 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
 
 COVERAGE_DEFINE(ofproto_dpif_ctlr_action);
 COVERAGE_DEFINE(ofproto_dpif_expired);
-COVERAGE_DEFINE(ofproto_dpif_no_packet_in);
 COVERAGE_DEFINE(ofproto_dpif_xlate);
 COVERAGE_DEFINE(facet_changed_rule);
 COVERAGE_DEFINE(facet_invalidated);
@@ -71,7 +70,8 @@ COVERAGE_DEFINE(facet_suppress);
 
 /* Number of implemented OpenFlow tables. */
 enum { N_TABLES = 255 };
-BUILD_ASSERT_DECL(N_TABLES >= 1 && N_TABLES <= 255);
+enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
+BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
 
 struct ofport_dpif;
 struct ofproto_dpif;
@@ -106,7 +106,10 @@ static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
 }
 
 static struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *,
-                                          const struct flow *, uint8_t table);
+                                          const struct flow *);
+static struct rule_dpif *rule_dpif_lookup__(struct ofproto_dpif *,
+                                            const struct flow *,
+                                            uint8_t table);
 
 static void rule_credit_stats(struct rule_dpif *,
                               const struct dpif_flow_stats *);
@@ -426,7 +429,7 @@ static struct facet *facet_find(struct ofproto_dpif *,
                                 const struct flow *, uint32_t hash);
 static struct facet *facet_lookup_valid(struct ofproto_dpif *,
                                         const struct flow *, uint32_t hash);
-static bool facet_revalidate(struct facet *);
+static void facet_revalidate(struct facet *);
 static bool facet_check_consistency(struct facet *);
 
 static void facet_flush_stats(struct facet *);
@@ -534,6 +537,10 @@ struct ofproto_dpif {
     struct dpif *dpif;
     int max_ports;
 
+    /* Special OpenFlow rules. */
+    struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
+    struct rule_dpif *no_packet_in_rule; /* Drops flow table misses. */
+
     /* Statistics. */
     uint64_t n_matches;
 
@@ -653,6 +660,8 @@ del(const char *type, const char *name)
 \f
 /* Basic life-cycle. */
 
+static int add_internal_flows(struct ofproto_dpif *);
+
 static struct ofproto *
 alloc(void)
 {
@@ -735,10 +744,73 @@ construct(struct ofproto *ofproto_)
     memset(&ofproto->stats, 0, sizeof ofproto->stats);
 
     ofproto_init_tables(ofproto_, N_TABLES);
+    error = add_internal_flows(ofproto);
+    ofproto->up.tables[TBL_INTERNAL].flags = OFTABLE_HIDDEN | OFTABLE_READONLY;
+
+    return error;
+}
+
+static int
+add_internal_flow(struct ofproto_dpif *ofproto, int id,
+                  const struct ofpbuf *actions, struct rule_dpif **rulep)
+{
+    struct ofputil_flow_mod fm;
+    int error;
+
+    cls_rule_init_catchall(&fm.cr, 0);
+    cls_rule_set_reg(&fm.cr, 0, id);
+    fm.cookie = htonll(0);
+    fm.cookie_mask = htonll(0);
+    fm.table_id = TBL_INTERNAL;
+    fm.command = OFPFC_ADD;
+    fm.idle_timeout = 0;
+    fm.hard_timeout = 0;
+    fm.buffer_id = 0;
+    fm.out_port = 0;
+    fm.flags = 0;
+    fm.actions = actions->data;
+    fm.n_actions = actions->size / sizeof(union ofp_action);
+
+    error = ofproto_flow_mod(&ofproto->up, &fm);
+    if (error) {
+        VLOG_ERR_RL(&rl, "failed to add internal flow %d (%s)",
+                    id, ofperr_to_string(error));
+        return error;
+    }
+
+    *rulep = rule_dpif_lookup__(ofproto, &fm.cr.flow, TBL_INTERNAL);
+    assert(*rulep != NULL);
 
     return 0;
 }
 
+static int
+add_internal_flows(struct ofproto_dpif *ofproto)
+{
+    struct nx_action_controller *nac;
+    uint64_t actions_stub[128 / 8];
+    struct ofpbuf actions;
+    int error;
+    int id;
+
+    ofpbuf_use_stack(&actions, actions_stub, sizeof actions_stub);
+    id = 1;
+
+    nac = ofputil_put_NXAST_CONTROLLER(&actions);
+    nac->max_len = htons(UINT16_MAX);
+    nac->controller_id = htons(0);
+    nac->reason = OFPR_NO_MATCH;
+    error = add_internal_flow(ofproto, id++, &actions, &ofproto->miss_rule);
+    if (error) {
+        return error;
+    }
+
+    ofpbuf_clear(&actions);
+    error = add_internal_flow(ofproto, id++, &actions,
+                              &ofproto->no_packet_in_rule);
+    return error;
+}
+
 static void
 complete_operations(struct ofproto_dpif *ofproto)
 {
@@ -863,13 +935,13 @@ run(struct ofproto *ofproto_)
         || !tag_set_is_empty(&ofproto->revalidate_set)) {
         struct tag_set revalidate_set = ofproto->revalidate_set;
         bool revalidate_all = ofproto->need_revalidate;
-        struct facet *facet, *next;
+        struct facet *facet;
 
         /* Clear the revalidation flags. */
         tag_set_init(&ofproto->revalidate_set);
         ofproto->need_revalidate = false;
 
-        HMAP_FOR_EACH_SAFE (facet, next, hmap_node, &ofproto->facets) {
+        HMAP_FOR_EACH (facet, hmap_node, &ofproto->facets) {
             if (revalidate_all
                 || tag_set_intersects(&revalidate_set, facet->tags)) {
                 facet_revalidate(facet);
@@ -1084,7 +1156,8 @@ port_reconfigured(struct ofport *port_, enum ofputil_port_config old_config)
     enum ofputil_port_config changed = old_config ^ port->up.pp.config;
 
     if (changed & (OFPUTIL_PC_NO_RECV | OFPUTIL_PC_NO_RECV_STP |
-                   OFPUTIL_PC_NO_FWD | OFPUTIL_PC_NO_FLOOD)) {
+                   OFPUTIL_PC_NO_FWD | OFPUTIL_PC_NO_FLOOD |
+                   OFPUTIL_PC_NO_PACKET_IN)) {
         ofproto->need_revalidate = true;
 
         if (changed & OFPUTIL_PC_NO_FLOOD && port->bundle) {
@@ -2795,30 +2868,6 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
     }
 }
 
-/* Handles flow miss 'miss' on 'ofproto'.  The flow does not match any flow in
- * the OpenFlow flow table. */
-static void
-handle_flow_miss_no_rule(struct ofproto_dpif *ofproto, struct flow_miss *miss)
-{
-    uint16_t in_port = miss->flow.in_port;
-    struct ofport_dpif *port = get_ofp_port(ofproto, in_port);
-
-    if (!port) {
-        VLOG_WARN_RL(&rl, "packet-in on unknown port %"PRIu16, in_port);
-    }
-
-    if (port && port->up.pp.config & OFPUTIL_PC_NO_PACKET_IN) {
-        /* XXX install 'drop' flow entry */
-        COVERAGE_INC(ofproto_dpif_no_packet_in);
-    } else {
-        const struct ofpbuf *packet;
-
-        LIST_FOR_EACH (packet, list_node, &miss->packets) {
-            send_packet_in_miss(ofproto, packet, &miss->flow);
-        }
-    }
-}
-
 /* Handles flow miss 'miss' on 'ofproto'.  May add any required datapath
  * operations to 'ops', incrementing '*n_ops' for each new op. */
 static void
@@ -2834,11 +2883,9 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
 
     facet = facet_lookup_valid(ofproto, &miss->flow, hash);
     if (!facet) {
-        struct rule_dpif *rule = rule_dpif_lookup(ofproto, &miss->flow, 0);
-        if (!rule) {
-            handle_flow_miss_no_rule(ofproto, miss);
-            return;
-        } else if (!flow_miss_should_make_facet(ofproto, miss, hash)) {
+        struct rule_dpif *rule = rule_dpif_lookup(ofproto, &miss->flow);
+
+        if (!flow_miss_should_make_facet(ofproto, miss, hash)) {
             handle_flow_miss_without_facet(miss, rule, ops, n_ops);
             return;
         }
@@ -3671,16 +3718,13 @@ static struct facet *
 facet_lookup_valid(struct ofproto_dpif *ofproto, const struct flow *flow,
                    uint32_t hash)
 {
-    struct facet *facet = facet_find(ofproto, flow, hash);
+    struct facet *facet;
 
-    /* The facet we found might not be valid, since we could be in need of
-     * revalidation.  If it is not valid, don't return it. */
+    facet = facet_find(ofproto, flow, hash);
     if (facet
         && (ofproto->need_revalidate
-            || tag_set_intersects(&ofproto->revalidate_set, facet->tags))
-        && !facet_revalidate(facet)) {
-        COVERAGE_INC(facet_invalidated);
-        return NULL;
+            || tag_set_intersects(&ofproto->revalidate_set, facet->tags))) {
+        facet_revalidate(facet);
     }
 
     return facet;
@@ -3702,17 +3746,10 @@ facet_check_consistency(struct facet *facet)
     bool ok;
 
     /* Check the rule for consistency. */
-    rule = rule_dpif_lookup(ofproto, &facet->flow, 0);
-    if (!rule) {
-        if (!VLOG_DROP_WARN(&rl)) {
-            char *s = flow_to_string(&facet->flow);
-            VLOG_WARN("%s: facet should not exist", s);
-            free(s);
-        }
-        return false;
-    } else if (rule != facet->rule) {
+    rule = rule_dpif_lookup(ofproto, &facet->flow);
+    ok = rule == facet->rule;
+    if (!ok) {
         may_log = !VLOG_DROP_WARN(&rl);
-        ok = false;
         if (may_log) {
             struct ds s;
 
@@ -3729,8 +3766,6 @@ facet_check_consistency(struct facet *facet)
             VLOG_WARN("%s", ds_cstr(&s));
             ds_destroy(&s);
         }
-    } else {
-        ok = true;
     }
 
     /* Check the datapath actions for consistency. */
@@ -3814,12 +3849,8 @@ facet_check_consistency(struct facet *facet)
  *     'facet' to the new rule and recompiles its actions.
  *
  *   - If the rule found is the same as 'facet''s current rule, leaves 'facet'
- *     where it is and recompiles its actions anyway.
- *
- *   - If there is none, destroys 'facet'.
- *
- * Returns true if 'facet' still exists, false if it has been destroyed. */
-static bool
+ *     where it is and recompiles its actions anyway. */
+static void
 facet_revalidate(struct facet *facet)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
@@ -3840,13 +3871,7 @@ facet_revalidate(struct facet *facet)
 
     COVERAGE_INC(facet_revalidate);
 
-    /* Determine the new rule. */
-    new_rule = rule_dpif_lookup(ofproto, &facet->flow, 0);
-    if (!new_rule) {
-        /* No new rule, so delete the facet. */
-        facet_remove(facet);
-        return false;
-    }
+    new_rule = rule_dpif_lookup(ofproto, &facet->flow);
 
     /* Calculate new datapath actions.
      *
@@ -3929,8 +3954,6 @@ facet_revalidate(struct facet *facet)
         facet->used = new_rule->up.created;
         facet->prev_used = facet->used;
     }
-
-    return true;
 }
 
 /* Updates 'facet''s used time.  Caller is responsible for calling
@@ -4293,8 +4316,31 @@ subfacet_update_stats(struct subfacet *subfacet,
 /* Rules. */
 
 static struct rule_dpif *
-rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
-                 uint8_t table_id)
+rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow)
+{
+    struct ofport_dpif *port;
+    struct rule_dpif *rule;
+
+    rule = rule_dpif_lookup__(ofproto, flow, 0);
+    if (rule) {
+        return rule;
+    }
+
+    port = get_ofp_port(ofproto, flow->in_port);
+    if (!port) {
+        VLOG_WARN_RL(&rl, "packet-in on unknown port %"PRIu16, flow->in_port);
+        return ofproto->miss_rule;
+    }
+
+    if (port->up.pp.config & OFPUTIL_PC_NO_PACKET_IN) {
+        return ofproto->no_packet_in_rule;
+    }
+    return ofproto->miss_rule;
+}
+
+static struct rule_dpif *
+rule_dpif_lookup__(struct ofproto_dpif *ofproto, const struct flow *flow,
+                   uint8_t table_id)
 {
     struct cls_rule *cls_rule;
     struct classifier *cls;
@@ -4702,7 +4748,7 @@ xlate_table_action(struct action_xlate_ctx *ctx,
         /* 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);
+        rule = rule_dpif_lookup__(ofproto, &ctx->flow, table_id);
 
         /* Tag the flow. */
         if (table_id > 0 && table_id < N_TABLES) {
@@ -6522,8 +6568,16 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
     flow_format(ds, flow);
     ds_put_char(ds, '\n');
 
-    rule = rule_dpif_lookup(ofproto, flow, 0);
+    rule = rule_dpif_lookup(ofproto, flow);
+
     trace_format_rule(ds, 0, 0, rule);
+    if (rule == ofproto->miss_rule) {
+        ds_put_cstr(ds, "\nNo match, flow generates \"packet in\"s.\n");
+    } else if (rule == ofproto->no_packet_in_rule) {
+        ds_put_cstr(ds, "\nNo match, packets dropped because "
+                    "OFPPC_NO_PACKET_IN is set on in_port.\n");
+    }
+
     if (rule) {
         uint64_t odp_actions_stub[1024 / 8];
         struct ofpbuf odp_actions;
index 9009b91502cc01f0095c64e3e1a827d32ed48022..84868d710241ba88ae22c44ac065bed472a9a3d4 100644 (file)
@@ -229,12 +229,14 @@ OVS_VSWITCHD_START
   0: classifier: wild=0x3fffff, max=1000000, active=0
                lookup=0, matched=0"
  x=1
- while test $x -lt 255; do
+ while test $x -lt 254; do
    printf "  %d: %-8s: wild=0x3fffff, max=1000000, active=0
                lookup=0, matched=0
 " $x table$x
    x=`expr $x + 1`
- done) > expout
+ done
+ echo "  254: table254: wild=0x3fffff, max=1000000, active=2
+               lookup=0, matched=0") > expout
 AT_CHECK([ovs-ofctl dump-tables br0], [0], [expout])
 # Change the configuration.
 AT_CHECK(