From 88d4914da1ad3c85416305a9a6e68cc073acd407 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 10 Sep 2013 21:31:59 -0700 Subject: [PATCH] ofproto: Mark immutable members of struct rule 'const'. 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 Acked-by: Ethan Jackson --- 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 f89715edb..d0b950660 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4778,7 +4778,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 6c9af0e48..2c1329a7b 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -221,8 +221,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. */ @@ -234,7 +239,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. */ bool send_flow_removed; /* Send a flow removed message? */ uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 14aa4416e..58042c23c 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2367,7 +2367,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); @@ -3742,8 +3742,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; @@ -3755,7 +3755,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->send_flow_removed = (fm->flags & OFPUTIL_FF_SEND_FLOW_REM) != 0; rule->actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len); list_init(&rule->meter_list_node); @@ -5861,7 +5861,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); @@ -5919,7 +5919,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); } -- 2.20.1