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 <b...@nicira.com>
---
ofproto/ofproto-dpif.c | 2 +-
ofproto/ofproto-provider.h | 15 +++++++++++----
ofproto/ofproto.c | 12 ++++++------
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f661035..ba2e671 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4772,7 +4772,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 d477434..b3c9b63 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -222,11 +222,19 @@ struct oftable {
* With few exceptions, ofproto implementations may look at these fields but
* should not modify them. */
struct rule {
- struct list ofproto_node; /* Owned by ofproto base code. */
- 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;
+ /* Used internally by ofproto.c to collect lists of rules for flow_mods and
+ * flow stats requests. */
+ struct list ofproto_node;
+
struct ofoperation *pending; /* Operation now in progress, if nonnull. */
ovs_be64 flow_cookie; /* Controller-issued identifier. Guarded by
@@ -236,7 +244,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 507c9a6..b7ba52d 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2344,7 +2344,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);
@@ -3665,8 +3665,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;
@@ -3678,7 +3678,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);
@@ -6209,7 +6209,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);
@@ -6267,7 +6267,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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
