Now that it is clear that struct cls_classifier itself does not need RCU indirection and pvector is defined in its own header, it is possible get rid of the indirection from struct classifier to struct cls_classifier.
Suggested-by: YAMAMOTO Takashi <yamam...@valinux.co.jp> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> --- lib/classifier.c | 120 ++++++++++++++--------------------------------- lib/classifier.h | 29 ++++++++++-- tests/test-classifier.c | 35 +++++++------- 3 files changed, 76 insertions(+), 108 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index fe38a55..723749a 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -26,52 +26,23 @@ #include "list.h" #include "odp-util.h" #include "ofp-util.h" -#include "ovs-thread.h" #include "packets.h" -#include "pvector.h" #include "tag.h" #include "util.h" #include "vlog.h" VLOG_DEFINE_THIS_MODULE(classifier); -struct trie_node; struct trie_ctx; /* Ports trie depends on both ports sharing the same ovs_be32. */ #define TP_PORTS_OFS32 (offsetof(struct flow, tp_src) / 4) BUILD_ASSERT_DECL(TP_PORTS_OFS32 == offsetof(struct flow, tp_dst) / 4); -typedef OVSRCU_TYPE(struct trie_node *) rcu_trie_ptr; - -/* Prefix trie for a 'field' */ -struct cls_trie { - const struct mf_field *field; /* Trie field, or NULL. */ - rcu_trie_ptr root; /* NULL if none. */ -}; - -enum { - CLS_MAX_INDICES = 3 /* Maximum number of lookup indices per subtable. */ -}; - -struct cls_classifier { - struct ovs_mutex mutex; - int n_rules OVS_GUARDED; /* Total number of rules. */ - uint8_t n_flow_segments; - uint8_t flow_segments[CLS_MAX_INDICES]; /* Flow segment boundaries to use - * for staged lookup. */ - struct cmap subtables_map; /* Contains "struct cls_subtable"s. */ - struct pvector subtables; - struct cmap partitions; /* Contains "struct cls_partition"s. */ - struct cls_trie tries[CLS_MAX_TRIES]; /* Prefix tries. */ - unsigned int n_tries; -}; - /* A set of rules that all have the same fields wildcarded. */ struct cls_subtable { /* The fields are only used by writers and iterators. */ - struct cmap_node cmap_node; /* Within struct cls_classifier - * 'subtables_map'. */ + struct cmap_node cmap_node; /* Within struct classifier 'subtables_map'. */ /* The fields are only used by writers. */ int n_rules OVS_GUARDED; /* Number of rules, including @@ -100,8 +71,7 @@ struct cls_subtable { * field) with tags for the "cls_subtable"s that contain rules that match that * metadata value. */ struct cls_partition { - struct cmap_node cmap_node; /* In struct cls_classifier's 'partitions' - * map. */ + struct cmap_node cmap_node; /* In struct classifier's 'partitions' map. */ ovs_be64 metadata; /* metadata value for this partition. */ tag_type tags; /* OR of each flow's cls_subtable tag. */ struct tag_tracker tracker OVS_GUARDED; /* Tracks the bits in 'tags'. */ @@ -143,15 +113,15 @@ cls_match_alloc(struct cls_rule *rule) return cls_match; } -static struct cls_subtable *find_subtable(const struct cls_classifier *cls, +static struct cls_subtable *find_subtable(const struct classifier *cls, const struct minimask *) OVS_REQUIRES(cls->mutex); -static struct cls_subtable *insert_subtable(struct cls_classifier *cls, +static struct cls_subtable *insert_subtable(struct classifier *cls, const struct minimask *) OVS_REQUIRES(cls->mutex); -static void destroy_subtable(struct cls_classifier *cls, struct cls_subtable *) +static void destroy_subtable(struct classifier *cls, struct cls_subtable *) OVS_REQUIRES(cls->mutex); -static struct cls_match *insert_rule(struct cls_classifier *cls, +static struct cls_match *insert_rule(struct classifier *cls, struct cls_subtable *, struct cls_rule *) OVS_REQUIRES(cls->mutex); @@ -177,7 +147,7 @@ static struct cls_match *next_rule_in_list(struct cls_match *); static unsigned int minimask_get_prefix_len(const struct minimask *, const struct mf_field *); -static void trie_init(struct cls_classifier *cls, int trie_idx, +static void trie_init(struct classifier *cls, int trie_idx, const struct mf_field *) OVS_REQUIRES(cls->mutex); static unsigned int trie_lookup(const struct cls_trie *, const struct flow *, @@ -476,16 +446,11 @@ cls_rule_is_catchall(const struct cls_rule *rule) /* Initializes 'cls' as a classifier that initially contains no classification * rules. */ void -classifier_init(struct classifier *cls_, const uint8_t *flow_segments) - OVS_EXCLUDED(cls_->cls->mutex) +classifier_init(struct classifier *cls, const uint8_t *flow_segments) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = xmalloc(sizeof *cls); - ovs_mutex_init(&cls->mutex); - ovs_mutex_lock(&cls->mutex); - cls_->cls = cls; - cls->n_rules = 0; cmap_init(&cls->subtables_map); pvector_init(&cls->subtables); @@ -508,19 +473,14 @@ classifier_init(struct classifier *cls_, const uint8_t *flow_segments) * caller's responsibility. * May only be called after all the readers have been terminated. */ void -classifier_destroy(struct classifier *cls_) - OVS_EXCLUDED(cls_->cls->mutex) +classifier_destroy(struct classifier *cls) + OVS_EXCLUDED(cls->mutex) { - if (cls_) { - struct cls_classifier *cls = cls_->cls; + if (cls) { struct cls_partition *partition, *next_partition; struct cls_subtable *subtable, *next_subtable; int i; - if (!cls) { - return; - } - ovs_mutex_lock(&cls->mutex); for (i = 0; i < cls->n_tries; i++) { trie_destroy(&cls->tries[i].root); @@ -541,7 +501,6 @@ classifier_destroy(struct classifier *cls_) pvector_destroy(&cls->subtables); ovs_mutex_unlock(&cls->mutex); ovs_mutex_destroy(&cls->mutex); - free(cls); } } @@ -550,12 +509,11 @@ BUILD_ASSERT_DECL(MFF_N_IDS <= 64); /* Set the fields for which prefix lookup should be performed. */ bool -classifier_set_prefix_fields(struct classifier *cls_, +classifier_set_prefix_fields(struct classifier *cls, const enum mf_field_id *trie_fields, unsigned int n_fields) - OVS_EXCLUDED(cls_->cls->mutex) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = cls_->cls; uint64_t fields = 0; const struct mf_field * new_fields[CLS_MAX_TRIES]; int i, n_tries = 0; @@ -633,8 +591,7 @@ classifier_set_prefix_fields(struct classifier *cls_, } static void -trie_init(struct cls_classifier *cls, int trie_idx, - const struct mf_field *field) +trie_init(struct classifier *cls, int trie_idx, const struct mf_field *field) OVS_REQUIRES(cls->mutex) { struct cls_trie *trie = &cls->tries[trie_idx]; @@ -675,7 +632,7 @@ trie_init(struct cls_classifier *cls, int trie_idx, bool classifier_is_empty(const struct classifier *cls) { - return cmap_is_empty(&cls->cls->subtables_map); + return cmap_is_empty(&cls->subtables_map); } /* Returns the number of rules in 'cls'. */ @@ -685,7 +642,7 @@ classifier_count(const struct classifier *cls) { /* n_rules is an int, so in the presence of concurrent writers this will * return either the old or a new value. */ - return cls->cls->n_rules; + return cls->n_rules; } static uint32_t @@ -696,8 +653,7 @@ hash_metadata(ovs_be64 metadata_) } static struct cls_partition * -find_partition(const struct cls_classifier *cls, ovs_be64 metadata, - uint32_t hash) +find_partition(const struct classifier *cls, ovs_be64 metadata, uint32_t hash) { struct cls_partition *partition; @@ -711,7 +667,7 @@ find_partition(const struct cls_classifier *cls, ovs_be64 metadata, } static struct cls_partition * -create_partition(struct cls_classifier *cls, struct cls_subtable *subtable, +create_partition(struct classifier *cls, struct cls_subtable *subtable, ovs_be64 metadata) OVS_REQUIRES(cls->mutex) { @@ -749,10 +705,9 @@ static inline ovs_be32 minimatch_get_ports(const struct minimatch *match) * rule, even rules that cannot have any effect because the new rule matches a * superset of their flows and has higher priority. */ struct cls_rule * -classifier_replace(struct classifier *cls_, struct cls_rule *rule) - OVS_EXCLUDED(cls_->cls->mutex) +classifier_replace(struct classifier *cls, struct cls_rule *rule) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = cls_->cls; struct cls_match *old_rule; struct cls_subtable *subtable; struct cls_rule *old_cls_rule = NULL; @@ -823,10 +778,9 @@ classifier_insert(struct classifier *cls, struct cls_rule *rule) * 'rule' with cls_rule_destroy(), freeing the memory block in which 'rule' * resides, etc., as necessary. */ void -classifier_remove(struct classifier *cls_, struct cls_rule *rule) - OVS_EXCLUDED(cls_->cls->mutex) +classifier_remove(struct classifier *cls, struct cls_rule *rule) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = cls_->cls; struct cls_partition *partition; struct cls_match *cls_match = rule->cls_match; struct cls_match *head; @@ -944,10 +898,9 @@ trie_ctx_init(struct trie_ctx *ctx, const struct cls_trie *trie) * earlier, 'wc' should have been initialized (e.g., by * flow_wildcards_init_catchall()). */ struct cls_rule * -classifier_lookup(const struct classifier *cls_, const struct flow *flow, +classifier_lookup(const struct classifier *cls, const struct flow *flow, struct flow_wildcards *wc) { - struct cls_classifier *cls = cls_->cls; const struct cls_partition *partition; tag_type tags; int64_t best_priority = -1; @@ -1056,11 +1009,10 @@ find_match_miniflow(const struct cls_subtable *subtable, * 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_, +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; @@ -1094,11 +1046,10 @@ classifier_lookup_miniflow_batch(const struct classifier *cls_, * matching criteria as 'target'. Returns a null pointer if 'cls' doesn't * contain an exact match. */ struct cls_rule * -classifier_find_rule_exactly(const struct classifier *cls_, +classifier_find_rule_exactly(const struct classifier *cls, const struct cls_rule *target) - OVS_EXCLUDED(cls_->cls->mutex) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = cls_->cls; struct cls_match *head, *rule; struct cls_subtable *subtable; @@ -1149,11 +1100,10 @@ classifier_find_match_exactly(const struct classifier *cls, * considered to overlap if both rules have the same priority and a packet * could match both. */ bool -classifier_rule_overlaps(const struct classifier *cls_, +classifier_rule_overlaps(const struct classifier *cls, const struct cls_rule *target) - OVS_EXCLUDED(cls_->cls->mutex) + OVS_EXCLUDED(cls->mutex) { - struct cls_classifier *cls = cls_->cls; struct cls_subtable *subtable; int64_t stop_at_priority = (int64_t)target->priority - 1; @@ -1277,7 +1227,7 @@ struct cls_cursor cls_cursor_init(const struct classifier *cls, struct cls_rule *cls_rule = NULL; cursor.safe = safe; - cursor.cls = cls->cls; + cursor.cls = cls; cursor.target = target && !cls_rule_is_catchall(target) ? target : NULL; /* Find first rule. */ @@ -1358,7 +1308,7 @@ cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_) } static struct cls_subtable * -find_subtable(const struct cls_classifier *cls, const struct minimask *mask) +find_subtable(const struct classifier *cls, const struct minimask *mask) OVS_REQUIRES(cls->mutex) { struct cls_subtable *subtable; @@ -1374,7 +1324,7 @@ find_subtable(const struct cls_classifier *cls, const struct minimask *mask) /* The new subtable will be visible to the readers only after this. */ static struct cls_subtable * -insert_subtable(struct cls_classifier *cls, const struct minimask *mask) +insert_subtable(struct classifier *cls, const struct minimask *mask) OVS_REQUIRES(cls->mutex) { uint32_t hash = minimask_hash(mask, 0); @@ -1437,7 +1387,7 @@ insert_subtable(struct cls_classifier *cls, const struct minimask *mask) } static void -destroy_subtable(struct cls_classifier *cls, struct cls_subtable *subtable) +destroy_subtable(struct classifier *cls, struct cls_subtable *subtable) OVS_REQUIRES(cls->mutex) { int i; @@ -1741,7 +1691,7 @@ find_equal(struct cls_subtable *subtable, const struct miniflow *flow, * the subtable until they see the updated partitions. */ static struct cls_match * -insert_rule(struct cls_classifier *cls, struct cls_subtable *subtable, +insert_rule(struct classifier *cls, struct cls_subtable *subtable, struct cls_rule *new_rule) OVS_REQUIRES(cls->mutex) { @@ -1967,7 +1917,7 @@ trie_node_rcu_realloc(const struct trie_node *node) return new_node; } -/* May only be called while holding the cls_classifier mutex. */ +/* May only be called while holding the classifier mutex. */ static void trie_destroy(rcu_trie_ptr *trie) { diff --git a/lib/classifier.h b/lib/classifier.h index d0a408b..1159610 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -216,23 +216,43 @@ #include "cmap.h" #include "match.h" #include "meta-flow.h" +#include "ovs-thread.h" +#include "pvector.h" #ifdef __cplusplus extern "C" { #endif /* Classifier internal data structures. */ -struct cls_classifier; struct cls_subtable; struct cls_match; +struct trie_node; +typedef OVSRCU_TYPE(struct trie_node *) rcu_trie_ptr; + +/* Prefix trie for a 'field' */ +struct cls_trie { + const struct mf_field *field; /* Trie field, or NULL. */ + rcu_trie_ptr root; /* NULL if none. */ +}; + enum { - CLS_MAX_TRIES = 3 /* Maximum number of prefix trees per classifier. */ + CLS_MAX_INDICES = 3, /* Maximum number of lookup indices per subtable. */ + CLS_MAX_TRIES = 3 /* Maximum number of prefix trees per classifier. */ }; /* A flow classifier. */ struct classifier { - struct cls_classifier *cls; + struct ovs_mutex mutex; + int n_rules OVS_GUARDED; /* Total number of rules. */ + uint8_t n_flow_segments; + uint8_t flow_segments[CLS_MAX_INDICES]; /* Flow segment boundaries to use + * for staged lookup. */ + struct cmap subtables_map; /* Contains "struct cls_subtable"s. */ + struct pvector subtables; + struct cmap partitions; /* Contains "struct cls_partition"s. */ + struct cls_trie tries[CLS_MAX_TRIES]; /* Prefix tries. */ + unsigned int n_tries; }; /* A rule to be inserted to the classifier. */ @@ -291,7 +311,7 @@ struct cls_rule *classifier_find_match_exactly(const struct classifier *, /* Iteration. */ struct cls_cursor { - const struct cls_classifier *cls; + const struct classifier *cls; const struct cls_subtable *subtable; const struct cls_rule *target; struct cmap_cursor subtables; @@ -299,7 +319,6 @@ struct cls_cursor { bool safe; }; - /* Iteration requires mutual exclusion of writers. We do this by taking * a mutex for the duration of the iteration, except for the * 'SAFE' variant, where we release the mutex for the body of the loop. */ diff --git a/tests/test-classifier.c b/tests/test-classifier.c index 1d5515c..253e9b3 100644 --- a/tests/test-classifier.c +++ b/tests/test-classifier.c @@ -432,7 +432,7 @@ compare_classifiers(struct classifier *cls, struct tcls *tcls) flow.nw_tos = nw_dscp_values[get_value(&x, N_NW_DSCP_VALUES)]; /* This assertion is here to suppress a GCC 4.9 array-bounds warning */ - ovs_assert(cls->cls->n_tries <= CLS_MAX_TRIES); + ovs_assert(cls->n_tries <= CLS_MAX_TRIES); cr0 = classifier_lookup(cls, &flow, &wc); cr1 = tcls_lookup(tcls, &flow); @@ -462,7 +462,7 @@ destroy_classifier(struct classifier *cls) } static void -pvector_verify(struct pvector *pvec) +pvector_verify(const struct pvector *pvec) { void *ptr OVS_UNUSED; unsigned int priority, prev_priority = UINT_MAX; @@ -495,9 +495,8 @@ trie_verify(const rcu_trie_ptr *trie, unsigned int ofs, unsigned int n_bits) } static void -verify_tries(struct classifier *cls_) +verify_tries(struct classifier *cls) { - struct cls_classifier *cls = cls_->cls; unsigned int n_rules = 0; int i; @@ -521,8 +520,8 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, int found_dups = 0; int found_rules2 = 0; - pvector_verify(&cls->cls->subtables); - CMAP_FOR_EACH (table, cmap_node, &cls->cls->subtables_map) { + pvector_verify(&cls->subtables); + CMAP_FOR_EACH (table, cmap_node, &cls->subtables_map) { const struct cls_match *head; unsigned int max_priority = 0; unsigned int max_count = 0; @@ -530,7 +529,7 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, const struct cls_subtable *iter; /* Locate the subtable from 'subtables'. */ - PVECTOR_FOR_EACH (iter, &cls->cls->subtables) { + PVECTOR_FOR_EACH (iter, &cls->subtables) { if (iter == table) { if (found) { VLOG_ABORT("Subtable %p duplicated in 'subtables'.", @@ -545,10 +544,10 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, assert(!cmap_is_empty(&table->rules)); - ovs_mutex_lock(&cls->cls->mutex); + ovs_mutex_lock(&cls->mutex); assert(trie_verify(&table->ports_trie, 0, table->ports_mask_len) == (table->ports_mask_len ? table->n_rules : 0)); - ovs_mutex_unlock(&cls->cls->mutex); + ovs_mutex_unlock(&cls->mutex); found_tables++; CMAP_FOR_EACH (head, cmap_node, &table->rules) { @@ -563,7 +562,7 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, } found_rules++; - ovs_mutex_lock(&cls->cls->mutex); + ovs_mutex_lock(&cls->mutex); LIST_FOR_EACH (rule, list, &head->list) { assert(rule->priority < prev_priority); assert(rule->priority <= table->max_priority); @@ -571,22 +570,22 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, prev_priority = rule->priority; found_rules++; found_dups++; - ovs_mutex_unlock(&cls->cls->mutex); + ovs_mutex_unlock(&cls->mutex); assert(classifier_find_rule_exactly(cls, rule->cls_rule) == rule->cls_rule); - ovs_mutex_lock(&cls->cls->mutex); + ovs_mutex_lock(&cls->mutex); } - ovs_mutex_unlock(&cls->cls->mutex); + ovs_mutex_unlock(&cls->mutex); } - ovs_mutex_lock(&cls->cls->mutex); + ovs_mutex_lock(&cls->mutex); assert(table->max_priority == max_priority); assert(table->max_count == max_count); - ovs_mutex_unlock(&cls->cls->mutex); + ovs_mutex_unlock(&cls->mutex); } - assert(found_tables == cmap_count(&cls->cls->subtables_map)); - assert(found_tables == pvector_count(&cls->cls->subtables)); - assert(n_tables == -1 || n_tables == cmap_count(&cls->cls->subtables_map)); + assert(found_tables == cmap_count(&cls->subtables_map)); + assert(found_tables == pvector_count(&cls->subtables)); + assert(n_tables == -1 || n_tables == cmap_count(&cls->subtables_map)); assert(n_rules == -1 || found_rules == n_rules); assert(n_dups == -1 || found_dups == n_dups); -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev