[ovs-dev] [PATCH 1/2] classifier: Avoid need for 64-bit priority temporary values in lookup.

Jarno Rajahalme jrajahalme at nicira.com
Thu Oct 30 16:29:25 UTC 2014


On Oct 29, 2014, at 11:46 PM, Ben Pfaff <blp at nicira.com> wrote:

> I have mixed feelings about this change.  I think it might be better to
> change the classifier priority type to "int", then document that negative
> values are reserved for use by the classifier.
> 

I agree that using an “int” would be cleaner. I think the only current “user” of negative values is the test-classifier, so there should not be any fallout due to changing this. Do you want me to do that?

  Jarno

> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/classifier.c | 11 +++++------
> lib/pvector.h    | 18 +++++++++---------
> 2 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/classifier.c b/lib/classifier.c
> index 9f306b9..284eab8 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -668,7 +668,7 @@ classifier_lookup(const struct classifier *cls, const struct flow *flow,
> {
>     const struct cls_partition *partition;
>     tag_type tags;
> -    int64_t best_priority = -1;
> +    unsigned int min_priority = 0;
>     const struct cls_match *best;
>     struct trie_ctx trie_ctx[CLS_MAX_TRIES];
>     struct cls_subtable *subtable;
> @@ -708,7 +708,7 @@ classifier_lookup(const struct classifier *cls, const struct flow *flow,
>     }
> 
>     best = NULL;
> -    PVECTOR_FOR_EACH_PRIORITY(subtable, best_priority, 2,
> +    PVECTOR_FOR_EACH_PRIORITY(subtable, min_priority, 2,
>                               sizeof(struct cls_subtable), &cls->subtables) {
>         struct cls_match *rule;
> 
> @@ -717,8 +717,8 @@ classifier_lookup(const struct classifier *cls, const struct flow *flow,
>         }
> 
>         rule = find_match_wc(subtable, flow, trie_ctx, cls->n_tries, wc);
> -        if (rule && (int64_t)rule->priority > best_priority) {
> -            best_priority = (int64_t)rule->priority;
> +        if (rule->priority >= min_priority) {
> +            min_priority = rule->priority + 1;
>             best = rule;
>         }
>     }
> @@ -789,11 +789,10 @@ classifier_rule_overlaps(const struct classifier *cls,
>     OVS_EXCLUDED(cls->mutex)
> {
>     struct cls_subtable *subtable;
> -    int64_t stop_at_priority = (int64_t)target->priority - 1;
> 
>     ovs_mutex_lock(&cls->mutex);
>     /* Iterate subtables in the descending max priority order. */
> -    PVECTOR_FOR_EACH_PRIORITY (subtable, stop_at_priority, 2,
> +    PVECTOR_FOR_EACH_PRIORITY (subtable, target->priority, 2,
>                                sizeof(struct cls_subtable), &cls->subtables) {
>         uint32_t storage[FLOW_U32S];
>         struct minimask mask;
> diff --git a/lib/pvector.h b/lib/pvector.h
> index 61d71b9..1c00bcb 100644
> --- a/lib/pvector.h
> +++ b/lib/pvector.h
> @@ -118,8 +118,8 @@ void pvector_remove(struct pvector *, void *);
>  * on a new instance.  To see any of the modifications, a new iteration loop
>  * has to be started.
>  *
> - * The PVECTOR_FOR_EACH_PRIORITY limits the iteration to entries with higher
> - * than given priority and allows for object lookahead.
> + * The PVECTOR_FOR_EACH_PRIORITY limits the iteration to entries with a
> + * minimum priority and allows for object lookahead.
>  *
>  * The iteration loop must be completed without entering the OVS RCU quiescent
>  * period.  That is, an old iteration loop must not be continued after any
> @@ -135,20 +135,20 @@ static inline struct pvector_cursor pvector_cursor_init(const struct pvector *,
>                                                         size_t n_ahead,
>                                                         size_t obj_size);
> static inline void *pvector_cursor_next(struct pvector_cursor *,
> -                                        int64_t stop_at_priority,
> +                                        unsigned int min_priority,
>                                         size_t n_ahead, size_t obj_size);
> static inline void pvector_cursor_lookahead(const struct pvector_cursor *,
>                                             int n, size_t size);
> 
> #define PVECTOR_FOR_EACH(PTR, PVECTOR)                                  \
>     for (struct pvector_cursor cursor__ = pvector_cursor_init(PVECTOR, 0, 0); \
> -         ((PTR) = pvector_cursor_next(&cursor__, -1, 0, 0)) != NULL; )
> +         ((PTR) = pvector_cursor_next(&cursor__, 0, 0, 0)) != NULL; )
> 
> -/* Loop while priority is higher than 'PRIORITY' and prefetch objects
> +/* Loop while priority is at least 'MIN_PRIORITY' and prefetch objects
>  * of size 'SZ' 'N' objects ahead from the current object. */
> -#define PVECTOR_FOR_EACH_PRIORITY(PTR, PRIORITY, N, SZ, PVECTOR)        \
> +#define PVECTOR_FOR_EACH_PRIORITY(PTR, MIN_PRIORITY, N, SZ, PVECTOR)        \
>     for (struct pvector_cursor cursor__ = pvector_cursor_init(PVECTOR, N, SZ); \
> -         ((PTR) = pvector_cursor_next(&cursor__, PRIORITY, N, SZ)) != NULL; )
> +         ((PTR) = pvector_cursor_next(&cursor__, MIN_PRIORITY, N, SZ)) != NULL; )
> 
> 
> /* Inline implementations. */
> @@ -176,11 +176,11 @@ pvector_cursor_init(const struct pvector *pvec,
> }
> 
> static inline void *pvector_cursor_next(struct pvector_cursor *cursor,
> -                                        int64_t stop_at_priority,
> +                                        unsigned int min_priority,
>                                         size_t n_ahead, size_t obj_size)
> {
>     if (++cursor->entry_idx < cursor->size &&
> -        cursor->vector[cursor->entry_idx].priority > stop_at_priority) {
> +        cursor->vector[cursor->entry_idx].priority >= min_priority) {
>         if (n_ahead) {
>             pvector_cursor_lookahead(cursor, n_ahead, obj_size);
>         }
> -- 
> 2.1.0
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list