ofproto: Fix abstraction of OpenFlow multiple table support.

The ofproto code until now has assumed to some extent that the switch has
exactly two tables, one that handles exact-match flow and the other that
handles all other flows.  Commit 840ee2faf "wdp: Add new wdp_class member
function 'get_table_stats'" did part of the work on getting rid of that
assumption.  This commit does the rest of the work.

Now the only assumption left is that a switch has no more than 32 tables.
This seems reasonable enough to me; OpenFlow allows for no more 255.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f7ce9db..7150d96 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1669,7 +1669,7 @@ flow_stats_cb(struct wdp_rule *rule, void *cbdata_)
 
     ofs = append_stats_reply(len, cbdata->ofconn, &cbdata->msg);
     ofs->length = htons(len);
-    ofs->table_id = rule->cr.flow.wildcards ? TABLEID_CLASSIFIER : TABLEID_HASH;
+    ofs->table_id = rule->ofp_table_id;
     ofs->pad = 0;
     flow_to_match(&rule->cr.flow, cbdata->ofproto->tun_id_from_cookie,
                   &ofs->match);
@@ -1685,12 +1685,11 @@ flow_stats_cb(struct wdp_rule *rule, void *cbdata_)
     memcpy(ofs->actions, rule->actions, act_len);
 }
 
