[ovs-dev] [threaded-learning v2 20/25] ofproto: Mark immutable members of struct rule 'const'.

[Ben Pfaff]

One difficult part of make flow_mods thread-safe is sorting out which
members of each structure can be modified under which locks and,
especially, documenting this in a way that makes it hard for programmers to
get it wrong later.  The compiler provides some tools for us, however, and
'const' is one of the nicer ones since it is standardized rather than part
of a compiler extension.

This commit makes use of 'const' to mark the immutable members of struct
rule, which is definitely the most confusing structure regarding thread
safety simply because it has so many members that use different forms of
synchronization.  It also adds a bunch of CONST_CASTs to allow these
members to be initialized and destroyed where necessary.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif.c     |    2 +-
 ofproto/ofproto-provider.h |   10 +++++++---
 ofproto/ofproto.c          |   12 ++++++------
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 81adb0a..5b60e36 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4777,7 +4777,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
                           const struct flow *flow, struct flow_wildcards *wc,
                           uint8_t table_id, struct rule_dpif **rule)
 {
-    struct cls_rule *cls_rule;
+    const struct cls_rule *cls_rule;
     struct classifier *cls;
     bool frag;
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 6583294e..a3640bd 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -227,8 +227,13 @@ struct oftable {
  * With few exceptions, ofproto implementations may look at these fields but
  * should not modify them. */
 struct rule {
-    struct ofproto *ofproto;     /* The ofproto that contains this rule. */
-    struct cls_rule cr;          /* In owning ofproto's classifier. */
+    /* Where this rule resides in an OpenFlow switch.
+     *
+     * These are immutable once the rule is constructed, hence 'const'. */
+    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. */
+
     atomic_uint ref_count;
 
     struct ofoperation *pending; /* Operation now in progress, if nonnull. */
@@ -240,7 +245,6 @@ struct rule {
     long long int created;       /* Creation time. */
     long long int modified;      /* Time of last modification. */
     long long int used;          /* Last use; time created if never used. */
-    uint8_t table_id;            /* Index in ofproto's 'tables' array. */
     enum ofputil_flow_mod_flags flags;
 
     uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 882d503..160d6b0 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2377,7 +2377,7 @@ ofproto_rule_unref(struct rule *rule)
 static void
 ofproto_rule_destroy__(struct rule *rule)
 {
-    cls_rule_destroy(&rule->cr);
+    cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
     rule_actions_unref(rule->actions);
     ovs_mutex_destroy(&rule->mutex);
     rule->ofproto->ofproto_class->rule_dealloc(rule);
@@ -3759,8 +3759,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     }
 
     /* Initialize base state. */
-    rule->ofproto = ofproto;
-    cls_rule_move(&rule->cr, &cr);
+    *CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto;
+    cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), &cr);
     atomic_init(&rule->ref_count, 1);
     rule->pending = NULL;
     rule->flow_cookie = fm->new_cookie;
@@ -3772,7 +3772,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     rule->hard_timeout = fm->hard_timeout;
     ovs_mutex_unlock(&rule->mutex);
 
-    rule->table_id = table - ofproto->tables;
+    *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables;
     rule->flags = fm->flags & OFPUTIL_FF_STATE;
     rule->actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
     list_init(&rule->meter_list_node);
@@ -6335,7 +6335,7 @@ oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls,
                       struct rule *rule)
     OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->mutex)
 {
-    classifier_remove(cls, &rule->cr);
+    classifier_remove(cls, CONST_CAST(struct cls_rule *, &rule->cr));
 
     ovs_mutex_lock(&ofproto_mutex);
     cookies_remove(ofproto, rule);
@@ -6393,7 +6393,7 @@ oftable_insert_rule(struct rule *rule)
         list_insert(&meter->rules, &rule->meter_list_node);
     }
     ovs_rwlock_wrlock(&table->cls.rwlock);
-    classifier_insert(&table->cls, &rule->cr);
+    classifier_insert(&table->cls, CONST_CAST(struct cls_rule *, &rule->cr));
     ovs_rwlock_unlock(&table->cls.rwlock);
     eviction_group_add_rule(rule);
 }
-- 
1.7.10.4