ofproto: Properly initialize table_id when creating rules.

Nothing was initializing table_id, and there was no "hook" to choose an
appropriate table.  This fixes both problems.

Found by inspection.

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 63bc379..fd3f212 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3926,6 +3926,7 @@ const struct ofproto_class ofproto_dpif_class = {
     port_poll,
     port_poll_wait,
     port_is_lacp_current,
+    NULL,                       /* rule_choose_table */
     rule_alloc,
     rule_construct,
     rule_destruct,

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ac646e5..4c370d3 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -64,7 +64,8 @@ COVERAGE_DEFINE(ofproto_update_port);
 static void ofport_destroy__(struct ofport *);
 static void ofport_destroy(struct ofport *);
 
-static int rule_create(struct ofproto *, const struct cls_rule *,
+static int rule_create(struct ofproto *,
+                       const struct cls_rule *, uint8_t table_id,
                        const union ofp_action *, size_t n_actions,
                        uint16_t idle_timeout, uint16_t hard_timeout,
                        ovs_be64 flow_cookie, bool send_flow_removed,
@@ -871,7 +872,7 @@ ofproto_add_flow(struct ofproto *p, const struct cls_rule *cls_rule,
                  const union ofp_action *actions, size_t n_actions)
 {
     struct rule *rule;
-    rule_create(p, cls_rule, actions, n_actions, 0, 0, 0, false, &rule);
+    rule_create(p, cls_rule, 0, actions, n_actions, 0, 0, 0, false, &rule);
 }
 
 /* Searches for a rule with matching criteria exactly equal to 'target' in
@@ -1222,7 +1223,8 @@ init_ports(struct ofproto *p)
  * flow table, and stores the new rule into '*rulep'.  Returns 0 on success,
  * otherwise a positive errno value or OpenFlow error code. */
 static int
-rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
+rule_create(struct ofproto *ofproto,
+            const struct cls_rule *cls_rule, uint8_t table_id,
             const union ofp_action *actions, size_t n_actions,
             uint16_t idle_timeout, uint16_t hard_timeout,
             ovs_be64 flow_cookie, bool send_flow_removed,
@@ -1231,6 +1233,20 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
     struct rule *rule;
     int error;
 
+    if (table_id == 0xff) {
+        if (ofproto->n_tables > 1) {
+            error = ofproto->ofproto_class->rule_choose_table(ofproto,
+                                                              cls_rule,
+                                                              &table_id);
+            if (error) {
+                return error;
+            }
+            assert(table_id < ofproto->n_tables);
+        } else {
+            table_id = 0;
+        }
+    }
+
     rule = ofproto->ofproto_class->rule_alloc();
     if (!rule) {
         error = ENOMEM;
@@ -1239,6 +1255,7 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
 
     rule->ofproto = ofproto;
     rule->cr = *cls_rule;
+    rule->table_id = table_id;
     rule->flow_cookie = flow_cookie;
     rule->created = time_msec();
     rule->idle_timeout = idle_timeout;
@@ -2209,7 +2226,7 @@ add_flow(struct ofconn *ofconn, struct flow_mod *fm)
     }
 
     buf_err = ofconn_pktbuf_retrieve(ofconn, fm->buffer_id, &packet, &in_port);
-    error = rule_create(p, &fm->cr, fm->actions, fm->n_actions,
+    error = rule_create(p, &fm->cr, fm->table_id, fm->actions, fm->n_actions,
                         fm->idle_timeout, fm->hard_timeout, fm->cookie,
                         fm->flags & OFPFF_SEND_FLOW_REM, &rule);
     if (error) {

diff --git a/ofproto/private.h b/ofproto/private.h
index db0edef..7a41a10 100644 (file)
--- a/ofproto/private.h
+++ b/ofproto/private.h
@@ -506,6 +506,25 @@ struct ofproto_class {
 /* ## OpenFlow Rule Functions ## */
 /* ## ----------------------- ## */
 
+    /* Chooses an appropriate table for 'cls_rule' within 'ofproto'.  On
+     * success, stores the table ID into '*table_idp' and returns 0.  On
+     * failure, returns an OpenFlow error code (as returned by ofp_mkerr()).
+     *
+     * The choice of table should be a function of 'cls_rule' and 'ofproto''s
+     * datapath capabilities.  It should not depend on the flows already in
+     * 'ofproto''s flow tables.  Failure implies that an OpenFlow rule with
+     * 'cls_rule' as its matching condition can never be inserted into
+     * 'ofproto', even starting from an empty flow table.
+     *
+     * If multiple tables are candidates for inserting the flow, the function
+     * should choose one arbitrarily (but deterministically).
+     *
+     * This function will never be called for an ofproto that has only one
+     * table, so it may be NULL in that case. */
+    int (*rule_choose_table)(const struct ofproto *ofproto,
+                             const struct cls_rule *cls_rule,
+                             uint8_t *table_idp);
+
     /* Life-cycle functions for a "struct rule" (see "Life Cycle" above).
      *
      * ->rule_construct() should first check whether the rule is acceptable: