Here's a new version. The most interesting thing is what I've done around
oftable_remove_rule(). It wasn't strictly necessary, but it made clang thread
safety a lot happier.
---
lib/classifier.c | 3 ++
lib/classifier.h | 57 ++++++++++++++++++++-----------
ofproto/ofproto-dpif.c | 36 ++++++++++++++++++--
ofproto/ofproto-provider.h | 3 +-
ofproto/ofproto.c | 81 +++++++++++++++++++++++++++++++++++++-------
tests/test-classifier.c | 19 +++++++++--
utilities/ovs-ofctl.c | 8 +++++
7 files changed, 170 insertions(+), 37 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c
index a717bd3..556278f 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -25,6 +25,7 @@
#include "odp-util.h"
#include "ofp-util.h"
#include "packets.h"
+#include "ovs-thread.h"
static struct cls_table *find_table(const struct classifier *,
const struct minimask *);
@@ -143,6 +144,7 @@ classifier_init(struct classifier *cls)
cls->n_rules = 0;
hmap_init(&cls->tables);
list_init(&cls->tables_priority);
+ ovs_rwlock_init(&cls->rwlock);
}
/* Destroys 'cls'. Rules within 'cls', if any, are not freed; this is the
@@ -157,6 +159,7 @@ classifier_destroy(struct classifier *cls)
destroy_table(cls, table);
}
hmap_destroy(&cls->tables);
+ ovs_rwlock_destroy(&cls->rwlock);
}
}
diff --git a/lib/classifier.h b/lib/classifier.h
index a14a24d..3f89196 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -24,7 +24,14 @@
* a hash map from fixed field values to "struct cls_rule",
* which can contain a list of otherwise identical rules
* with lower priorities.
- */
+ *
+ * Thread-safety
+ * =============
+ *
+ * When locked properly, the classifier is thread safe as long as the following
+ * conditions are satisfied.
+ * - Only the main thread calls functions requiring a write lock.
+ * - Only the main thread is allowed to iterate over rules. */
#include "flow.h"
#include "hmap.h"
@@ -32,6 +39,8 @@
#include "match.h"
#include "openflow/nicira-ext.h"
#include "openflow/openflow.h"
+#include "ovs-thread.h"
+#include "util.h"
#ifdef __cplusplus
extern "C" {
@@ -42,6 +51,7 @@ struct classifier {
int n_rules; /* Total number of rules. */
struct hmap tables; /* Contains "struct cls_table"s. */
struct list tables_priority; /* Tables in descending priority order */
+ struct ovs_rwlock rwlock;
};
/* A set of rules that all have the same fields wildcarded. */
@@ -88,26 +98,35 @@ bool cls_rule_is_catchall(const struct cls_rule *);
bool cls_rule_is_loose_match(const struct cls_rule *rule,
const struct minimatch *criteria);
-void classifier_init(struct classifier *);
+void classifier_init(struct classifier *cls);
void classifier_destroy(struct classifier *);
-bool classifier_is_empty(const struct classifier *);
-int classifier_count(const struct classifier *);
-void classifier_insert(struct classifier *, struct cls_rule *);
-struct cls_rule *classifier_replace(struct classifier *, struct cls_rule *);
-void classifier_remove(struct classifier *, struct cls_rule *);
-struct cls_rule *classifier_lookup(const struct classifier *,
+bool classifier_is_empty(const struct classifier *cls)
+ OVS_REQ_RDLOCK(cls->rwlock);
+int classifier_count(const struct classifier *cls)
+ OVS_REQ_RDLOCK(cls->rwlock);
+void classifier_insert(struct classifier *cls, struct cls_rule *)
+ OVS_REQ_WRLOCK(cls->rwlock);
+struct cls_rule *classifier_replace(struct classifier *cls, struct cls_rule *)
+ OVS_REQ_WRLOCK(cls->rwlock);
+void classifier_remove(struct classifier *cls, struct cls_rule *)
+ OVS_REQ_WRLOCK(cls->rwlock);
+struct cls_rule *classifier_lookup(const struct classifier *cls,
const struct flow *,
- struct flow_wildcards *);
-bool classifier_rule_overlaps(const struct classifier *,
- const struct cls_rule *);
+ struct flow_wildcards *)
+ OVS_REQ_RDLOCK(cls->rwlock);
+bool classifier_rule_overlaps(const struct classifier *cls,
+ const struct cls_rule *)
+ OVS_REQ_RDLOCK(cls->rwlock);
typedef void cls_cb_func(struct cls_rule *, void *aux);
-struct cls_rule *classifier_find_rule_exactly(const struct classifier *,
- const struct cls_rule *);
-struct cls_rule *classifier_find_match_exactly(const struct classifier *,
+struct cls_rule *classifier_find_rule_exactly(const struct classifier *cls,
+ const struct cls_rule *)
+ OVS_REQ_RDLOCK(cls->rwlock);
+struct cls_rule *classifier_find_match_exactly(const struct classifier *cls,
const struct match *,
- unsigned int priority);
+ unsigned int priority)
+ OVS_REQ_RDLOCK(cls->rwlock);
/* Iteration. */
@@ -117,10 +136,10 @@ struct cls_cursor {
const struct cls_rule *target;
};
-void cls_cursor_init(struct cls_cursor *, const struct classifier *,
- const struct cls_rule *match);
-struct cls_rule *cls_cursor_first(struct cls_cursor *);
-struct cls_rule *cls_cursor_next(struct cls_cursor *, struct cls_rule *);
+void cls_cursor_init(struct cls_cursor *cursor, const struct classifier *cls,
+ const struct cls_rule *match) OVS_REQ_RDLOCK(cls->rwlock);
+struct cls_rule *cls_cursor_first(struct cls_cursor *cursor);
+struct cls_rule *cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *);
#define CLS_CURSOR_FOR_EACH(RULE, MEMBER, CURSOR) \
for (ASSIGN_CONTAINER(RULE, cls_cursor_first(CURSOR), MEMBER); \
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6ef2eb8..68042ef 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -829,7 +829,11 @@ type_run(const char *type)
ofport->may_enable);
}
+ /* Only ofproto-dpif cares about the facet classifier so we just
+ * lock cls_cursor_init() to appease the thread safety analysis. */
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
CLS_CURSOR_FOR_EACH_SAFE (facet, next, cr, &cursor) {
facet_revalidate(facet);
run_fast_rl();
@@ -1453,10 +1457,12 @@ destruct(struct ofproto *ofproto_)
OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
struct cls_cursor cursor;
+ ovs_rwlock_wrlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, up.cr, &cursor) {
- ofproto_rule_destroy(&rule->up);
+ ofproto_rule_destroy(&ofproto->up, &table->cls, &rule->up);
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
ovs_mutex_lock(&ofproto->flow_mod_mutex);
@@ -1617,6 +1623,7 @@ run(struct ofproto *ofproto_)
ovs_rwlock_unlock(&ofproto->ml->rwlock);
/* Check the consistency of a random facet, to aid debugging. */
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
if (time_msec() >= ofproto->consistency_rl
&& !classifier_is_empty(&ofproto->facets)
&& !ofproto->backer->need_revalidate) {
@@ -1636,6 +1643,7 @@ run(struct ofproto *ofproto_)
ofproto->backer->need_revalidate = REV_INCONSISTENCY;
}
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
return 0;
}
@@ -1688,12 +1696,16 @@ get_memory_usage(const struct ofproto *ofproto_, struct
simap *usage)
size_t n_subfacets = 0;
struct facet *facet;
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
simap_increase(usage, "facets", classifier_count(&ofproto->facets));
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
n_subfacets += list_size(&facet->subfacets);
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
simap_increase(usage, "subfacets", n_subfacets);
}
@@ -4364,7 +4376,9 @@ facet_create(const struct flow_miss *miss, struct
rule_dpif *rule,
match_init(&match, &facet->flow, &facet->xout.wc);
cls_rule_init(&facet->cr, &match, OFP_DEFAULT_PRIORITY);
+ ovs_rwlock_wrlock(&ofproto->facets.rwlock);
classifier_insert(&ofproto->facets, &facet->cr);
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
facet->nf_flow.output_iface = facet->xout.nf_output_iface;
facet->fail_open = rule->up.cr.priority == FAIL_OPEN_PRIORITY;
@@ -4432,7 +4446,9 @@ facet_remove(struct facet *facet)
&facet->subfacets) {
subfacet_destroy__(subfacet);
}
+ ovs_rwlock_wrlock(&facet->ofproto->facets.rwlock);
classifier_remove(&facet->ofproto->facets, &facet->cr);
+ ovs_rwlock_unlock(&facet->ofproto->facets.rwlock);
cls_rule_destroy(&facet->cr);
facet_free(facet);
}
@@ -4576,7 +4592,11 @@ facet_flush_stats(struct facet *facet)
static struct facet *
facet_find(struct ofproto_dpif *ofproto, const struct flow *flow)
{
- struct cls_rule *cr = classifier_lookup(&ofproto->facets, flow, NULL);
+ struct cls_rule *cr;
+
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
+ cr = classifier_lookup(&ofproto->facets, flow, NULL);
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
return cr ? CONTAINER_OF(cr, struct facet, cr) : NULL;
}
@@ -4821,6 +4841,7 @@ push_all_stats__(bool run_fast)
struct cls_cursor cursor;
struct facet *facet;
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
facet_push_stats(facet, false);
@@ -4828,6 +4849,7 @@ push_all_stats__(bool run_fast)
run_fast_rl();
}
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
}
rl = time_msec() + 100;
@@ -5148,14 +5170,18 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
struct flow ofpc_normal_flow = *flow;
ofpc_normal_flow.tp_src = htons(0);
ofpc_normal_flow.tp_dst = htons(0);
+ ovs_rwlock_rdlock(&cls->rwlock);
cls_rule = classifier_lookup(cls, &ofpc_normal_flow, wc);
+ ovs_rwlock_unlock(&cls->rwlock);
} else if (frag && ofproto->up.frag_handling == OFPC_FRAG_DROP) {
cls_rule = &ofproto->drop_frags_rule->up.cr;
if (wc) {
flow_wildcards_init_exact(wc);
}
} else {
+ ovs_rwlock_rdlock(&cls->rwlock);
cls_rule = classifier_lookup(cls, flow, wc);
+ ovs_rwlock_unlock(&cls->rwlock);
}
return rule_dpif_cast(rule_from_cls_rule(cls_rule));
}
@@ -5483,10 +5509,12 @@ send_netflow_active_timeouts(struct ofproto_dpif
*ofproto)
struct cls_cursor cursor;
struct facet *facet;
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
send_active_timeout(ofproto, facet);
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
}
static struct ofproto_dpif *
@@ -5885,12 +5913,14 @@ ofproto_dpif_self_check__(struct ofproto_dpif *ofproto,
struct ds *reply)
int errors;
errors = 0;
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
if (!facet_check_consistency(facet)) {
errors++;
}
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
if (errors) {
ofproto->backer->need_revalidate = REV_INCONSISTENCY;
}
@@ -6111,6 +6141,7 @@ ofproto_unixctl_dpif_dump_megaflows(struct unixctl_conn
*conn,
return;
}
+ ovs_rwlock_rdlock(&ofproto->facets.rwlock);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
cls_rule_format(&facet->cr, &ds);
@@ -6133,6 +6164,7 @@ ofproto_unixctl_dpif_dump_megaflows(struct unixctl_conn
*conn,
}
ds_put_cstr(&ds, "\n");
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
ds_chomp(&ds, '\n');
unixctl_command_reply(conn, ds_cstr(&ds));
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 68f4a80..f081482 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -266,7 +266,8 @@ rule_from_cls_rule(const struct cls_rule *cls_rule)
void ofproto_rule_update_used(struct rule *, long long int used);
void ofproto_rule_expire(struct rule *, uint8_t reason);
-void ofproto_rule_destroy(struct rule *);
+void ofproto_rule_destroy(struct ofproto *, struct classifier *cls,
+ struct rule *) OVS_REQ_WRLOCK(cls->rwlock);
bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index fad8746..3cdc72c 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -153,6 +153,9 @@ static void oftable_enable_eviction(struct oftable *,
size_t n_fields);
static void oftable_remove_rule(struct rule *);
+static void oftable_remove_rule__(struct ofproto *ofproto,
+ struct classifier *cls, struct rule *rule)
+ OVS_REQ_WRLOCK(cls->rwlock);
static struct rule *oftable_replace_rule(struct rule *);
static void oftable_substitute_rule(struct rule *old, struct rule *new);
@@ -1018,6 +1021,7 @@ ofproto_configure_table(struct ofproto *ofproto, int
table_id,
}
table->max_flows = s->max_flows;
+ ovs_rwlock_rdlock(&table->cls.rwlock);
if (classifier_count(&table->cls) > table->max_flows
&& table->eviction_fields) {
/* 'table' contains more flows than allowed. We might not be able to
@@ -1033,6 +1037,7 @@ ofproto_configure_table(struct ofproto *ofproto, int
table_id,
break;
}
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
bool
@@ -1066,15 +1071,17 @@ ofproto_flush__(struct ofproto *ofproto)
continue;
}
+ ovs_rwlock_wrlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
if (!rule->pending) {
ofoperation_create(group, rule, OFOPERATION_DELETE,
OFPRR_DELETE);
- oftable_remove_rule(rule);
+ oftable_remove_rule__(ofproto, &table->cls, rule);
ofproto->ofproto_class->rule_destruct(rule);
}
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
ofopgroup_submit(group);
}
@@ -1387,7 +1394,9 @@ ofproto_get_memory_usage(const struct ofproto *ofproto,
struct simap *usage)
n_rules = 0;
OFPROTO_FOR_EACH_TABLE (table, ofproto) {
+ ovs_rwlock_rdlock(&table->cls.rwlock);
n_rules += classifier_count(&table->cls);
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
simap_increase(usage, "rules", n_rules);
@@ -1614,8 +1623,10 @@ ofproto_add_flow(struct ofproto *ofproto, const struct
match *match,
{
const struct rule *rule;
+ ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
rule = rule_from_cls_rule(classifier_find_match_exactly(
&ofproto->tables[0].cls, match, priority));
+ ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
ofpacts, ofpacts_len)) {
struct ofputil_flow_mod fm;
@@ -1652,8 +1663,10 @@ ofproto_delete_flow(struct ofproto *ofproto,
{
struct rule *rule;
+ ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
rule = rule_from_cls_rule(classifier_find_match_exactly(
&ofproto->tables[0].cls, target, priority));
+ ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
if (!rule) {
/* No such rule -> success. */
return true;
@@ -2181,10 +2194,11 @@ ofproto_rule_destroy__(struct rule *rule)
* This function should only be called from an ofproto implementation's
* ->destruct() function. It is not suitable elsewhere. */
void
-ofproto_rule_destroy(struct rule *rule)
+ofproto_rule_destroy(struct ofproto *ofproto, struct classifier *cls,
+ struct rule *rule) OVS_REQ_WRLOCK(cls->rwlock)
{
ovs_assert(!rule->pending);
- oftable_remove_rule(rule);
+ oftable_remove_rule__(ofproto, cls, rule);
ofproto_rule_destroy__(rule);
}
@@ -2616,7 +2630,9 @@ handle_table_stats_request(struct ofconn *ofconn,
ots[i].instructions = htonl(OFPIT11_ALL);
ots[i].config = htonl(OFPTC11_TABLE_MISS_MASK);
ots[i].max_entries = htonl(1000000); /* An arbitrary big number. */
+ ovs_rwlock_rdlock(&p->tables[i].cls.rwlock);
ots[i].active_count = htonl(classifier_count(&p->tables[i].cls));
+ ovs_rwlock_unlock(&p->tables[i].cls.rwlock);
}
p->ofproto_class->get_tables(p, ots);
@@ -2883,9 +2899,11 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t
table_id,
struct cls_cursor cursor;
struct rule *rule;
+ ovs_rwlock_rdlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, &cr);
CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
if (rule->pending) {
+ ovs_rwlock_unlock(&table->cls.rwlock);
error = OFPROTO_POSTPONE;
goto exit;
}
@@ -2895,6 +2913,7 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t
table_id,
list_push_back(rules, &rule->ofproto_node);
}
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
exit:
@@ -2959,8 +2978,10 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t
table_id,
FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
struct rule *rule;
+ ovs_rwlock_rdlock(&table->cls.rwlock);
rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls,
&cr));
+ ovs_rwlock_unlock(&table->cls.rwlock);
if (rule) {
if (rule->pending) {
error = OFPROTO_POSTPONE;
@@ -3080,10 +3101,12 @@ ofproto_get_all_flows(struct ofproto *p, struct ds
*results)
struct cls_cursor cursor;
struct rule *rule;
+ ovs_rwlock_rdlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, NULL);
CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
flow_stats_ds(rule, results);
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
}
@@ -3313,6 +3336,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
struct rule *victim;
struct rule *rule;
uint8_t table_id;
+ bool overlaps;
int error;
error = check_table_id(ofproto, fm->table_id);
@@ -3369,8 +3393,10 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
}
/* Check for overlap, if requested. */
- if (fm->flags & OFPFF_CHECK_OVERLAP
- && classifier_rule_overlaps(&table->cls, &rule->cr)) {
+ ovs_rwlock_rdlock(&table->cls.rwlock);
+ overlaps = classifier_rule_overlaps(&table->cls, &rule->cr);
+ ovs_rwlock_unlock(&table->cls.rwlock);
+ if (fm->flags & OFPFF_CHECK_OVERLAP && overlaps) {
cls_rule_destroy(&rule->cr);
ofproto->ofproto_class->rule_dealloc(rule);
return OFPERR_OFPFMFC_OVERLAP;
@@ -3413,8 +3439,12 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
} else {
struct ofoperation *op;
struct rule *evict;
+ size_t n_rules;
- if (classifier_count(&table->cls) > table->max_flows) {
+ ovs_rwlock_rdlock(&table->cls.rwlock);
+ n_rules = classifier_count(&table->cls);
+ ovs_rwlock_unlock(&table->cls.rwlock);
+ if (n_rules > table->max_flows) {
bool was_evictable;
was_evictable = rule->evictable;
@@ -4099,11 +4129,13 @@ ofproto_collect_ofmonitor_refresh_rules(const struct
ofmonitor *m,
struct cls_cursor cursor;
struct rule *rule;
+ ovs_rwlock_rdlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, &target);
CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
ovs_assert(!rule->pending); /* XXX */
ofproto_collect_ofmonitor_refresh_rule(m, rule, seqno, rules);
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
HMAP_FOR_EACH (op, hmap_node, &ofproto->deletions) {
@@ -5085,9 +5117,17 @@ ofproto_evict(struct ofproto *ofproto)
group = ofopgroup_create_unattached(ofproto);
OFPROTO_FOR_EACH_TABLE (table, ofproto) {
- while (classifier_count(&table->cls) > table->max_flows
- && table->eviction_fields) {
+ while (table->eviction_fields) {
struct rule *rule;
+ size_t n_rules;
+
+ ovs_rwlock_rdlock(&table->cls.rwlock);
+ n_rules = classifier_count(&table->cls);
+ ovs_rwlock_unlock(&table->cls.rwlock);
+
+ if (n_rules <= table->max_flows) {
+ break;
+ }
rule = choose_rule_to_evict(table);
if (!rule || rule->pending) {
@@ -5304,7 +5344,9 @@ oftable_init(struct oftable *table)
static void
oftable_destroy(struct oftable *table)
{
+ ovs_rwlock_rdlock(&table->cls.rwlock);
ovs_assert(classifier_is_empty(&table->cls));
+ ovs_rwlock_unlock(&table->cls.rwlock);
oftable_disable_eviction(table);
classifier_destroy(&table->cls);
free(table->name);
@@ -5384,20 +5426,20 @@ oftable_enable_eviction(struct oftable *table,
hmap_init(&table->eviction_groups_by_id);
heap_init(&table->eviction_groups_by_size);
+ ovs_rwlock_rdlock(&table->cls.rwlock);
cls_cursor_init(&cursor, &table->cls, NULL);
CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
eviction_group_add_rule(rule);
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
/* Removes 'rule' from the oftable that contains it. */
static void
-oftable_remove_rule(struct rule *rule)
+oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls,
+ struct rule *rule) OVS_REQ_WRLOCK(cls->rwlock)
{
- struct ofproto *ofproto = rule->ofproto;
- struct oftable *table = &ofproto->tables[rule->table_id];
-
- classifier_remove(&table->cls, &rule->cr);
+ classifier_remove(cls, &rule->cr);
if (rule->meter_id) {
list_remove(&rule->meter_list_node);
}
@@ -5413,6 +5455,17 @@ oftable_remove_rule(struct rule *rule)
}
}
+static void
+oftable_remove_rule(struct rule *rule)
+{
+ struct ofproto *ofproto = rule->ofproto;
+ struct oftable *table = &ofproto->tables[rule->table_id];
+
+ ovs_rwlock_wrlock(&table->cls.rwlock);
+ oftable_remove_rule__(ofproto, &table->cls, rule);
+ ovs_rwlock_unlock(&table->cls.rwlock);
+}
+
/* Inserts 'rule' into its oftable. Removes any existing rule from 'rule''s
* oftable that has an identical cls_rule. Returns the rule that was removed,
* if any, and otherwise NULL. */
@@ -5438,7 +5491,9 @@ oftable_replace_rule(struct rule *rule)
struct meter *meter = ofproto->meters[rule->meter_id];
list_insert(&meter->rules, &rule->meter_list_node);
}
+ ovs_rwlock_wrlock(&table->cls.rwlock);
victim = rule_from_cls_rule(classifier_replace(&table->cls, &rule->cr));
+ ovs_rwlock_unlock(&table->cls.rwlock);
if (victim) {
if (victim->meter_id) {
list_remove(&victim->meter_list_node);
diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 7fb1447..1d3d2d4 100644
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -396,6 +396,7 @@ get_value(unsigned int *x, unsigned n_values)
static void
compare_classifiers(struct classifier *cls, struct tcls *tcls)
+ OVS_REQ_RDLOCK(cls->rwlock)
{
static const int confidence = 500;
unsigned int i;
@@ -444,17 +445,19 @@ destroy_classifier(struct classifier *cls)
struct test_rule *rule, *next_rule;
struct cls_cursor cursor;
+ ovs_rwlock_wrlock(&cls->rwlock);
cls_cursor_init(&cursor, cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cls_rule, &cursor) {
classifier_remove(cls, &rule->cls_rule);
free_rule(rule);
}
+ ovs_rwlock_unlock(&cls->rwlock);
classifier_destroy(cls);
}
static void
-check_tables(const struct classifier *cls,
- int n_tables, int n_rules, int n_dups)
+check_tables(const struct classifier *cls, int n_tables, int n_rules,
+ int n_dups) OVS_REQ_RDLOCK(cls->rwlock)
{
const struct cls_table *table;
struct test_rule *test_rule;
@@ -610,10 +613,12 @@ test_empty(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
struct tcls tcls;
classifier_init(&cls);
+ ovs_rwlock_rdlock(&cls.rwlock);
tcls_init(&tcls);
assert(classifier_is_empty(&cls));
assert(tcls_is_empty(&tcls));
compare_classifiers(&cls, &tcls);
+ ovs_rwlock_unlock(&cls.rwlock);
classifier_destroy(&cls);
tcls_destroy(&tcls);
}
@@ -640,6 +645,7 @@ test_single_rule(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
hash_bytes(&wc_fields, sizeof wc_fields, 0), 0);
classifier_init(&cls);
+ ovs_rwlock_wrlock(&cls.rwlock);
tcls_init(&tcls);
tcls_rule = tcls_insert(&tcls, rule);
@@ -654,6 +660,7 @@ test_single_rule(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
compare_classifiers(&cls, &tcls);
free_rule(rule);
+ ovs_rwlock_unlock(&cls.rwlock);
classifier_destroy(&cls);
tcls_destroy(&tcls);
}
@@ -677,6 +684,7 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
rule2->aux += 5;
classifier_init(&cls);
+ ovs_rwlock_wrlock(&cls.rwlock);
tcls_init(&tcls);
tcls_insert(&tcls, rule1);
classifier_insert(&cls, &rule1->cls_rule);
@@ -692,6 +700,7 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
check_tables(&cls, 1, 1, 0);
compare_classifiers(&cls, &tcls);
tcls_destroy(&tcls);
+ ovs_rwlock_unlock(&cls.rwlock);
destroy_classifier(&cls);
}
}
@@ -787,6 +796,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char
*argv[] OVS_UNUSED)
}
classifier_init(&cls);
+ ovs_rwlock_wrlock(&cls.rwlock);
tcls_init(&tcls);
for (i = 0; i < ARRAY_SIZE(ops); i++) {
@@ -825,6 +835,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char
*argv[] OVS_UNUSED)
compare_classifiers(&cls, &tcls);
}
+ ovs_rwlock_unlock(&cls.rwlock);
classifier_destroy(&cls);
tcls_destroy(&tcls);
@@ -887,6 +898,7 @@ test_many_rules_in_one_table(int argc OVS_UNUSED, char
*argv[] OVS_UNUSED)
} while ((1 << count_ones(value_mask)) < N_RULES);
classifier_init(&cls);
+ ovs_rwlock_wrlock(&cls.rwlock);
tcls_init(&tcls);
for (i = 0; i < N_RULES; i++) {
@@ -913,6 +925,7 @@ test_many_rules_in_one_table(int argc OVS_UNUSED, char
*argv[] OVS_UNUSED)
compare_classifiers(&cls, &tcls);
}
+ ovs_rwlock_unlock(&cls.rwlock);
classifier_destroy(&cls);
tcls_destroy(&tcls);
}
@@ -947,6 +960,7 @@ test_many_rules_in_n_tables(int n_tables)
shuffle(priorities, ARRAY_SIZE(priorities));
classifier_init(&cls);
+ ovs_rwlock_wrlock(&cls.rwlock);
tcls_init(&tcls);
for (i = 0; i < MAX_RULES; i++) {
@@ -979,6 +993,7 @@ test_many_rules_in_n_tables(int n_tables)
free_rule(target);
}
+ ovs_rwlock_unlock(&cls.rwlock);
destroy_classifier(&cls);
tcls_destroy(&tcls);
}
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 35a2ca7..899ce4e 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1941,11 +1941,13 @@ fte_free_all(struct classifier *cls)
struct cls_cursor cursor;
struct fte *fte, *next;
+ ovs_rwlock_wrlock(&cls->rwlock);
cls_cursor_init(&cursor, cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (fte, next, rule, &cursor) {
classifier_remove(cls, &fte->rule);
fte_free(fte);
}
+ ovs_rwlock_unlock(&cls->rwlock);
classifier_destroy(cls);
}
@@ -1964,7 +1966,9 @@ fte_insert(struct classifier *cls, const struct match
*match,
cls_rule_init(&fte->rule, match, priority);
fte->versions[index] = version;
+ ovs_rwlock_wrlock(&cls->rwlock);
old = fte_from_cls_rule(classifier_replace(cls, &fte->rule));
+ ovs_rwlock_unlock(&cls->rwlock);
if (old) {
fte_version_free(old->versions[index]);
fte->versions[!index] = old->versions[!index];
@@ -2174,6 +2178,7 @@ ofctl_replace_flows(int argc OVS_UNUSED, char *argv[])
list_init(&requests);
/* Delete flows that exist on the switch but not in the file. */
+ ovs_rwlock_rdlock(&cls.rwlock);
cls_cursor_init(&cursor, &cls, NULL);
CLS_CURSOR_FOR_EACH (fte, rule, &cursor) {
struct fte_version *file_ver = fte->versions[FILE_IDX];
@@ -2197,6 +2202,7 @@ ofctl_replace_flows(int argc OVS_UNUSED, char *argv[])
fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol, &requests);
}
}
+ ovs_rwlock_unlock(&cls.rwlock);
transact_multiple_noreply(vconn, &requests);
vconn_close(vconn);
@@ -2238,6 +2244,7 @@ ofctl_diff_flows(int argc OVS_UNUSED, char *argv[])
ds_init(&a_s);
ds_init(&b_s);
+ ovs_rwlock_rdlock(&cls.rwlock);
cls_cursor_init(&cursor, &cls, NULL);
CLS_CURSOR_FOR_EACH (fte, rule, &cursor) {
struct fte_version *a = fte->versions[0];
@@ -2257,6 +2264,7 @@ ofctl_diff_flows(int argc OVS_UNUSED, char *argv[])
}
}
}
+ ovs_rwlock_unlock(&cls.rwlock);
ds_destroy(&a_s);
ds_destroy(&b_s);
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
