[ovs-dev] [PATCH 2/2] classifier: Make versioning more explicit.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Jul 1 19:45:05 UTC 2015
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 <blp at nicira.com>
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
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 db6a169..a8a4780 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 8bbc736..5ffe756 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 532487e..df55bb4 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 79c9631..9fb6b17 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 e40a80e..92d4176 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -130,6 +130,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'.
@@ -3757,7 +3758,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;
@@ -3908,7 +3910,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. */
@@ -3957,7 +3959,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);
}
}
@@ -4013,7 +4016,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);
}
@@ -4507,16 +4511,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;
}
@@ -4708,7 +4713,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,
@@ -4818,7 +4823,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) {
@@ -5504,11 +5509,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);
}
}
@@ -7675,7 +7680,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);
@@ -7684,7 +7689,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 2fe9a5d..56d5575 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 fc6ea4b..f57a4ec 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 5af1f13..f9a67c7 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2508,10 +2508,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;
--
1.7.10.4
More information about the dev
mailing list