On Nov 13, 2014, at 3:50 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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.
>
I folded your incremental in, thanks!
Pushing in a moment,
Jarno
> 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
