[ovs-dev] [PATCH v3 5/5] classifier: Defer pvector publication.
Jarno Rajahalme
jrajahalme at nicira.com
Tue Nov 11 20:01:03 UTC 2014
This patch adds a new 'defer' parameter to the classifier modification
API, and a new function classifier_publish(), that can be used to make
deferred modifications visible to classifier lookups. Calling any of
the modification functions (classifier_insert(), classifier_replace(),
and classifier_remove()) with 'defer' as 'false' will make also
earlier deferred modifications visible to lookups.
Currently the deferring is limited to the visibility of the subtable
vector changes. pvector now processes modifications mostly in a
working copy, which needs to be explicitly published with
pvector_publish(). pvector_publish() sorts the working copy and
removes gaps before publishing it.
This change helps avoiding O(n**2) memory behavior in corner cases,
where large number of rules with different masks are inserted or
deleted.
VMware-BZ: #1322017
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/classifier.c | 32 ++++++++---
lib/classifier.h | 14 ++++-
lib/dpif-netdev.c | 2 +
lib/ovs-router.c | 11 ++--
lib/pvector.c | 139 +++++++++++++++++++++++------------------------
lib/pvector.h | 25 +++++++--
ofproto/ofproto.c | 105 +++++++++++++++++++----------------
tests/test-classifier.c | 27 ++++-----
utilities/ovs-ofctl.c | 6 +-
9 files changed, 208 insertions(+), 153 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c
index 0d91035..bc70aa3 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -499,7 +499,8 @@ subtable_replace_head_rule(struct classifier *cls OVS_UNUSED,
* subtable list will only be visible after the subtable's max priority is
* updated. */
const struct cls_rule *
-classifier_replace(struct classifier *cls, const struct cls_rule *rule)
+classifier_replace(struct classifier *cls, const struct cls_rule *rule,
+ bool defer)
{
struct cls_subtable *subtable;
struct cls_match *head;
@@ -595,7 +596,11 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule)
ovsrcu_postpone(free, iter);
old->cls_match = NULL;
- /* No change in subtable's max priority or max count. */
+ /* No change in subtable's max priority or max count,
+ * but earlier calls may have deferred publication. */
+ if (!defer) {
+ pvector_publish(&cls->subtables);
+ }
/* Make rule visible to iterators. */
rculist_replace(&CONST_CAST(struct cls_rule *, rule)->node,
@@ -626,6 +631,11 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule)
/* Nothing was replaced. */
cls->n_rules++;
+
+ if (!defer) {
+ pvector_publish(&cls->subtables);
+ }
+
return NULL;
}
@@ -636,10 +646,11 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule)
* fixed fields, and priority). Use classifier_find_rule_exactly() to find
* such a rule. */
void
-classifier_insert(struct classifier *cls, const struct cls_rule *rule)
+classifier_insert(struct classifier *cls, const struct cls_rule *rule,
+ bool defer)
{
- const struct cls_rule *displaced_rule = classifier_replace(cls, rule);
- ovs_assert(!displaced_rule);
+ const struct cls_rule *old_rule = classifier_replace(cls, rule, defer);
+ ovs_assert(!old_rule);
}
/* Removes 'rule' from 'cls'. It is the caller's responsibility to destroy
@@ -651,7 +662,8 @@ classifier_insert(struct classifier *cls, const struct cls_rule *rule)
* Returns the removed rule, or NULL, if it was already removed.
*/
const struct cls_rule *
-classifier_remove(struct classifier *cls, const struct cls_rule *rule)
+classifier_remove(struct classifier *cls, const struct cls_rule *rule,
+ bool defer)
{
struct cls_partition *partition;
struct cls_match *cls_match;
@@ -665,7 +677,8 @@ classifier_remove(struct classifier *cls, const struct cls_rule *rule)
cls_match = rule->cls_match;
if (!cls_match) {
- return NULL;
+ rule = NULL;
+ goto out;
}
/* Mark as removed. */
CONST_CAST(struct cls_rule *, rule)->cls_match = NULL;
@@ -759,9 +772,14 @@ check_priority:
pvector_change_priority(&cls->subtables, subtable, max_priority);
}
}
+
free:
ovsrcu_postpone(free, cls_match);
cls->n_rules--;
+out:
+ if (!defer) {
+ pvector_publish(&cls->subtables);
+ }
return rule;
}
diff --git a/lib/classifier.h b/lib/classifier.h
index a11de5c..7a88108 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -283,11 +283,13 @@ void classifier_destroy(struct classifier *);
bool classifier_set_prefix_fields(struct classifier *,
const enum mf_field_id *trie_fields,
unsigned int n_trie_fields);
-void classifier_insert(struct classifier *, const struct cls_rule *);
+void classifier_insert(struct classifier *, const struct cls_rule *,
+ bool defer);
const struct cls_rule *classifier_replace(struct classifier *,
- const struct cls_rule *);
+ const struct cls_rule *, bool defer);
const struct cls_rule *classifier_remove(struct classifier *,
- const struct cls_rule *);
+ const struct cls_rule *, bool defer);
+static inline void classifier_publish(struct classifier *);
/* Lookups. These are RCU protected and may run concurrently with modifiers
* and each other. */
@@ -343,4 +345,10 @@ void cls_cursor_advance(struct cls_cursor *);
}
#endif
+
+static inline void
+classifier_publish(struct classifier *cls)
+{
+ pvector_publish(&cls->subtables);
+}
#endif /* classifier.h */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bee9330..8886a42 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3290,6 +3290,7 @@ dpcls_create_subtable(struct dpcls *cls, const struct netdev_flow_key *mask)
netdev_flow_key_clone(&subtable->mask, mask);
cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash);
pvector_insert(&cls->subtables, subtable, 0);
+ pvector_publish(&cls->subtables);
return subtable;
}
@@ -3332,6 +3333,7 @@ dpcls_remove(struct dpcls *cls, struct dpcls_rule *rule)
if (cmap_remove(&subtable->rules, &rule->cmap_node, rule->flow.hash)
== 0) {
dpcls_destroy_subtable(cls, subtable);
+ pvector_publish(&cls->subtables);
}
}
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index 0ce68ba..f515542 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -117,7 +117,7 @@ ovs_router_insert__(uint8_t priority, ovs_be32 ip_dst, uint8_t plen,
cls_rule_init(&p->cr, &match, priority); /* Longest prefix matches first. */
ovs_mutex_lock(&mutex);
- cr = classifier_replace(&cls, &p->cr);
+ cr = classifier_replace(&cls, &p->cr, false);
ovs_mutex_unlock(&mutex);
if (cr) {
@@ -149,7 +149,7 @@ rt_entry_delete(uint8_t priority, ovs_be32 ip_dst, uint8_t plen)
if (cr) {
/* Remove it. */
ovs_mutex_lock(&mutex);
- cr = classifier_remove(&cls, cr);
+ cr = classifier_remove(&cls, cr, false);
ovs_mutex_unlock(&mutex);
if (cr) {
@@ -262,13 +262,14 @@ ovs_router_flush(void)
{
struct ovs_router_entry *rt;
+ ovs_mutex_lock(&mutex);
CLS_FOR_EACH(rt, cr, &cls) {
if (rt->priority == rt->plen) {
- ovs_mutex_lock(&mutex);
- classifier_remove(&cls, &rt->cr);
- ovs_mutex_unlock(&mutex);
+ classifier_remove(&cls, &rt->cr, true);
}
}
+ classifier_publish(&cls);
+ ovs_mutex_unlock(&mutex);
}
/* May not be called more than once. */
diff --git a/lib/pvector.c b/lib/pvector.c
index 3f2e9e8..fd30819 100644
--- a/lib/pvector.c
+++ b/lib/pvector.c
@@ -38,7 +38,14 @@ pvector_impl_alloc(size_t size)
static struct pvector_impl *
pvector_impl_dup(struct pvector_impl *old)
{
- return xmemdup(old, sizeof *old + old->allocated * sizeof old->vector[0]);
+ struct pvector_impl *impl;
+ size_t alloc = old->size + PVECTOR_EXTRA_ALLOC;
+
+ impl = xmalloc(sizeof *impl + alloc * sizeof impl->vector[0]);
+ impl->allocated = alloc;
+ impl->size = old->size;
+ memcpy(impl->vector, old->vector, old->size * sizeof old->vector[0]);
+ return impl;
}
/* Initializes 'pvec' as an empty concurrent priority vector. */
@@ -46,6 +53,7 @@ void
pvector_init(struct pvector *pvec)
{
ovsrcu_set(&pvec->impl, pvector_impl_alloc(PVECTOR_EXTRA_ALLOC));
+ pvec->temp = NULL;
}
/* Destroys 'pvec'.
@@ -55,6 +63,8 @@ pvector_init(struct pvector *pvec)
void
pvector_destroy(struct pvector *pvec)
{
+ free(pvec->temp);
+ pvec->temp = NULL;
ovsrcu_postpone(free, pvector_impl_get(pvec));
ovsrcu_set(&pvec->impl, NULL); /* Poison. */
}
@@ -81,23 +91,10 @@ static void
pvector_impl_sort(struct pvector_impl *impl)
{
qsort(impl->vector, impl->size, sizeof *impl->vector, pvector_entry_cmp);
-}
-
-/* Returns the index with priority equal or lower than 'target_priority',
- * which will be one past the vector if none exists. */
-static int
-pvector_impl_find_priority(struct pvector_impl *impl,
- int target_priority)
-{
- const struct pvector_entry *entry;
- int index;
-
- PVECTOR_IMPL_FOR_EACH (entry, index, impl) {
- if (entry->priority <= target_priority) {
- break;
- }
+ /* Trim gaps. */
+ while (impl->size && impl->vector[impl->size - 1].priority == INT_MIN) {
+ impl->size = impl->size - 1;
}
- return index;
}
/* Returns the index of the 'ptr' in the vector, or -1 if none is found. */
@@ -118,15 +115,13 @@ pvector_impl_find(struct pvector_impl *impl, void *target)
void
pvector_insert(struct pvector *pvec, void *ptr, int priority)
{
- struct pvector_impl *old, *new;
- int index;
+ struct pvector_impl *temp = pvec->temp;
+ struct pvector_impl *old = pvector_impl_get(pvec);
ovs_assert(ptr != NULL);
- old = pvector_impl_get(pvec);
-
/* Check if can add to the end without reallocation. */
- if (old->allocated > old->size &&
+ if (!temp && old->allocated > old->size &&
(!old->size || priority <= old->vector[old->size - 1].priority)) {
old->vector[old->size].ptr = ptr;
old->vector[old->size].priority = priority;
@@ -135,78 +130,80 @@ pvector_insert(struct pvector *pvec, void *ptr, int priority)
atomic_thread_fence(memory_order_release);
++old->size;
} else {
- new = pvector_impl_alloc(old->size + 1 + PVECTOR_EXTRA_ALLOC);
-
- index = pvector_impl_find_priority(old, priority);
- /* Now at the insertion index. */
- memcpy(new->vector, old->vector, index * sizeof old->vector[0]);
- new->vector[index].ptr = ptr;
- new->vector[index].priority = priority;
- memcpy(&new->vector[index + 1], &old->vector[index],
- (old->size - index) * sizeof old->vector[0]);
- new->size = old->size + 1;
-
- ovsrcu_set(&pvec->impl, new);
- ovsrcu_postpone(free, old);
+ if (!temp) {
+ temp = pvector_impl_dup(old);
+ pvec->temp = temp;
+ } else if (temp->size == temp->allocated) {
+ temp = pvector_impl_dup(temp);
+ free(pvec->temp);
+ pvec->temp = temp;
+ }
+ /* Insert at the end, publish will sort. */
+ temp->vector[temp->size].ptr = ptr;
+ temp->vector[temp->size].priority = priority;
+ temp->size += 1;
}
}
void
pvector_remove(struct pvector *pvec, void *ptr)
{
- struct pvector_impl *old, *new;
+ struct pvector_impl *temp = pvec->temp;
int index;
- old = pvector_impl_get(pvec);
-
- ovs_assert(old->size > 0);
-
- index = pvector_impl_find(old, ptr);
+ if (!temp) {
+ temp = pvector_impl_dup(pvector_impl_get(pvec));
+ pvec->temp = temp;
+ }
+ ovs_assert(temp->size > 0);
+ index = pvector_impl_find(temp, ptr);
ovs_assert(index >= 0);
- /* Now at the index of the entry to be deleted. */
-
- /* We do not try to delete the last entry without reallocation so that
- * the readers can read the 'size' once in the beginning of each iteration.
- */
-
- /* Keep extra space for insertions to the end. */
- new = pvector_impl_alloc(old->size - 1 + PVECTOR_EXTRA_ALLOC);
-
- memcpy(new->vector, old->vector, index * sizeof old->vector[0]);
- memcpy(&new->vector[index], &old->vector[index + 1],
- (old->size - (index + 1)) * sizeof old->vector[0]);
-
- new->size = old->size - 1;
-
- ovsrcu_set(&pvec->impl, new);
- ovsrcu_postpone(free, old);
+ /* Now at the index of the entry to be deleted.
+ * Clear in place, publish will sort and clean these off. */
+ temp->vector[index].ptr = NULL;
+ temp->vector[index].priority = INT_MIN;
}
/* Change entry's 'priority' and keep the vector ordered. */
void
pvector_change_priority(struct pvector *pvec, void *ptr, int priority)
{
- struct pvector_impl *old = pvector_impl_get(pvec);
- int index = pvector_impl_find(old, ptr);
+ struct pvector_impl *old = pvec->temp;
+ int index;
+
+ if (!old) {
+ old = pvector_impl_get(pvec);
+ }
+
+ index = pvector_impl_find(old, ptr);
ovs_assert(index >= 0);
/* Now at the index of the entry to be updated. */
+ /* Check if can not update in place. */
if ((priority > old->vector[index].priority && index > 0
&& priority > old->vector[index - 1].priority)
|| (priority < old->vector[index].priority && index < old->size - 1
&& priority < old->vector[index + 1].priority)) {
- /* Have to reallocate to reorder. */
- struct pvector_impl *new = pvector_impl_dup(old);
+ /* Have to use a temp. */
+ if (!pvec->temp) {
+ /* Have to reallocate to reorder. */
+ pvec->temp = pvector_impl_dup(old);
+ old = pvec->temp;
+ /* Publish will sort. */
+ }
+ }
+ old->vector[index].priority = priority;
+}
- new->vector[index].priority = priority;
- pvector_impl_sort(new);
+/* Make the modified pvector available for iteration. */
+void pvector_publish__(struct pvector *pvec)
+{
+ struct pvector_impl *temp = pvec->temp;
- ovsrcu_set(&pvec->impl, new);
- ovsrcu_postpone(free, old);
- } else {
- /* Can update in place. Readers are free to use either value,
- * so we do not try to synchronize here. */
- old->vector[index].priority = priority;
- }
+ pvec->temp = NULL;
+ pvector_impl_sort(temp); /* Also removes gaps. */
+ ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
+ &pvec->impl));
+ ovsrcu_set(&pvec->impl, temp);
}
diff --git a/lib/pvector.h b/lib/pvector.h
index 0d94b56..04e77f5 100644
--- a/lib/pvector.h
+++ b/lib/pvector.h
@@ -19,6 +19,7 @@
#include <stdbool.h>
#include <stdint.h>
+#include <stdlib.h>
#include "ovs-rcu.h"
#include "util.h"
@@ -68,21 +69,25 @@ struct pvector_impl {
/* Concurrent priority vector. */
struct pvector {
OVSRCU_TYPE(struct pvector_impl *) impl;
+ struct pvector_impl *temp;
};
/* Initialization. */
void pvector_init(struct pvector *);
void pvector_destroy(struct pvector *);
-/* Count. */
-static inline size_t pvector_count(const struct pvector *);
-static inline bool pvector_is_empty(const struct pvector *);
-
-/* Insertion and deletion. */
+/* Insertion and deletion. These work on 'temp'. */
void pvector_insert(struct pvector *, void *, int priority);
void pvector_change_priority(struct pvector *, void *, int priority);
void pvector_remove(struct pvector *, void *);
+/* Make the modified pvector available for iteration. */
+static inline void pvector_publish(struct pvector *);
+
+/* Count. These operate on the published pvector. */
+static inline size_t pvector_count(const struct pvector *);
+static inline bool pvector_is_empty(const struct pvector *);
+
/* Iteration.
*
*
@@ -216,4 +221,14 @@ static inline bool pvector_is_empty(const struct pvector *pvec)
return pvector_count(pvec) == 0;
}
+void pvector_publish__(struct pvector *);
+
+/* Make the modified pvector available for iteration. */
+static inline void pvector_publish(struct pvector *pvec)
+{
+ if (pvec->temp) {
+ pvector_publish__(pvec);
+ }
+}
+
#endif /* pvector.h */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dad4bb0..56e9bca 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -89,8 +89,6 @@ static void oftable_enable_eviction(struct oftable *,
size_t n_fields);
static void oftable_remove_rule(struct rule *rule) OVS_REQUIRES(ofproto_mutex);
-static void oftable_remove_rule__(struct ofproto *, struct rule *)
- OVS_REQUIRES(ofproto_mutex);
/* A set of rules within a single OpenFlow table (oftable) that have the same
* values for the oftable's eviction_fields. A rule to be evicted, when one is
@@ -237,6 +235,8 @@ struct ofport_usage {
/* rule. */
static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
static bool rule_is_readonly(const struct rule *);
+static void ofproto_rule_remove__(struct ofproto *, struct rule *)
+ OVS_REQUIRES(ofproto_mutex);
/* The source of a flow_mod request, in the code that processes flow_mods.
*
@@ -1317,18 +1317,6 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops)
connmgr_get_snoops(ofproto->connmgr, snoops);
}
-static void
-ofproto_rule_delete__(struct rule *rule, uint8_t reason)
- OVS_REQUIRES(ofproto_mutex)
-{
- struct rule_collection rules;
-
- rules.rules = rules.stub;
- rules.n = 1;
- rules.stub[0] = rule;
- delete_flows__(&rules, reason, NULL);
-}
-
/* Deletes 'rule' from 'ofproto'.
*
* Within an ofproto implementation, this function allows an ofproto
@@ -1346,7 +1334,7 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
* switch is being deleted and any OpenFlow channels have been or soon will
* be killed. */
ovs_mutex_lock(&ofproto_mutex);
- oftable_remove_rule__(ofproto, rule);
+ oftable_remove_rule(rule);
ofproto->ofproto_class->rule_delete(rule);
ovs_mutex_unlock(&ofproto_mutex);
}
@@ -1370,15 +1358,20 @@ ofproto_flush__(struct ofproto *ofproto)
ovs_mutex_lock(&ofproto_mutex);
OFPROTO_FOR_EACH_TABLE (table, ofproto) {
+ struct rule_collection rules;
struct rule *rule;
if (table->flags & OFTABLE_HIDDEN) {
continue;
}
+ rule_collection_init(&rules);
+
CLS_FOR_EACH (rule, cr, &table->cls) {
- ofproto_rule_delete__(rule, OFPRR_DELETE);
+ rule_collection_add(&rules, rule);
}
+ delete_flows__(&rules, OFPRR_DELETE, NULL);
+ rule_collection_destroy(&rules);
}
/* XXX: Concurrent handler threads may insert new learned flows based on
* learn actions of the now deleted flows right after we release
@@ -4102,29 +4095,32 @@ handle_queue_stats_request(struct ofconn *ofconn,
return error;
}
-static bool
-should_evict_a_rule(struct oftable *table, unsigned int extra_space)
- OVS_REQUIRES(ofproto_mutex)
- OVS_NO_THREAD_SAFETY_ANALYSIS
-{
- return classifier_count(&table->cls) + extra_space > table->max_flows;
-}
-
static enum ofperr
evict_rules_from_table(struct oftable *table, unsigned int extra_space)
OVS_REQUIRES(ofproto_mutex)
{
- while (should_evict_a_rule(table, extra_space)) {
+ enum ofperr error = 0;
+ struct rule_collection rules;
+ unsigned int count = classifier_count(&table->cls) + extra_space;
+ unsigned int max_flows = table->max_flows;
+
+ rule_collection_init(&rules);
+
+ while (count-- > max_flows) {
struct rule *rule;
if (!choose_rule_to_evict(table, &rule)) {
- return OFPERR_OFPFMFC_TABLE_FULL;
+ error = OFPERR_OFPFMFC_TABLE_FULL;
+ break;
} else {
- ofproto_rule_delete__(rule, OFPRR_EVICTION);
+ eviction_group_remove_rule(rule);
+ rule_collection_add(&rules, rule);
}
}
+ delete_flows__(&rules, OFPRR_EVICTION, NULL);
+ rule_collection_destroy(&rules);
- return 0;
+ return error;
}
/* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
@@ -4272,7 +4268,7 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
meter_insert_rule(rule);
}
- classifier_insert(&table->cls, &rule->cr);
+ classifier_insert(&table->cls, &rule->cr, true);
error = ofproto->ofproto_class->rule_insert(rule);
if (error) {
@@ -4280,6 +4276,9 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
ofproto_rule_unref(rule);
return error;
}
+
+ classifier_publish(&table->cls);
+
learned_cookies_inc(ofproto, actions);
if (minimask_get_vid_mask(&rule->cr.match.mask) == VLAN_VID_MASK) {
@@ -4498,21 +4497,29 @@ delete_flows__(const struct rule_collection *rules,
if (rules->n) {
struct list dead_cookies = LIST_INITIALIZER(&dead_cookies);
struct ofproto *ofproto = rules->rules[0]->ofproto;
+ struct rule *rule, *next;
size_t i;
- for (i = 0; i < rules->n; i++) {
- struct rule *rule = rules->rules[i];
- const struct rule_actions *actions = rule_get_actions(rule);
+ for (i = 0, next = rules->rules[0];
+ rule = next, next = (++i < rules->n) ? rules->rules[i] : NULL,
+ rule; ) {
+ struct classifier *cls = &ofproto->tables[rule->table_id].cls;
+ uint8_t next_table = next ? next->table_id : UINT8_MAX;
ofproto_rule_send_removed(rule, reason);
ofmonitor_report(ofproto->connmgr, rule, NXFME_DELETED, reason,
req ? req->ofconn : NULL, req ? req->xid : 0,
NULL);
- oftable_remove_rule(rule);
+
+ classifier_remove(cls, &rule->cr, next_table == rule->table_id);
+
+ ofproto_rule_remove__(ofproto, rule);
+
ofproto->ofproto_class->rule_delete(rule);
- learned_cookies_dec(ofproto, actions, &dead_cookies);
+ learned_cookies_dec(ofproto, rule_get_actions(rule),
+ &dead_cookies);
}
learned_cookies_flush(ofproto, &dead_cookies);
ofmonitor_flush(ofproto->connmgr);
@@ -4538,7 +4545,7 @@ delete_flows_loose(struct ofproto *ofproto,
error = collect_rules_loose(ofproto, &criteria, &rules);
rule_criteria_destroy(&criteria);
- if (!error && rules.n > 0) {
+ if (!error) {
delete_flows__(&rules, fm->delete_reason, req);
}
rule_collection_destroy(&rules);
@@ -4564,7 +4571,7 @@ delete_flow_strict(struct ofproto *ofproto, const struct ofputil_flow_mod *fm,
error = collect_rules_strict(ofproto, &criteria, &rules);
rule_criteria_destroy(&criteria);
- if (!error && rules.n > 0) {
+ if (!error) {
delete_flows__(&rules, fm->delete_reason, req);
}
rule_collection_destroy(&rules);
@@ -4611,7 +4618,12 @@ void
ofproto_rule_expire(struct rule *rule, uint8_t reason)
OVS_REQUIRES(ofproto_mutex)
{
- ofproto_rule_delete__(rule, reason);
+ struct rule_collection rules;
+
+ rules.rules = rules.stub;
+ rules.n = 1;
+ rules.stub[0] = rule;
+ delete_flows__(&rules, reason, NULL);
}
/* Reduces '*timeout' to no more than 'max'. A value of zero in either case
@@ -5293,9 +5305,7 @@ handle_delete_meter(struct ofconn *ofconn, struct ofputil_meter_mod *mm)
}
}
}
- if (rules.n > 0) {
- delete_flows__(&rules, OFPRR_METER_DELETE, NULL);
- }
+ delete_flows__(&rules, OFPRR_METER_DELETE, NULL);
/* Delete the meters. */
meter_delete(ofproto, first, last);
@@ -6642,15 +6652,12 @@ oftable_enable_eviction(struct oftable *table,
}
}
-/* Removes 'rule' from the oftable that contains it. */
+/* Removes 'rule' from the ofproto data structures AFTER caller has removed
+ * it from the classifier. */
static void
-oftable_remove_rule__(struct ofproto *ofproto, struct rule *rule)
+ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
OVS_REQUIRES(ofproto_mutex)
{
- struct classifier *cls = &ofproto->tables[rule->table_id].cls;
-
- classifier_remove(cls, &rule->cr);
-
cookies_remove(ofproto, rule);
eviction_group_remove_rule(rule);
@@ -6667,7 +6674,11 @@ static void
oftable_remove_rule(struct rule *rule)
OVS_REQUIRES(ofproto_mutex)
{
- oftable_remove_rule__(rule->ofproto, rule);
+ struct classifier *cls = &rule->ofproto->tables[rule->table_id].cls;
+
+ if (classifier_remove(cls, &rule->cr, false)) {
+ ofproto_rule_remove__(rule->ofproto, rule);
+ }
}
/* unixctl commands. */
diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 273b0b9..b254d2a 100644
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -454,7 +454,7 @@ destroy_classifier(struct classifier *cls)
struct test_rule *rule;
CLS_FOR_EACH (rule, cls_rule, cls) {
- if (classifier_remove(cls, &rule->cls_rule)) {
+ if (classifier_remove(cls, &rule->cls_rule, true)) {
ovsrcu_postpone(free_rule, rule);
}
}
@@ -735,11 +735,11 @@ test_single_rule(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_init(&tcls);
tcls_rule = tcls_insert(&tcls, rule);
- classifier_insert(&cls, &rule->cls_rule);
+ classifier_insert(&cls, &rule->cls_rule, false);
compare_classifiers(&cls, &tcls);
check_tables(&cls, 1, 1, 0);
- classifier_remove(&cls, &rule->cls_rule);
+ classifier_remove(&cls, &rule->cls_rule, false);
tcls_remove(&tcls, tcls_rule);
assert(classifier_is_empty(&cls));
assert(tcls_is_empty(&tcls));
@@ -772,7 +772,7 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
set_prefix_fields(&cls);
tcls_init(&tcls);
tcls_insert(&tcls, rule1);
- classifier_insert(&cls, &rule1->cls_rule);
+ classifier_insert(&cls, &rule1->cls_rule, false);
compare_classifiers(&cls, &tcls);
check_tables(&cls, 1, 1, 0);
tcls_destroy(&tcls);
@@ -781,11 +781,12 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_insert(&tcls, rule2);
assert(test_rule_from_cls_rule(
- classifier_replace(&cls, &rule2->cls_rule)) == rule1);
+ classifier_replace(&cls, &rule2->cls_rule, false))
+ == rule1);
ovsrcu_postpone(free_rule, rule1);
compare_classifiers(&cls, &tcls);
check_tables(&cls, 1, 1, 0);
- classifier_remove(&cls, &rule2->cls_rule);
+ classifier_remove(&cls, &rule2->cls_rule, true);
tcls_destroy(&tcls);
destroy_classifier(&cls);
@@ -895,7 +896,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_rules[j] = tcls_insert(&tcls, rules[j]);
displaced_rule = test_rule_from_cls_rule(
- classifier_replace(&cls, &rules[j]->cls_rule));
+ classifier_replace(&cls, &rules[j]->cls_rule, false));
if (pri_rules[pris[j]] >= 0) {
int k = pri_rules[pris[j]];
assert(displaced_rule != NULL);
@@ -907,7 +908,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
}
pri_rules[pris[j]] = j;
} else {
- classifier_remove(&cls, &rules[j]->cls_rule);
+ classifier_remove(&cls, &rules[j]->cls_rule, false);
tcls_remove(&tcls, tcls_rules[j]);
tcls_rules[j] = NULL;
pri_rules[pris[j]] = -1;
@@ -922,7 +923,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
}
for (i = 0; i < N_RULES; i++) {
- if (classifier_remove(&cls, &rules[i]->cls_rule)) {
+ if (classifier_remove(&cls, &rules[i]->cls_rule, true)) {
ovsrcu_postpone(free_rule, rules[i]);
}
}
@@ -997,7 +998,7 @@ test_many_rules_in_one_table(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
rules[i] = make_rule(wcf, priority, value_pats[i]);
tcls_rules[i] = tcls_insert(&tcls, rules[i]);
- classifier_insert(&cls, &rules[i]->cls_rule);
+ classifier_insert(&cls, &rules[i]->cls_rule, false);
compare_classifiers(&cls, &tcls);
check_tables(&cls, 1, i + 1, 0);
@@ -1005,7 +1006,7 @@ test_many_rules_in_one_table(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
for (i = 0; i < N_RULES; i++) {
tcls_remove(&tcls, tcls_rules[i]);
- classifier_remove(&cls, &rules[i]->cls_rule);
+ classifier_remove(&cls, &rules[i]->cls_rule, false);
compare_classifiers(&cls, &tcls);
ovsrcu_postpone(free_rule, rules[i]);
@@ -1056,7 +1057,7 @@ test_many_rules_in_n_tables(int n_tables)
int value_pat = random_uint32() & ((1u << CLS_N_FIELDS) - 1);
rule = make_rule(wcf, priority, value_pat);
tcls_insert(&tcls, rule);
- classifier_insert(&cls, &rule->cls_rule);
+ classifier_insert(&cls, &rule->cls_rule, false);
compare_classifiers(&cls, &tcls);
check_tables(&cls, -1, i + 1, -1);
}
@@ -1068,7 +1069,7 @@ test_many_rules_in_n_tables(int n_tables)
target = clone_rule(tcls.rules[random_range(tcls.n_rules)]);
CLS_FOR_EACH_TARGET (rule, cls_rule, &cls, &target->cls_rule) {
- if (classifier_remove(&cls, &rule->cls_rule)) {
+ if (classifier_remove(&cls, &rule->cls_rule, false)) {
ovsrcu_postpone(free_rule, rule);
}
}
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index a24bd68..eb7da0e 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2355,7 +2355,7 @@ fte_free_all(struct classifier *cls)
struct fte *fte;
CLS_FOR_EACH (fte, rule, cls) {
- classifier_remove(cls, &fte->rule);
+ classifier_remove(cls, &fte->rule, true);
ovsrcu_postpone(fte_free, fte);
}
classifier_destroy(cls);
@@ -2376,7 +2376,7 @@ fte_insert(struct classifier *cls, const struct match *match,
cls_rule_init(&fte->rule, match, priority);
fte->versions[index] = version;
- old = fte_from_cls_rule(classifier_replace(cls, &fte->rule));
+ old = fte_from_cls_rule(classifier_replace(cls, &fte->rule, true));
if (old) {
fte->versions[!index] = old->versions[!index];
old->versions[!index] = NULL;
@@ -2428,6 +2428,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index)
fte_insert(cls, &fm.match, fm.priority, version, index);
}
+ classifier_publish(cls);
ds_destroy(&s);
if (file != stdin) {
@@ -2530,6 +2531,7 @@ read_flows_from_switch(struct vconn *vconn,
fte_insert(cls, &fs.match, fs.priority, version, index);
}
+ classifier_publish(cls);
ofpbuf_uninit(&ofpacts);
}
--
1.7.10.4
More information about the dev
mailing list