[ovs-dev] [PATCH] classifier: refactor classifier_remove and introduce classifier_remove_assert
Yifeng Sun
pkusunyifeng at gmail.com
Tue Jan 30 15:56:07 UTC 2018
The return type of classifier_remove is changed to bool. Besides,
classifier_remove_assert is introduced to assert that the classifier
must contain the rule. This patch is based on Ben's advice.
Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
---
lib/classifier.c | 21 +++++++++++++++++----
lib/classifier.h | 6 ++++--
lib/ovs-router.c | 15 +++++----------
lib/tnl-ports.c | 5 ++---
ofproto/ofproto.c | 14 ++++----------
5 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c
index 16c451da1b30..4a4aacfd6208 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -701,9 +701,9 @@ classifier_insert(struct classifier *cls, const struct cls_rule *rule,
*
* Does nothing if 'rule' has been already removed, or was never inserted.
*
- * Returns the removed rule, or NULL, if it was already removed.
+ * Returns true on success, or false, if it was already removed.
*/
-const struct cls_rule *
+bool
classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule)
{
struct cls_match *rule, *prev, *next, *head;
@@ -716,7 +716,7 @@ classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule)
rule = get_cls_match_protected(cls_rule);
if (!rule) {
- return NULL;
+ return false;
}
/* Mark as removed. */
ovsrcu_set(&CONST_CAST(struct cls_rule *, cls_rule)->cls_match, NULL);
@@ -820,7 +820,20 @@ check_priority:
ovsrcu_postpone(cls_match_free_cb, rule);
cls->n_rules--;
- return cls_rule;
+ return true;
+}
+
+/* Removes 'rule' from 'cls'. It is the caller's responsibility to destroy
+ * 'rule' with cls_rule_destroy(), freeing the memory block in which 'rule'
+ * resides, etc., as necessary.
+ *
+ * Asserts that the rule must be in the classifier.
+ */
+void
+classifier_remove_assert(struct classifier *cls,
+ const struct cls_rule *cls_rule)
+{
+ ovs_assert(classifier_remove(cls, cls_rule));
}
/* Prefix tree context. Valid when 'lookup_done' is true. Can skip all
diff --git a/lib/classifier.h b/lib/classifier.h
index f0ea5a9cb8b2..7699d58e1b07 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -387,8 +387,10 @@ const struct cls_rule *classifier_replace(struct classifier *,
ovs_version_t,
const struct cls_conjunction *,
size_t n_conjunctions);
-const struct cls_rule *classifier_remove(struct classifier *,
- const struct cls_rule *);
+bool classifier_remove(struct classifier *,
+ const struct cls_rule *);
+void classifier_remove_assert(struct classifier *,
+ const struct cls_rule *);
static inline void classifier_defer(struct classifier *);
static inline void classifier_publish(struct classifier *);
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index cd2ab15fb003..a7d55c754d16 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -245,19 +245,15 @@ ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen,
ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw);
}
-static bool
+static void
__rt_entry_delete(const struct cls_rule *cr)
{
struct ovs_router_entry *p = ovs_router_entry_cast(cr);
tnl_port_map_delete_ipdev(p->output_bridge);
/* Remove it. */
- cr = classifier_remove(&cls, cr);
- if (cr) {
- ovsrcu_postpone(rt_entry_free, ovs_router_entry_cast(cr));
- return true;
- }
- return false;
+ classifier_remove_assert(&cls, cr);
+ ovsrcu_postpone(rt_entry_free, p);
}
static bool
@@ -267,7 +263,6 @@ rt_entry_delete(uint32_t mark, uint8_t priority,
const struct cls_rule *cr;
struct cls_rule rule;
struct match match;
- bool res = false;
rt_init_match(&match, mark, ip6_dst, plen);
@@ -277,12 +272,12 @@ rt_entry_delete(uint32_t mark, uint8_t priority,
cr = classifier_find_rule_exactly(&cls, &rule, OVS_VERSION_MAX);
if (cr) {
ovs_mutex_lock(&mutex);
- res = __rt_entry_delete(cr);
+ __rt_entry_delete(cr);
ovs_mutex_unlock(&mutex);
}
cls_rule_destroy(&rule);
- return res;
+ return (cr != NULL);
}
static bool
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 04d2b3f7c6cf..b814f7a0a50a 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -223,9 +223,8 @@ tnl_port_unref(const struct cls_rule *cr)
struct tnl_port_in *p = tnl_port_cast(cr);
if (cr && ovs_refcount_unref_relaxed(&p->ref_cnt) == 1) {
- if (classifier_remove(&cls, cr)) {
- ovsrcu_postpone(tnl_port_free, p);
- }
+ classifier_remove_assert(&cls, cr);
+ ovsrcu_postpone(tnl_port_free, p);
}
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 4f17f79d286f..536636393850 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1520,10 +1520,8 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
/* Make sure there is no postponed removal of the rule. */
ovs_assert(cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX));
- if (!classifier_remove(&rule->ofproto->tables[rule->table_id].cls,
- &rule->cr)) {
- OVS_NOT_REACHED();
- }
+ classifier_remove_assert(&rule->ofproto->tables[rule->table_id].cls,
+ &rule->cr);
ofproto_rule_remove__(rule->ofproto, rule);
if (ofproto->ofproto_class->rule_delete) {
ofproto->ofproto_class->rule_delete(rule);
@@ -2885,9 +2883,7 @@ remove_rule_rcu__(struct rule *rule)
struct oftable *table = &ofproto->tables[rule->table_id];
ovs_assert(!cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX));
- if (!classifier_remove(&table->cls, &rule->cr)) {
- OVS_NOT_REACHED();
- }
+ classifier_remove_assert(&table->cls, &rule->cr);
if (ofproto->ofproto_class->rule_delete) {
ofproto->ofproto_class->rule_delete(rule);
}
@@ -5231,9 +5227,7 @@ replace_rule_revert(struct ofproto *ofproto,
}
/* Remove the new rule immediately. It was never visible to lookups. */
- if (!classifier_remove(&table->cls, &new_rule->cr)) {
- OVS_NOT_REACHED();
- }
+ classifier_remove_assert(&table->cls, &new_rule->cr);
ofproto_rule_remove__(ofproto, new_rule);
ofproto_rule_unref(new_rule);
}
--
2.7.4
More information about the dev
mailing list