mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-09-01 06:45:17 +00:00
Code Issues Releases Wiki Activity

classifier: Prepare for "struct cls_rule" needing to be destroyed.

Browse Source 
Until now, "struct cls_rule" didn't own any data outside its own memory
block.  An upcoming commit will make "struct cls_rule" sometimes own blocks
of memory, so it needs "destroy" and to a lesser extent "clone" functions.
This commit adds these in advance, even though they are mostly no-ops, to
make it possible to separately review the memory management.

Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
Ben Pfaff
2012-08-20 11:29:43 -07:00
parent 81a76618be
commit 48d28ac161
5 changed files with 80 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
lib/classifier.c
View File

@@ -56,6 +56,8 @@ static struct cls_rule *next_rule_in_list(struct cls_rule *);
/* Initializes 'rule' to match packets specified by 'match' at the given
* 'priority'.
*
* The caller must eventually destroy 'rule' with cls_rule_destroy().
*
* 'match' must satisfy the invariant described in the comment at the
* definition of struct match.
*
@@ -69,6 +71,25 @@ cls_rule_init(struct cls_rule *rule,
rule->priority = priority;
}
/* Initializes 'dst' as a copy of 'src'.
*
* The caller must eventually destroy 'rule' with cls_rule_destroy(). */
void
cls_rule_clone(struct cls_rule *dst, const struct cls_rule *src)
{
*dst = *src;
}
/* Frees memory referenced by 'rule'. Doesn't free 'rule' itself (it's
* normally embedded into a larger structure).
*
* ('rule' must not currently be in a classifier.) */
void
cls_rule_destroy(struct cls_rule *rule OVS_UNUSED)
{
/* Nothing to do yet. */
}
/* Returns true if 'a' and 'b' match the same packets at the same priority,
* false if they differ in some way. */
bool
@@ -137,7 +158,8 @@ classifier_count(const struct classifier *cls)
* 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 freeing it, etc., as necessary.
* is thus responsible for destroying it with cls_rule_destroy(), freeing the
* memory block in which it resides, etc., as necessary.
*
* 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
@@ -175,8 +197,9 @@ classifier_insert(struct classifier *cls, struct cls_rule *rule)
assert(!displaced_rule);
}
/* Removes 'rule' from 'cls'. It is the caller's responsibility to free
* 'rule', if this is desirable. */
/* Removes 'rule' from 'cls'. It is the caller's responsibility to destroy
* '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)
{
@@ -261,6 +284,7 @@ classifier_find_match_exactly(const struct classifier *cls,
cls_rule_init(&cr, target, priority);
retval = classifier_find_rule_exactly(cls, &cr);
cls_rule_destroy(&cr);
return retval;
}

2
lib/classifier.h
View File

@@ -70,6 +70,8 @@ struct cls_rule {
void cls_rule_init(struct cls_rule *,
const struct match *, unsigned int priority);
void cls_rule_clone(struct cls_rule *, const struct cls_rule *);
void cls_rule_destroy(struct cls_rule *);
bool cls_rule_equal(const struct cls_rule *, const struct cls_rule *);
uint32_t cls_rule_hash(const struct cls_rule *, uint32_t basis);

18
ofproto/ofproto.c
View File

@@ -1901,6 +1901,7 @@ static void
ofproto_rule_destroy__(struct rule *rule)
{
if (rule) {
cls_rule_destroy(&rule->cr);
free(rule->ofpacts);
rule->ofproto->ofproto_class->rule_dealloc(rule);
}
@@ -2457,7 +2458,8 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
cls_cursor_init(&cursor, &table->cls, &cr);
CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
if (rule->pending) {
return OFPROTO_POSTPONE;
error = OFPROTO_POSTPONE;
goto exit;
}
if (!ofproto_rule_is_hidden(rule)
&& ofproto_rule_has_out_port(rule, out_port)
@@ -2466,7 +2468,10 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
}
}
}
return 0;
exit:
cls_rule_destroy(&cr);
return error;
}
/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
@@ -2504,7 +2509,8 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
&cr));
if (rule) {
if (rule->pending) {
return OFPROTO_POSTPONE;
error = OFPROTO_POSTPONE;
goto exit;
}
if (!ofproto_rule_is_hidden(rule)
&& ofproto_rule_has_out_port(rule, out_port)
@@ -2513,6 +2519,9 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
}
}
}
exit:
cls_rule_destroy(&cr);
return 0;
}
@@ -2922,6 +2931,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
/* Serialize against pending deletion. */
if (is_flow_deletion_pending(ofproto, &cr, table - ofproto->tables)) {
cls_rule_destroy(&rule->cr);
ofproto->ofproto_class->rule_dealloc(rule);
return OFPROTO_POSTPONE;
}
@@ -2929,6 +2939,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
/* Check for overlap, if requested. */
if (fm->flags & OFPFF_CHECK_OVERLAP
&& classifier_rule_overlaps(&table->cls, &rule->cr)) {
cls_rule_destroy(&rule->cr);
ofproto->ofproto_class->rule_dealloc(rule);
return OFPERR_OFPFMFC_OVERLAP;
}
@@ -3644,6 +3655,7 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m,
ofproto_collect_ofmonitor_refresh_rule(m, rule, seqno, rules);
}
}
cls_rule_destroy(&target);
}
static void

