[ovs-dev] [batch v2 4/5] classifier: Add a batched miniflow lookup function.

[Ethan Jackson]

I've made the suggested changes.  Thanks for the review.

Ethan

On Mon, Jun 30, 2014 at 1:40 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> Minor comments below, otherwise:
>
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
>
>   Jarno
>
> On Jun 30, 2014, at 1:11 PM, Ethan Jackson <ethan at nicira.com> wrote:
>
>> Used in a future patch.
>>
>> Signed-off-by: Ethan Jackson <ethan at nicira.com>
>> ---
>> lib/classifier.c  | 47 ++++++++++++++++++++++++++++++-----------------
>> lib/classifier.h  |  5 +++--
>> lib/dpif-netdev.c |  2 +-
>> 3 files changed, 34 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/classifier.c b/lib/classifier.c
>> index 330cd67..3479367 100644
>> --- a/lib/classifier.c
>> +++ b/lib/classifier.c
>> @@ -942,32 +942,45 @@ find_match_miniflow(const struct cls_subtable *subtable,
>>     return NULL;
>> }
>>
>> -/* Finds and returns the highest-priority rule in 'cls' that matches
>> - * 'miniflow'.  Returns a null pointer if no rules in 'cls' match 'flow'.
>> - * If multiple rules of equal priority match 'flow', returns one arbitrarily.
>> +/* For each flow miniflow in 'flows' performs a classifier lookup writing the
>
> “flow miniflow” ?
>
>> + * result into the corresponding slot in 'rules'.  If a particular entry in
>> + * 'flows' is NULL it is skipped.
>>  *
>> - * This function is optimized for the userspace datapath, which only ever has
>> - * one priority value for it's flows!
>> - */
>> -struct cls_rule *classifier_lookup_miniflow_first(const struct classifier *cls_,
>> -                                                  const struct miniflow *flow)
>> + * This function is optimized for use in the userspace datapath and therefore
>> + * does not implement a lot of features available in the standard
>> + * classifier_lookup() function.  Specifically, it does not implement
>> + * priorities, instead returning any rule which matches the flow. */
>> +void
>> +classifier_lookup_miniflow_batch(const struct classifier *cls_,
>> +                                 const struct miniflow **flows,
>> +                                 struct cls_rule **rules, size_t len)
>> {
>>     struct cls_classifier *cls = cls_->cls;
>>     struct cls_subtable *subtable;
>> +    size_t i, begin = 0;
>>
>> +    memset(rules, 0, len * sizeof *rules);
>>     PVECTOR_FOR_EACH (subtable, &cls->subtables) {
>> -        struct cls_match *rule;
>> +        for (i = begin; i < len; i++) {
>> +            struct cls_match *match;
>> +            uint32_t hash;
>>
>> -        rule = find_match_miniflow(subtable, flow,
>> -                                   miniflow_hash_in_minimask(flow,
>> -                                                             &subtable->mask,
>> -                                                             0));
>> -        if (rule) {
>> -            return rule->cls_rule;
>> +            if (OVS_UNLIKELY(rules[i] || !flows[i])) {
>> +                continue;
>> +            }
>> +
>> +            hash = miniflow_hash_in_minimask(flows[i], &subtable->mask, 0);
>> +            match = find_match_miniflow(subtable, flows[i], hash);
>> +            if (OVS_UNLIKELY(match)) {
>> +                rules[i] = match->cls_rule;
>> +            }
>>         }
>> -    }
>>
>> -    return NULL;
>> +        for (; begin < len && (rules[begin] || !flows[begin]); begin++);
>
> I would find this more readable as a while loop with "begin++;” as the body.
>
>> +        if (begin == len) {
>> +            break;
>> +        }
>> +    }
>> }
>>
>> /* Finds and returns a rule in 'cls' with exactly the same priority and
>> diff --git a/lib/classifier.h b/lib/classifier.h
>> index 326ca08..602d504 100644
>> --- a/lib/classifier.h
>> +++ b/lib/classifier.h
>> @@ -286,8 +286,9 @@ struct cls_rule *classifier_lookup(const struct classifier *cls,
>>                                    const struct flow *,
>>                                    struct flow_wildcards *)
>>     OVS_REQ_RDLOCK(cls->rwlock);
>> -struct cls_rule *classifier_lookup_miniflow_first(const struct classifier *cls,
>> -                                                  const struct miniflow *)
>> +void classifier_lookup_miniflow_batch(const struct classifier *cls,
>> +                                      const struct miniflow **flows,
>> +                                      struct cls_rule **rules, size_t len)
>>     OVS_REQ_RDLOCK(cls->rwlock);
>> bool classifier_rule_overlaps(const struct classifier *cls,
>>                               const struct cls_rule *)
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 4deb763..3a2b68f 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1078,7 +1078,7 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp, const struct miniflow *key)
>>     struct dp_netdev_flow *netdev_flow;
>>     struct cls_rule *rule;
>>
>> -    rule = classifier_lookup_miniflow_first(&dp->cls, key);
>> +    classifier_lookup_miniflow_batch(&dp->cls, &key, &rule, 1);
>>     netdev_flow = dp_netdev_flow_cast(rule);
>>
>>     return netdev_flow;
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>