On Thu, Nov 13, 2014 at 11:56:13AM -0800, Jarno Rajahalme wrote:
> There is no point in adding duplicate information into prefix tries.
>
> Also, since the lower-priority duplicate rules are not visible to
> lookups, they do not need to be in staged lookup indices directly
> either (the head rule is).
>
> Finally, now that cmap operations return the number of elements in the
> cmap, subtable's 'n_rules' member is not needed any more.
>
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
Acked-by: Ben Pfaff <b...@nicira.com>
I played with the code a little to improve my understanding of it, and
ended up with the following incremental change. You're welcome to use
it or ignore it, as you like.
diff --git a/lib/classifier.c b/lib/classifier.c
index 5b01a69..37ac880 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -571,50 +571,49 @@ classifier_replace(struct classifier *cls, struct
cls_rule *rule)
cmap_insert(&subtable->indices[i], &new->index_nodes[i], ihash[i]);
}
n_rules = cmap_insert(&subtable->rules, &new->cmap_node, hash);
-
} else { /* Equal rules exist in the classifier already. */
struct cls_match *iter;
- struct cls_rule *old = NULL;
/* Scan the list for the insertion point that will keep the list in
* order of decreasing priority. */
FOR_EACH_RULE_IN_LIST_PROTECTED (iter, head) {
if (rule->priority >= iter->priority) {
- if (rule->priority == iter->priority) {
- old = CONST_CAST(struct cls_rule *, iter->cls_rule);
- }
break;
}
}
/* 'iter' now at the insertion point or NULL it at end. */
- if (old) {
- rculist_replace(&new->list, &iter->list);
- } else {
- if (!iter) {
- rculist_push_back(&head->list, &new->list);
+ if (iter) {
+ struct cls_rule *old;
+
+ if (rule->priority == iter->priority) {
+ rculist_replace(&new->list, &iter->list);
+ old = CONST_CAST(struct cls_rule *, iter->cls_rule);
} else {
- /* Insert 'rule' right before 'iter'. */
rculist_insert(&iter->list, &new->list);
+ old = NULL;
}
- }
- /* Replace the existing head in data structures, if rule is the new
- * head. */
- if (iter == head) {
- subtable_replace_head_rule(cls, subtable, head, new, hash, ihash);
- }
+ /* Replace the existing head in data structures, if rule is the new
+ * head. */
+ if (iter == head) {
+ subtable_replace_head_rule(cls, subtable, head,
+ new, hash, ihash);
+ }
- if (old) {
- ovsrcu_postpone(free, iter);
- old->cls_match = NULL;
+ if (old) {
+ 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. */
- /* Return displaced rule. Caller is responsible for keeping it
- * around until all threads quiesce. */
- ovs_mutex_unlock(&cls->mutex);
- return old;
+ /* Return displaced rule. Caller is responsible for keeping it
+ * around until all threads quiesce. */
+ ovs_mutex_unlock(&cls->mutex);
+ return old;
+ }
+ } else {
+ rculist_push_back(&head->list, &new->list);
}
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
