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 <pkusunyif...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev