From bd53aa1723a007415d26fdd8fb7da05acc06aad0 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 6 Jul 2015 11:45:54 -0700 Subject: [PATCH] classifier: Make versioning more explicit. Now that struct cls_match has 'add_version' the 'version' in cls_match was largely redundant. Remove 'version' from struct cls_rule, and add it to function prototypes that need it. This makes versioning more explicit (or less indirect) in the API. Suggested-by: Ben Pfaff Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- lib/classifier.c | 120 ++++++++++++++++++---------------------- lib/classifier.h | 36 ++++++------ lib/ovs-router.c | 8 +-- lib/tnl-ports.c | 4 +- ofproto/ofproto.c | 32 ++++++----- tests/test-classifier.c | 80 +++++++++++++-------------- tests/test-ovn.c | 5 +- utilities/ovs-ofctl.c | 5 +- 8 files changed, 142 insertions(+), 148 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index db6a16916..a8a4780f6 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -87,7 +87,7 @@ cls_conjunction_set_alloc(struct cls_match *match, } static struct cls_match * -cls_match_alloc(const struct cls_rule *rule, +cls_match_alloc(const struct cls_rule *rule, cls_version_t version, const struct cls_conjunction conj[], size_t n) { int count = count_1bits(rule->match.flow.map); @@ -99,9 +99,9 @@ cls_match_alloc(const struct cls_rule *rule, ovsrcu_init(&cls_match->next, NULL); *CONST_CAST(const struct cls_rule **, &cls_match->cls_rule) = rule; *CONST_CAST(int *, &cls_match->priority) = rule->priority; - *CONST_CAST(cls_version_t *, &cls_match->add_version) = rule->version; - atomic_init(&cls_match->remove_version, rule->version); /* Initially - invisible. */ + *CONST_CAST(cls_version_t *, &cls_match->add_version) = version; + atomic_init(&cls_match->remove_version, version); /* Initially + * invisible. */ miniflow_clone_inline(CONST_CAST(struct miniflow *, &cls_match->flow), &rule->match.flow, count); ovsrcu_set_hidden(&cls_match->conj_set, @@ -163,12 +163,10 @@ static bool mask_prefix_bits_set(const struct flow_wildcards *, /* cls_rule. */ static inline void -cls_rule_init__(struct cls_rule *rule, unsigned int priority, - cls_version_t version) +cls_rule_init__(struct cls_rule *rule, unsigned int priority) { rculist_init(&rule->node); *CONST_CAST(int *, &rule->priority) = priority; - *CONST_CAST(cls_version_t *, &rule->version) = version; rule->cls_match = NULL; } @@ -181,41 +179,29 @@ cls_rule_init__(struct cls_rule *rule, unsigned int priority, * Clients should not use priority INT_MIN. (OpenFlow uses priorities between * 0 and UINT16_MAX, inclusive.) */ void -cls_rule_init(struct cls_rule *rule, const struct match *match, int priority, - cls_version_t version) +cls_rule_init(struct cls_rule *rule, const struct match *match, int priority) { - cls_rule_init__(rule, priority, version); + cls_rule_init__(rule, priority); minimatch_init(CONST_CAST(struct minimatch *, &rule->match), match); } /* Same as cls_rule_init() for initialization from a "struct minimatch". */ void cls_rule_init_from_minimatch(struct cls_rule *rule, - const struct minimatch *match, int priority, - cls_version_t version) + const struct minimatch *match, int priority) { - cls_rule_init__(rule, priority, version); + cls_rule_init__(rule, priority); minimatch_clone(CONST_CAST(struct minimatch *, &rule->match), match); } -/* Initializes 'dst' as a copy of 'src', but with 'version'. - * - * The caller must eventually destroy 'dst' with cls_rule_destroy(). */ -void -cls_rule_clone_in_version(struct cls_rule *dst, const struct cls_rule *src, - cls_version_t version) -{ - cls_rule_init__(dst, src->priority, version); - minimatch_clone(CONST_CAST(struct minimatch *, &dst->match), &src->match); -} - /* Initializes 'dst' as a copy of 'src'. * * The caller must eventually destroy 'dst' with cls_rule_destroy(). */ void cls_rule_clone(struct cls_rule *dst, const struct cls_rule *src) { - cls_rule_clone_in_version(dst, src, src->version); + cls_rule_init__(dst, src->priority); + minimatch_clone(CONST_CAST(struct minimatch *, &dst->match), &src->match); } /* Initializes 'dst' with the data in 'src', destroying 'src'. @@ -226,7 +212,7 @@ cls_rule_clone(struct cls_rule *dst, const struct cls_rule *src) void cls_rule_move(struct cls_rule *dst, struct cls_rule *src) { - cls_rule_init__(dst, src->priority, src->version); + cls_rule_init__(dst, src->priority); minimatch_move(CONST_CAST(struct minimatch *, &dst->match), CONST_CAST(struct minimatch *, &src->match)); } @@ -312,7 +298,7 @@ cls_rule_make_invisible_in_version(const struct cls_rule *rule, cls_match_set_remove_version(rule->cls_match, remove_version); } -/* This undoes the change made by cls_rule_make_invisible_after_version(). +/* This undoes the change made by cls_rule_make_invisible_in_version(). * * 'rule' must be in a classifier. */ void @@ -572,14 +558,15 @@ subtable_replace_head_rule(struct classifier *cls OVS_UNUSED, cmap_replace(&subtable->rules, &head->cmap_node, &new->cmap_node, hash); } -/* Inserts 'rule' into 'cls'. Until 'rule' is removed from 'cls', the caller - * must not modify or free it. +/* Inserts 'rule' into 'cls' in 'version'. Until 'rule' is removed from 'cls', + * the caller must not modify or free it. * * If 'cls' already contains an identical rule (including wildcards, values of - * fixed fields, and priority), replaces the old rule by 'rule' and returns the - * rule that was replaced. The caller takes ownership of the returned rule and - * is thus responsible for destroying it with cls_rule_destroy(), after RCU - * grace period has passed (see ovsrcu_postpone()). + * fixed fields, and priority) that is visible in 'version', replaces the old + * rule by 'rule' and returns the rule that was replaced. The caller takes + * ownership of the returned rule and is thus responsible for destroying it + * with cls_rule_destroy(), after RCU grace period has passed (see + * ovsrcu_postpone()). * * Returns NULL if 'cls' does not contain a rule with an identical key, after * inserting the new rule. In this case, no rules are displaced by the new @@ -588,6 +575,7 @@ subtable_replace_head_rule(struct classifier *cls OVS_UNUSED, */ const struct cls_rule * classifier_replace(struct classifier *cls, const struct cls_rule *rule, + cls_version_t version, const struct cls_conjunction *conjs, size_t n_conjs) { struct cls_match *new; @@ -601,7 +589,7 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule, int i; /* 'new' is initially invisible to lookups. */ - new = cls_match_alloc(rule, conjs, n_conjs); + new = cls_match_alloc(rule, version, conjs, n_conjs); CONST_CAST(struct cls_rule *, rule)->cls_match = new; @@ -773,10 +761,11 @@ classifier_replace(struct classifier *cls, const struct cls_rule *rule, * such a rule. */ void classifier_insert(struct classifier *cls, const struct cls_rule *rule, - const struct cls_conjunction conj[], size_t n_conj) + cls_version_t version, const struct cls_conjunction conj[], + size_t n_conj) { const struct cls_rule *displaced_rule - = classifier_replace(cls, rule, conj, n_conj); + = classifier_replace(cls, rule, version, conj, n_conj); ovs_assert(!displaced_rule); } @@ -1292,12 +1281,13 @@ classifier_lookup(const struct classifier *cls, cls_version_t version, } /* Finds and returns a rule in 'cls' with exactly the same priority and - * matching criteria as 'target', and that is visible in 'target->version. + * matching criteria as 'target', and that is visible in 'version'. * Only one such rule may ever exist. Returns a null pointer if 'cls' doesn't * contain an exact match. */ const struct cls_rule * classifier_find_rule_exactly(const struct classifier *cls, - const struct cls_rule *target) + const struct cls_rule *target, + cls_version_t version) { const struct cls_match *head, *rule; const struct cls_subtable *subtable; @@ -1318,7 +1308,7 @@ classifier_find_rule_exactly(const struct classifier *cls, break; /* Not found. */ } if (rule->priority == target->priority - && cls_match_visible_in_version(rule, target->version)) { + && cls_match_visible_in_version(rule, version)) { return rule->cls_rule; } } @@ -1337,16 +1327,16 @@ classifier_find_match_exactly(const struct classifier *cls, const struct cls_rule *retval; struct cls_rule cr; - cls_rule_init(&cr, target, priority, version); - retval = classifier_find_rule_exactly(cls, &cr); + cls_rule_init(&cr, target, priority); + retval = classifier_find_rule_exactly(cls, &cr, version); cls_rule_destroy(&cr); return retval; } -/* Checks if 'target' would overlap any other rule in 'cls'. Two rules are - * considered to overlap if both rules have the same priority and a packet - * could match both, and if both rules are visible in the same version. +/* Checks if 'target' would overlap any other rule in 'cls' in 'version'. Two + * rules are considered to overlap if both rules have the same priority and a + * packet could match both, and if both rules are visible in the same version. * * A trivial example of overlapping rules is two rules matching disjoint sets * of fields. E.g., if one rule matches only on port number, while another only @@ -1354,7 +1344,7 @@ classifier_find_match_exactly(const struct classifier *cls, * dl_type could match both, if the rules also have the same priority. */ bool classifier_rule_overlaps(const struct classifier *cls, - const struct cls_rule *target) + const struct cls_rule *target, cls_version_t version) { struct cls_subtable *subtable; @@ -1371,8 +1361,7 @@ classifier_rule_overlaps(const struct classifier *cls, if (rule->priority == target->priority && miniflow_equal_in_minimask(&target->match.flow, &rule->match.flow, &mask) - && cls_match_visible_in_version(rule->cls_match, - target->version)) { + && cls_match_visible_in_version(rule->cls_match, version)) { return true; } } @@ -1424,17 +1413,15 @@ cls_rule_is_loose_match(const struct cls_rule *rule, /* Iteration. */ -/* Rule may only match a target if it is visible in target's version. For NULL - * target we only return rules that are not invisible in any version. */ static bool -rule_matches(const struct cls_rule *rule, const struct cls_rule *target) +rule_matches(const struct cls_rule *rule, const struct cls_rule *target, + cls_version_t version) { - /* Iterators never see duplicate rules with the same priority. */ - return target - ? (miniflow_equal_in_minimask(&rule->match.flow, &target->match.flow, - &target->match.mask) - && cls_match_visible_in_version(rule->cls_match, target->version)) - : !cls_match_is_eventually_invisible(rule->cls_match); + /* Rule may only match a target if it is visible in target's version. */ + return cls_match_visible_in_version(rule->cls_match, version) + && (!target || miniflow_equal_in_minimask(&rule->match.flow, + &target->match.flow, + &target->match.mask)); } static const struct cls_rule * @@ -1446,7 +1433,7 @@ search_subtable(const struct cls_subtable *subtable, const struct cls_rule *rule; RCULIST_FOR_EACH (rule, node, &subtable->rules_list) { - if (rule_matches(rule, cursor->target)) { + if (rule_matches(rule, cursor->target, cursor->version)) { return rule; } } @@ -1455,27 +1442,26 @@ search_subtable(const struct cls_subtable *subtable, } /* Initializes 'cursor' for iterating through rules in 'cls', and returns the - * first matching cls_rule via '*pnode', or NULL if there are no matches. + * cursor. * - * - If 'target' is null, or if the 'target' is a catchall target and the - * target's version is CLS_MAX_VERSION, the cursor will visit every rule - * in 'cls' that is not invisible in any version. + * - If 'target' is null, or if the 'target' is a catchall target, the + * cursor will visit every rule in 'cls' that is visible in 'version'. * * - If 'target' is nonnull, the cursor will visit each 'rule' in 'cls' * such that cls_rule_is_loose_match(rule, target) returns true and that - * the rule is visible in 'target->version'. + * the rule is visible in 'version'. * * Ignores target->priority. */ struct cls_cursor -cls_cursor_start(const struct classifier *cls, const struct cls_rule *target) +cls_cursor_start(const struct classifier *cls, const struct cls_rule *target, + cls_version_t version) { struct cls_cursor cursor; struct cls_subtable *subtable; cursor.cls = cls; - cursor.target = target && (!cls_rule_is_catchall(target) - || target->version != CLS_MAX_VERSION) - ? target : NULL; + cursor.target = target && !cls_rule_is_catchall(target) ? target : NULL; + cursor.version = version; cursor.rule = NULL; /* Find first rule. */ @@ -1502,7 +1488,7 @@ cls_cursor_next(struct cls_cursor *cursor) rule = cursor->rule; subtable = cursor->subtable; RCULIST_FOR_EACH_CONTINUE (rule, node, &subtable->rules_list) { - if (rule_matches(rule, cursor->target)) { + if (rule_matches(rule, cursor->target, cursor->version)) { return rule; } } diff --git a/lib/classifier.h b/lib/classifier.h index 8bbc7366b..5ffe756b1 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -357,18 +357,14 @@ struct cls_conjunction { struct cls_rule { struct rculist node; /* In struct cls_subtable 'rules_list'. */ const int priority; /* Larger numbers are higher priorities. */ - const cls_version_t version; /* Version in which the rule was added. */ struct cls_match *cls_match; /* NULL if not in a classifier. */ const struct minimatch match; /* Matching rule. */ }; -void cls_rule_init(struct cls_rule *, const struct match *, int priority, - cls_version_t); +void cls_rule_init(struct cls_rule *, const struct match *, int priority); void cls_rule_init_from_minimatch(struct cls_rule *, const struct minimatch *, - int priority, cls_version_t); + int priority); void cls_rule_clone(struct cls_rule *, const struct cls_rule *); -void cls_rule_clone_in_version(struct cls_rule *, const struct cls_rule *, - cls_version_t); void cls_rule_move(struct cls_rule *dst, struct cls_rule *src); void cls_rule_destroy(struct cls_rule *); @@ -395,9 +391,11 @@ bool classifier_set_prefix_fields(struct classifier *, const enum mf_field_id *trie_fields, unsigned int n_trie_fields); void classifier_insert(struct classifier *, const struct cls_rule *, - const struct cls_conjunction *, size_t n_conjunctions); + cls_version_t, const struct cls_conjunction *, + size_t n_conjunctions); const struct cls_rule *classifier_replace(struct classifier *, const struct cls_rule *, + cls_version_t, const struct cls_conjunction *, size_t n_conjunctions); const struct cls_rule *classifier_remove(struct classifier *, @@ -411,9 +409,10 @@ const struct cls_rule *classifier_lookup(const struct classifier *, cls_version_t, struct flow *, struct flow_wildcards *); bool classifier_rule_overlaps(const struct classifier *, - const struct cls_rule *); + const struct cls_rule *, cls_version_t); const struct cls_rule *classifier_find_rule_exactly(const struct classifier *, - const struct cls_rule *); + const struct cls_rule *, + cls_version_t); const struct cls_rule *classifier_find_match_exactly(const struct classifier *, const struct match *, int priority, @@ -437,18 +436,20 @@ struct cls_cursor { const struct classifier *cls; const struct cls_subtable *subtable; const struct cls_rule *target; + cls_version_t version; /* Version to iterate. */ struct pvector_cursor subtables; const struct cls_rule *rule; }; -struct cls_cursor cls_cursor_start(const struct classifier *cls, - const struct cls_rule *target); +struct cls_cursor cls_cursor_start(const struct classifier *, + const struct cls_rule *target, + cls_version_t); void cls_cursor_advance(struct cls_cursor *); #define CLS_FOR_EACH(RULE, MEMBER, CLS) \ - CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, NULL) -#define CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, TARGET) \ - for (struct cls_cursor cursor__ = cls_cursor_start(CLS, TARGET); \ + CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, NULL, CLS_MAX_VERSION) +#define CLS_FOR_EACH_TARGET(RULE, MEMBER, CLS, TARGET, VERSION) \ + for (struct cls_cursor cursor__ = cls_cursor_start(CLS, TARGET, VERSION); \ (cursor__.rule \ ? (INIT_CONTAINER(RULE, cursor__.rule, MEMBER), \ cls_cursor_advance(&cursor__), \ @@ -456,9 +457,6 @@ void cls_cursor_advance(struct cls_cursor *); : false); \ ) -#ifdef __cplusplus -} -#endif static inline void classifier_defer(struct classifier *cls) @@ -472,4 +470,8 @@ classifier_publish(struct classifier *cls) cls->publish = true; pvector_publish(&cls->subtables); } + +#ifdef __cplusplus +} +#endif #endif /* classifier.h */ diff --git a/lib/ovs-router.c b/lib/ovs-router.c index 532487e8f..df55bb4f0 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -116,10 +116,10 @@ ovs_router_insert__(uint8_t priority, ovs_be32 ip_dst, uint8_t plen, p->plen = plen; p->priority = priority; /* Longest prefix matches first. */ - cls_rule_init(&p->cr, &match, priority, CLS_MIN_VERSION); + cls_rule_init(&p->cr, &match, priority); ovs_mutex_lock(&mutex); - cr = classifier_replace(&cls, &p->cr, NULL, 0); + cr = classifier_replace(&cls, &p->cr, CLS_MIN_VERSION, NULL, 0); ovs_mutex_unlock(&mutex); if (cr) { @@ -145,10 +145,10 @@ rt_entry_delete(uint8_t priority, ovs_be32 ip_dst, uint8_t plen) rt_init_match(&match, ip_dst, plen); - cls_rule_init(&rule, &match, priority, CLS_MIN_VERSION); + cls_rule_init(&rule, &match, priority); /* Find the exact rule. */ - cr = classifier_find_rule_exactly(&cls, &rule); + cr = classifier_find_rule_exactly(&cls, &rule, CLS_MAX_VERSION); if (cr) { /* Remove it. */ ovs_mutex_lock(&mutex); diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c index 35dd0a5a7..9a87b7432 100644 --- a/lib/tnl-ports.c +++ b/lib/tnl-ports.c @@ -94,11 +94,11 @@ tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port, const char dev_name[]) match.wc.masks.nw_frag = 0xff; /* XXX: No fragments support. */ match.wc.masks.tp_dst = OVS_BE16_MAX; - cls_rule_init(&p->cr, &match, 0, CLS_MIN_VERSION); /* Priority == 0. */ + cls_rule_init(&p->cr, &match, 0); /* Priority == 0. */ ovs_refcount_init(&p->ref_cnt); ovs_strlcpy(p->dev_name, dev_name, sizeof p->dev_name); - classifier_insert(&cls, &p->cr, NULL, 0); + classifier_insert(&cls, &p->cr, CLS_MIN_VERSION, NULL, 0); } ovs_mutex_unlock(&mutex); } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 6a61ea656..7c3948ba3 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -137,6 +137,7 @@ struct rule_criteria { * collect_rules_loose() and "strict" way by collect_rules_strict(), as * defined in the OpenFlow spec. */ struct cls_rule cr; + cls_version_t version; /* Matching criteria for the OpenFlow cookie. Consider a bit B in a rule's * cookie and the corresponding bits C in 'cookie' and M in 'cookie_mask'. @@ -3802,7 +3803,8 @@ rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id, uint32_t out_group) { criteria->table_id = table_id; - cls_rule_init(&criteria->cr, match, priority, version); + cls_rule_init(&criteria->cr, match, priority); + criteria->version = version; criteria->cookie = cookie; criteria->cookie_mask = cookie_mask; criteria->out_port = out_port; @@ -3953,7 +3955,7 @@ collect_rule(struct rule *rule, const struct rule_criteria *c, && ofproto_rule_has_out_group(rule, c->out_group) && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask) && (!rule_is_hidden(rule) || c->include_hidden) - && cls_rule_visible_in_version(&rule->cr, c->cr.version)) { + && cls_rule_visible_in_version(&rule->cr, c->version)) { /* Rule matches all the criteria... */ if (!rule_is_readonly(rule) || c->include_readonly) { /* ...add it. */ @@ -4002,7 +4004,8 @@ collect_rules_loose(struct ofproto *ofproto, FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) { struct rule *rule; - CLS_FOR_EACH_TARGET (rule, cr, &table->cls, &criteria->cr) { + CLS_FOR_EACH_TARGET (rule, cr, &table->cls, &criteria->cr, + criteria->version) { collect_rule(rule, criteria, rules, &n_readonly); } } @@ -4058,7 +4061,8 @@ collect_rules_strict(struct ofproto *ofproto, struct rule *rule; rule = rule_from_cls_rule(classifier_find_rule_exactly( - &table->cls, &criteria->cr)); + &table->cls, &criteria->cr, + criteria->version)); if (rule) { collect_rule(rule, criteria, rules, &n_readonly); } @@ -4552,16 +4556,17 @@ add_flow_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm) return OFPERR_OFPBRC_EPERM; } - cls_rule_init(&cr, &fm->match, fm->priority, ofm->version); + cls_rule_init(&cr, &fm->match, fm->priority); /* Check for the existence of an identical rule. * This will not return rules earlier marked for removal. */ - rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, &cr)); + rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, &cr, + ofm->version)); *old_rule = rule; if (!rule) { /* Check for overlap, if requested. */ if (fm->flags & OFPUTIL_FF_CHECK_OVERLAP - && classifier_rule_overlaps(&table->cls, &cr)) { + && classifier_rule_overlaps(&table->cls, &cr, ofm->version)) { cls_rule_destroy(&cr); return OFPERR_OFPFMFC_OVERLAP; } @@ -4752,7 +4757,7 @@ replace_rule_start(struct ofproto *ofproto, cls_version_t version, ofproto_rule_insert__(ofproto, new_rule); /* Make the new rule visible for classifier lookups only from the next * version. */ - classifier_insert(&table->cls, &new_rule->cr, conjs, n_conjs); + classifier_insert(&table->cls, &new_rule->cr, version, conjs, n_conjs); } static void replace_rule_revert(struct ofproto *ofproto, @@ -4862,7 +4867,7 @@ modify_flows_start__(struct ofproto *ofproto, struct ofproto_flow_mod *ofm) struct rule *new_rule; struct cls_rule cr; - cls_rule_clone_in_version(&cr, &old_rule->cr, ofm->version); + cls_rule_clone(&cr, &old_rule->cr); error = replace_rule_create(ofproto, fm, &cr, old_rule->table_id, old_rule, &new_rule); if (!error) { @@ -5548,11 +5553,11 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m, const struct oftable *table; struct cls_rule target; - cls_rule_init_from_minimatch(&target, &m->match, 0, CLS_MAX_VERSION); + cls_rule_init_from_minimatch(&target, &m->match, 0); FOR_EACH_MATCHING_TABLE (table, m->table_id, ofproto) { struct rule *rule; - CLS_FOR_EACH_TARGET (rule, cr, &table->cls, &target) { + CLS_FOR_EACH_TARGET (rule, cr, &table->cls, &target, CLS_MAX_VERSION) { ofproto_collect_ofmonitor_refresh_rule(m, rule, seqno, rules); } } @@ -7776,7 +7781,7 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap) match_init_catchall(&match); match_set_vlan_vid_masked(&match, htons(VLAN_CFI), htons(VLAN_CFI)); - cls_rule_init(&target, &match, 0, CLS_MAX_VERSION); + cls_rule_init(&target, &match, 0); free(ofproto->vlan_bitmap); ofproto->vlan_bitmap = bitmap_allocate(4096); @@ -7785,7 +7790,8 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap) OFPROTO_FOR_EACH_TABLE (oftable, ofproto) { struct rule *rule; - CLS_FOR_EACH_TARGET (rule, cr, &oftable->cls, &target) { + CLS_FOR_EACH_TARGET (rule, cr, &oftable->cls, &target, + CLS_MAX_VERSION) { if (minimask_get_vid_mask(&rule->cr.match.mask) == VLAN_VID_MASK) { uint16_t vid = miniflow_get_vid(&rule->cr.match.flow); diff --git a/tests/test-classifier.c b/tests/test-classifier.c index 2fe9a5d55..56d55755e 100644 --- a/tests/test-classifier.c +++ b/tests/test-classifier.c @@ -110,8 +110,7 @@ test_rule_destroy(struct test_rule *rule) } } -static struct test_rule *make_rule(int wc_fields, int priority, int value_pat, - long long version); +static struct test_rule *make_rule(int wc_fields, int priority, int value_pat); static void free_rule(struct test_rule *); static struct test_rule *clone_rule(const struct test_rule *); @@ -403,7 +402,7 @@ get_value(unsigned int *x, unsigned n_values) static void compare_classifiers(struct classifier *cls, size_t n_invisible_rules, - long long version, struct tcls *tcls) + cls_version_t version, struct tcls *tcls) { static const int confidence = 500; unsigned int i; @@ -520,7 +519,7 @@ verify_tries(struct classifier *cls) static void check_tables(const struct classifier *cls, int n_tables, int n_rules, - int n_dups, int n_invisible, long long version) + int n_dups, int n_invisible, cls_version_t version) OVS_NO_THREAD_SAFETY_ANALYSIS { const struct cls_subtable *table; @@ -564,7 +563,7 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, CMAP_FOR_EACH (head, cmap_node, &table->rules) { int prev_priority = INT_MAX; - long long prev_version = 0; + cls_version_t prev_version = 0; const struct cls_match *rule, *prev; bool found_visible_rules_in_list = false; @@ -576,7 +575,7 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, } FOR_EACH_RULE_IN_LIST_PROTECTED(rule, prev, head) { - long long rule_version; + cls_version_t rule_version; const struct cls_rule *found_rule; /* Priority may not increase. */ @@ -601,24 +600,25 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, } /* Rule must be visible in the version it was inserted. */ - rule_version = rule->cls_rule->version; + rule_version = rule->add_version; assert(cls_match_visible_in_version(rule, rule_version)); /* We should always find the latest version of the rule, * unless all rules have been marked for removal. * Later versions must always be later in the list. */ - found_rule = classifier_find_rule_exactly(cls, rule->cls_rule); + found_rule = classifier_find_rule_exactly(cls, rule->cls_rule, + rule_version); if (found_rule && found_rule != rule->cls_rule) { assert(found_rule->priority == rule->priority); /* Found rule may not have a lower version. */ - assert(found_rule->version >= rule_version); + assert(found_rule->cls_match->add_version >= rule_version); /* This rule must not be visible in the found rule's * version. */ - assert(!cls_match_visible_in_version(rule, - found_rule->version)); + assert(!cls_match_visible_in_version( + rule, found_rule->cls_match->add_version)); } if (rule->priority == prev_priority) { @@ -659,7 +659,7 @@ check_tables(const struct classifier *cls, int n_tables, int n_rules, } static struct test_rule * -make_rule(int wc_fields, int priority, int value_pat, long long version) +make_rule(int wc_fields, int priority, int value_pat) { const struct cls_field *f; struct test_rule *rule; @@ -707,7 +707,7 @@ make_rule(int wc_fields, int priority, int value_pat, long long version) cls_rule_init(&rule->cls_rule, &match, wc_fields ? (priority == INT_MIN ? priority + 1 : priority == INT_MAX ? priority - 1 : priority) - : 0, version); + : 0); return rule; } @@ -801,14 +801,13 @@ test_single_rule(struct ovs_cmdl_context *ctx OVS_UNUSED) struct tcls tcls; rule = make_rule(wc_fields, - hash_bytes(&wc_fields, sizeof wc_fields, 0), 0, - CLS_MIN_VERSION); + hash_bytes(&wc_fields, sizeof wc_fields, 0), 0); classifier_init(&cls, flow_segment_u64s); set_prefix_fields(&cls); tcls_init(&tcls); tcls_rule = tcls_insert(&tcls, rule); - classifier_insert(&cls, &rule->cls_rule, NULL, 0); + classifier_insert(&cls, &rule->cls_rule, CLS_MIN_VERSION, NULL, 0); compare_classifiers(&cls, 0, CLS_MIN_VERSION, &tcls); check_tables(&cls, 1, 1, 0, 0, CLS_MIN_VERSION); @@ -836,10 +835,8 @@ test_rule_replacement(struct ovs_cmdl_context *ctx OVS_UNUSED) struct test_rule *rule2; struct tcls tcls; - rule1 = make_rule(wc_fields, OFP_DEFAULT_PRIORITY, UINT_MAX, - CLS_MIN_VERSION); - rule2 = make_rule(wc_fields, OFP_DEFAULT_PRIORITY, UINT_MAX, - CLS_MIN_VERSION); + rule1 = make_rule(wc_fields, OFP_DEFAULT_PRIORITY, UINT_MAX); + rule2 = make_rule(wc_fields, OFP_DEFAULT_PRIORITY, UINT_MAX); rule2->aux += 5; rule2->aux += 5; @@ -847,7 +844,7 @@ test_rule_replacement(struct ovs_cmdl_context *ctx OVS_UNUSED) set_prefix_fields(&cls); tcls_init(&tcls); tcls_insert(&tcls, rule1); - classifier_insert(&cls, &rule1->cls_rule, NULL, 0); + classifier_insert(&cls, &rule1->cls_rule, CLS_MIN_VERSION, NULL, 0); compare_classifiers(&cls, 0, CLS_MIN_VERSION, &tcls); check_tables(&cls, 1, 1, 0, 0, CLS_MIN_VERSION); tcls_destroy(&tcls); @@ -856,7 +853,7 @@ test_rule_replacement(struct ovs_cmdl_context *ctx OVS_UNUSED) tcls_insert(&tcls, rule2); assert(test_rule_from_cls_rule( - classifier_replace(&cls, &rule2->cls_rule, + classifier_replace(&cls, &rule2->cls_rule, CLS_MIN_VERSION, NULL, 0)) == rule1); ovsrcu_postpone(free_rule, rule1); compare_classifiers(&cls, 0, CLS_MIN_VERSION, &tcls); @@ -950,13 +947,13 @@ test_many_rules_in_one_list (struct ovs_cmdl_context *ctx OVS_UNUSED) int pri_rules[N_RULES]; struct classifier cls; struct tcls tcls; - long long version = CLS_MIN_VERSION; + cls_version_t version = CLS_MIN_VERSION; size_t n_invisible_rules = 0; n_permutations++; for (i = 0; i < N_RULES; i++) { - rules[i] = make_rule(456, pris[i], 0, version); + rules[i] = make_rule(456, pris[i], 0); tcls_rules[i] = NULL; pri_rules[i] = -1; } @@ -975,13 +972,12 @@ test_many_rules_in_one_list (struct ovs_cmdl_context *ctx OVS_UNUSED) tcls_rules[j] = tcls_insert(&tcls, rules[j]); if (versioned) { /* Insert the new rule in the next version. */ - *CONST_CAST(cls_version_t *, - &rules[j]->cls_rule.version) - = ++version; + ++version; displaced_rule = test_rule_from_cls_rule( classifier_find_rule_exactly(&cls, - &rules[j]->cls_rule)); + &rules[j]->cls_rule, + version)); if (displaced_rule) { /* Mark the old rule for removal after the current * version. */ @@ -990,11 +986,12 @@ test_many_rules_in_one_list (struct ovs_cmdl_context *ctx OVS_UNUSED) n_invisible_rules++; removable_rule = &displaced_rule->cls_rule; } - classifier_insert(&cls, &rules[j]->cls_rule, NULL, 0); + classifier_insert(&cls, &rules[j]->cls_rule, version, + NULL, 0); } else { displaced_rule = test_rule_from_cls_rule( classifier_replace(&cls, &rules[j]->cls_rule, - NULL, 0)); + version, NULL, 0)); } if (pri_rules[pris[j]] >= 0) { int k = pri_rules[pris[j]]; @@ -1010,9 +1007,9 @@ test_many_rules_in_one_list (struct ovs_cmdl_context *ctx OVS_UNUSED) if (versioned) { /* Mark the rule for removal after the current * version. */ - cls_rule_make_invisible_in_version( - &rules[j]->cls_rule, version + 1); ++version; + cls_rule_make_invisible_in_version( + &rules[j]->cls_rule, version); n_invisible_rules++; removable_rule = &rules[j]->cls_rule; } else { @@ -1093,7 +1090,7 @@ test_many_rules_in_one_table(struct ovs_cmdl_context *ctx OVS_UNUSED) struct test_rule *tcls_rules[N_RULES]; struct classifier cls; struct tcls tcls; - long long version = CLS_MIN_VERSION; + cls_version_t version = CLS_MIN_VERSION; size_t n_invisible_rules = 0; int value_pats[N_RULES]; int value_mask; @@ -1117,10 +1114,10 @@ test_many_rules_in_one_table(struct ovs_cmdl_context *ctx OVS_UNUSED) } while (array_contains(value_pats, i, value_pats[i])); ++version; - rules[i] = make_rule(wcf, priority, value_pats[i], version); + rules[i] = make_rule(wcf, priority, value_pats[i]); tcls_rules[i] = tcls_insert(&tcls, rules[i]); - classifier_insert(&cls, &rules[i]->cls_rule, NULL, 0); + classifier_insert(&cls, &rules[i]->cls_rule, version, NULL, 0); compare_classifiers(&cls, n_invisible_rules, version, &tcls); check_tables(&cls, 1, i + 1, 0, n_invisible_rules, version); @@ -1130,9 +1127,9 @@ test_many_rules_in_one_table(struct ovs_cmdl_context *ctx OVS_UNUSED) tcls_remove(&tcls, tcls_rules[i]); if (versioned) { /* Mark the rule for removal after the current version. */ - cls_rule_make_invisible_in_version(&rules[i]->cls_rule, - version + 1); ++version; + cls_rule_make_invisible_in_version(&rules[i]->cls_rule, + version); n_invisible_rules++; } else { classifier_remove(&cls, &rules[i]->cls_rule); @@ -1182,7 +1179,7 @@ test_many_rules_in_n_tables(int n_tables) int priorities[MAX_RULES]; struct classifier cls; struct tcls tcls; - long long version = CLS_MIN_VERSION; + cls_version_t version = CLS_MIN_VERSION; size_t n_invisible_rules = 0; struct ovs_list list = OVS_LIST_INITIALIZER(&list); @@ -1201,9 +1198,9 @@ test_many_rules_in_n_tables(int n_tables) int priority = priorities[i]; int wcf = wcfs[random_range(n_tables)]; int value_pat = random_uint32() & ((1u << CLS_N_FIELDS) - 1); - rule = make_rule(wcf, priority, value_pat, version); + rule = make_rule(wcf, priority, value_pat); tcls_insert(&tcls, rule); - classifier_insert(&cls, &rule->cls_rule, NULL, 0); + classifier_insert(&cls, &rule->cls_rule, version, NULL, 0); compare_classifiers(&cls, n_invisible_rules, version, &tcls); check_tables(&cls, -1, i + 1, -1, n_invisible_rules, version); } @@ -1215,7 +1212,8 @@ test_many_rules_in_n_tables(int n_tables) target = clone_rule(tcls.rules[random_range(tcls.n_rules)]); - CLS_FOR_EACH_TARGET (rule, cls_rule, &cls, &target->cls_rule) { + CLS_FOR_EACH_TARGET (rule, cls_rule, &cls, &target->cls_rule, + version) { if (versioned) { /* Mark the rule for removal after the current version. */ cls_rule_make_invisible_in_version(&rule->cls_rule, diff --git a/tests/test-ovn.c b/tests/test-ovn.c index fc6ea4b73..f57a4ec65 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -889,8 +889,9 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, classifier_init(&cls, NULL); HMAP_FOR_EACH (m, hmap_node, &matches) { test_rule = xmalloc(sizeof *test_rule); - cls_rule_init(&test_rule->cr, &m->match, 0, CLS_MIN_VERSION); - classifier_insert(&cls, &test_rule->cr, m->conjunctions, m->n); + cls_rule_init(&test_rule->cr, &m->match, 0); + classifier_insert(&cls, &test_rule->cr, CLS_MIN_VERSION, + m->conjunctions, m->n); } } for (int subst = 0; subst < 1 << (n_bits * n_vars); subst++) { diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 566a6b42f..0bc708928 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2521,10 +2521,11 @@ fte_insert(struct classifier *cls, const struct match *match, struct fte *old, *fte; fte = xzalloc(sizeof *fte); - cls_rule_init(&fte->rule, match, priority, CLS_MIN_VERSION); + cls_rule_init(&fte->rule, match, priority); fte->versions[index] = version; - old = fte_from_cls_rule(classifier_replace(cls, &fte->rule, NULL, 0)); + old = fte_from_cls_rule(classifier_replace(cls, &fte->rule, + CLS_MIN_VERSION, NULL, 0)); if (old) { fte->versions[!index] = old->versions[!index]; old->versions[!index] = NULL; -- 2.20.1