mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 06:15:47 +00:00
Code Issues Releases Wiki Activity

ovs-ofctl: Avoid printing false differences on "ovs-ofctl diff-flows".

Browse Source 
It is possible for "struct ofpact"s to differ bytewise even if they are
equivalent when converted to another representation, such as OpenFlow 1.0
action format or a string representation.  This can cause "ovs-ofctl
diff-flows" to print surprising false "differences", e.g. as in the bug
report:

    - actions=resubmit(,1)
    + actions=resubmit(,1)

This commit fixes the problem by comparing not just the ofpacts but also
the string representation and printing a difference only if both differ.

Bug #8899.
Reported-by: Luca Giraudo <lgiraudo@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
Ben Pfaff
2012-07-23 10:16:31 -07:00
parent 4b9f923273
commit 98f7f427bf
2 changed files with 59 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
tests/ovs-ofctl.at
View File

@@ -1962,3 +1962,28 @@ AT_CHECK([ovs-ofctl diff-flows add-flows.txt br0 | sort], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
dnl ofpacts that differ bytewise don't necessarily differ when
dnl converted to another representation, such as OpenFlow 1.0
dnl or to a string. "resubmit(,1)" is an example of an action
dnl of this type: "ofpact_resubmit"s can differ in their "compat"
dnl values even though this doesn't affect the string format.
dnl
dnl This test checks that "ovs-ofctl diff-flows" doesn't report
dnl false ofpacts differences.
AT_SETUP([ovs-ofctl diff-flows - suppress false differences])
OVS_VSWITCHD_START
AT_DATA([flows.txt], [actions=resubmit(,1)
])
AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
AT_CHECK([ovs-ofctl diff-flows br0 flows.txt])
AT_CHECK([ovs-ofctl add-flow br0 idle_timeout=60,dl_vlan=9,actions=output:1])
AT_CHECK([ovs-ofctl diff-flows br0 flows.txt], [2], [dnl
-dl_vlan=9 idle_timeout=60 actions=output:1
])
AT_CHECK([ovs-ofctl add-flow br0 hard_timeout=120,cookie=1234,dl_vlan=9,actions=output:1])
AT_CHECK([ovs-ofctl diff-flows flows.txt br0], [2], [dnl
+dl_vlan=9 cookie=0x4d2 hard_timeout=120 actions=output:1
])
OVS_VSWITCHD_STOP
AT_CLEANUP

56
utilities/ovs-ofctl.c
View File

@@ -1740,27 +1740,33 @@ fte_version_equals(const struct fte_version *a, const struct fte_version *b)
b->ofpacts, b->ofpacts_len));
}
/* Prints 'version' on stdout. Expects the caller to have printed the rule
* associated with the version. */
/* Clears 's', then if 's' has a version 'index', formats 'fte' and version
* 'index' into 's', followed by a new-line. */
static void
fte_version_print(const struct fte_version *version)
fte_version_format(const struct fte *fte, int index, struct ds *s)
{
struct ds s;
const struct fte_version *version = fte->versions[index];
ds_clear(s);
if (!version) {
return;
}
cls_rule_format(&fte->rule, s);
if (version->cookie != htonll(0)) {
printf(" cookie=0x%"PRIx64, ntohll(version->cookie));
ds_put_format(s, " cookie=0x%"PRIx64, ntohll(version->cookie));
}
if (version->idle_timeout != OFP_FLOW_PERMANENT) {
printf(" idle_timeout=%"PRIu16, version->idle_timeout);
ds_put_format(s, " idle_timeout=%"PRIu16, version->idle_timeout);
}
if (version->hard_timeout != OFP_FLOW_PERMANENT) {
printf(" hard_timeout=%"PRIu16, version->hard_timeout);
ds_put_format(s, " hard_timeout=%"PRIu16, version->hard_timeout);
}
ds_init(&s);
ofpacts_format(version->ofpacts, version->ofpacts_len, &s);
printf(" %s\n", ds_cstr(&s));
ds_destroy(&s);
ds_put_char(s, ' ');
ofpacts_format(version->ofpacts, version->ofpacts_len, s);
ds_put_char(s, '\n');
}
static struct fte *
@@ -2067,33 +2073,39 @@ ofctl_diff_flows(int argc OVS_UNUSED, char *argv[])
bool differences = false;
struct cls_cursor cursor;
struct classifier cls;
struct ds a_s, b_s;
struct fte *fte;
classifier_init(&cls);
read_flows_from_source(argv[1], &cls, 0);
read_flows_from_source(argv[2], &cls, 1);
ds_init(&a_s);
ds_init(&b_s);
cls_cursor_init(&cursor, &cls, NULL);
CLS_CURSOR_FOR_EACH (fte, rule, &cursor) {
struct fte_version *a = fte->versions[0];
struct fte_version *b = fte->versions[1];
if (!a || !b || !fte_version_equals(a, b)) {
char *rule_s = cls_rule_to_string(&fte->rule);
if (a) {
printf("-%s", rule_s);
fte_version_print(a);
fte_version_format(fte, 0, &a_s);
fte_version_format(fte, 1, &b_s);
if (strcmp(ds_cstr(&a_s), ds_cstr(&b_s))) {
if (a_s.length) {
printf("-%s", ds_cstr(&a_s));
}
if (b_s.length) {
printf("+%s", ds_cstr(&b_s));
}
differences = true;
}
if (b) {
printf("+%s", rule_s);
fte_version_print(b);
}
free(rule_s);
differences = true;
}
}
ds_destroy(&a_s);
ds_destroy(&b_s);
fte_free_all(&cls);
if (differences) {