ovs-ofctl: Report attempts to add (remove, etc.) non-normalized flows.

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@nicira.com>
Bug #5029.

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 29137ac..52fdbf2 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -845,6 +845,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;
@@ -861,6 +862,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);
+
     if (fm.table_id != 0xff && !*flow_mod_table_id) {
         struct ofpbuf *sff = ofputil_make_flow_mod_table_id(true);
         list_push_back(packets, &sff->list_node);

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 3927550..dc863e7 100644 (file)
--- 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 997cc30..6d636c1 100644 (file)
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -491,3 +491,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