The ovs-ofctl "diff-flows" and "replace-flows" command compare the flows
in two flow tables. Until now, the "replace-flows" command has considered
certain almost meaningless differences related to the version of OpenFlow
used to add a flow as significant, which caused it to replace a flow by an
identical-in-practice version, e.g. in the following, the "replace-flows"
command prints a FLOW_MOD that adds the flow that was already added
previously:
$ cat > flows
actions=resubmit(,1)
$ ovs-vsctl add-br br0
$ ovs-ofctl del-flows br0
$ ovs-ofctl add-flows br0 flows
$ ovs-ofctl -vvconn replace-flows br0 flows 2>&1 | grep FLOW_MOD
Re-adding an existing flow has some effects, for example, it resets the
flow's duration, so it's better to avoid it.
This commit fixes the problem using the same trick previously used for a
similar problem with the "diff-flows" command, which was fixed in commit
98f7f427bf8b ("ovs-ofctl: Avoid printing false differences on "ovs-ofctl
diff-flows".").
Reported-by: Kevin Lin <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
include/openvswitch/ofp-actions.h | 2 ++
lib/ofp-actions.c | 24 ++++++++++++++++++++++++
utilities/ovs-ofctl.c | 18 ++++++++----------
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 7b4aa9201d9b..0dcb6452ea02 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -1013,6 +1013,8 @@ bool ofpacts_output_to_group(const struct ofpact[],
size_t ofpacts_len,
uint32_t group_id);
bool ofpacts_equal(const struct ofpact a[], size_t a_len,
const struct ofpact b[], size_t b_len);
+bool ofpacts_equal_stringwise(const struct ofpact a[], size_t a_len,
+ const struct ofpact b[], size_t b_len);
const struct mf_field *ofpact_get_mf_dst(const struct ofpact *ofpact);
uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ae27d9d88080..868b511dc908 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -8316,6 +8316,8 @@ ofpacts_output_to_group(const struct ofpact *ofpacts,
size_t ofpacts_len,
return false;
}
+/* Returns true if the 'a_len' bytes of actions in 'a' and the 'b_len' bytes of
+ * actions in 'b' are bytewise identical. */
bool
ofpacts_equal(const struct ofpact *a, size_t a_len,
const struct ofpact *b, size_t b_len)
@@ -8323,6 +8325,28 @@ ofpacts_equal(const struct ofpact *a, size_t a_len,
return a_len == b_len && !memcmp(a, b, a_len);
}
+/* Returns true if the 'a_len' bytes of actions in 'a' and the 'b_len' bytes of
+ * actions in 'b' are identical when formatted as strings. (Converting actions
+ * to string form suppresses some rarely meaningful differences, such as the
+ * 'compat' member of actions.) */
+bool
+ofpacts_equal_stringwise(const struct ofpact *a, size_t a_len,
+ const struct ofpact *b, size_t b_len)
+{
+ struct ds a_s = DS_EMPTY_INITIALIZER;
+ struct ds b_s = DS_EMPTY_INITIALIZER;
+
+ ofpacts_format(a, a_len, NULL, &a_s);
+ ofpacts_format(b, b_len, NULL, &b_s);
+
+ bool equal = !strcmp(ds_cstr(&a_s), ds_cstr(&b_s));
+
+ ds_destroy(&a_s);
+ ds_destroy(&b_s);
+
+ return equal;
+}
+
/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
* 'ofpacts'. If found, returns its meter ID; if not, returns 0.
*
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6fb2cc08de50..5b7f1b02104f 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -3146,8 +3146,8 @@ fte_version_equals(const struct fte_version *a, const
struct fte_version *b)
&& a->hard_timeout == b->hard_timeout
&& a->importance == b->importance
&& a->table_id == b->table_id
- && ofpacts_equal(a->ofpacts, a->ofpacts_len,
- b->ofpacts, b->ofpacts_len));
+ && ofpacts_equal_stringwise(a->ofpacts, a->ofpacts_len,
+ b->ofpacts, b->ofpacts_len));
}
/* Clears 's', then if 's' has a version 'index', formats 'fte' and version
@@ -3656,15 +3656,13 @@ ofctl_diff_flows(struct ovs_cmdl_context *ctx)
if (!a || !b || !fte_version_equals(a, b)) {
fte_version_format(&fte_state, fte, 0, &a_s);
fte_version_format(&fte_state, 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 (a_s.length) {
+ printf("-%s", ds_cstr(&a_s));
+ }
+ if (b_s.length) {
+ printf("+%s", ds_cstr(&b_s));
}
+ differences = true;
}
}
}
--
2.10.2
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
