On Oct 29, 2014, at 11:46 PM, Ben Pfaff <b...@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 <b...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
