ofproto: Change rule's 'removed' member to a tri-state 'state'.

As a rule may not be re-inserted to ofproto data structures, it is
cleaner to have three states for the rule, rather than just two.  This
will be useful for managing learned flows in later patches.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

2 files changed, 22 insertions, 8 deletions

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index b11bf12c0..ef8ed6710 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -323,6 +323,19 @@ struct oftable {
  *      'rule->mutex', and safely written only by coding holding ofproto_mutex
  *      AND 'rule->mutex'.  These are marked OVS_GUARDED.
  */
+enum OVS_PACKED_ENUM rule_state {
+    RULE_INITIALIZED, /* Rule has been initialized, but not inserted to the
+                       * ofproto data structures.  Versioning makes sure the
+                       * rule is not visible to lookups by other threads, even
+                       * if the rule is added to a classifier. */
+    RULE_INSERTED,    /* Rule has been inserted to ofproto data structures and
+                       * may be visible to lookups by other threads. */
+    RULE_REMOVED,     /* Rule has been removed from ofproto data structures,
+                       * and may still be visible to lookups by other threads
+                       * until they quiesce, after which the rule will be
+                       * removed from the classifier as well. */
+};
+
 struct rule {
     /* Where this rule resides in an OpenFlow switch.
      *
@@ -330,8 +343,8 @@ struct rule {
     struct ofproto *const ofproto; /* The ofproto that contains this rule. */
     const struct cls_rule cr;      /* In owning ofproto's classifier. */
     const uint8_t table_id;        /* Index in ofproto's 'tables' array. */
-    bool removed;                  /* Rule has been removed from the ofproto
-                                    * data structures. */
+
+    enum rule_state state;
 
     /* Protects members marked OVS_GUARDED.
      * Readers only need to hold this mutex.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cfc4d4121..762c42d33 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1478,7 +1478,7 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
      * be killed. */
     ovs_mutex_lock(&ofproto_mutex);
 
-    if (!rule->removed) {
+    if (rule->state == RULE_INSERTED) {
         /* Make sure there is no postponed removal of the rule. */
         ovs_assert(cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX));
 
@@ -4816,7 +4816,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr,
         return error;
     }
 
-    rule->removed = true;   /* Not yet in ofproto data structures. */
+    rule->state = RULE_INITIALIZED;
 
     *new_rule = rule;
     return 0;
@@ -8100,7 +8100,8 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct rule *rule)
 {
     const struct rule_actions *actions = rule_get_actions(rule);
 
-    ovs_assert(rule->removed);
+    /* A rule may not be reinserted. */
+    ovs_assert(rule->state == RULE_INITIALIZED);
 
     if (rule->hard_timeout || rule->idle_timeout) {
         ovs_list_insert(&ofproto->expirable, &rule->expirable);
@@ -8123,7 +8124,7 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct rule *rule)
         }
     }
 
-    rule->removed = false;
+    rule->state = RULE_INSERTED;
 }
 
 /* Removes 'rule' from the ofproto data structures.  Caller may have deferred
@@ -8132,7 +8133,7 @@ static void
 ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
     OVS_REQUIRES(ofproto_mutex)
 {
-    ovs_assert(!rule->removed);
+    ovs_assert(rule->state == RULE_INSERTED);
 
     cookies_remove(ofproto, rule);
 
@@ -8168,7 +8169,7 @@ ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
         }
     }
 
-    rule->removed = true;
+    rule->state = RULE_REMOVED;
 }
 
 /* unixctl commands. */