ovs-ofctl: Avoid unnecessary flow replacement in "replace-flows" command.

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 <kevin@quilt.io>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>

1 files changed, 8 insertions, 10 deletions

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6fb2cc08d..5b7f1b021 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;
             }
         }
     }