[ovs-dev] [warnings2 4/4] ovs-ofctl: Report attempts to add (remove, etc.) non-normalized flows.

[Ben Pfaff]

Commit 0b3f27253 (ovs-ofctl: Warn about flows not in normal form) made
ovs-ofctl warn about non-normalized flows, that is, flows some of whose
specified fields will be ignored by the switch.  This was convenient for
users, who are understandably confused by flow normalization.  However,
later commit 8050b31d6 (ofp-parse: Refactor flow parsing) accidentally
deleted the warning.  This commit restores it and adds a test to ensure
that it doesn't get deleted again later.

Reported-by: Reid Price <reid at nicira.com>
Bug #5029.
---
 lib/ofp-parse.c         |    7 +++++++
 tests/ofproto-macros.at |    1 +
 tests/ovs-ofctl.at      |   12 ++++++++++++
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index f45c450..d60e94a 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -851,6 +851,7 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
 {
     bool is_del = command == OFPFC_DELETE || command == OFPFC_DELETE_STRICT;
     enum nx_flow_format min_format, next_format;
+    struct cls_rule rule_copy;
     struct ofpbuf actions;
     struct ofpbuf *ofm;
     struct flow_mod fm;
@@ -867,6 +868,12 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
         *cur_format = next_format;
     }
 
+    /* Normalize a copy of the rule.  This ensures that non-normalized flows
+     * get logged but doesn't affect what gets sent to the switch, so that the
+     * switch can do whatever it likes with the flow. */
+    rule_copy = fm.cr;
+    ofputil_normalize_rule(&rule_copy, next_format);
+
     ofm = ofputil_encode_flow_mod(&fm, *cur_format);
     list_push_back(packets, &ofm->list_node);
 
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index fabdb84..c165559 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -1,5 +1,6 @@
 m4_define([STRIP_XIDS], [[sed 's/ (xid=0x[0-9a-fA-F]*)//']])
 m4_define([STRIP_DURATION], [[sed 's/\bduration=[0-9.]*s/duration=?s/']])
+m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m'])
 
 m4_define([OFPROTO_START],
   [OVS_RUNDIR=$PWD; export OVS_RUNDIR
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index c23945f..acb353e 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -490,3 +490,15 @@ AT_CHECK([ovs-ofctl -F openflow10 dump-flows unix:br0.mgmt reg0=0xabcdef], [1],
 ])
 OFPROTO_STOP
 AT_CLEANUP
+
+dnl Check that add-flow reports non-normalized flows (feature #5029).
+AT_SETUP([ovs-ofctl add-flow reports non-normalized flows])
+OFPROTO_START
+AT_CHECK([ovs-ofctl TESTABLE_LOG add-flow br0 nw_src=1.2.3.4,actions=5],
+  [0], [], [dnl
+ofp_util|INFO|normalization changed ofp_match, details:
+ofp_util|INFO| pre: nw_src=1.2.3.4
+ofp_util|INFO|post: @&t@
+])
+OFPROTO_STOP
+AT_CLEANUP
-- 
1.7.4.4