46
tests/test-classifier.c
View File

@@ -95,6 +95,11 @@ test_rule_from_cls_rule(const struct cls_rule *rule)
return rule ? CONTAINER_OF(rule, struct test_rule, cls_rule) : NULL;
}
static struct test_rule *make_rule(int wc_fields, unsigned int priority,
int value_pat);
static void free_rule(struct test_rule *);
static struct test_rule *clone_rule(const struct test_rule *);
/* Trivial (linear) classifier. */
struct tcls {
size_t n_rules;
@@ -138,8 +143,8 @@ tcls_insert(struct tcls *tcls, const struct test_rule *rule)
const struct cls_rule *pos = &tcls->rules[i]->cls_rule;
if (cls_rule_equal(pos, &rule->cls_rule)) {
/* Exact match. */
free(tcls->rules[i]);
tcls->rules[i] = xmemdup(rule, sizeof *rule);
free_rule(tcls->rules[i]);
tcls->rules[i] = clone_rule(rule);
return tcls->rules[i];
} else if (pos->priority < rule->cls_rule.priority) {
break;
@@ -154,7 +159,7 @@ tcls_insert(struct tcls *tcls, const struct test_rule *rule)
memmove(&tcls->rules[i + 1], &tcls->rules[i],
sizeof *tcls->rules * (tcls->n_rules - i));
}
tcls->rules[i] = xmemdup(rule, sizeof *rule);
tcls->rules[i] = clone_rule(rule);
tcls->n_rules++;
return tcls->rules[i];
}
@@ -422,7 +427,7 @@ destroy_classifier(struct classifier *cls)
cls_cursor_init(&cursor, cls, NULL);
CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cls_rule, &cursor) {
classifier_remove(cls, &rule->cls_rule);
free(rule);
free_rule(rule);
}
classifier_destroy(cls);
}
@@ -522,6 +527,24 @@ make_rule(int wc_fields, unsigned int priority, int value_pat)
return rule;
}
static struct test_rule *
clone_rule(const struct test_rule *src)
{
struct test_rule *dst;
dst = xmalloc(sizeof *dst);
dst->aux = src->aux;
cls_rule_clone(&dst->cls_rule, &src->cls_rule);
return dst;
}
static void
free_rule(struct test_rule *rule)
{
cls_rule_destroy(&rule->cls_rule);
free(rule);
}
static void
shuffle(unsigned int *p, size_t n)
{
@@ -584,7 +607,7 @@ test_single_rule(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
assert(tcls_is_empty(&tcls));
compare_classifiers(&cls, &tcls);
free(rule);
free_rule(rule);
classifier_destroy(&cls);
tcls_destroy(&tcls);
}
@@ -619,7 +642,7 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_insert(&tcls, rule2);
assert(test_rule_from_cls_rule(
classifier_replace(&cls, &rule2->cls_rule)) == rule1);
free(rule1);
free_rule(rule1);
check_tables(&cls, 1, 1, 0);
compare_classifiers(&cls, &tcls);
tcls_destroy(&tcls);
@@ -760,7 +783,7 @@ test_many_rules_in_one_list (int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_destroy(&tcls);
for (i = 0; i < N_RULES; i++) {
free(rules[i]);
free_rule(rules[i]);
}
} while (next_permutation(ops, ARRAY_SIZE(ops)));
assert(n_permutations == (factorial(N_RULES * 2) >> N_RULES));
@@ -838,7 +861,7 @@ test_many_rules_in_one_table(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
for (i = 0; i < N_RULES; i++) {
tcls_remove(&tcls, tcls_rules[i]);
classifier_remove(&cls, &rules[i]->cls_rule);
free(rules[i]);
free_rule(rules[i]);
check_tables(&cls, i < N_RULES - 1, N_RULES - (i + 1), 0);
compare_classifiers(&cls, &tcls);
@@ -897,18 +920,17 @@ test_many_rules_in_n_tables(int n_tables)
struct test_rule *target;
struct cls_cursor cursor;
target = xmemdup(tcls.rules[rand() % tcls.n_rules],
sizeof(struct test_rule));
target = clone_rule(tcls.rules[rand() % tcls.n_rules]);
cls_cursor_init(&cursor, &cls, &target->cls_rule);
CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cls_rule, &cursor) {
classifier_remove(&cls, &rule->cls_rule);
free(rule);
free_rule(rule);
}
tcls_delete_matches(&tcls, &target->cls_rule);
compare_classifiers(&cls, &tcls);
check_tables(&cls, -1, -1, -1);
free(target);
free_rule(target);
}
destroy_classifier(&cls);

2
utilities/ovs-ofctl.c
View File

@@ -1778,6 +1778,7 @@ fte_free(struct fte *fte)
if (fte) {
fte_version_free(fte->versions[0]);
fte_version_free(fte->versions[1]);
cls_rule_destroy(&fte->rule);
free(fte);
}
}
@@ -1816,6 +1817,7 @@ fte_insert(struct classifier *cls, const struct match *match,
if (old) {
fte_version_free(old->versions[index]);
fte->versions[!index] = old->versions[!index];
cls_rule_destroy(&old->rule);
free(old);
}
}