I've made the suggested changes. Thanks for the review. Ethan
On Mon, Jun 30, 2014 at 1:40 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote: > Minor comments below, otherwise: > > Acked-by: Jarno Rajahalme <jrajaha...@nicira.com> > > Jarno > > On Jun 30, 2014, at 1:11 PM, Ethan Jackson <et...@nicira.com> wrote: > >> Used in a future patch. >> >> Signed-off-by: Ethan Jackson <et...@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@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev