Signed-off-by: Ethan Jackson <et...@nicira.com>
---
lib/classifier.c | 3 +++
lib/classifier.h | 55 +++++++++++++++++++++++++++++++----------------
ofproto/ofproto-dpif.c | 18 +++++++++++++++-
ofproto/ofproto.c | 43 +++++++++++++++++++++++++++++++-----
tests/test-classifier.c | 19 ++++++++++++++--
utilities/ovs-ofctl.c | 4 ++++
6 files changed, 116 insertions(+), 26 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..51d7c32 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 *,
+void cls_cursor_init(struct cls_cursor *cursor, const struct classifier *cls,
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 *);
+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 b484a07..a466bc9 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1619,6 +1619,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) {
@@ -1638,6 +1639,7 @@ run(struct ofproto *ofproto_)
ofproto->backer->need_revalidate = REV_INCONSISTENCY;
}
}
+ ovs_rwlock_unlock(&ofproto->facets.rwlock);
return 0;
}
@@ -1690,7 +1692,9 @@ 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);
cls_cursor_init(&cursor, &ofproto->facets, NULL);
CLS_CURSOR_FOR_EACH (facet, cr, &cursor) {
@@ -4360,7 +4364,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;
@@ -4428,7 +4434,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);
}
@@ -4572,7 +4580,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;
}
@@ -5137,14 +5149,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));
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0625ccf..49fcaa9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1017,6 +1017,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
@@ -1032,6 +1033,7 @@ ofproto_configure_table(struct ofproto *ofproto, int
table_id,
break;
}
}
+ ovs_rwlock_unlock(&table->cls.rwlock);
}
bool
@@ -1385,7 +1387,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);
@@ -1612,8 +1616,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;
@@ -1650,8 +1656,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;
@@ -2613,7 +2621,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);
@@ -2956,8 +2966,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;
@@ -3306,6 +3318,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);
@@ -3362,8 +3375,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;
@@ -3401,8 +3416,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;
@@ -5069,9 +5088,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) {
@@ -5282,7 +5309,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);
@@ -5375,7 +5404,9 @@ 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);
classifier_remove(&table->cls, &rule->cr);
+ ovs_rwlock_unlock(&table->cls.rwlock);
if (rule->meter_id) {
list_remove(&rule->meter_list_node);
}
@@ -5408,7 +5439,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 68b73bf..5ee7ed1 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1891,7 +1891,9 @@ fte_free_all(struct classifier *cls)
cls_cursor_init(&cursor, cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (fte, next, rule, &cursor) {
+ ovs_rwlock_wrlock(&cls->rwlock);
classifier_remove(cls, &fte->rule);
+ ovs_rwlock_unlock(&cls->rwlock);
fte_free(fte);
}
classifier_destroy(cls);
@@ -1912,7 +1914,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];
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
