From 49a0e0eb3f85502ae6bc91151e345e67eb79314c 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 81adb0ab4..5b60e3660 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 6583294ee..a3640bd8e 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 71f4a0ef7..1621687d6 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2379,7 +2379,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); @@ -3761,8 +3761,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; @@ -3774,7 +3774,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); @@ -6340,7 +6340,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); @@ -6398,7 +6398,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.43.0