[ovs-dev] [bug 7550 3/9] ofproto: Implement "hidden" and "readonly" OpenFlow tables.

Ben Pfaff blp at nicira.com
Wed Dec 21 21:03:51 UTC 2011


A "hidden" table is one that OpenFlow operations affect only if the
table_id is explicitly specified, that is, operations that affect
all tables ignore it.

A "read-only" table is one that OpenFlow operations are not allowed
to modify.

I intend to use these flags in an upcoming commit for implementing
tables internal to the Open vSwitch implementation.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-provider.h |    6 ++++
 ofproto/ofproto.c          |   65 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 0c514c6..936361f 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -101,8 +101,14 @@ struct ofport {
 
 void ofproto_port_set_state(struct ofport *, ovs_be32 state);
 
+enum oftable_flags {
+    OFTABLE_HIDDEN = 1 << 0,   /* Hide from most OpenFlow operations. */
+    OFTABLE_READONLY = 1 << 1  /* Don't allow OpenFlow to change this table. */
+};
+
 /* A flow table within a "struct ofproto". */
 struct oftable {
+    enum oftable_flags flags;
     struct classifier cls;      /* Contains "struct rule"s. */
 };
 
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f589a4a..2216e12 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -133,6 +133,8 @@ static void ofproto_destroy__(struct ofproto *);
 
 static void ofproto_rule_destroy__(struct rule *);
 static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
+static bool rule_is_modifiable(const struct rule *);
+static bool rule_is_hidden(const struct rule *);
 
 static void ofopgroup_destroy(struct ofopgroup *);
 
@@ -368,6 +370,7 @@ ofproto_init_tables(struct ofproto *ofproto, int n_tables)
     ofproto->n_tables = n_tables;
     ofproto->tables = xmalloc(n_tables * sizeof *ofproto->tables);
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
+        table->flags = 0;
         classifier_init(&table->cls);
     }
 }
@@ -818,6 +821,10 @@ ofproto_flush__(struct ofproto *ofproto)
         struct rule *rule, *next_rule;
         struct cls_cursor cursor;
 
+        if (table->flags & OFTABLE_HIDDEN) {
+            continue;
+        }
+
         cls_cursor_init(&cursor, &table->cls, NULL);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
             if (!rule->pending) {
@@ -1709,6 +1716,18 @@ rule_is_hidden(const struct rule *rule)
 {
     return rule->cr.priority > UINT16_MAX;
 }
+
+static enum oftable_flags
+rule_get_flags(const struct rule *rule)
+{
+    return rule->ofproto->tables[rule->table_id].flags;
+}
+
+static bool
+rule_is_modifiable(const struct rule *rule)
+{
+    return !(rule_get_flags(rule) & OFTABLE_READONLY);
+}
 
 static int
 handle_echo_request(struct ofconn *ofconn, const struct ofp_header *oh)
@@ -2036,10 +2055,26 @@ check_table_id(const struct ofproto *ofproto, uint8_t table_id)
 }
 
 static struct oftable *
+next_visible_table(struct ofproto *ofproto, uint8_t table_id)
+{
+    struct oftable *table;
+
+    for (table = &ofproto->tables[table_id];
+         table < &ofproto->tables[ofproto->n_tables];
+         table++) {
+        if (!(table->flags & OFTABLE_HIDDEN)) {
+            return table;
+        }
+    }
+
+    return NULL;
+}
+
+static struct oftable *
 first_matching_table(struct ofproto *ofproto, uint8_t table_id)
 {
     if (table_id == 0xff) {
-        return &ofproto->tables[0];
+        return next_visible_table(ofproto, 0);
     } else if (table_id < ofproto->n_tables) {
         return &ofproto->tables[table_id];
     } else {
@@ -2051,18 +2086,19 @@ static struct oftable *
 next_matching_table(struct ofproto *ofproto,
                     struct oftable *table, uint8_t table_id)
 {
-    return (table_id == 0xff && table < &ofproto->tables[ofproto->n_tables - 1]
-            ? table + 1
+    return (table_id == 0xff
+            ? next_visible_table(ofproto, (table - ofproto->tables) + 1)
             : NULL);
 }
 
 /* Assigns TABLE to each oftable, in turn, that matches TABLE_ID in OFPROTO:
  *
  *   - If TABLE_ID is 0xff, this iterates over every classifier table in
- *     OFPROTO.
+ *     OFPROTO, skipping tables marked OFTABLE_HIDDEN.
  *
  *   - If TABLE_ID is the number of a table in OFPROTO, then the loop iterates
- *     only once, for that table.
+ *     only once, for that table.  (This can be used to access tables marked
+ *     OFTABLE_HIDDEN.)
  *
  *   - Otherwise, TABLE_ID isn't valid for OFPROTO, so the loop won't be
  *     entered at all.  (Perhaps you should have validated TABLE_ID with
@@ -2493,6 +2529,10 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
         return ofp_mkerr_nicira(OFPET_FLOW_MOD_FAILED, NXFMFC_BAD_TABLE_ID);
     }
 
+    if (table->flags & OFTABLE_READONLY) {
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_EPERM);
+    }
+
     /* Check for overlap, if requested. */
     if (fm->flags & OFPFF_CHECK_OVERLAP
         && classifier_rule_overlaps(&table->cls, &fm->cr)) {
@@ -2525,7 +2565,9 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 
     /* Insert new rule. */
     victim = rule_from_cls_rule(classifier_replace(&table->cls, &rule->cr));
-    if (victim && victim->pending) {
+    if (victim && !rule_is_modifiable(victim)) {
+        error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_EPERM);
+    } else if (victim && victim->pending) {
         error = OFPROTO_POSTPONE;
     } else {
         group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
@@ -2567,9 +2609,18 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
 {
     struct ofopgroup *group;
     struct rule *rule;
+    int error;
 
     group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
+    error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_EPERM);
     LIST_FOR_EACH (rule, ofproto_node, rules) {
+        if (rule_is_modifiable(rule)) {
+            /* At least one rule is modifiable, don't report EPERM error. */
+            error = 0;
+        } else {
+            continue;
+        }
+
         if (!ofputil_actions_equal(fm->actions, fm->n_actions,
                                    rule->actions, rule->n_actions)) {
             ofoperation_create(group, rule, OFOPERATION_MODIFY);
@@ -2585,7 +2636,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
     }
     ofopgroup_submit(group);
 
-    return 0;
+    return error;
 }
 
 /* Implements OFPFC_MODIFY.  Returns 0 on success or an OpenFlow error code as
-- 
1.7.2.5




More information about the dev mailing list