-static int
+static unsigned int
 table_id_to_include(uint8_t table_id)
 {
-    return (table_id == TABLEID_HASH ? CLS_INC_EXACT
-            : table_id == TABLEID_CLASSIFIER ? CLS_INC_WILD
-            : table_id == 0xff ? CLS_INC_ALL
+    return (table_id == 0xff ? UINT_MAX
+            : table_id < 32 ? 1u << table_id
             : 0);
 }
 
@@ -1714,7 +1713,7 @@ handle_flow_stats_request(struct ofproto *p, struct ofconn *ofconn,
     cbdata.out_port = fsr->out_port;
     cbdata.msg = start_stats_reply(osr, 1024);
     flow_from_match(&fsr->match, 0, false, 0, &target);
-    wdp_flow_for_each_match(p->wdp, &target,
+    wdp_flow_for_each_match(p->wdp, &target, 
                             table_id_to_include(fsr->table_id),
                             flow_stats_cb, &cbdata);
     queue_tx(cbdata.msg, ofconn, ofconn->reply_counter);
@@ -1765,7 +1764,7 @@ ofproto_get_all_flows(struct ofproto *p, struct ds *results)
     cbdata.results = results;
 
     flow_from_match(&match, 0, false, 0, &target);
-    wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL,
+    wdp_flow_for_each_match(p->wdp, &target, UINT_MAX,
                             flow_stats_ds_cb, &cbdata);
 }
 
@@ -2090,7 +2089,7 @@ modify_flows_loose(struct ofproto *p, struct ofconn *ofconn,
     flow_from_match(&ofm->match, 0, p->tun_id_from_cookie, ofm->cookie,
                     &target);
 
-    wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL,
+    wdp_flow_for_each_match(p->wdp, &target, UINT_MAX,
                             modify_flows_cb, &cbdata);
     if (cbdata.match) {
         /* This credits the packet to whichever flow happened to happened to
@@ -2186,7 +2185,7 @@ delete_flows_loose(struct ofproto *p, const struct ofp_flow_mod *ofm)
     flow_from_match(&ofm->match, 0, p->tun_id_from_cookie, ofm->cookie,
                     &target);
 
-    wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL,
+    wdp_flow_for_each_match(p->wdp, &target, UINT_MAX,
                             delete_flows_cb, &cbdata);
 }
 

diff --git a/ofproto/wdp-provider.h b/ofproto/wdp-provider.h
index 6414a80..414ce6d 100644 (file)
--- a/ofproto/wdp-provider.h
+++ b/ofproto/wdp-provider.h
@@ -269,9 +269,9 @@ struct wdp_class {
      * flow that matches the specified search criteria to 'callback' along with
      * 'aux'.
      *
-     * Flows are filtered out in two ways.  First, based on 'include':
-     * Exact-match flows are excluded unless CLS_INC_EXACT is in 'include'.
-     * Wildcarded flows are excluded unless CLS_INC_WILD is in 'include'.
+     * Flows are filtered out in two ways.  First, based on the bit-mask in
+     * 'include': wdp_rule 'wr' is included only if 'include & (1u <<
+     * wr->ofp_table_id)' is nonzero.
      *
      * Flows are also filtered out based on 'target': on a field-by-field
      * basis, a flow is included if 'target' wildcards that field or if the
@@ -285,7 +285,7 @@ struct wdp_class {
      * 'callback' must not delete flows from 'wdp' other than its argument
      * flow, nor may it insert new flows into 'wdp'. */
     void (*flow_for_each_match)(const struct wdp *wdp, const flow_t *flow,
-                                int include,
+                                unsigned int include,
                                 wdp_flow_cb_func *callback, void *aux);
 
     /* Retrieves flow statistics for 'rule', which must be in 'wdp''s flow

diff --git a/ofproto/wdp-xflow.c b/ofproto/wdp-xflow.c
index 71f0455..35d6ed2 100644 (file)
--- a/ofproto/wdp-xflow.c
+++ b/ofproto/wdp-xflow.c
@@ -51,6 +51,11 @@
 #define THIS_MODULE VLM_wdp_xflow
 #include "vlog.h"
 
+enum {
+    TABLEID_HASH = 0,
+    TABLEID_CLASSIFIER = 1
+};
+
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 \f
 /* Maximum numbers of rules. */
@@ -355,8 +360,8 @@ wx_rule_has_out_port(const struct wx_rule *rule, uint16_t out_port)
 }
 #endif
 
-/* Caller is responsible for initializing the 'cr' member of the returned
- * rule. */
+/* Caller is responsible for initializing the 'cr' and ofp_table_id members of
+ * the returned rule. */
 static struct wx_rule *
 wx_rule_create(struct wx_rule *super,
                const union ofp_action *actions, size_t n_actions,
@@ -478,6 +483,8 @@ wx_rule_create_subrule(struct wx *wx, struct wx_rule *rule, const flow_t *flow)
     subrule = wx_rule_create(rule, NULL, 0,
                              rule->wr.idle_timeout,
                              rule->wr.hard_timeout);
+    /* Subrules aren't really in any OpenFlow table, so don't bother with
+     * subrule->wr.ofp_table_id. */
     COVERAGE_INC(wx_subrule_create);
     cls_rule_from_flow(flow, &subrule->wr.cr);
     classifier_insert_exact(&wx->cls, &subrule->wr.cr);
@@ -1581,15 +1588,24 @@ wx_for_each_thunk(struct cls_rule *cls_rule, void *aux_)
 
 static void
 wx_flow_for_each_match(const struct wdp *wdp, const flow_t *target,
-                       int include,
+                       unsigned int include,
                        wdp_flow_cb_func *client_callback, void *client_aux)
 {
     struct wx *wx = wx_cast(wdp);
     struct wx_for_each_thunk_aux aux;
+    int cls_include;
+
+    cls_include = 0;
+    if (include & (1u << TABLEID_HASH)) {
+        cls_include |= CLS_INC_EXACT;
+    }
+    if (include & (1u << TABLEID_CLASSIFIER)) {
+        cls_include |= CLS_INC_WILD;
+    }
 
     aux.client_callback = client_callback;
     aux.client_aux = client_aux;
-    classifier_for_each_match(&wx->cls, target, include,
+    classifier_for_each_match(&wx->cls, target, cls_include,
                               wx_for_each_thunk, &aux);
 }
 
@@ -1708,6 +1724,8 @@ wx_flow_put(struct wdp *wdp, const struct wdp_flow_put *put,
     rule = wx_rule_create(NULL, put->actions, put->n_actions,
                           put->idle_timeout, put->hard_timeout);
     cls_rule_from_flow(put->flow, &rule->wr.cr);
+    rule->wr.ofp_table_id = (put->flow->wildcards
+                             ? TABLEID_CLASSIFIER : TABLEID_HASH);
     wx_rule_insert(wx, rule, NULL, 0);
 
     if (old_stats) {

diff --git a/ofproto/wdp.c b/ofproto/wdp.c
index 72519ee..9312918 100644 (file)
--- a/ofproto/wdp.c
+++ b/ofproto/wdp.c
@@ -48,7 +48,8 @@
 /* Initializes a new 'struct wdp_rule', copying in the 'n_actions' elements of
  * 'actions'.
  *
- * The caller is responsible for initializing 'rule->cr'. */
+ * The caller is responsible for initializing 'rule->cr'.  The caller must also
+ * fill in 'rule->ofp_table_id', if the wdp has more than one table. */
 void
 wdp_rule_init(struct wdp_rule *rule, const union ofp_action *actions,
               size_t n_actions)
@@ -58,6 +59,7 @@ wdp_rule_init(struct wdp_rule *rule, const union ofp_action *actions,
     rule->created = time_msec();
     rule->idle_timeout = 0;
     rule->hard_timeout = 0;
+    rule->ofp_table_id = 0;
     rule->client_data = NULL;
 }
 
@@ -792,12 +794,30 @@ wdp_flow_match(struct wdp *wdp, const flow_t *flow)
     return wdp->wdp_class->flow_match(wdp, flow);
 }
 
+/* Iterates through all of the flows in 'wdp''s flow table, passing each flow
+ * that matches the specified search criteria to 'callback' along with 'aux'.
+ *
+ * Flows are filtered out in two ways.  First, based on the bit-mask in
+ * 'include': wdp_rule 'wr' is included only if 'include & (1u <<
+ * wr->ofp_table_id)' is nonzero.
+ *
+ * Flows are also filtered out based on 'target': on a field-by-field basis, a
+ * flow is included if 'target' wildcards that field or if the flow and
+ * 'target' both have the same exact value for the field.  A flow is excluded
+ * if any field does not match based on these criteria.
+ *
+ * Ignores 'target->priority'.
+ *
+ * 'callback' is allowed to delete the rule that is passed as its argument.  It
+ * may modify any flow in 'wdp', e.g. changing their actions.  'callback' must
+ * not delete flows from 'wdp' other than its argument flow, nor may it insert
+ * new flows into 'wdp'. */
 void
 wdp_flow_for_each_match(const struct wdp *wdp, const flow_t *target,
-                        int include, wdp_flow_cb_func *callback, void *aux)
+                        unsigned int include,
+                        wdp_flow_cb_func *callback, void *aux)
 {
-    wdp->wdp_class->flow_for_each_match(wdp, target, include,
-                                        callback, aux); 
+    wdp->wdp_class->flow_for_each_match(wdp, target, include, callback, aux);
 }
 
 int

diff --git a/ofproto/wdp.h b/ofproto/wdp.h
index b20079a..4272119 100644 (file)
--- a/ofproto/wdp.h
+++ b/ofproto/wdp.h
@@ -25,11 +25,6 @@
 extern "C" {
 #endif
 
-enum {
-    TABLEID_HASH = 0,
-    TABLEID_CLASSIFIER = 1
-};
-
 struct ofhooks;
 struct ofpbuf;
 struct svec;
@@ -59,6 +54,8 @@ struct wdp_rule {
     long long int created;      /* Time created, in ms since the epoch. */
     uint16_t idle_timeout;      /* In seconds from time of last use. */
     uint16_t hard_timeout;      /* In seconds from time of creation. */
+    uint8_t ofp_table_id;       /* OpenFlow table_id in e.g. ofp_flow_stats.
+                                 * Supported range is at most 0...31. */
 
     /* OpenFlow actions.
      *
@@ -149,7 +146,8 @@ struct wdp_rule *wdp_flow_match(struct wdp *, const flow_t *);
 
 typedef void wdp_flow_cb_func(struct wdp_rule *, void *aux);
 void wdp_flow_for_each_match(const struct wdp *, const flow_t *,
-                               int include, wdp_flow_cb_func *, void *aux);
+                             unsigned int include, wdp_flow_cb_func *,
+                             void *aux);
 
 int wdp_flow_get_stats(const struct wdp *, const struct wdp_rule *,
                          struct wdp_flow_stats *);